Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test".
(Note: "add to whitelist" is no longer supported. Please update configurations in kubernetes/test-infra/jenkins/job-configs/kubernetes-jenkins-pull instead.)

This message may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

Otherwise, if this message is too spammy, please complain to ixdy.

I agree replenish quota could be made more efficient.

I am interested in exploring a means to track what resources on a quota are dirty so that when a quota is synced, we only recalculate the dirty set rather than all resources on the quota. There was a recent PR to reduce the number of queries on sync (see #29134), but I agree there is more than can and should be done.

I have a concern with this PR that the underlying expectation is that the computation of usage for the object is cheap in UsageFunc. It is true for existing Kubernetes resources tracked under quota, but its not true for some complex resources in OpenShift that shares this same core framework. There are some scenarios where calculating the usage of an object may require fetching additional information to some kind of roll-up.

Since this PR ignores the usage values, and just wants to know the set of potential keys, if we had a way of telling the UsageFunc to not worry about calculating values I would have fewer concerns.

As for the scenario this PR helps, did you have one in mind where the set of resources tracked by an Evaluator differed from the runtime.Object in a material way that showed problems? Best I can think of is a quota that tracked services.loadbalancers would now be able to ignore changes to services that are not of that type. That seems useful, so maybe a unit test for that scenario would be good to ensure replenishQuota works as you desire?

I also agree that ReplenishmentFunc was vague on the object, and agree that it should have been the oldObject so can you update the typedef to take that into account?

/cc @deads2k - fyi

Hi @derekwaynecarr , first thanks for your guidance above, i have thought about carefully, the UsageFunc
func seems not useful, it can not solve the problem of efficiency in essence. As the sync process focus on resource quota using queue, not for special resource set, so only change this architecture could make replenish quota more efficient, but this need a well thought out plan. Now in this pr, i only do a optimize for replenish quota of service, for example, if if a service goes from node port -> node port, although it was quota tracked, wo still need not do replenish quota , only a servcie type has changed, wo would do replenish quota.

hi @derekwaynecarr , did you checked this pr?

@derekwaynecarr

ok to test

thanks for the update and test case.

GCE e2e build/test passed for commit 3973856.

• Test Results
• Build Log
• Test Artifacts
• Internal Jenkins Results

Automatic merge from submit-queue