-
Notifications
You must be signed in to change notification settings - Fork 673
feat: add params for initContainers, and hostNetwork #2363
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the contribution! 🎉
The change looks good. I've left a couple of comments. Also, may I ask you to sign the CLA please?
operations/pyroscope/helm/pyroscope/templates/deployments-statefulsets.yaml
Outdated
Show resolved
Hide resolved
I signed the CLA. I removed serviceAnnotations (already brought by: #2364) I removed the v1beta for pod disruption budget |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you also please run make helm/check
and make helm/docs
to update rendered manifests and the documentation?
operations/pyroscope/helm/pyroscope/templates/deployments-statefulsets.yaml
Outdated
Show resolved
Hide resolved
f7cabdd
to
c1a6a61
Compare
Done :) |
operations/pyroscope/helm/pyroscope/templates/deployments-statefulsets.yaml
Outdated
Show resolved
Hide resolved
operations/pyroscope/helm/pyroscope/rendered/micro-services.yaml
Outdated
Show resolved
Hide resolved
94cb75c
to
8b7bc95
Compare
I added a "-n default" to the helm/check helm commands in the makefile. Because by default, it takes the current namespace set in the kube context What do you think ? |
perfect ! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
No description provided.