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

Add schema annotations to values.yaml, generate schema using helm-schema #153

Draft
wants to merge 17 commits into
base: main
Choose a base branch
from

Conversation

marians
Copy link
Member

@marians marians commented May 23, 2024

Towards giantswarm/roadmap#3459

This PR serves as an example to demonstrate: What changes are needed to an unprepared values.yaml so that it can be used with helm-schema to generate a useful values schema (values.schema.json file).

Details

The values.schema.json file is generated using the following command:

helm-schema -k additionalProperties

helm-schema version was 0.11.2

For consistency, I also applied this schemalint command:

schemalint normalize helm/hello-world/values.schema.json -o helm/hello-world/values.schema.json --force

The README.md file in the chart folder has been re-generated using the helm-docs command without any araguments.

@marians marians self-assigned this May 23, 2024
@marians marians changed the title Helm schema annotations Add schema annotations to values.yaml, generate schema using helm-schema May 23, 2024
Comment on lines +238 to +240
"^[a-zA-Z0-9/-_\\.]+$": {
"$ref": "https://schema.giantswarm.io/labelvalue/v0.0.1"
}
Copy link
Member Author

@marians marians May 23, 2024

Choose a reason for hiding this comment

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

This is an example where we use the $ref key to point to an external schema, to avoid repetition.

https://schema.giantswarm.io/labelvalue/v0.0.1 is a schema we created and published exactly for this purpose.

Copy link
Member

@Gacko Gacko left a comment

Choose a reason for hiding this comment

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

Just leaving some comments. Is this PR still active?

# type: integer
# minimum: 1
# @schema
minReplicas: 3
Copy link
Member

Choose a reason for hiding this comment

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

Please do not increase this. This app is used for several tests, also with kind clusters, and this change might lead to pods not being able to be scheduled due to resource limits.

# minimum: 1
# @schema
# Make sure to set a value that is higher than the minReplicas value.
maxReplicas: 10
Copy link
Member

Choose a reason for hiding this comment

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

Please do not decrease this. This app has also been used for load tests in the past, at least from me, and I therefore increased it to 100 at some point.

Comment on lines -71 to -72
cpu: 200m
memory: 50Mi
Copy link
Member

@Gacko Gacko Jul 17, 2024

Choose a reason for hiding this comment

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

Basically the same as for the max replicas: Those values were tuned with load tests. IIRC a single hello world pod takes this amount of resources to handle 1.000 requests per second at 100 connections or something like that. So this ratio is kind of tested and memory consumption will probably never be a multiple of CPU consumption.

@Gacko
Copy link
Member

Gacko commented Jul 31, 2024

I wanted to kindly ask if you're still working on this PR or if we can also close it for now. :)

@marians
Copy link
Member Author

marians commented Aug 1, 2024

I wanted to kindly ask if you're still working on this PR or if we can also close it for now. :)

Hi there! Thanks for the review. I'd like to keep the PR open, as Honeybadger is going to come back to this. We are using this app as a reference for how to possibly maintain values schema for apps in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants