bitnami / charts

Bitnami Helm Charts
https://bitnami.com
Other
8.61k stars 8.98k forks source link

[bitnami/flux] Flux application version is wrong #21087

Open stefanprodan opened 7 months ago

stefanprodan commented 7 months ago

Name and Version

bitnami/flux

What architecture are you using?

None

What steps will reproduce the bug?

Hello, Flux maintainer here 👋

I noticed that the Flux chart sets the application version wrong here https://github.com/bitnami/charts/blob/9dde49c88c7a45d7fa36df5b81c285a627bad896/bitnami/flux/Chart.yaml#L23

It should be 2.1.2, Flux 1.1.2 is deprecated and archived.

What is the expected behavior?

The app version should be set to 2.1.2

What do you see instead?

The app version is set to 1.1.2

javsalgar commented 7 months ago

Hi!

Thank you so much for reporting. Currently, this is a limitation of how our pipeline currently works. The appVersion is calculated using the version of the, let's call it the "main container". For example, in the WordPress helm chart, the main container is WordPress so it uses the WordPress version as appVersion.

In other multi-container applications like Harbor, we use the component "harbor-core" as the main container (in this case it does not matter because all of the harbor containers share the same versioning).

In this case, it was more difficult because we are not packaging a container of the flux2 repository (as, AFAIK, this is the CLI and it is not used in the chart), so we needed to choose one of the containers we package. In this case, we selected the fluxcd-source-controller container as the "main container". At the moment this is something that we cannot easily change, as the automated system will immediately replace the appVersion with the version in source-controller.

In order to workaround this, we would need to add the flux CLI to the chart and use it as the "main container", though it is not clear to me how this would make sense in the chart. We'd be grateful to hear of any thoughts on how to solve this issue.

stefanprodan commented 7 months ago

@javsalgar see here how the CLI container is used as a pre-install hook: https://github.com/fluxcd-community/helm-charts/blob/main/charts/flux2/templates/pre-install-job.yaml

stefanprodan commented 7 months ago

Another option is to use the CLI container for Helm test, that would run flux check. This way you don't force people into running the CLI pod, as tests are opt-in.

javsalgar commented 7 months ago

Thank you so much for the input! What does this pre-install-job do exactly?

stefanprodan commented 7 months ago

pre-install-job runs flux check --pre which gets the cluster version and errors out if it's not supported by Flux. As I mentioned in https://github.com/bitnami/charts/issues/21087#issuecomment-1821327562 a better option would be to add flux check as a Helm test, when not run with --pre, the check command verifies that the Flux CRDs are registered correctly in etcd and that the Flux controllers are healthy.

FraPazGal commented 7 months ago

Hello @stefanprodan, given our pipeline architecture, using the CLI for testing purposes wouldn't require adding it to the chart's containers (as our pipeline sees them) and wouldn't solve the current issue.

On the other hand, using a pre-install job would indeed requiere adding the cli container to the chart's definition. This would then allow us to fix the versioning mismatch, though it requires a somewhat lengthy process which involves adding the cli to our containers catalog. Let me discuss this with the engineering and product teams and get back as soon as possible.

stefanprodan commented 7 months ago

using the CLI for testing purposes wouldn't require adding it to the chart's containers

I was referring to Helm test, not your CI process.

carrodher commented 7 months ago

Hi Stefan, you're right about the versioning, the appVersion should be 2.1.2 right now. As my teammates mentioned, currently we have a limitation so the version needs to come from a bundled container, it can't be hardcoded or manually introduced, that's the reason why 1.1.2 is the current version.

We will figure out how to track and include the CLI container so the appVersion matches the upstream product version.