diff --git a/README.md b/README.md index f9da5677..6e2617f7 100644 --- a/README.md +++ b/README.md @@ -640,12 +640,13 @@ The command resolves the operator namespace from an explicit `--user-config`, `./cluster-config.yaml`, or an explicit `--config-dir`, then the `nvidia-network-operator` default. Custom installation namespaces must be supplied by flag or config; in-cluster objects never select this destructive -target. The command deletes every namespaced custom resource in that namespace, -removes the known cluster-scoped Network Operator CRs, waits for their -finalizers, and uninstalls the `network-operator` Helm release last. The -namespace, CRDs, unrelated Secrets, files on disk, and custom resources outside -the namespace are preserved; Helm metadata and chart-managed resources are -removed with the release. +target. The command discovers every namespaced custom resource in that +namespace and the known cluster-scoped Network Operator CRs, sends deletion +requests to the complete set before monitoring any CR for finalizer completion, +and re-sweeps both scopes before uninstalling the `network-operator` Helm +release last. The namespace, CRDs, unrelated Secrets, files on disk, and custom +resources outside the namespace are preserved; Helm metadata and chart-managed +resources are removed with the release. Pass `--keep-helm-chart` to delete the custom resources while leaving the Helm release installed. Cleanup is destructive and confirms the resolved target in diff --git a/docs/reference/cli.md b/docs/reference/cli.md index f0b3eec1..d3c7fca7 100644 --- a/docs/reference/cli.md +++ b/docs/reference/cli.md @@ -117,9 +117,10 @@ Discovery also accepts the profile and Spectrum-X flags below. Explicit flags ov ## Clean Flags -`l8k clean` deletes every namespaced custom-resource instance in the resolved -Network Operator namespace, then deletes the known cluster-scoped Network -Operator CRs. It waits for their finalizers before uninstalling the +`l8k clean` discovers every namespaced custom-resource instance in the resolved +Network Operator namespace and the known cluster-scoped Network Operator CRs. +It sends deletion requests to the complete set before monitoring any CR for +finalizer completion, then re-sweeps both scopes before uninstalling the `network-operator` Helm release. It preserves the namespace, CRDs, unrelated Secrets, generated files, and resources outside the namespace. Helm release metadata and chart-managed resources are removed with the release. diff --git a/docs/user/cleanup.md b/docs/user/cleanup.md index 7b03a2db..276a2a35 100644 --- a/docs/user/cleanup.md +++ b/docs/user/cleanup.md @@ -47,15 +47,18 @@ l8k clean --kubeconfig ~/.kube/config \ Cleanup performs these operations in order: -1. Discover every namespaced CRD served by the cluster and delete all of its - custom-resource instances in the resolved operator namespace. -2. Delete all instances of the Network Operator's known cluster-scoped CR +1. Discover every namespaced custom-resource instance in the resolved operator + namespace and every instance of the Network Operator's known cluster-scoped kinds: `HostDeviceNetwork`, `IPoIBNetwork`, `MacvlanNetwork`, - `NicNodePolicy`, and `NicClusterPolicy`. `NicClusterPolicy` is deleted last. -3. Re-scan the operator namespace and the known cluster-scoped kinds, removing - any custom resources created or exposed during policy teardown. -4. Wait until all selected custom resources are gone, including finalizer - processing. + `NicNodePolicy`, and `NicClusterPolicy`. +2. Send a background deletion request to every discovered custom resource + before waiting on any one of them. The `NicClusterPolicy` request is sent + after the other known cluster-scoped kinds. +3. Monitor the complete deletion set until every custom resource is gone, + including finalizer processing. Sending all requests first prevents one + CR's finalizer from blocking on another CR that has not entered deletion. +4. Re-scan both scopes and repeat the delete-all-then-wait sequence for any + custom resources created or exposed during policy teardown. 5. Uninstall the `network-operator` Helm release and wait for Helm-managed resources to be removed. diff --git a/pkg/networkoperatorplugin/clean.go b/pkg/networkoperatorplugin/clean.go index 6bd3d3ab..46fb68ad 100644 --- a/pkg/networkoperatorplugin/clean.go +++ b/pkg/networkoperatorplugin/clean.go @@ -37,8 +37,8 @@ var clusterScopedCleanKinds = []schema.GroupVersionKind{ {Group: "mellanox.com", Version: "v1alpha1", Kind: "IPoIBNetwork"}, {Group: "mellanox.com", Version: "v1alpha1", Kind: "MacvlanNetwork"}, {Group: "mellanox.com", Version: "v1alpha1", Kind: "NicNodePolicy"}, - // NicClusterPolicy is last so its controller remains available while the - // dependent custom resources above process deletion and finalizers. + // Signal NicClusterPolicy deletion last, after every dependent custom + // resource has already received its deletion request. {Group: "mellanox.com", Version: "v1alpha1", Kind: "NicClusterPolicy"}, } @@ -150,18 +150,18 @@ func deleteNetworkOperatorCustomResources( return 0, err } - deleted := 0 - for _, refs := range [][]cleanObjectRef{namespaced, clusterScoped} { - count, err := deleteAndWait(ctx, kubeClient, refs, pollInterval, uiOutput) - deleted += count - if err != nil { - return deleted, err - } + // Signal deletion for both scopes before waiting on any one resource. A + // finalizer on one CR may depend on another CR entering deletion, so + // interleaving scope-by-scope deletion and waiting can deadlock cleanup. + refs := append(namespaced, clusterScoped...) + deleted, err := deleteAllAndWait(ctx, kubeClient, refs, pollInterval, uiOutput) + if err != nil { + return deleted, err } // Controllers can remove or briefly recreate service CRs while the root - // policies disappear. Sweep both scopes until they are empty after deleting - // NicClusterPolicy so no operator-generated CR is left behind. + // policies disappear. Sweep both scopes until they are empty, preserving the + // same delete-all-then-wait ordering for every pass. for { remainingNamespaced, err := listNamespacedCustomResources(ctx, kubeClient, namespace) if err != nil { @@ -171,15 +171,14 @@ func deleteNetworkOperatorCustomResources( if err != nil { return deleted, err } - if len(remainingNamespaced) == 0 && len(remainingClusterScoped) == 0 { + remaining := append(remainingNamespaced, remainingClusterScoped...) + if len(remaining) == 0 { return deleted, nil } - for _, refs := range [][]cleanObjectRef{remainingNamespaced, remainingClusterScoped} { - count, err := deleteAndWait(ctx, kubeClient, refs, pollInterval, uiOutput) - deleted += count - if err != nil { - return deleted, err - } + count, err := deleteAllAndWait(ctx, kubeClient, remaining, pollInterval, uiOutput) + deleted += count + if err != nil { + return deleted, err } } } @@ -276,7 +275,10 @@ func servedStorageVersion(crd *apiextv1.CustomResourceDefinition) string { return "" } -func deleteAndWait( +// deleteAllAndWait sends a background deletion request to every ref before it +// starts polling. Keep these phases separate: finalizers can depend on another +// custom resource entering deletion. +func deleteAllAndWait( ctx context.Context, kubeClient client.Client, refs []cleanObjectRef, diff --git a/pkg/networkoperatorplugin/clean_test.go b/pkg/networkoperatorplugin/clean_test.go index a629a968..7e645b5e 100644 --- a/pkg/networkoperatorplugin/clean_test.go +++ b/pkg/networkoperatorplugin/clean_test.go @@ -136,6 +136,43 @@ func TestDeleteNetworkOperatorCustomResources(t *testing.T) { context.Background(), client.ObjectKey{Name: crd.Name}, actualCRD)) } +func TestDeleteNetworkOperatorCustomResourcesSignalsAllBeforeWaiting(t *testing.T) { + namespaced := testCleanObject(testNamespacedGVK, "operator-system", "namespaced") + clusterScoped := testCleanObject(clusterScopedCleanKinds[3], "", "cluster-scoped") + baseClient := newCleanTestClient(t, testNamespacedCRD(), namespaced, clusterScoped) + targets := []*unstructured.Unstructured{namespaced, clusterScoped} + var deletionSignals []string + kubeClient := interceptor.NewClient(baseClient, interceptor.Funcs{ + Delete: func( + ctx context.Context, + underlying client.WithWatch, + obj client.Object, + opts ...client.DeleteOption, + ) error { + deletionSignals = append(deletionSignals, obj.GetName()) + if len(deletionSignals) < len(targets) { + // Simulate a finalizer that keeps the first CR until the other CR + // has also entered deletion. + return nil + } + for _, target := range targets { + if err := underlying.Delete(ctx, target.DeepCopy(), opts...); err != nil { + return err + } + } + return nil + }, + }) + + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() + deleted, err := deleteNetworkOperatorCustomResources( + ctx, kubeClient, "operator-system", time.Millisecond, ui.NewSilent()) + require.NoError(t, err) + assert.Equal(t, 2, deleted) + assert.Equal(t, []string{"namespaced", "cluster-scoped"}, deletionSignals) +} + func TestDeleteNetworkOperatorCustomResourcesSweepsRecreatedClusterResource(t *testing.T) { original := testCleanObject(clusterScopedCleanKinds[3], "", "original-node-policy") nicClusterPolicy := testCleanObject(clusterScopedCleanKinds[4], "", "nic-cluster-policy") diff --git a/skills/k8s-launch-kit-clean/SKILL.md b/skills/k8s-launch-kit-clean/SKILL.md index 5ee952b6..f67902b0 100644 --- a/skills/k8s-launch-kit-clean/SKILL.md +++ b/skills/k8s-launch-kit-clean/SKILL.md @@ -69,6 +69,9 @@ The command: namespace. - Deletes all `HostDeviceNetwork`, `IPoIBNetwork`, `MacvlanNetwork`, `NicNodePolicy`, and `NicClusterPolicy` instances cluster-wide. +- Sends deletion requests to the complete namespaced and cluster-scoped set + before monitoring any CR for finalizer completion, then uses the same + delete-all-then-wait ordering for re-sweeps. - Keeps controllers installed until CR deletion and finalizers complete. - Uninstalls the `network-operator` Helm release last unless `--keep-helm-chart` is set.