Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 7 additions & 0 deletions chart/templates/nodewright-crd.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -847,6 +847,13 @@ spec:
image:
description: Image for the package
type: string
interruptPending:
description: |-
InterruptPending is set at apply or upgrade for a package that declares its own interrupt,
and cleared once that interrupt has completed or the package reaches post-interrupt. While set, it is still owed on this
node, so a config change that matches no configInterrupts does not let it finish without it.
Entries written before this field existed read as not pending.
type: boolean
name:
description: Name is the name of the package
type: string
Expand Down
4 changes: 4 additions & 0 deletions chart/templates/skyhook-crd.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -816,6 +816,10 @@ spec:
image:
description: Image for the package
type: string
interruptPending:
description: InterruptPending mirrors the nodewright.nvidia.com
field of the same name.
type: boolean
name:
description: Name is the name of the package
type: string
Expand Down
2 changes: 1 addition & 1 deletion docs/architecture/lifecycle.md
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,7 @@ the spec and it converges again:
|---|---|
| Package `version` increases | Package re-enters at `upgrade` |
| Package `version` decreases | With `uninstall.enabled`, rejected unless the package was explicitly uninstalled first; without it, the new version applies |
| A `configMap` key | Nothing happens until every node is complete or one is erroring at `config`, `interrupt` or `post-interrupt`. Then every node at `config`, `interrupt`, `post-interrupt` or complete re-enters at `config`, and its unfinished Jobs are cleared; `configInterrupts` decides whether that costs an interrupt. Nodes at `uninstall`, `apply` or `upgrade`, or without the package, are not reset and pick up the new ConfigMap when they reach `config` |
| A `configMap` key | Nothing happens until every node is complete or one is erroring at `config`, `interrupt` or `post-interrupt`. Then every node at `config`, `interrupt`, `post-interrupt` or complete re-enters at `config`, and its unfinished Jobs are cleared; the matching `configInterrupts` entries, plus the package's own `interrupt` on any node that has not yet run it, decide whether that costs an interrupt. Nodes at `uninstall`, `apply` or `upgrade`, or without the package, are not reset and pick up the new ConfigMap when they reach `config` |
| `nodeSelectors` | Newly matching nodes are enrolled; newly excluded nodes leave scope — their host changes remain |
| `image` or `containerSHA` only | No stage change — `version` is the ordering key |

Expand Down
5 changes: 4 additions & 1 deletion docs/user-guide/custom-resource.md
Original file line number Diff line number Diff line change
Expand Up @@ -506,6 +506,9 @@ not cost you a node reboot, while the key that does need one still gets it.
literal key that does not exist, or a glob that matches nothing, is rejected by
the webhook — this catches the typo that would otherwise silently skip an
interrupt.
- A config change adds to the package's own [`interrupt`](#interrupt), it does not
replace it: on a node where the package's own interrupt has not yet run, that
interrupt still runs, merged with the matching `configInterrupts` entries.

For secrets, do **not** put them in `configMap`. See
[Providing Secrets to Packages](providing-secrets.md).
Expand Down Expand Up @@ -644,7 +647,7 @@ To end a failing interrupt sooner, or to run it again after the verdict:

A failing `uninstall-interrupt` cannot be ended by a rerun or a reset. Both remove the package's entry, an absent entry reads as already uninstalled, and so the operator abandons the uninstall interrupt rather than running it again; `package rerun --stage` cannot target `uninstall-interrupt` either. Fix the cause and delete the failed Job instead.

