Implement ClusterPolicy deletion and resource watches#2212
Open
retr0-kernel wants to merge 2 commits intoNVIDIA:mainfrom
Open
Implement ClusterPolicy deletion and resource watches#2212retr0-kernel wants to merge 2 commits intoNVIDIA:mainfrom
retr0-kernel wants to merge 2 commits intoNVIDIA:mainfrom
Conversation
- Handle deletion of the active singleton ClusterPolicy and cycle to the next available one (previously a TODO with no implementation). - Replace the placeholder secondary-resource watch (TODO comment) with proper watches for all resource types owned by ClusterPolicy: DaemonSet, Deployment, ConfigMap, Service, ServiceAccount. - Use RequeueAfter (5 s) instead of the deprecated Requeue field throughout the new code path. Signed-off-by: Krish Srivastava <krish22092003@gmail.com>
controllers: implement ClusterPolicy TODOs
retr0-kernel
requested review from
cdesiniotis,
karthikvetrivel,
rahulait,
rajathagasthya,
shivamerla and
tariq1890
as code owners
Author
|
Hi Reviewers, |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
controllers: implement ClusterPolicy TODOs
TODO 1 -> Handle deletion of the active singleton ClusterPolicy
What changed and why:
ClusterPolicyhas
DeletionTimestamp != nil(Kubernetes sets this when the object is beingdeleted but still has finalizers).
clusterPolicyCtrlto a zero value so the internal stateis clean and a new singleton election can happen on the next reconcile.
RequeueAfter: 5s(non-deprecated; replaces the originally suggestedRequeue: true) so the controller re-runs after the object is fully removedand can pick up the next available
ClusterPolicyfrom the cluster.a separate
ifblock immediately after the deletion guard.TODO 2 -> Replace placeholder secondary-resource watches
What changed and why:
TODO(user): Modify this to be the types you create)has been removed.
Deployment,ConfigMap,Service, andServiceAccountall four are managed by
ClusterPolicyControllerviaobject_controls.go(e.g.
Deployment(),Service(),ConfigMaps(),ServiceAccount()).TypedEnqueueRequestForOwnerwithOnlyControllerOwner()soonly resources whose
ownerReference.controller=truepoints to aClusterPolicytrigger a reconcile matching how ownership is set viacontrollerutil.SetControllerReferenceinobject_controls.go.manually deleting a Service or ConfigMap) would not trigger reconciliation and
the operator would not self-heal.
Refactor -> Extract
buildNodeMapFnandbuildNodePredicateThe inline closures inside
addWatchNewGPUNodehave been extracted into twonamed package-level helpers:
buildNodeMapFn(r)-> returns the map function that enqueues reconcile requestsfor all
ClusterPolicyobjects when a relevant Node event fires.buildNodePredicate()-> returns theTypedFuncs[*corev1.Node]predicate(Create / Update / Delete filters).
addWatchNewGPUNodenow calls these helpers and wrapsUpdateFuncto add thelogger call, keeping the predicate itself logger-free and independently testable.
This is a pure refactor with no behaviour change it exists to make the logic
unit-testable without a real
ctrl.Manager.Removed TODO comment on
Reconciledoc commentThe scaffolding comment
TODO(user): Modify the Reconcile function to compare the state specified by...has been replaced with an accurate description of whatthe function actually does, since the implementation is complete.
Checklist
make lint)make validate-generated-assets)make validate-modules)Testing
Build verified:
Existing tests unaffected: