-
Notifications
You must be signed in to change notification settings - Fork 14.8k
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
Validate Helm values.yaml #10664
Validate Helm values.yaml #10664
Conversation
We have some errors Can you look at it?
|
chart/values.schema.json
Outdated
"config": { | ||
"description": "Settings to go into the mounted airflow.cfg", | ||
"type": "object", | ||
"properties": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here similar, but a little more complex. We expect there will be a map, which has a map that contains arbitrary text strings. Map<String, Map<String, String>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I do a
helm install airflow . --namespace airflow --set defaultAirflowRepository=apache/airflow --set images.airflow.repository=apache/airflow --set images.airflow.tag=master-python3.6-kubernetes -v 1 --set defaultAirflowTag=master-python3.6-kubernetes -v 1 --set config.api.auth_backend=airflow.api.auth.backend.default --dry-run
I get the following errors:
Error: values don't meet the specifications of the schema(s) in the following chart(s):
airflow:
- config.elasticsearch_configs.max_retries: Invalid type. Expected: string, given: integer
- config.elasticsearch_configs.timeout: Invalid type. Expected: string, given: integer
- config.scheduler.run_duration: Invalid type. Expected: string, given: integer
- config.scheduler.scheduler_heartbeat_sec: Invalid type. Expected: string, given: integer
- config.scheduler.statsd_port: Invalid type. Expected: string, given: integer
And indeed we have the following values:
config:
scheduler:
scheduler_heartbeat_sec: 5
statsd_port: 9125
run_duration: 41460
elasticsearch_configs:
max_retries: 3
timeout: 30
Should we allow primitive types as well (integer, number and boolean) ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. We should allow other types also.
It would be fantastic if we could generate documentation for README.md based on this, but we should do it in a separate PR. It has a lot of potential. |
Indeed, this is a great way to keep both in sync. Let me know if I can help with this as well :) |
@mik-laj @dimberman Do you have any feedback on this PR ? Also, should I clean a bit the commit history ? |
We use squash and merge so it's not important to us. If you wish, you can do so, but it is not required. I can see your branch is a bit outdated. Can you do a rebase? |
@dimberman Can you look at it? This is a nice improvement for the Helm Chart. |
147744a
to
c4bb235
Compare
@schnie Can I ask for review? |
Tackle #10634