A configMap change does not retry the interrupt either. When the package's configMap changes while it is `erroring` at `config`, `interrupt` or `post-interrupt`, the operator deletes its interrupt Job and sends the package back through `config` with the changed keys recorded. From then on the package's own `interrupt` is not consulted: only its `configInterrupts` entries matching the changed keys decide whether an interrupt runs. With no match the package completes at `config` without its interrupt ever having succeeded, and the node is uncordoned; with a match, that entry runs instead. A configMap change is not applied at all while the package is mid-uninstall.
A configMap change retries the interrupt only on a node that still owes it. When the package's configMap changes while it is `erroring` at `config`, `interrupt` or `post-interrupt`, the operator deletes its interrupt Job and sends the package back through `config` with the changed keys recorded. On a node where the package's own `interrupt` has not yet run (the node's `interruptPending` flag is still set; it is cleared once the interrupt completes or the package reaches `post-interrupt`), that interrupt still runs after `config`, merged with any `configInterrupts` entry matching the changed keys; the highest-priority one wins (a `reboot` over a service restart). On a node where it already ran, only the matching `configInterrupts` entries decide whether an interrupt runs, so a change that matches none completes at `config` without one. A package whose node entry predates the `interruptPending` flag reads as already done. A configMap change is not applied at all while the package is mid-uninstall.

### `uninstall`

Expand Down
55 changes: 45 additions & 10 deletions operator/api/nodewright/v1alpha1/nodewright_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -548,8 +548,17 @@ type NodeWrightStatus struct {

type NodeState map[string]PackageStatus

// Upsert adds or updates specified state for package in the node state
// Upsert adds or updates specified state for package in the node state, carrying the entry's
// InterruptPending flag through unchanged apart from clearing it once the interrupt completes
// or at post-interrupt.
func (ns *NodeState) Upsert(_package PackageRef, image string, state State, stage Stage, restarts int32, containerSHA string) bool {
return ns.UpsertOwing(_package, image, state, stage, restarts, containerSHA, false)
}

// UpsertOwing is Upsert for a package that declares its own interrupt when ownInterrupt is set:
// an entry written at apply or upgrade then owes that interrupt until it completes or the
// package reaches post-interrupt.
func (ns *NodeState) UpsertOwing(_package PackageRef, image string, state State, stage Stage, restarts int32, containerSHA string, ownInterrupt bool) bool {

if *ns == nil {
*ns = make(map[string]PackageStatus)
Expand All @@ -566,6 +575,15 @@ func (ns *NodeState) Upsert(_package PackageRef, image string, state State, stag
}

existing, ok := (*ns)[_package.GetUniqueName()]

status.InterruptPending = ok && existing.InterruptPending
switch {
case ownInterrupt && (stage == StageApply || stage == StageUpgrade):
status.InterruptPending = true
case stage == StagePostInterrupt, stage == StageInterrupt && state == StateComplete:
status.InterruptPending = false
}

if ok && status.Equal(&existing) {
return false
}
Expand All @@ -575,6 +593,16 @@ func (ns *NodeState) Upsert(_package PackageRef, image string, state State, stag
return true
}

// OwesOwnInterrupt reports whether the package declares an interrupt that has not yet run on
// this node: the entry is missing or still pending.
func (ns *NodeState) OwesOwnInterrupt(_package Package) bool {
if !_package.HasInterrupt() {
return false
}
status, ok := (*ns)[_package.GetUniqueName()]
return !ok || status.InterruptPending
}

// RemoveState removes specified package from Node State
func (ns *NodeState) RemoveState(_package PackageRef) bool {
if *ns == nil {
Expand Down Expand Up @@ -653,17 +681,15 @@ func (ns *NodeState) IsComplete(packages Packages, interrupt map[string][]*Inter
return false
}

// HasInterrupt determines whether package has an interrupt based off of config updates and interrupts passed to it
// HasInterrupt determines whether package has an interrupt based off of config updates and interrupts passed to it.
// With config updates recorded, the interrupts matching the changed keys apply, plus the package's own
// while this node still owes it, so a config change cannot finish a package that never ran its interrupt.
func (ns *NodeState) HasInterrupt(_package Package, interrupt map[string][]*Interrupt, config map[string][]string) bool {
var hasInterrupt bool

if len(config[_package.Name]) > 0 {
hasInterrupt = len(interrupt[_package.Name]) > 0
} else {
hasInterrupt = _package.HasInterrupt()
return len(interrupt[_package.Name]) > 0 || ns.OwesOwnInterrupt(_package)
}

return hasInterrupt
return _package.HasInterrupt()
}

func (ns *NodeState) NextStage(_package *Package, interrupt map[string][]*Interrupt, config map[string][]string) *Stage {
Expand Down Expand Up @@ -787,6 +813,7 @@ func (ns *NodeState) ProgressSkipped(packages Packages, interrupt map[string][]*

if s.Stage == StageInterrupt && s.State == StateSkipped {
s.State = StateComplete
s.InterruptPending = false // nothing is left to run at (interrupt, complete)
(*ns)[f.GetUniqueName()] = s
ret = true
}
Expand Down Expand Up @@ -825,17 +852,25 @@ type PackageStatus struct {

// Restarts are the number of times a package restarted
Restarts int32 `json:"restarts,omitempty"`

// InterruptPending is set at apply or upgrade for a package that declares its own interrupt,
// and cleared once that interrupt has completed or the package reaches post-interrupt. While set, it is still owed on this
// node, so a config change that matches no configInterrupts does not let it finish without it.
// Entries written before this field existed read as not pending.
//+optional
InterruptPending bool `json:"interruptPending,omitempty"`
}

// Equal checks name, version, image, containerSHA, stage, state, and restarts
// Equal checks name, version, image, containerSHA, stage, state, restarts, and interruptPending
func (left *PackageStatus) Equal(right *PackageStatus) bool {
return left.Name == right.Name &&
left.Version == right.Version &&
left.Image == right.Image &&
left.ContainerSHA == right.ContainerSHA &&
left.Stage == right.Stage &&
left.State == right.State &&
left.Restarts == right.Restarts
left.Restarts == right.Restarts &&
left.InterruptPending == right.InterruptPending
}

// IsInterruptStage reports whether the package is in one of the two interrupt-phase
Expand Down
98 changes: 98 additions & 0 deletions operator/api/nodewright/v1alpha1/nodewright_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,104 @@ var _ = Describe("NodeWrightSpec BuildGraph", func() {

var _ = Describe("NodeState", func() {

Describe("owed own interrupt", func() {
ref := PackageRef{Name: "foo", Version: "1.0.0"}
withReboot := Package{PackageRef: ref, Interrupt: &Interrupt{Type: REBOOT}}
plain := Package{PackageRef: ref}
unmatched := map[string][]string{"foo": {"app.conf"}}
noMatch := map[string][]*Interrupt{}

pending := func(ns NodeState) bool { return ns["foo|1.0.0"].InterruptPending }

It("sets it on apply and upgrade, carries it through config and interrupt, clears it at post-interrupt", func() {
ns := NodeState{}
ns.UpsertOwing(ref, "img", StateInProgress, StageApply, 0, "", true)
Expect(pending(ns)).To(BeTrue())
ns.Upsert(ref, "img", StateComplete, StageConfig, 0, "")
Expect(pending(ns)).To(BeTrue())
ns.Upsert(ref, "img", StateInProgress, StageInterrupt, 0, "")
Expect(pending(ns)).To(BeTrue())
ns.Upsert(ref, "img", StateInProgress, StagePostInterrupt, 0, "")
Expect(pending(ns)).To(BeFalse())

// a config reset keeps it cleared
ns.Upsert(ref, "img", StateInProgress, StageConfig, 0, "")
Expect(pending(ns)).To(BeFalse())

// a new apply or upgrade owes it again
ns.UpsertOwing(ref, "img", StateInProgress, StageUpgrade, 0, "", true)
Expect(pending(ns)).To(BeTrue())
})

It("clears it once the interrupt completes, so a config reset at (interrupt, complete) does not rerun it", func() {
ns := NodeState{}
ns.UpsertOwing(ref, "img", StateInProgress, StageApply, 0, "", true)
ns.Upsert(ref, "img", StateInProgress, StageInterrupt, 0, "")
ns.Upsert(ref, "img", StateComplete, StageInterrupt, 0, "")
Expect(pending(ns)).To(BeFalse())

ns.Upsert(ref, "img", StateInProgress, StageConfig, 0, "")
Expect(ns.HasInterrupt(withReboot, noMatch, unmatched)).To(BeFalse())
Expect(ns.IsPackageComplete(withReboot, noMatch, unmatched)).To(BeFalse()) // in_progress at config, not complete
})

It("clears it when a skipped interrupt is promoted to complete", func() {
ns := NodeState{"foo|1.0.0": {Name: "foo", Version: "1.0.0", Stage: StageInterrupt, State: StateSkipped, InterruptPending: true}}
Expect(ns.ProgressSkipped(Packages{"foo": withReboot}, noMatch, unmatched)).To(BeTrue())
Expect(pending(ns)).To(BeFalse())
Expect(ns["foo|1.0.0"].State).To(Equal(StateComplete))
})

It("leaves a package without its own interrupt unflagged and keeps a carried flag on plain upserts", func() {
ns := NodeState{}
ns.UpsertOwing(ref, "img", StateInProgress, StageApply, 0, "", false)
Expect(pending(ns)).To(BeFalse())

ns = NodeState{}
ns.UpsertOwing(ref, "img", StateInProgress, StageApply, 0, "", true)
ns.Upsert(ref, "img", StateComplete, StageApply, 0, "")
Expect(pending(ns)).To(BeTrue())
})

It("reports a change when only the flag differs", func() {
ns := NodeState{"foo|1.0.0": PackageStatus{Name: "foo", Version: "1.0.0", Image: "img", Stage: StageApply, State: StateInProgress}}
Expect(ns.UpsertOwing(ref, "img", StateInProgress, StageApply, 0, "", true)).To(BeTrue())
Expect(ns.UpsertOwing(ref, "img", StateInProgress, StageApply, 0, "", true)).To(BeFalse())
})

It("owes it for a missing or pending entry and not for a settled one", func() {
Expect((&NodeState{}).OwesOwnInterrupt(withReboot)).To(BeTrue())
Expect((&NodeState{"foo|1.0.0": {InterruptPending: true}}).OwesOwnInterrupt(withReboot)).To(BeTrue())
Expect((&NodeState{"foo|1.0.0": {}}).OwesOwnInterrupt(withReboot)).To(BeFalse())
Expect((&NodeState{"foo|1.0.0": {InterruptPending: true}}).OwesOwnInterrupt(plain)).To(BeFalse())
})

It("keeps the own interrupt when a config change matches no configInterrupts", func() {
ns := NodeState{"foo|1.0.0": {Name: "foo", Version: "1.0.0", Stage: StageConfig, State: StateComplete, InterruptPending: true}}
Expect(ns.HasInterrupt(withReboot, noMatch, unmatched)).To(BeTrue())
Expect(ns.IsPackageComplete(withReboot, noMatch, unmatched)).To(BeFalse())
Expect(*ns.NextStage(&withReboot, noMatch, unmatched)).To(Equal(StageInterrupt))
})

It("finishes at config once the own interrupt already ran", func() {
ns := NodeState{"foo|1.0.0": {Name: "foo", Version: "1.0.0", Stage: StageConfig, State: StateComplete}}
Expect(ns.HasInterrupt(withReboot, noMatch, unmatched)).To(BeFalse())
Expect(ns.IsPackageComplete(withReboot, noMatch, unmatched)).To(BeTrue())
})

It("still runs a matching configInterrupt after the own interrupt ran", func() {
ns := NodeState{"foo|1.0.0": {Name: "foo", Version: "1.0.0", Stage: StageConfig, State: StateComplete}}
matched := map[string][]*Interrupt{"foo": {{Type: SERVICE}}}
Expect(ns.HasInterrupt(withReboot, matched, unmatched)).To(BeTrue())
})

It("ignores the flag when there are no config updates", func() {
ns := NodeState{"foo|1.0.0": {Name: "foo", Version: "1.0.0", Stage: StageConfig, State: StateComplete}}
Expect(ns.HasInterrupt(withReboot, noMatch, nil)).To(BeTrue())
Expect(ns.HasInterrupt(plain, noMatch, nil)).To(BeFalse())
})
})

Describe("RemoveState", func() {
It("should report no change when the state map is nil", func() {
var ns NodeState
Expand Down
2 changes: 2 additions & 0 deletions operator/api/v1alpha1/nodewright_conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,8 @@ func convertPackageStatus(in PackageStatus) nwv1.PackageStatus {
Stage: nwv1.Stage(in.Stage),
State: nwv1.State(in.State),
Restarts: in.Restarts,

InterruptPending: in.InterruptPending,
}
}

Expand Down
15 changes: 8 additions & 7 deletions operator/api/v1alpha1/nodewright_conversion_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,13 +129,14 @@ func fullSkyhook() *Skyhook {
NodeState: map[string]NodeState{
"node-1": {
"tuning|1.2.3": PackageStatus{
Name: "tuning",
Version: "1.2.3",
Image: "alpine",
ContainerSHA: "sha256:deadbeef",
Stage: StageConfig,
State: StateComplete,
Restarts: 2,
Name: "tuning",
Version: "1.2.3",
Image: "alpine",
ContainerSHA: "sha256:deadbeef",
Stage: StageConfig,
State: StateComplete,
Restarts: 2,
InterruptPending: true,
},
},
},
Expand Down
4 changes: 4 additions & 0 deletions operator/api/v1alpha1/skyhook_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -792,6 +792,10 @@ type PackageStatus struct {

// Restarts are the number of times a package restarted
Restarts int32 `json:"restarts,omitempty"`

// InterruptPending mirrors the nodewright.nvidia.com field of the same name.
//+optional
InterruptPending bool `json:"interruptPending,omitempty"`
}

// Equal checks name, version, image, containerSHA, stage, state, and restarts
Expand Down
9 changes: 8 additions & 1 deletion operator/cmd/cli/app/package/package_rerun.go
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ func rerunPackage(ctx context.Context, cmd *cobra.Command, kubeClient *client.Cl
}

// Update each node's annotation to trigger re-run
successCount, updateErrors := updateNodeAnnotations(ctx, kubeClient, nodesToUpdate, nodeStates, packageKey, annotationKey, opts)
successCount, updateErrors := updateNodeAnnotations(ctx, kubeClient, nodesToUpdate, nodeStates, packageKey, annotationKey, pkg.HasInterrupt(), opts)

printRerunResults(cmd, packageName, successCount, updateErrors)

Expand Down Expand Up @@ -297,6 +297,7 @@ func updateNodeAnnotations(
nodesToUpdate []string,
nodeStates map[string]v1alpha1.NodeState,
packageKey, annotationKey string,
ownInterrupt bool,
opts *rerunOptions,
) (int, []string) {
var updateErrors []string
Expand All @@ -309,6 +310,12 @@ func updateNodeAnnotations(
pkgStatus := nodeState[packageKey]
pkgStatus.Stage = v1alpha1.Stage(opts.stage)
pkgStatus.State = v1alpha1.StateInProgress
switch pkgStatus.Stage {
case v1alpha1.StageApply, v1alpha1.StageUpgrade:
pkgStatus.InterruptPending = ownInterrupt // a fresh apply owes the package's own interrupt again
case v1alpha1.StagePostInterrupt:
pkgStatus.InterruptPending = false
}
nodeState[packageKey] = pkgStatus
} else {
Comment thread
nko-cpu marked this conversation as resolved.
delete(nodeState, packageKey)
Expand Down
Loading
Loading