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

feat: move lint test chart to seperate action and trigger it on reg tests #2536

Merged
merged 9 commits into from
Feb 6, 2024

Conversation

kostasrim
Copy link
Contributor

  • move lint test chart to a separate action
  • use it in regression tests

@kostasrim
Copy link
Contributor Author

@kostasrim kostasrim self-assigned this Feb 4, 2024
@romange
Copy link
Collaborator

romange commented Feb 5, 2024

@Pothulapati what this step does? does it change release the new version of helm templates?

@Pothulapati
Copy link
Contributor

@romange This test just runs the Helm Charts against a kubernetes cluster to make sure things work (using https://github.com/helm/chart-testing). I don't think this workflow releases a new file.

Currently, Its failing on the Password Var deprecation and #2539 fixes that.

@romange
Copy link
Collaborator

romange commented Feb 5, 2024

Do you guys understand why some steps have if: github.event_name == 'workflow_dispatch' condition @kostasrim @Pothulapati ?

@Pothulapati
Copy link
Contributor

Pothulapati commented Feb 5, 2024

@romange Not exactly sure why they exist but my guess would be that as they involve running a Kubernetes cluster they thought it was not necessary to run them per PR and only when they want but for runtime bugs (like the password one), It's the only way to find them is to run them.

Considering that this new regression job runs on a schedule and on a trigger, I guess it makes sense to remove the toggle entirely and have this run everytime. wdyt?

@romange
Copy link
Collaborator

romange commented Feb 5, 2024

I agree, @kostasrim ^^

ct \
lint \
--config .ct.yaml \
${{github.event_name == 'workflow_dispatch' && '--all'}} ;
Copy link
Contributor Author

@kostasrim kostasrim Feb 5, 2024

Choose a reason for hiding this comment

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

I kept this because if the job gets triggered manually it will become true. I am not sure about the && -all` part, I am running the reg tests manually to see how it affects it and I will remove it if it's not used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kostasrim kostasrim merged commit 4c0055d into main Feb 6, 2024
14 of 15 checks passed
@kostasrim kostasrim deleted the add_helm_to_reg branch February 6, 2024 08:27
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.

None yet

4 participants