open-policy-agent / kube-mgmt

Sidecar for managing OPA instances in Kubernetes.
Apache License 2.0
238 stars 106 forks source link

redundant configmap patch operations #90

Closed jdef closed 2 years ago

jdef commented 3 years ago

https://github.com/open-policy-agent/kube-mgmt/blob/191879ecb3633b2d9ac849ec2ef441241a4ed0bd/pkg/configmap/configmap.go#L250

there appears to be no check on the configmap annotations here, to only issue a PATCH to apiserver if the desired value is different than the current. i started investigating this because our OPA policies rarely change, yet I see PATCH requests constantly issued to apiserver. this appears to, perhaps, be part of the problem.

tsandall commented 3 years ago

I suppose kube-mgmt could perform a bytes.Equal check on the configmap data. That said, what's the actual problem you're observing? Those updates are only sent when there's a watch even on the configmaps. Either something on the configmaps is changing or k8s is just firing spurious watch events...

jdef commented 3 years ago

"updates are only sent when there's a watch event..."

AFAICT there's a full resync that happens every minute: https://github.com/open-policy-agent/kube-mgmt/blob/191879ecb3633b2d9ac849ec2ef441241a4ed0bd/pkg/configmap/configmap.go#L146

.. which corresponds to the audit logs in my cluster; and the update is hitting the apiserver, even when there are no configmap changes.

jdef commented 3 years ago

from the cache API docs re: informers:

resyncPeriod: if non-zero, will re-list this often (you will get OnUpdate
  calls, even if nothing changed). Otherwise, re-list will be delayed as
  long as possible (until the upstream source closes the watch or times out,
  or you stop the controller).
tsandall commented 3 years ago

ah, @jdef you're right...sorry, it's been a while since I've looked at this code.

Feel free to submit a patch if you like.

jdef commented 3 years ago

I would, but I already have a stack of aging (~1yr) OSS patches that have yet to clear our legal department. As much as I'd like to patch this myself in OPA upstream, I'm afraid that I'll be unable to do so until corporate red tape has cleared.

I would be grateful if someone in the OPA community could spend 15m on this

On Tue, Mar 16, 2021 at 8:57 AM Torin Sandall @.***> wrote:

ah, @jdef https://github.com/jdef you're right...sorry, it's been a while since I've looked at this code.

Feel free to submit a patch if you like.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/open-policy-agent/kube-mgmt/issues/90#issuecomment-800235763, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAR5KLB537GCWK7J5IDG24TTD5IT3ANCNFSM4ZEJIXHA .

-- James DeFelice 585.241.9488 (voice) 650.649.6071 (fax)

jdef commented 3 years ago

/cc @pires

jdef commented 2 years ago

Was this resolved by a PR? If so, mind linking to it?

On Sat, May 7, 2022, 1:13 AM Ievgenii Shepeliuk @.***> wrote:

Closed #90 https://github.com/open-policy-agent/kube-mgmt/issues/90.

— Reply to this email directly, view it on GitHub https://github.com/open-policy-agent/kube-mgmt/issues/90#event-6567050282, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAR5KLG3A35A7NBXM3WCJJ3VIX3WXANCNFSM4ZEJIXHA . You are receiving this because you were mentioned.Message ID: @.***>

eshepelyuk commented 2 years ago

Was this resolved by a PR? If so, mind linking to it? On Sat, May 7, 2022, 1:13 AM Ievgenii Shepeliuk @.> wrote: Closed #90 <#90>. — Reply to this email directly, view it on GitHub <#90 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAR5KLG3A35A7NBXM3WCJJ3VIX3WXANCNFSM4ZEJIXHA . You are receiving this because you were mentioned.Message ID: @.>

The links are just above your comment.

jdef commented 2 years ago

Heh, thanks. Was looking at the email thread. Should have checked GH directly

On Sat, May 7, 2022, 10:52 AM Ievgenii Shepeliuk @.***> wrote:

Was this resolved by a PR? If so, mind linking to it? … <#m9087902201560492211> On Sat, May 7, 2022, 1:13 AM Ievgenii Shepeliuk @.> wrote: Closed #90 https://github.com/open-policy-agent/kube-mgmt/issues/90 <#90 https://github.com/open-policy-agent/kube-mgmt/issues/90>. — Reply to this email directly, view it on GitHub <#90 (comment) https://github.com/open-policy-agent/kube-mgmt/issues/90#event-6567050282>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAR5KLG3A35A7NBXM3WCJJ3VIX3WXANCNFSM4ZEJIXHA https://github.com/notifications/unsubscribe-auth/AAR5KLG3A35A7NBXM3WCJJ3VIX3WXANCNFSM4ZEJIXHA . You are receiving this because you were mentioned.Message ID: @.>

The links are just above your comment.

— Reply to this email directly, view it on GitHub https://github.com/open-policy-agent/kube-mgmt/issues/90#issuecomment-1120222836, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAR5KLFNW7ZOJMTAPHXZSE3VIZ7TBANCNFSM4ZEJIXHA . You are receiving this because you were mentioned.Message ID: @.***>