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

Helm: Added various configuration options for the Tetragon Operator Deployment #1817

Conversation

PhilipSchmid
Copy link
Collaborator

@PhilipSchmid PhilipSchmid commented Nov 30, 2023

I have added multiple configuration options for the Tetragon Operator Deployment, which are often crucial for enterprise-grade deployments that need to happen within certain security boundaries.

According to my current understanding, this should not break anything for existing users. The only chance I see a breakage happen is when somebody already configured .Values.serviceAccount.name in the past. The thing is, this value was never considered, and the SA was simply named {{ .Release.Name }} or {{ .Release.Name }}-operator-service-account. With this change, it would suddenly be considered for the actual SA naming.

  • helm: Added extraVolumes support for the Tetragon Operator
  • helm: Added extra labels for the Tetragon Operator Deployment and Pods
  • helm: Added configurable annotations for the Tetragon Operator Deployment and Pods
  • helm: Made the placement of Tetragon Operator Deployment Pods configurable
  • helm: Made update strategy configurable for the Tetragon Operator Deployment
  • helm: Made resource configurable for the Tetragon Operator Deployment
  • helm: Added pod/container securityContext to the Tetragon Operator Deployment
  • helm: Added priorityClassName to the Tetragon Operator Deployment
  • helm: Added imagePullSecret to the Tetragon Operator Deployment

Fixes: #1761

@PhilipSchmid PhilipSchmid requested a review from a team as a code owner November 30, 2023 17:01
Copy link

netlify bot commented Nov 30, 2023

Deploy Preview for tetragon ready!

Name Link
🔨 Latest commit 2aa5b16
🔍 Latest deploy log https://app.netlify.com/sites/tetragon/deploys/6569a005c8357900082dd59d
😎 Deploy Preview https://deploy-preview-1817--tetragon.netlify.app/docs/reference/helm-chart
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@tpapagian tpapagian added the release-note/minor This PR introduces a minor user-visible change label Nov 30, 2023
@lambdanis lambdanis added the area/helm Related to the Helm chart label Nov 30, 2023
Copy link
Contributor

@lambdanis lambdanis left a comment

Choose a reason for hiding this comment

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

This is great, thank you.

I left one comment about labels configuration, but more like food for thought.

install/kubernetes/values.yaml Show resolved Hide resolved
@lambdanis
Copy link
Contributor

@PhilipSchmid I see the CI is failing:

  • The generated docs are not up to date - ./install/kubernetes/test.sh should do the trick
  • The commit messages are too long

Could you fix these?

* Added support for dedicated SA for the Tetragon Operator
* Fixed serviceAccount.name usage as it wasn't considered before

Signed-off-by: Philip Schmid <[email protected]>
@PhilipSchmid PhilipSchmid force-pushed the pr/philip/separate_helm_flags_for_components branch from 54d5017 to 454d680 Compare November 30, 2023 19:43
@PhilipSchmid
Copy link
Collaborator Author

@lambdanis I just shortened the commit messages and checked that the .test.sh script no longer generates new changes. Unfortunately, the CI is now even more unhappy with this PR 😆.

@lambdanis
Copy link
Contributor

lambdanis commented Nov 30, 2023

Oh dear, it looks like some Quay hiccup maybe? The CI failures definitely don't looks related to the changes. I'll rerun.

@lambdanis
Copy link
Contributor

checked that the .test.sh script no longer generates new changes

Ok, I run on your branch and it looks like the trailing space here is the problem. I suppose your editor is removing trailing spaces automatically? If you could remove this space from the script, then it should pass :) (a bit surprising that nobody hit it before tbh)

@PhilipSchmid PhilipSchmid force-pushed the pr/philip/separate_helm_flags_for_components branch 2 times, most recently from 7a8dc1e to aee72c3 Compare December 1, 2023 08:42
Signed-off-by: Philip Schmid <[email protected]>
@PhilipSchmid PhilipSchmid force-pushed the pr/philip/separate_helm_flags_for_components branch from aee72c3 to 2aa5b16 Compare December 1, 2023 08:57
@lambdanis
Copy link
Contributor

After some flakes the CI finally passes, so I'm merging it, thank you @PhilipSchmid!

@lambdanis lambdanis merged commit 3fb7869 into cilium:main Dec 1, 2023
35 checks passed
@PhilipSchmid PhilipSchmid deleted the pr/philip/separate_helm_flags_for_components branch December 1, 2023 13:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/helm Related to the Helm chart release-note/minor This PR introduces a minor user-visible change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add custom annotations and labels to tetragon operator
3 participants