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(helm): allow to disable tls-checksum generation #7955

Merged
merged 2 commits into from
Oct 4, 2023

Conversation

lukidzi
Copy link
Contributor

@lukidzi lukidzi commented Oct 3, 2023

Checklist prior to review

It might be useful to not generate a new checksum for each secret change, sometimes we don't need to restart control plane or we are not using them. In this case, we don't need to restart. Also, Argo generates a checksum for each template and that can cause additional ReplicaSet for each deployment.

  • Link to relevant issue as well as docs and UI issues --
  • This will not break child repos: it doesn't hardcode values (.e.g "kumahq" as a image registry) and it will work on Windows, system specific functions like syscall.Mkfifo have equivalent implementation on the other OS --
  • Tests (Unit test, E2E tests, manual test on universal and k8s) --
  • Do you need to update UPGRADE.md? --
  • Does it need to be backported according to the backporting policy? (this GH action will add "backport" label based on these file globs, if you want to prevent it from adding the "backport" label use no-backport-autolabel label) --
  • Do you need to explicitly set a > Changelog: entry here or add a ci/ label to run fewer/more tests?

@lukidzi lukidzi requested a review from a team as a code owner October 3, 2023 16:18
@lukidzi lukidzi requested review from michaelbeaumont and slonka and removed request for a team October 3, 2023 16:18
Copy link
Contributor

@slonka slonka left a comment

Choose a reason for hiding this comment

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

don't need to restart control plane

can you be more specific here, in which cases?

we are not using them

can we just have a conditional checking for usage?

Argo generates a checksum for each template

yeah but the checksum there should be the same, right? if the underlying value of cp-webhooks-and-secrets.yaml did not change then sha256 also shouldn't change

deployments/charts/kuma/README.md Outdated Show resolved Hide resolved
@lukidzi
Copy link
Contributor Author

lukidzi commented Oct 4, 2023

don't need to restart control plane

can you be more specific here, in which cases?

Example: helm template, when you do helm template it doesn't connect to the cluster so even if the secret exists it doesn't know about it and generate new checksum

we are not using them

can we just have a conditional checking for usage?

It's not a trivial change and might breaks many things - each server API requires them and while going this way notice more changes than expected

Argo generates a checksum for each template

yeah but the checksum there should be the same, right? if the underlying value of cp-webhooks-and-secrets.yaml did not change then sha256 also shouldn't change

It's not correct. Because helm template doesn't connect to the cluster it generates new sha each time. While applying there is no changes but when service is using helm template (like argo) it generates new hash each time. That creates new replicaSet with new pod template, but the secret under might not change or even be used

Signed-off-by: Lukasz Dziedziak <[email protected]>
@lukidzi lukidzi merged commit 36bca4a into kumahq:master Oct 4, 2023
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