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

control-service: Update helm template license headers #1670

Merged
merged 1 commit into from
Feb 24, 2023

Conversation

DeltaMichael
Copy link
Contributor

@DeltaMichael DeltaMichael commented Feb 23, 2023

What

Update helm template license headers to use template-style comments Ignore helm templates in pre-commit yaml license hook Create new pre-commit license hook to add template-style comments to helm templates

Why

Regular yaml comments at the top of files break parsing for helm charts This breaks control-service deployments

How was this tested

Tested installing the charts on a local k8s cluster using helm

Ran helm upgrade --install --debug --wait --timeout 10m0s cicd-control-service . on the chart, which gave me the same output as the failing pipeline. Edited the comments until the apiVersion error disappeared.

Error: UPGRADE FAILED: error validating "": error validating data: apiVersion not set

https://gitlab.com/vmware-analytics/versatile-data-kit/-/jobs/3813924321

Copy link
Collaborator

@antoniivanov antoniivanov left a comment

Choose a reason for hiding this comment

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

Please do not include the downloaded charts in source control (the binary files). Those are generated automatically.

@DeltaMichael DeltaMichael force-pushed the person/mdilyan/helm-template-license branch from 0a4044d to 89ec5cd Compare February 23, 2023 13:58
What
Update helm template license headers to use template-style comments
Ignore helm templates in pre-commit yaml license hook
Create new pre-commit license hook to add template-style comments to helm templates

Why
Regular yaml comments at the top of files break parsing for helm charts
This breaks control-service deployments

How was this tested

Tested installing the charts on a local k8s cluster using helm

Signed-off-by: Dilyan Marinov <[email protected]>
@DeltaMichael DeltaMichael force-pushed the person/mdilyan/helm-template-license branch from 89ec5cd to dbf5629 Compare February 23, 2023 14:28
@antoniivanov
Copy link
Collaborator

Tested installing the charts on a local k8s cluster using helm

It is good idea when doing any manual testing to include the actual commands being executed in the 'manual' test and some relevant for verification output of the command. That makes it clearer what exactly was tested and easier to review if something could be tested better.

@DeltaMichael DeltaMichael merged commit 1516bcb into main Feb 24, 2023
@DeltaMichael DeltaMichael deleted the person/mdilyan/helm-template-license branch February 24, 2023 07:58
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.

4 participants