Create federated-resource-quota-enforcement-controller #6367

mszacillo · 2025-05-13T04:56:11Z

What type of PR is this?

Adds the federated-resource-quota-enforcement controller.

Reconciles in response to one of the following events:

Creation or update of a FederatedResourceQuota
Deletion of a ResourceBinding

At the moment I did not include the third planned reconcile: Periodically recalculate FRQ status, since I was worried about using RequeueAfter and hammering the controller with too many reconcile events. Perhaps we can add this as a follow-up iteration task.

/kind feature

What this PR does / why we need it:

Which issue(s) this PR fixes:
Part of #6350.

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

`karmada-controller-manager`: Added `federated-resource-quota-enforcement-controller` as part of ResourceQuotaEnforcement feature.

codecov-commenter · 2025-05-13T05:11:10Z

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 20.97902% with 113 lines in your changes missing coverage. Please review.

Please upload report for BASE (master@96d4577). Learn more about missing BASE report.
Report is 14 commits behind head on master.

Files with missing lines	Patch %	Lines
...federated_resource_quota_enforcement_controller.go	24.00%	94 Missing and 1 partial ⚠️
cmd/controller-manager/app/controllermanager.go	0.00%	13 Missing ⚠️
pkg/util/helper/binding.go	0.00%	5 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff            @@
##             master    #6367   +/-   ##
=========================================
  Coverage          ?   49.21%           
=========================================
  Files             ?      684           
  Lines             ?    55325           
  Branches          ?        0           
=========================================
  Hits              ?    27228           
  Misses            ?    26324           
  Partials          ?     1773

