A new controller adds/removes finalizer to VAC for protection

[carlory]

What type of PR is this?

/kind feature

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #119298

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Added a new controller, volumeattributesclass-protection-controller, into the kube-controller-manager.
The new controller manages a protective finalizer on VolumeAttributesClass objects.

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

- [KEP]: https://github.com/kubernetes/enhancements/issues/3751

[k8s-ci-robot]

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

[sftim]

Changelog suggestion (early feedback)

-kube-controller-manager adds a new controller named volumeattributesclass-protection-controller which adds/removes finalizer to VAC for protection.
+Added a new controller, volumeattributesclass-protection-controller, into the kube-controller-manager.
+The new controller manages a protective finalizer on VolumeAttributesClass objects.

[carlory]

/cc @sunnylovestiramisu @msau42

[carlory]

/cc @mikedanese

[liggitt]

is this going to use update or patch to remove the finalizer?

[carlory]

Yep, it follows the same pattern as pvcprotection and pvprotection.

• https://github.com/carlory/kubernetes/blob/kep-3751-finalizer/pkg/controller/volume/pvprotection/pv_protection_controller.go#L176
• https://github.com/carlory/kubernetes/blob/kep-3751-finalizer/pkg/controller/volume/pvcprotection/pvc_protection_controller.go#L206

/cc @msau42

Should I use a patch instead of an update in this PR?

[carlory]

/hold cancel

we discussed this at sig storage triage today. “Patch” is usually recommended, but “update” is fine here

https://docs.google.com/document/d/1n-dXXvCbHsPfO1yrKwT1qoC80KhsxHYKbRdChdzqeXY/edit?tab=t.0#heading=h.lc8t1s7x74pi

/cc @liggitt

[carlory]

/test pull-kubernetes-e2e-storage-kind-alpha-beta-features

[carlory]

/test pull-kubernetes-node-e2e-containerd

[carlory]

need to bump e2e hostpath manifests to fix csi bug
/hold

/test pull-kubernetes-e2e-storage-kind-alpha-beta-features

[carlory]

/test pull-kubernetes-e2e-storage-kind-alpha-beta-features

[carlory]

/test pull-kubernetes-e2e-storage-kind-alpha-beta-features

[carlory]

/cc @liggitt @deads2k

[msau42]

Does the controller only start when the feature gate is enabled? Because the beta API is optional, I believe if you start the informer and the API is not installed, then there will be lots of error messages.

[carlory]

Does the controller only start when the feature gate is enabled?

@msau42 Yes, it is.

// StartController starts a controller with a specified ControllerContext
// and performs required pre- and post- checks/actions
func StartController(ctx context.Context, controllerCtx ControllerContext, controllerDescriptor *ControllerDescriptor,
	unsecuredMux *mux.PathRecorderMux) (healthz.HealthChecker, error) {
	logger := klog.FromContext(ctx)
	controllerName := controllerDescriptor.Name()

	for _, featureGate := range controllerDescriptor.GetRequiredFeatureGates() {
		if !utilfeature.DefaultFeatureGate.Enabled(featureGate) {
			logger.Info("Controller is disabled by a feature gate", "controller", controllerName, "requiredFeatureGates", controllerDescriptor.GetRequiredFeatureGates())
			return nil, nil
		}
	}

[carlory]

the controller is gated with features.VolumeAttributesClass in its controller descriptor.

requiredFeatureGates: []featuregate.Feature{
    features.VolumeAttributesClass,
},

[msau42]

/lgtm

[k8s-ci-robot]

LGTM label has been added.

Git tree hash: 862a796a3c13e5479a4b5f5b8b9a55ff7daaf025

[carlory]

/assign @liggitt @deads2k

[deads2k]

It looks good. Remember that this will be suffer from cache lag, so it's a "usually correct and helpful" versus "security/correctness requirement we can rely upon".

/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: carlory, deads2k, msau42

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:

• cmd/kube-controller-manager/OWNERS [deads2k]
• pkg/controller/volume/OWNERS [deads2k,msau42]
• pkg/volume/OWNERS [msau42]
• plugin/pkg/auth/authorizer/OWNERS [deads2k]

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