Skip to content

Commit

Permalink
WIP: DEMO: makr dangling unwanted MachineConfig
Browse files Browse the repository at this point in the history
is this better than the current approach?

Signed-off-by: Francesco Romani <[email protected]>
  • Loading branch information
ffromani committed Oct 23, 2024
1 parent fce9b77 commit 50b481d
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 5 deletions.
16 changes: 13 additions & 3 deletions controllers/numaresourcesoperator_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -462,12 +462,22 @@ func (r *NUMAResourcesOperatorReconciler) syncNUMAResourcesOperatorResources(ctx
klog.V(4).InfoS("RTESync start", "trees", len(trees))
defer klog.V(4).Info("RTESync stop")

wantsCustomPolicy := annotations.IsCustomPolicyEnabled(instance.Annotations)

errorList := dangling.DeleteUnusedDaemonSets(r.Client, ctx, instance, trees)
if len(errorList) > 0 {
klog.ErrorS(fmt.Errorf("failed to delete unused daemonsets"), "errors", errorList)
}

errorList = dangling.DeleteUnusedMachineConfigs(r.Client, ctx, instance, trees)
errorList = dangling.DeleteUnusedMachineConfigs(r.Client, ctx, instance, trees, func(expected bool, _ client.Object) (bool, string) {
if !expected {
return false, "unexpected object"
}
if !wantsCustomPolicy {
return false, "removing custom SELinuxPolicy, using builtin"
}
return true, ""
})
if len(errorList) > 0 {
klog.ErrorS(fmt.Errorf("failed to delete unused machineconfigs"), "errors", errorList)
}
Expand Down Expand Up @@ -498,7 +508,7 @@ func (r *NUMAResourcesOperatorReconciler) syncNUMAResourcesOperatorResources(ctx
}
rteupdate.DaemonSetHashAnnotation(r.RTEManifests.DaemonSet, cmHash)
}
rteupdate.SecurityContextConstraint(r.RTEManifests.SecurityContextConstraint, annotations.IsCustomPolicyEnabled(instance.Annotations))
rteupdate.SecurityContextConstraint(r.RTEManifests.SecurityContextConstraint, wantsCustomPolicy)

processor := func(mcpName string, gdm *rtestate.GeneratedDesiredManifest) error {
err := daemonsetUpdater(mcpName, gdm)
Expand All @@ -510,7 +520,7 @@ func (r *NUMAResourcesOperatorReconciler) syncNUMAResourcesOperatorResources(ctx
}

existing := rtestate.FromClient(ctx, r.Client, r.Platform, r.RTEManifests, instance, trees, r.Namespace)
for _, objState := range existing.State(r.RTEManifests, processor, annotations.IsCustomPolicyEnabled(instance.Annotations)) {
for _, objState := range existing.State(r.RTEManifests, processor, wantsCustomPolicy) {
if objState.Error != nil {
// We are likely in the bootstrap scenario. In this case, which is expected once, everything is fine.
// If it happens past bootstrap, still carry on. We know what to do, and we do want to enforce the desired state.
Expand Down
21 changes: 19 additions & 2 deletions internal/dangling/dangling.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ import (
"github.com/openshift-kni/numaresources-operator/pkg/objectnames"
)

type KeepDanglingObjectFunc func(expected bool, obj client.Object) (bool, string)

func DeleteUnusedDaemonSets(cli client.Client, ctx context.Context, instance *nropv1.NUMAResourcesOperator, trees []nodegroupv1.Tree) []error {
klog.V(3).Info("Delete dangling Daemonsets start")
defer klog.V(3).Info("Delete dangling Daemonsets end")
Expand Down Expand Up @@ -78,7 +80,7 @@ func DeleteUnusedDaemonSets(cli client.Client, ctx context.Context, instance *nr
return errors
}

func DeleteUnusedMachineConfigs(cli client.Client, ctx context.Context, instance *nropv1.NUMAResourcesOperator, trees []nodegroupv1.Tree) []error {
func DeleteUnusedMachineConfigs(cli client.Client, ctx context.Context, instance *nropv1.NUMAResourcesOperator, trees []nodegroupv1.Tree, filterObjects ...KeepDanglingObjectFunc) []error {
klog.V(3).Info("Delete dangling Machineconfigs start")
defer klog.V(3).Info("Delete dangling Machineconfigs end")
var errors []error
Expand All @@ -100,7 +102,7 @@ func DeleteUnusedMachineConfigs(cli client.Client, ctx context.Context, instance
if !isOwnedBy(mc.GetObjectMeta(), instance) {
continue
}
if expectedMachineConfigNames.Has(mc.Name) {
if keepMachineConfig(expectedMachineConfigNames.Has(mc.Name), mc, filterObjects...) {
continue
}
if err := cli.Delete(ctx, &mc); err != nil {
Expand All @@ -117,6 +119,21 @@ func DeleteUnusedMachineConfigs(cli client.Client, ctx context.Context, instance
return errors
}

func keepMachineConfig(expected bool, mc machineconfigv1.MachineConfig, filterObjects ...KeepDanglingObjectFunc) bool {
if len(filterObjects) == 0 {
return expected
}
for _, filterObject := range filterObjects {
keep, reason := filterObject(expected, &mc)
if keep {
klog.V(5).InfoS("keeping dangling object", "MachineConfig", mc.Name, "reason", reason)
return true
}
}
klog.V(2).InfoS("deleting dangling object", "MachineConfig", mc.Name)
return false
}

func isOwnedBy(element metav1.Object, owner metav1.Object) bool {
for _, ref := range element.GetOwnerReferences() {
if ref.UID == owner.GetUID() {
Expand Down

0 comments on commit 50b481d

Please sign in to comment.