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: Add default divisor
for GOMEMLIMIT
and GOMAXPROCS
#49923
Conversation
The committers listed above are authorized under a signed CLA. |
😊 Welcome @Kellen275! This is either your first contribution to the Istio istio repo, or it's been You can learn more about the Istio working groups, Code of Conduct, and contribution guidelines Thanks for contributing! Courtesy of your friendly welcome wagon. |
Hi @Kellen275. Thanks for your PR. I'm waiting for a istio member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Is this really the only field that we don't have a default explicitly set? FWIW I think this is solveable already in argo. Not an expert but I think argoproj/argo-cd#11574 is the relevant link. Basically Argo is not aware of defaults by default. I think its a bit silly to expect every helm chart to explicitly specify every default value, and Argo pushes this problem to all charts in the world. But it is also cheap to fix so probably worth while. |
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.
we have this in many places, is there a reason only the cni chart was changed?
In my particular case, I'm attempting to sync the following istio charts: This CNI chart is the only one Argo is reporting a diff on. Admittedly I haven't explored the other istio charts or different configurations of the same charts. |
🚧 This issue or pull request has been closed due to not having had activity from an Istio team member since 2024-03-14. If you feel this issue or pull request deserves attention, please reopen the issue. Please see this wiki page for more information. Thank you for your contributions. Created by the issue and PR lifecycle manager. |
Please provide a description of this PR:
When deploying istio via ArgoCD with auto-sync, this field is flagged as being removed since Kubernetes appears to add the default after-the-fact.