Skip to content

Conversation

@pvannierop
Copy link
Contributor

@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
Contributor 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
Contributor 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
@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.

3 participants