Flag	Coverage Δ
unittests	`49.21% <20.97%> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

pkg/controllers/federatedresourcequota/federated_resource_quota_enforcement_controller.go

zhzhuang-zju · 2025-05-13T08:59:32Z

pkg/controllers/federatedresourcequota/federated_resource_quota_enforcement_controller.go

+			return []reconcile.Request{{
+				NamespacedName: types.NamespacedName{
+					Namespace: obj.GetNamespace(),
+					Name:      obj.GetName(),
+				},


When a deletion event of ResourceBinding is detected, it's possible to return a QuotaList instead of the ResourceBinding itself, allowing the reconcile logic to focus only on FederatedResourceQuota.

Here is an example:

karmada/pkg/controllers/federatedresourcequota/federated_resource_quota_sync_controller.go

Lines 104 to 131 in e2f0e6e

fn := handler.MapFunc(

func(ctx context.Context, obj client.Object) []reconcile.Request {

var requests []reconcile.Request

var clusterName string

switch obj.(type) {

case *clusterv1alpha1.Cluster:

clusterName = obj.GetName()

default:

return nil

}

FederatedResourceQuotaList := &policyv1alpha1.FederatedResourceQuotaList{}

if err := c.Client.List(ctx, FederatedResourceQuotaList); err != nil {

klog.Errorf("Failed to list FederatedResourceQuota, error: %v", err)

}

for _, federatedResourceQuota := range FederatedResourceQuotaList.Items {

hard := extractClusterHardResourceList(federatedResourceQuota.Spec, clusterName)

if hard == nil {

continue

}

requests = append(requests, reconcile.Request{

NamespacedName: types.NamespacedName{

Namespace: federatedResourceQuota.Namespace,

Name: federatedResourceQuota.Name,

},

})

}

Oh nice! I'll add this in. Thanks

Thought about this more - I think we do want to make a distinction between ResourceBinding and FederatedResourceQuota reconciles.

When reconciling the quota, we will only update the Overall field of the quota. When reconciling a ResourceBinding deletion, we will update both Overall and OverallUsed. That way the validation webhook is entirely responsible for updating the used resources, and there is a separation of concerns.

When reconciling the quota, we will only update the Overall field of the quota. When reconciling a ResourceBinding deletion, we will update both Overall and OverallUsed.

It's not entirely the case; when the quota's hard limits change, it will also be necessary to update the OverallUsed.

Good thinking! But not the pattern of how the controller works.

The reconciliation logic is fixed and only pushes the target object(FedeeratedResourceQuota) to the desired state. For this controller, it will list all ResourceBindings calculate the in-use quota, and then update the OverallUsed.

Changes to ResourceBinding are one of the factors that trigger the reconciliation of FederatedResourceQuoa.

Could there be cases where the validation webhook and the controller both update the overallUsed at the same time? I suppose the only time this would happen is if a few RBs are simultaneously created and deleted, or if RB are created around the same time the FRQ is updated, but could this cause conflicts?

Nevermind - if we trigger another reconciliation in response to failed updates, then these type of conflicts should not happen.

Signed-off-by: mszacillo <[email protected]>

RainbowMango

/assign

Copilot

Pull Request Overview

This PR introduces the new federated-resource-quota-enforcement controller to reconcile FederatedResourceQuota resources and ResourceBinding deletions. Key updates include:

Adding a new helper function in binding.go to list ResourceBindings by namespace.
Implementing the QuotaEnforcementController in its own package along with corresponding tests for resource usage calculations.
Registering the new controller in the controller manager.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
pkg/util/helper/binding.go	Added GetResourceBindingsByNamespace to list ResourceBindings filtered by namespace.
pkg/controllers/federatedresourcequota/federated_resource_quota_enforcement_controller_test.go	Added tests for calculating resource usage from ResourceBinding instances.
pkg/controllers/federatedresourcequota/federated_resource_quota_enforcement_controller.go	Introduced the controller logic including reconciliation and status update methods.
cmd/controller-manager/app/controllermanager.go	Registered the new federated-resource-quota-enforcement controller.

pkg/controllers/federatedresourcequota/federated_resource_quota_enforcement_controller.go

zhzhuang-zju

Thanks for your great work! @mszacillo
My opinion is for reference only; open to discussion 😄

pkg/controllers/federatedresourcequota/federated_resource_quota_enforcement_controller.go

RainbowMango

Looks great!

Signed-off-by: mszacillo <[email protected]>

RainbowMango

/lgtm

@zhzhuang-zju Do you have any further comments?

zhzhuang-zju · 2025-05-16T01:49:58Z

/lgtm

@zhzhuang-zju Do you have any further comments?

/lgtm

RainbowMango

/approve

karmada-bot · 2025-05-16T02:13:09Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: RainbowMango

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [RainbowMango]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

karmada-bot added the kind/feature Categorizes issue or PR as related to a new feature. label May 13, 2025

karmada-bot requested review from chaunceyjiang, pigletfly and whitewindmills May 13, 2025 04:56

karmada-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label May 13, 2025

mszacillo mentioned this pull request May 13, 2025

[Feature] FederatedResourceQuota Enhancement #6350

Open

21 tasks

zhzhuang-zju reviewed May 13, 2025

View reviewed changes

Create federated-resource-quota-enforcement-controller

20c2a18

Signed-off-by: mszacillo <[email protected]>

mszacillo force-pushed the frq-controller branch from 9367868 to 20c2a18 Compare May 13, 2025 14:32

RainbowMango reviewed May 14, 2025

View reviewed changes

karmada-bot assigned RainbowMango May 14, 2025

RainbowMango requested a review from Copilot May 14, 2025 07:33

Copilot AI reviewed May 14, 2025

View reviewed changes

pkg/controllers/federatedresourcequota/federated_resource_quota_enforcement_controller.go Outdated Show resolved Hide resolved

RainbowMango reviewed May 14, 2025

View reviewed changes

pkg/controllers/federatedresourcequota/federated_resource_quota_enforcement_controller.go Outdated Show resolved Hide resolved

zhzhuang-zju reviewed May 14, 2025

View reviewed changes

mszacillo force-pushed the frq-controller branch 4 times, most recently from 83133c1 to 56e5ef1 Compare May 14, 2025 20:36

zhzhuang-zju reviewed May 15, 2025

View reviewed changes

RainbowMango reviewed May 15, 2025

View reviewed changes

zhzhuang-zju mentioned this pull request May 15, 2025

Create RB ValidatingWebhook for FederatedResourceQuota check #6377

Merged

mszacillo force-pushed the frq-controller branch 2 times, most recently from 6f053aa to 0d42f6e Compare May 15, 2025 15:44

controller only reconcile quota, adding new events, fixing lint issues

713b6b7

Signed-off-by: mszacillo <[email protected]>

mszacillo force-pushed the frq-controller branch from 0d42f6e to 713b6b7 Compare May 15, 2025 16:53

RainbowMango reviewed May 16, 2025

View reviewed changes

karmada-bot added the lgtm Indicates that a PR is ready to be merged. label May 16, 2025

RainbowMango added this to the v1.14 milestone May 16, 2025

karmada-bot assigned zhzhuang-zju May 16, 2025

RainbowMango approved these changes May 16, 2025

View reviewed changes

karmada-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 16, 2025

karmada-bot merged commit cb26864 into karmada-io:master May 16, 2025
24 checks passed

	fn := handler.MapFunc(
	func(ctx context.Context, obj client.Object) []reconcile.Request {
	var requests []reconcile.Request
	var clusterName string
	switch obj.(type) {
	case *clusterv1alpha1.Cluster:
	clusterName = obj.GetName()
	default:
	return nil
	}

	FederatedResourceQuotaList := &policyv1alpha1.FederatedResourceQuotaList{}
	if err := c.Client.List(ctx, FederatedResourceQuotaList); err != nil {
	klog.Errorf("Failed to list FederatedResourceQuota, error: %v", err)
	}

	for _, federatedResourceQuota := range FederatedResourceQuotaList.Items {
	hard := extractClusterHardResourceList(federatedResourceQuota.Spec, clusterName)
	if hard == nil {
	continue
	}
	requests = append(requests, reconcile.Request{
	NamespacedName: types.NamespacedName{
	Namespace: federatedResourceQuota.Namespace,
	Name: federatedResourceQuota.Name,
	},
	})
	}

Create federated-resource-quota-enforcement-controller #6367

Create federated-resource-quota-enforcement-controller #6367

Uh oh!

Conversation

mszacillo commented May 13, 2025 • edited by RainbowMango Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-commenter commented May 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

zhzhuang-zju May 13, 2025

Choose a reason for hiding this comment

Uh oh!

mszacillo May 13, 2025

Choose a reason for hiding this comment

Uh oh!

mszacillo May 14, 2025

Choose a reason for hiding this comment

Uh oh!

zhzhuang-zju May 14, 2025

Choose a reason for hiding this comment

Uh oh!

RainbowMango May 14, 2025

Choose a reason for hiding this comment

Uh oh!

mszacillo May 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RainbowMango left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

zhzhuang-zju left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

RainbowMango left a comment

Choose a reason for hiding this comment

Uh oh!

RainbowMango left a comment

Choose a reason for hiding this comment

Uh oh!

zhzhuang-zju commented May 16, 2025

Uh oh!

RainbowMango left a comment

Choose a reason for hiding this comment

Uh oh!

karmada-bot commented May 16, 2025

Uh oh!

Uh oh!

Uh oh!

mszacillo commented May 13, 2025 •

edited by RainbowMango

Loading

codecov-commenter commented May 13, 2025 •

edited

Loading

mszacillo May 14, 2025 •

edited

Loading