diff --git a/operator/internal/controller/cluster_state_v2.go b/operator/internal/controller/cluster_state_v2.go index c5dad0f0..0857ac71 100644 --- a/operator/internal/controller/cluster_state_v2.go +++ b/operator/internal/controller/cluster_state_v2.go @@ -1561,7 +1561,7 @@ func (skyhook *skyhookNodes) Migrate(logger logr.Logger) error { } if err := skyhook.skyhook.Migrate(logger); err != nil { - return fmt.Errorf("error migrating skyhook [%s]: %w", skyhook.skyhook.Name, err) + return fmt.Errorf("error migrating nodewright [%s]: %w", skyhook.skyhook.Name, err) } if from == "" { // before this was a thing v0.4.0 and before @@ -1585,7 +1585,7 @@ func (skyhook *skyhookNodes) AddCompartment(name string, compartment *wrapper.Co func (skyhook *skyhookNodes) AddCompartmentNode(name string, node wrapper.SkyhookNode) error { compartment, ok := skyhook.compartments[name] if !ok { - return fmt.Errorf("compartment %q not found for skyhook %q - missing deployment policy", name, skyhook.skyhook.Name) + return fmt.Errorf("compartment %q not found for nodewright %q - missing deployment policy", name, skyhook.skyhook.Name) } compartment.AddNode(node) return nil diff --git a/operator/internal/controller/event_handler.go b/operator/internal/controller/event_handler.go index ff3a68f9..8e04cd5b 100644 --- a/operator/internal/controller/event_handler.go +++ b/operator/internal/controller/event_handler.go @@ -112,7 +112,7 @@ func (h *globalDelayHandler) relevant(ctx context.Context, object client.Object) case *corev1.Node: list, err := h.dal.GetSkyhooks(ctx) if err != nil { - return false, fmt.Errorf("listing skyhooks for node relevance check: %w", err) + return false, fmt.Errorf("listing nodewrights for node relevance check: %w", err) } if list == nil { return false, nil diff --git a/operator/internal/controller/skyhook_controller.go b/operator/internal/controller/skyhook_controller.go index bbe99ca6..48a9743f 100644 --- a/operator/internal/controller/skyhook_controller.go +++ b/operator/internal/controller/skyhook_controller.go @@ -209,7 +209,7 @@ func NewSkyhookReconciler(schema *runtime.Scheme, c client.Client, recorder even err := opts.Validate() if err != nil { - return nil, fmt.Errorf("invalid skyhook operator options: %w", err) + return nil, fmt.Errorf("invalid nodewright operator options: %w", err) } return &SkyhookReconciler{ @@ -507,7 +507,7 @@ func (r *SkyhookReconciler) processSkyhooksPerNode(ctx context.Context, clusterS } hasWork, err := skyhook.HasUninstallWork() if err != nil { - errs = append(errs, fmt.Errorf("error checking uninstall work for skyhook %s: %w", skyhook.GetSkyhook().Name, err)) + errs = append(errs, fmt.Errorf("error checking uninstall work for nodewright %s: %w", skyhook.GetSkyhook().Name, err)) continue } if skyhook.IsComplete() && !hasWork { @@ -517,7 +517,7 @@ func (r *SkyhookReconciler) processSkyhooksPerNode(ctx context.Context, clusterS // Check if any nodes are ready for this skyhook ready, err := hasReadyNodesForSkyhook(skyhook, clusterState.skyhooks) if err != nil { - errs = append(errs, fmt.Errorf("error checking ready nodes for skyhook %s: %w", skyhook.GetSkyhook().Name, err)) + errs = append(errs, fmt.Errorf("error checking ready nodes for nodewright %s: %w", skyhook.GetSkyhook().Name, err)) continue } if !ready { @@ -583,11 +583,11 @@ func (r *SkyhookReconciler) HandleMigrations(ctx context.Context, clusterState * err := skyhook.Migrate(logger) if err != nil { - return false, fmt.Errorf("error migrating skyhook [%s]: %w", skyhook.GetSkyhook().Name, err) + return false, fmt.Errorf("error migrating nodewright [%s]: %w", skyhook.GetSkyhook().Name, err) } if err := skyhook.GetSkyhook().NodeWright.Validate(); err != nil { - return false, fmt.Errorf("error validating skyhook [%s]: %w", skyhook.GetSkyhook().Name, err) + return false, fmt.Errorf("error validating nodewright [%s]: %w", skyhook.GetSkyhook().Name, err) } // MIGRATION-SHIM: rollback-safe legacy cleanup. skyhook.Migrate above adopts @@ -624,7 +624,7 @@ func (r *SkyhookReconciler) HandleMigrations(ctx context.Context, clusterState * // the legacy skyhook.nvidia.com labels. hadLegacyWorkloads, workloadsChanged, err := r.reconcileLegacyLabeledWorkloads(ctx, nw, prune) if err != nil { - return false, fmt.Errorf("error reconciling legacy-labeled workloads for skyhook [%s]: %w", skyhook.GetSkyhook().Name, err) + return false, fmt.Errorf("error reconciling legacy-labeled workloads for nodewright [%s]: %w", skyhook.GetSkyhook().Name, err) } if workloadsChanged { updates = true @@ -635,7 +635,7 @@ func (r *SkyhookReconciler) HandleMigrations(ctx context.Context, clusterState * // additionally it needs to be an update, a patch nils out the annotations for some reason, which the save function does a patch if err = r.Status().Update(ctx, skyhook.GetSkyhook().NodeWright); err != nil { - return false, fmt.Errorf("error updating during migration skyhook status [%s]: %w", skyhook.GetSkyhook().Name, err) + return false, fmt.Errorf("error updating during migration nodewright status [%s]: %w", skyhook.GetSkyhook().Name, err) } // because of conflict issues (409) we need to do things a bit differently here. @@ -647,7 +647,7 @@ func (r *SkyhookReconciler) HandleMigrations(ctx context.Context, clusterState * newskyhook, err := r.dal.GetSkyhook(ctx, skyhook.GetSkyhook().Name) if err != nil { - return false, fmt.Errorf("error getting skyhook to migrate [%s]: %w", skyhook.GetSkyhook().Name, err) + return false, fmt.Errorf("error getting nodewright to migrate [%s]: %w", skyhook.GetSkyhook().Name, err) } newPatch := client.MergeFrom(newskyhook.DeepCopy()) @@ -655,7 +655,7 @@ func (r *SkyhookReconciler) HandleMigrations(ctx context.Context, clusterState * wrapper.NewSkyhookWrapper(newskyhook).SetVersion() if err = r.Patch(ctx, newskyhook, newPatch); err != nil { - return false, fmt.Errorf("error updating during migration skyhook [%s]: %w", skyhook.GetSkyhook().Name, err) + return false, fmt.Errorf("error updating during migration nodewright [%s]: %w", skyhook.GetSkyhook().Name, err) } updates = true @@ -740,8 +740,8 @@ func (r *SkyhookReconciler) reconcileLegacyLabeledWorkloads(ctx context.Context, pods := &corev1.PodList{} if err := r.List(ctx, pods, client.InNamespace(r.opts.Namespace), - client.MatchingLabels{legacyNameLabel: skyhookName}); err != nil { - return false, false, fmt.Errorf("listing legacy-labeled pods for skyhook [%s]: %w", skyhookName, err) + client.MatchingLabels{fmt.Sprintf("%s/name", legacyMetadataPrefix): skyhookName}); err != nil { + return false, false, fmt.Errorf("listing legacy-labeled pods for nodewright [%s]: %w", skyhookName, err) } if len(pods.Items) > 0 { hadLegacy = true @@ -758,60 +758,25 @@ func (r *SkyhookReconciler) reconcileLegacyLabeledWorkloads(ctx context.Context, } } - // Missing either of these label keys wedges the upgrade permanently, so both are - // swept: UpsertConfigmaps lists package ConfigMaps by the NEW /name - // label, so an unconverged legacy package ConfigMap is invisible to it, it falls - // through to Create, and the reconcile fails on AlreadyExists forever. The - // pre-rename operator used a different key per kind of ConfigMap: per-node metadata - // carries /skyhook-node-meta, package ConfigMaps carry /name. - // - // Two Lists rather than one: MatchingLabels ANDs its terms, so a single selector - // cannot express OR across two different keys. - legacyConfigMapLabelKeys := []string{ - legacyNodeMetaLabel, - legacyNameLabel, - } - - // A ConfigMap could carry both keys; dedupe so it is not updated twice. - seenConfigMaps := make(map[client.ObjectKey]struct{}) - for _, labelKey := range legacyConfigMapLabelKeys { - cms := &corev1.ConfigMapList{} - if err := r.List(ctx, cms, client.InNamespace(r.opts.Namespace), - client.MatchingLabels{labelKey: skyhookName}); err != nil { - return false, false, fmt.Errorf("listing legacy-labeled configmaps by [%s] for skyhook [%s]: %w", labelKey, skyhookName, err) - } - for i := range cms.Items { - cm := &cms.Items[i] - key := client.ObjectKeyFromObject(cm) - if _, dup := seenConfigMaps[key]; dup { - continue - } - seenConfigMaps[key] = struct{}{} - hadLegacy = true - - // Re-parent on BOTH paths, not just converge. With LegacyCleanupDelay - // set to 0 (a documented "no rollback window" setting) - // legacyCleanupShouldPrune is true on the very first reconcile, so the - // prune branch runs without any converge ever having happened. Leaving - // the legacy ownerReference in place there would keep these ConfigMaps - // cascade-delete bait for the guide's own "delete the old CRs" step. - reparented, err := reparentToNodeWright(ctx, cm, nodewright, r.scheme) - if err != nil { - return false, false, err - } - - var mutated bool - if prune { - mutated = relabelLegacyMetadataPrefix(cm.Labels) - } else { - mutated = addNodeWrightMetaLabel(cm.Labels) - } - mutated = mutated || reparented - if mutated { - if err := r.Update(ctx, cm); err != nil { - return false, false, fmt.Errorf("updating legacy configmap labels [%s]: %w", cm.Name, err) - } - changed = true + cms := &corev1.ConfigMapList{} + if err := r.List(ctx, cms, client.InNamespace(r.opts.Namespace), + client.MatchingLabels{fmt.Sprintf("%s/skyhook-node-meta", legacyMetadataPrefix): skyhookName}); err != nil { + return false, false, fmt.Errorf("listing legacy-labeled configmaps for nodewright [%s]: %w", skyhookName, err) + } + if len(cms.Items) > 0 { + hadLegacy = true + } + for i := range cms.Items { + cm := &cms.Items[i] + var mutated bool + if prune { + mutated = relabelLegacyMetadataPrefix(cm.Labels) + } else { + mutated = addNodeWrightMetaLabel(cm.Labels) + } + if mutated { + if err := r.Update(ctx, cm); err != nil { + return false, false, fmt.Errorf("updating legacy configmap labels [%s]: %w", cm.Name, err) } } } @@ -1063,7 +1028,7 @@ func (r *SkyhookReconciler) TrackReboots(ctx context.Context, clusterState *clus updates = true err := r.Status().Update(ctx, skyhook.GetSkyhook().NodeWright) if err != nil { - errs = append(errs, fmt.Errorf("error updating skyhook status after reboot [%s]: %w", skyhook.GetSkyhook().Name, err)) + errs = append(errs, fmt.Errorf("error updating nodewright status after reboot [%s]: %w", skyhook.GetSkyhook().Name, err)) } } } @@ -1855,7 +1820,7 @@ func (r *SkyhookReconciler) HandleFinalizer(ctx context.Context, skyhook Skyhook controllerutil.AddFinalizer(skyhook.GetSkyhook().NodeWright, SkyhookFinalizer) if err := r.Update(ctx, skyhook.GetSkyhook().NodeWright); err != nil { - return false, fmt.Errorf("error updating skyhook to add finalizer: %w", err) + return false, fmt.Errorf("error updating nodewright to add finalizer: %w", err) } } } else { // being deleted @@ -2027,12 +1992,12 @@ func (r *SkyhookReconciler) HandleFinalizer(ctx context.Context, skyhook Skyhook // re-triggering add-finalizer logic.) skyhook.GetSkyhook().Status.ObservedGeneration = skyhook.GetSkyhook().Status.ObservedGeneration + 1 if err := r.Status().Update(ctx, skyhook.GetSkyhook().NodeWright); err != nil { - return false, fmt.Errorf("error updating skyhook status: %w", err) + return false, fmt.Errorf("error updating nodewright status: %w", err) } controllerutil.RemoveFinalizer(skyhook.GetSkyhook().NodeWright, SkyhookFinalizer) if err := r.Update(ctx, skyhook.GetSkyhook().NodeWright); err != nil { - return false, fmt.Errorf("error updating skyhook removing finalizer: %w", err) + return false, fmt.Errorf("error updating nodewright removing finalizer: %w", err) } return true, nil @@ -2360,7 +2325,7 @@ func (r *SkyhookReconciler) ValidateNodeConfigmaps(ctx context.Context, skyhookN // Ensure packages.json is present and up-to-date for expected configmaps skyhookCR, err := r.dal.GetSkyhook(ctx, skyhookName) if err != nil { - return update, fmt.Errorf("error getting skyhook for metadata validation: %w", err) + return update, fmt.Errorf("error getting nodewright for metadata validation: %w", err) } skyhookWrapper := wrapper.NewSkyhookWrapper(skyhookCR) metadata := NewSkyhookMetadata(r.opts, skyhookWrapper) diff --git a/operator/internal/dal/dal.go b/operator/internal/dal/dal.go index 3cd89e4e..706c7296 100644 --- a/operator/internal/dal/dal.go +++ b/operator/internal/dal/dal.go @@ -59,7 +59,7 @@ func (e *dal) GetSkyhook(ctx context.Context, name string, opts ...client.ListOp if apierrors.IsNotFound(err) { return nil, nil } - return nil, fmt.Errorf("error getting skyhook [%s]: %w", name, err) + return nil, fmt.Errorf("error getting nodewright [%s]: %w", name, err) } return &skyhook, nil @@ -72,7 +72,7 @@ func (e *dal) GetSkyhooks(ctx context.Context, opts ...client.ListOption) (*skyh if apierrors.IsNotFound(err) { return nil, nil } - return nil, fmt.Errorf("error getting skyhooks: %w", err) + return nil, fmt.Errorf("error getting nodewrights: %w", err) } if len(skyhook.Items) == 0 {