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

Implemented a possibility to disable gops via helm #2961

Merged
merged 1 commit into from
Oct 2, 2024

Conversation

XelK
Copy link
Contributor

@XelK XelK commented Sep 30, 2024

Fixes

Description

Implemented the possibility to disable gops server when installing tetragon via helm

Changelog

Add an "enabled" switch to enable/disable the gops server via the Helm chart. It is now disabled by default.

@XelK XelK requested a review from a team as a code owner September 30, 2024 11:08
@XelK XelK requested a review from jrfastab September 30, 2024 11:08
@mtardy mtardy added the release-note/misc This PR makes changes that have no direct user impact. label Sep 30, 2024
@mtardy mtardy requested review from mtardy and removed request for jrfastab September 30, 2024 11:11
@mtardy
Copy link
Member

mtardy commented Sep 30, 2024

Thanks for the patch following the discussion we had on Slack :)! However, it looks like your commit is not signed-off, to improve tracking of who did what, the Tetragon project uses a "sign-off" procedure which you can learn more about in the Cilium project documentation.

Concretely it consists of adding your real name and an email address to your git config and using git commit -s to write your commit message. You can use git commit --amend -s to add the signature to existing commits.

See more details on how to comply with the "sign-off" procedure

To add your name and email to your git config:

git config user.email "[email protected]"
git config user.name "firstname lastname"

Note: add the --global flag after config to configure this by default for repositories on your host.

Then you can use for new commits:

git commit -s

Or for existing commits on which you want to add the signed-off-by statement:

git commit --amend -s

For more information, see this extract from git-commit(1) man page regarding the -s flag:

-s, --signoff, --no-signoff
   Add a Signed-off-by trailer by the committer at the end of the commit log
   message. The meaning of a signoff depends on the project to which you're
   committing. For example, it may certify that the committer has the rights to
   submit the work under the project's license or agrees to some contributor
   representation, such as a Developer Certificate of Origin. (See
   http://developercertificate.org for the one used by the Linux kernel and Git
   projects.) Consult the documentation or leadership of the project to which
   you're contributing to understand how the signoffs are used in that project.

   The --no-signoff option can be used to countermand an earlier --signoff
   option on the command line.

@XelK XelK force-pushed the pr/XelK/helm-gops branch from 2174006 to 753a6f7 Compare September 30, 2024 11:42
@mtardy
Copy link
Member

mtardy commented Sep 30, 2024

Hey, please take a look at the CI output to update your patch and please in the end squash your commits into only one :)! Thanks

@XelK XelK force-pushed the pr/XelK/helm-gops branch from 6afb809 to 293db8d Compare September 30, 2024 20:11
Copy link

netlify bot commented Sep 30, 2024

Deploy Preview for tetragon ready!

Name Link
🔨 Latest commit d439e0f
🔍 Latest deploy log https://app.netlify.com/sites/tetragon/deploys/66fc3eea2291080008cb8d48
😎 Deploy Preview https://deploy-preview-2961--tetragon.netlify.app
📱 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.

@@ -175,6 +175,8 @@ tetragon:
# -- The address at which to expose gRPC. Examples: localhost:54321, unix:///var/run/cilum/tetragon/tetragon.sock
address: "localhost:54321"
gops:
# -- Whether to enable exposing gops server.
enabled: false
Copy link
Member

Choose a reason for hiding this comment

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

It will be user-facing change however, I imagine that the previous situation was making this enabled by default. However I think disabling it by default might be correct indeed!

@mtardy mtardy added release-note/minor This PR introduces a minor user-visible change and removed release-note/misc This PR makes changes that have no direct user impact. labels Oct 1, 2024
@mtardy
Copy link
Member

mtardy commented Oct 1, 2024

it looks like e2e test is assuming gops is enabled by default

@XelK XelK force-pushed the pr/XelK/helm-gops branch from 293db8d to d439e0f Compare October 1, 2024 18:26
Copy link
Member

@mtardy mtardy left a comment

Choose a reason for hiding this comment

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

Thanks a lot for the work, if you want we could maybe disable that by default, thus we would need small changes in the e2e test framework.

@mtardy mtardy merged commit 94359f5 into cilium:main Oct 2, 2024
46 checks passed
@XelK XelK deleted the pr/XelK/helm-gops branch October 2, 2024 18:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note/minor This PR introduces a minor user-visible change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants