Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] deploy dir values.yaml files for supervisor,concierge as schemas #1680

Closed
wants to merge 4 commits into from

Conversation

benjaminapetersen
Copy link
Member

@benjaminapetersen benjaminapetersen commented Sep 18, 2023

To improve the values.yaml file by enhancing it into a schema (and to prepare for the Carvel package work) the following files are converted:

  • supervisor/values.yaml
  • concierge/values.yaml

The openapi-v3 schema generation can be validated via:

# generate an openapi doc via the schema w/o error
ytt --file ./deploy/supervisor --data-values-schema-inspect --output openapi-v3
# generate an openapi doc via the schema w/o error
ytt --file ./deploy/supervisor --data-values-schema-inspect --output openapi-v3

⚠️ These should be dumped out into ./deploy/{supervisor,concierge}/ but not inside the ./deploy/{supervisor,concierge}/config directories as kapp will be confused when deploying as they are not kubernetes resources.

The basic template generation can be validated via:

# output of supervisor templates render w/o error
ytt --file ./deploy/supervisor \
    --data-value "app_name=test-app-name" \
    --data-value "namespace=test-app-namespace" \
    --data-value "api_group_suffix=pinnnnnnnipeeeed.dev" \
    --data-value "image_repo=pinniped.local/test/build/tttteeeessst" \
    --data-value "image_tag=12345" \
    --data-value "log_level=debug" \
    --data-value-yaml "custom_labels={}" \
    --data-value-yaml "service_https_nodeport_port=1234" \
    --data-value-yaml "service_https_nodeport_nodeport=4567" \
    --data-value-yaml "service_https_clusterip_port=8901"

# output of concierge templates render w/o error
ytt --file ./deploy/concierge \
    --data-value "app_name=test-app-name" \
    --data-value "namespace=test-app-namespace" \
    --data-value "api_group_suffix=pinnnnnnnipeeeed.dev" \
    --data-value "log_level=debug" \
    --data-value-yaml "custom_labels={}" \
    --data-value "image_repo=pinniped.local/test/build/tttteeeessst" \
    --data-value "image_tag=12345" \
    --data-value "discovery_url=1.2.3.4.5:7891"

The real test now is:

  • Run the above ytt against main and this branch for supervisor and diff the files. We want the same (or perhaps close, with documented differences) output.
  • Run the above ytt against main and this branch for concierge and diff the files. We want the same (or perhaps close, with documented differences) output.

Open:

  • Should we generate and save the openapi-v3 schema files as well for both supervisor and concierge? It is generally expected to be included in a Package.
  • Should we add something to CI to ensure that the openapi-v3 generation is done and kept up to date?

@codecov
Copy link

codecov bot commented Sep 18, 2023

Codecov Report

Merging #1680 (a861bb4) into main (bed9a74) will increase coverage by 0.00%.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1680   +/-   ##
=======================================
  Coverage   79.21%   79.22%           
=======================================
  Files         163      163           
  Lines       15758    15758           
=======================================
+ Hits        12483    12484    +1     
  Misses       2959     2959           
+ Partials      316      315    -1     

see 1 file with indirect coverage changes

@benjaminapetersen benjaminapetersen changed the title deploy dir values.yaml files as schemas deploy dir values.yaml files for supervisor,concierge as schemas Sep 18, 2023
@joshuatcasey
Copy link
Member

This seems reasonable. You can run ./hack/prepare-for-integration-tests.sh without errors, which indicates (hopefully) that all the same defaults are in place.

#@ Optional."
#@schema/desc image_pull_dockerconfigjson_desc
#@schema/nullable
image_pull_dockerconfigjson: {"auths":{"https://registry.example.com":{"username":"USERNAME","password":"PASSWORD","auth":"BASE64_ENCODED_USERNAME_COLON_PASSWORD"}}}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This appears to cause CI/CD issues

#@ Optional."
#@schema/desc image_pull_dockerconfigjson_desc
#@schema/nullable
image_pull_dockerconfigjson: {"auths":{"https://registry.example.com":{"username":"USERNAME","password":"PASSWORD","auth":"BASE64_ENCODED_USERNAME_COLON_PASSWORD"}}}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This appears to cause CI/CD issues

@benjaminapetersen benjaminapetersen changed the title deploy dir values.yaml files for supervisor,concierge as schemas [WIP] deploy dir values.yaml files for supervisor,concierge as schemas Sep 27, 2023
@benjaminapetersen
Copy link
Member Author

Revamping this work, will open a new PR.

@cfryanr cfryanr deleted the ben/deploy-values-to-schema branch June 21, 2024 15:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants