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

Rework helmfile dependencies during deployment #306

Merged
merged 1 commit into from
Aug 7, 2024

Conversation

pvannierop
Copy link

@pvannierop pvannierop commented Jul 31, 2024

In this PR the method by which dependencies are evaluated by helmfile is reworked. Instead of different helmfiles treated in sequence (00-init, 10-base, ...) there is one helmfile (sic, see next) where dependencies are declared with needs members. The only service that is created before all others is cert-manager (and prometheus); omitting this will cause other services to throw errors.

@pvannierop pvannierop requested a review from keyvaann July 31, 2024 12:45
@pvannierop pvannierop self-assigned this Jul 31, 2024
@pvannierop
Copy link
Author

@keyvaann For Kafka related services, we still have wait: true in the helmfile. Do you think we should remove these?

@pvannierop pvannierop force-pushed the feature/needs-helmfile-rework branch 2 times, most recently from c98535b to 3941841 Compare July 31, 2024 12:56
@keyvaann
Copy link
Collaborator

If we have the needs for Kakfa services then I don't think we need the wait: true

@keyvaann
Copy link
Collaborator

You can remove the helmfile.d directory and create helmfile.yaml in the root directory.

@keyvaann
Copy link
Collaborator

keyvaann commented Jul 31, 2024

Why there are 2 helmfiles?

@pvannierop
Copy link
Author

pvannierop commented Aug 1, 2024

This is to make sure that cert-manager is installed first. Cert-manager is needed to install many services and it would be very cumbersome declare it as needs in the 00-services.yaml. For cert-manager I rely on the glob order.

@keyvaann keyvaann force-pushed the feature/needs-helmfile-rework branch from 3941841 to daa0dda Compare August 7, 2024 07:55
Copy link
Collaborator

@keyvaann keyvaann left a comment

Choose a reason for hiding this comment

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

LGTM

@keyvaann keyvaann merged commit 2529086 into dev Aug 7, 2024
2 of 3 checks passed
@keyvaann keyvaann deleted the feature/needs-helmfile-rework branch August 7, 2024 07:55
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.

2 participants