Skip to content

fix(operator): rename skyhook -> nodewright in fmt.Errorf text #414

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mohityadav8 wants to merge 6 commits into
base: main
Choose a base branch
from
Open

fix(operator): rename skyhook -> nodewright in fmt.Errorf text #414

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f88c407
fix(operator): rename skyhook -> nodewright in fmt.Errorf text
mohityadav8 Aug 6, 2026
8f75cf6
Merge branch 'main' into fix/403-errorf-text-rename
lockwobr Aug 6, 2026
f49c695
Merge branch 'main' into fix/403-errorf-text-rename
mohityadav8 Aug 6, 2026
3cf763b
Merge branch 'main' into fix/403-errorf-text-rename
ayuskauskas Aug 11, 2026
0f3e8fc
Merge branch 'main' into fix/403-errorf-text-rename
lockwobr Aug 13, 2026
679d571
Merge branch 'main' into fix/403-errorf-text-rename
lockwobr Aug 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions operator/internal/controller/cluster_state_v2.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion operator/internal/controller/event_handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
105 changes: 35 additions & 70 deletions operator/internal/controller/skyhook_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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.
Expand All @@ -647,15 +647,15 @@ 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())

// set version
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
Expand Down Expand Up @@ -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
Expand All @@ -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 <nodewright>/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 <legacy>/skyhook-node-meta, package ConfigMaps carry <legacy>/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{}

@ayuskauskas ayuskauskas Aug 11, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This block is a revert of #380, not a rename. On main this sweeps legacy ConfigMaps by both <legacy>/skyhook-node-meta and <legacy>/name (two Lists + dedupe, because MatchingLabels ANDs), and calls reparentToNodeWright on each. Here only skyhook-node-meta is listed, so package ConfigMaps labeled <legacy>/name never converge and UpsertConfigmaps wedges on AlreadyExists; legacy ownerReferences also survive, making these ConfigMaps cascade-delete bait when the old Skyhook CR is removed.

The changed = true after a successful r.Update(ctx, cm) is also gone, so workloadsChanged returns false even when ConfigMaps were relabeled.

It came in via the f49c695a merge of main@84df6448 — restoring main's version of this function and re-applying just the fmt.Errorf renames should resolve it.

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)
}
}
}
Expand Down Expand Up @@ -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))
}
}
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions operator/internal/dal/dal.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand Down
Loading