search

apache / airflow

Apache Airflow - A platform to programmatically author, schedule, and monitor workflows
https://airflow.apache.org/
Apache License 2.0
36.66k stars 14.19k forks source link

Run helm chart through kubeconform -strict as part of the CI process #37919

Closed brokenjacobs closed 7 months ago

brokenjacobs commented 7 months ago

Description

There have been several releases in a row that have broken helm chart deployments of airflow using tools like fluxcd. If you do a helm install of the chart on the command line it works, but there are errors in the yaml output causing strict validation to fail. I suggest adding a check to PR's that update the helm chart to do the equivalent of:

helm template -f values.yaml ./ | kubeconform -strict

I'm not sure if I can just go add random github actions to this project, but I would suggest an approach like this: https://github.com/marketplace/actions/helm-kubeval

And we would need to pull the keda crds into the repo for use with kubeconform for these checks. A quick sanity check of this approach with the 1.13.0 release:

❯ helm template -f values.yaml ./ | kubeconform -strict
stdin - Deployment release-name-scheduler failed validation: error unmarshalling resource: error converting YAML to JSON: yaml: unmarshal errors:
  line 43: key "cluster-autoscaler.kubernetes.io/safe-to-evict" already set in map
stdin - StatefulSet release-name-triggerer failed validation: error unmarshalling resource: error converting YAML to JSON: yaml: unmarshal errors:
  line 36: key "cluster-autoscaler.kubernetes.io/safe-to-evict" already set in map

I'm willing to submit a PR if someone can give me guidance as to if using an action for this is acceptable.

Use case/motivation

Preventing future errors in the helm chart through automated testing.

Related issues

https://github.com/apache/airflow/issues/37872 https://github.com/apache/airflow/issues/37794

Are you willing to submit a PR?

Code of Conduct

brokenjacobs commented 7 months ago

@potiuk thoughts?

potiuk commented 7 months ago

Good idea - but random actions are not good (ASF rule/security}. Also we prefer our pre-commit for all static checks (even if it requires wrting a few lines of python). because then it is run automatically when you did pre-commit install LONG time before you push stuff to CI and it can be optimized to only run when any helm/* files change locally - so it adds almost no overhead for most local commits

potiuk commented 7 months ago

In this case we can simply run kubeconform docker image as pre-commit (possibly even without writing Python code just running appropriate docker run as pre-commit

potiuk commented 7 months ago

Or even running pre-commit hook https://github.com/zrootorg/kubeconform-precommit-hook

potiuk commented 7 months ago

I went ahead and copied the approach from helm lint https://github.com/apache/airflow/issues/37919 - this one will automatically install helm using our breeze k8s setup-env and run the pipe with docker image of kubeconform.

brokenjacobs commented 7 months ago

Awesome. Thank you! Hopefully this will help stabilize the releases.