-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
fix(helm-operator): do not send empty patch requests to kube apiserver #4957
Conversation
…ut of the 3-way merge. Signed-off-by: Cheuk Lam <chlam4@hotmail.com>
03f4495
to
e2f4317
Compare
/lgtm |
looks reasonable to me, allowing the tests to run |
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.
@chlam4 thanks for the bugfix. Can you add a changelog fragment? Then I'll approve.
…sts. Signed-off-by: Cheuk Lam <chlam4@hotmail.com>
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
/cherry-pick v1.8.x |
@estroz: once the present PR merges, I will cherry-pick it on top of v1.8.x in a new PR and assign it to you. In response to this:
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. |
Co-authored-by: Eric Stroczynski <estroczy@redhat.com> Signed-off-by: Cheuk Lam <chlam4@hotmail.com>
a422921
to
9c82087
Compare
New changes are detected. LGTM label has been removed. |
@estroz I had to force push the last commit to fulfill the DCO sign-off requirement. Do you mind re-authorizing the workflows? Thanks in advance! |
@estroz: new pull request created: #4971 In response to this:
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. |
@estroz: new pull request created: #4972 In response to this:
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. |
The change filters out empty patches generated from the 3-way merge, which could be in the form of the "{}" byte array that represents an empty map. Reference: operator-framework/operator-sdk#4957 Co-authored-by: Cheuk Lam
The change filters out empty patches generated from the 3-way merge, which could be in the form of the "{}" byte array that represents an empty map. Reference: operator-framework/operator-sdk#4957 Co-authored-by: Cheuk Lam
#139) The change filters out empty patches generated from the 3-way merge, which could be in the form of the "{}" byte array that represents an empty map. Reference: operator-framework/operator-sdk#4957 Co-authored-by: Cheuk Lam
Description of the change:
This change is in the helm operator when generating patch requests during each reconcile cycle. The change filters out empty patches generated from the 3-way merge, which could be in the form of the "{}" byte array that represents an empty map. This byte array is neither nil or zero-length, so the release reconciler sees it as non-empty and sends out a patch request to the kube apiserver.
This addresses #4956.
Motivation for the change:
We have a deployment scenario with hundreds of helm operators, each generating ~60 empty patch requests even there are no changes. That caused heavy traffic onto the kube apiserver.
Testing done: