diff --git a/Makefile b/Makefile index b3beaad..8ceb1ed 100644 --- a/Makefile +++ b/Makefile @@ -52,13 +52,25 @@ unit: manifests generate setup-envtest ## Run unit tests (envtest). V=1 for verb KUBEBUILDER_ASSETS="$(shell "$(ENVTEST)" use $(ENVTEST_K8S_VERSION) --bin-dir "$(LOCALBIN)" -p path)" go test $(if $(V),-v) $(if $(RUN),-run $(RUN)) $$(go list ./... | grep -v /test/e2e) .PHONY: lint -lint: golangci-lint ## Run golangci-lint linter. +lint: golangci-lint ## Run golangci-lint linter and go-lines "$(GOLANGCI_LINT)" run + $(MAKE) go-lines .PHONY: lint-fix lint-fix: golangci-lint ## Run golangci-lint linter and perform fixes. "$(GOLANGCI_LINT)" run --fix +.PHONY: go-lines +go-lines: get-golines + @if [ "$$("$(GOLINES_BIN)" --dry-run . | wc -l)" -gt 0 ]; then \ + echo "Run make go-lines-fix"; \ + exit 1; \ + fi + +.PHONY: go-lines-fix +go-lines-fix: get-golines + "$(GOLINES_BIN)" -w . + .PHONY: e2e e2e: ## Run e2e tests (requires: make deploy-bink). V=1 for verbose. RUN= to filter. # NB: we `cd` here instead of passing a package path to `go test` so that `-v` @@ -180,6 +192,7 @@ KUSTOMIZE ?= $(LOCALBIN)/kustomize CONTROLLER_GEN ?= $(LOCALBIN)/controller-gen ENVTEST ?= $(LOCALBIN)/setup-envtest GOLANGCI_LINT = $(LOCALBIN)/golangci-lint +GOLINES_BIN = $(LOCALBIN)/golines YQ ?= $(LOCALBIN)/yq KUSTOMIZE_VERSION ?= v5.8.1 @@ -224,6 +237,11 @@ golangci-lint: $(GOLANGCI_LINT) ## Download golangci-lint locally if necessary. $(GOLANGCI_LINT): $(LOCALBIN) $(call go-install-tool,$(GOLANGCI_LINT),github.com/golangci/golangci-lint/v2/cmd/golangci-lint,$(GOLANGCI_LINT_VERSION)) +.PHONY: get-golines +get-golines: $(GOLINES_BIN) ## Download golines locally if necessary. +$(GOLINES_BIN): $(LOCALBIN) + $(call go-install-tool,$(GOLINES_BIN),github.com/golangci/golines,latest) + .PHONY: yq yq: $(YQ) ## Download yq locally if necessary. $(YQ): $(LOCALBIN) diff --git a/cmd/controller/main.go b/cmd/controller/main.go index 64537d7..7b09f39 100644 --- a/cmd/controller/main.go +++ b/cmd/controller/main.go @@ -35,10 +35,24 @@ func main() { var probeAddr string var tagResolutionInterval time.Duration var allowInsecureRegistry bool - flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.") - flag.DurationVar(&tagResolutionInterval, "tag-resolution-interval", 5*time.Minute, "How often to re-resolve tag-based image refs.") - flag.BoolVar(&allowInsecureRegistry, "allow-insecure-registry", false, - "Allow falling back to HTTP when resolving tag-based image refs against registries that do not serve TLS.") + flag.StringVar( + &probeAddr, + "health-probe-bind-address", + ":8081", + "The address the probe endpoint binds to.", + ) + flag.DurationVar( + &tagResolutionInterval, + "tag-resolution-interval", + 5*time.Minute, + "How often to re-resolve tag-based image refs.", + ) + flag.BoolVar( + &allowInsecureRegistry, + "allow-insecure-registry", + false, + "Allow falling back to HTTP when resolving tag-based image refs against registries that do not serve TLS.", + ) flag.BoolVar(&enableLeaderElection, "leader-elect", false, "Enable leader election for controller manager. "+ "Enabling this will ensure there is only one active controller manager.") diff --git a/cmd/daemon/main.go b/cmd/daemon/main.go index ed49e8f..84ac526 100644 --- a/cmd/daemon/main.go +++ b/cmd/daemon/main.go @@ -35,7 +35,12 @@ func init() { func main() { var pollInterval time.Duration - flag.DurationVar(&pollInterval, "bootc-poll-interval", 5*time.Minute, "Interval for polling bootc status as a fallback to fsnotify") + flag.DurationVar( + &pollInterval, + "bootc-poll-interval", + 5*time.Minute, + "Interval for polling bootc status as a fallback to fsnotify", + ) opts := zap.Options{ Development: true, @@ -47,7 +52,10 @@ func main() { nodeName := os.Getenv("NODE_NAME") if nodeName == "" { - setupLog.Error(fmt.Errorf("NODE_NAME not set"), "NODE_NAME environment variable is required") + setupLog.Error( + fmt.Errorf("NODE_NAME not set"), + "NODE_NAME environment variable is required", + ) os.Exit(1) } diff --git a/internal/controller/bootcnodepool_controller.go b/internal/controller/bootcnodepool_controller.go index 59eb7fe..2433605 100644 --- a/internal/controller/bootcnodepool_controller.go +++ b/internal/controller/bootcnodepool_controller.go @@ -105,7 +105,10 @@ func (r *BootcNodePoolReconciler) SetupWithManager(mgr ctrl.Manager) error { // pool that owns it. The second set is needed so the owning pool can clean up // when a node's labels change such that it no longer matches, or when the node // is deleted entirely. -func (r *BootcNodePoolReconciler) mapNodeToPoolRequests(ctx context.Context, obj client.Object) []reconcile.Request { +func (r *BootcNodePoolReconciler) mapNodeToPoolRequests( + ctx context.Context, + obj client.Object, +) []reconcile.Request { node, ok := obj.(*corev1.Node) if !ok { return nil @@ -218,7 +221,10 @@ func nodeUnschedulableChanged(oldNode, newNode *corev1.Node) bool { // Reconcile is part of the main kubernetes reconciliation loop which aims to // move the current state of the cluster closer to the desired state. -func (r *BootcNodePoolReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { +func (r *BootcNodePoolReconciler) Reconcile( + ctx context.Context, + req ctrl.Request, +) (ctrl.Result, error) { log := logf.FromContext(ctx).WithValues("pool", req.Name) // Fetch the pool. @@ -306,12 +312,17 @@ func (r *BootcNodePoolReconciler) Reconcile(ctx context.Context, req ctrl.Reques // resolveTargetDigest resolves the target digest from the pool's image // ref. Digest refs are extracted directly. Tag refs are resolved via // the registry, respecting the re-resolution interval. -func (r *BootcNodePoolReconciler) resolveTargetDigest(ctx context.Context, pool *bootcv1alpha1.BootcNodePool) (ctrl.Result, error) { +func (r *BootcNodePoolReconciler) resolveTargetDigest( + ctx context.Context, + pool *bootcv1alpha1.BootcNodePool, +) (ctrl.Result, error) { log := logf.FromContext(ctx) ref, err := parseImageRef(pool.Spec.Image.Ref) if err != nil { - return ctrl.Result{}, newInvalidSpecError(fmt.Sprintf("invalid image ref %q: %v", pool.Spec.Image.Ref, err)) + return ctrl.Result{}, newInvalidSpecError( + fmt.Sprintf("invalid image ref %q: %v", pool.Spec.Image.Ref, err), + ) } digested, ok := ref.(reference.Digested) @@ -325,7 +336,8 @@ func (r *BootcNodePoolReconciler) resolveTargetDigest(ctx context.Context, pool // Tag ref — check if resolution is due. now := time.Now() - if pool.Status.NextTagResolutionTime != nil && now.Before(pool.Status.NextTagResolutionTime.Time) { + if pool.Status.NextTagResolutionTime != nil && + now.Before(pool.Status.NextTagResolutionTime.Time) { remaining := pool.Status.NextTagResolutionTime.Sub(now) log.V(1).Info("Tag resolution not yet due", "remaining", remaining) return ctrl.Result{RequeueAfter: remaining}, nil @@ -375,7 +387,11 @@ func isInvalidSpecError(err error) bool { // setInvalidSpecCondition sets Degraded/InvalidSpec on the pool and // returns (Result, nil) so Reconcile stops without requeueing. -func (r *BootcNodePoolReconciler) setInvalidSpecCondition(ctx context.Context, pool *bootcv1alpha1.BootcNodePool, specErr error) (ctrl.Result, error) { +func (r *BootcNodePoolReconciler) setInvalidSpecCondition( + ctx context.Context, + pool *bootcv1alpha1.BootcNodePool, + specErr error, +) (ctrl.Result, error) { setPoolDegraded(pool, bootcv1alpha1.PoolInvalidSpec, specErr.Error()) if err := r.Status().Update(ctx, pool); err != nil { return ctrl.Result{}, fmt.Errorf("updating pool status: %w", err) @@ -386,7 +402,10 @@ func (r *BootcNodePoolReconciler) setInvalidSpecCondition(ctx context.Context, p // syncMembership reconciles the set of BootcNodes owned by this pool // against the set of Nodes matching the pool's nodeSelector. It returns // the owned BootcNodes after mutations (creates and deletes) are applied. -func (r *BootcNodePoolReconciler) syncMembership(ctx context.Context, pool *bootcv1alpha1.BootcNodePool) (map[string]*bootcv1alpha1.BootcNode, error) { +func (r *BootcNodePoolReconciler) syncMembership( + ctx context.Context, + pool *bootcv1alpha1.BootcNodePool, +) (map[string]*bootcv1alpha1.BootcNode, error) { log := logf.FromContext(ctx).WithValues("pool", pool.Name) // List all nodes matching the pool's selector. @@ -473,21 +492,30 @@ func (r *BootcNodePoolReconciler) syncMembership(ctx context.Context, pool *boot // listMatchingNodes returns all Nodes whose labels match the pool's // nodeSelector. -func (r *BootcNodePoolReconciler) listMatchingNodes(ctx context.Context, pool *bootcv1alpha1.BootcNodePool) ([]corev1.Node, error) { +func (r *BootcNodePoolReconciler) listMatchingNodes( + ctx context.Context, + pool *bootcv1alpha1.BootcNodePool, +) ([]corev1.Node, error) { selector, err := metav1.LabelSelectorAsSelector(pool.Spec.NodeSelector) if err != nil { return nil, newInvalidSpecError(fmt.Sprintf("invalid nodeSelector: %v", err)) } var nodeList corev1.NodeList - if err := r.List(ctx, &nodeList, client.MatchingLabelsSelector{Selector: selector}); err != nil { + if err := r.List( + ctx, + &nodeList, + client.MatchingLabelsSelector{Selector: selector}, + ); err != nil { return nil, fmt.Errorf("listing nodes: %w", err) } return nodeList.Items, nil } // listAllBootcNodes returns all BootcNodes keyed by name. -func (r *BootcNodePoolReconciler) listAllBootcNodes(ctx context.Context) (map[string]*bootcv1alpha1.BootcNode, error) { +func (r *BootcNodePoolReconciler) listAllBootcNodes( + ctx context.Context, +) (map[string]*bootcv1alpha1.BootcNode, error) { var bnList bootcv1alpha1.BootcNodeList if err := r.List(ctx, &bnList); err != nil { return nil, fmt.Errorf("listing BootcNodes: %w", err) @@ -501,7 +529,11 @@ func (r *BootcNodePoolReconciler) listAllBootcNodes(ctx context.Context) (map[st } // syncBootcNodeSpec updates a BootcNode's spec fields to match the pool. -func (r *BootcNodePoolReconciler) syncBootcNodeSpec(ctx context.Context, pool *bootcv1alpha1.BootcNodePool, bn *bootcv1alpha1.BootcNode) error { +func (r *BootcNodePoolReconciler) syncBootcNodeSpec( + ctx context.Context, + pool *bootcv1alpha1.BootcNodePool, + bn *bootcv1alpha1.BootcNode, +) error { modified := bn.DeepCopy() desiredImage := desiredImageFromPool(pool) needPatch := false @@ -540,7 +572,11 @@ func desiredImageFromPool(pool *bootcv1alpha1.BootcNodePool) string { // createBootcNode creates a BootcNode for a node joining the pool and // labels the node as managed. -func (r *BootcNodePoolReconciler) createBootcNode(ctx context.Context, pool *bootcv1alpha1.BootcNodePool, node *corev1.Node) (*bootcv1alpha1.BootcNode, error) { +func (r *BootcNodePoolReconciler) createBootcNode( + ctx context.Context, + pool *bootcv1alpha1.BootcNodePool, + node *corev1.Node, +) (*bootcv1alpha1.BootcNode, error) { bn := &bootcv1alpha1.BootcNode{ ObjectMeta: metav1.ObjectMeta{ Name: node.Name, @@ -575,7 +611,11 @@ func (r *BootcNodePoolReconciler) createBootcNode(ctx context.Context, pool *boo } // ensureManagedLabel adds or removes the bootc.dev/managed label on a Node. -func (r *BootcNodePoolReconciler) ensureManagedLabel(ctx context.Context, node *corev1.Node, managed bool) error { +func (r *BootcNodePoolReconciler) ensureManagedLabel( + ctx context.Context, + node *corev1.Node, + managed bool, +) error { _, hasLabel := node.Labels[bootcv1alpha1.LabelManaged] if managed && hasLabel { return nil @@ -602,7 +642,10 @@ func (r *BootcNodePoolReconciler) ensureManagedLabel(ctx context.Context, node * // removeBootcNode deletes a BootcNode for a node leaving the pool, // removes the managed label, and restores prior cordon state. -func (r *BootcNodePoolReconciler) removeBootcNode(ctx context.Context, bn *bootcv1alpha1.BootcNode) error { +func (r *BootcNodePoolReconciler) removeBootcNode( + ctx context.Context, + bn *bootcv1alpha1.BootcNode, +) error { // Cancel any active drain goroutine for this node. The goroutine will // exit on its own and send a result on the channel, but since we've // removed the entry from the map, collectDrainResults will never see @@ -643,7 +686,11 @@ func (r *BootcNodePoolReconciler) removeBootcNode(ctx context.Context, bn *bootc // restoreCordonState uncordons the Node if the BootcNode's was-cordoned // annotation indicates the operator cordoned it. If the annotation is absent // or "true" (node was already cordoned before us), this is a no-op. -func (r *BootcNodePoolReconciler) restoreCordonState(ctx context.Context, bn *bootcv1alpha1.BootcNode, node *corev1.Node) error { +func (r *BootcNodePoolReconciler) restoreCordonState( + ctx context.Context, + bn *bootcv1alpha1.BootcNode, + node *corev1.Node, +) error { if bn.Annotations[bootcv1alpha1.AnnotationWasCordoned] != "false" { return nil } diff --git a/internal/controller/membership_test.go b/internal/controller/membership_test.go index d97b7f4..bc10415 100644 --- a/internal/controller/membership_test.go +++ b/internal/controller/membership_test.go @@ -140,7 +140,11 @@ func TestMembershipCreatesBootcNodes(t *testing.T) { // Wait for BootcNode to be deleted. g.Eventually(func() error { - return k8sClient.Get(ctx, client.ObjectKey{Name: "mem-worker-1"}, &bootcv1alpha1.BootcNode{}) + return k8sClient.Get( + ctx, + client.ObjectKey{Name: "mem-worker-1"}, + &bootcv1alpha1.BootcNode{}, + ) }).Should(MatchError(apierrors.IsNotFound, "IsNotFound")) // Verify managed label is removed. @@ -154,7 +158,11 @@ func TestMembershipCreatesBootcNodes(t *testing.T) { g.Expect(k8sClient.Delete(ctx, node2)).To(Succeed()) g.Eventually(func() error { - return k8sClient.Get(ctx, client.ObjectKey{Name: "mem-worker-2"}, &bootcv1alpha1.BootcNode{}) + return k8sClient.Get( + ctx, + client.ObjectKey{Name: "mem-worker-2"}, + &bootcv1alpha1.BootcNode{}, + ) }).Should(MatchError(apierrors.IsNotFound, "IsNotFound")) } @@ -216,7 +224,10 @@ func TestMembershipConflictDetection(t *testing.T) { // node1: pool1 only, node2: pool2 only, node3: both (contested). node1 := testutil.NewK8sNode("mem-conflict-1", map[string]string{"pool1": "true"}) node2 := testutil.NewK8sNode("mem-conflict-2", map[string]string{"pool2": "true"}) - node3 := testutil.NewK8sNode("mem-conflict-3", map[string]string{"pool1": "true", "pool2": "true"}) + node3 := testutil.NewK8sNode( + "mem-conflict-3", + map[string]string{"pool1": "true", "pool2": "true"}, + ) for _, n := range []*corev1.Node{node1, node2, node3} { g.Expect(k8sClient.Create(ctx, n)).To(Succeed()) t.Cleanup(func() { diff --git a/internal/controller/rollout.go b/internal/controller/rollout.go index 17c9d1e..0872cf3 100644 --- a/internal/controller/rollout.go +++ b/internal/controller/rollout.go @@ -57,7 +57,11 @@ func (rs *rolloutState) nodeCount() int { } // driveRollout is the main function that advances the rollout state machine. -func (r *BootcNodePoolReconciler) driveRollout(ctx context.Context, pool *bootcv1alpha1.BootcNodePool, ownedBootcNodes map[string]*bootcv1alpha1.BootcNode) (*rolloutState, error) { +func (r *BootcNodePoolReconciler) driveRollout( + ctx context.Context, + pool *bootcv1alpha1.BootcNodePool, + ownedBootcNodes map[string]*bootcv1alpha1.BootcNode, +) (*rolloutState, error) { log := logf.FromContext(ctx) // Process drain results first. This isn't really ordering dependent, @@ -96,8 +100,14 @@ func (r *BootcNodePoolReconciler) driveRollout(ctx context.Context, pool *bootcv details[i] = u.name + ": " + u.reason } slices.Sort(details) - setPoolDegraded(pool, bootcv1alpha1.PoolRolloutHalted, - fmt.Sprintf("Rollout halted: 2+ unhealthy nodes in reboot slots (%s)", strings.Join(details, ", "))) + setPoolDegraded( + pool, + bootcv1alpha1.PoolRolloutHalted, + fmt.Sprintf( + "Rollout halted: 2+ unhealthy nodes in reboot slots (%s)", + strings.Join(details, ", "), + ), + ) log.Info("Rollout halted: 2+ unhealthy nodes in reboot slots", "unhealthyInSlots", len(unhealthy)) @@ -163,7 +173,11 @@ func (r *BootcNodePoolReconciler) driveRollout(ctx context.Context, pool *bootcv // annotation on the BootcNode, records prior cordon state in the // was-cordoned annotation, and cordons the node. All operations are // idempotent. -func (r *BootcNodePoolReconciler) assignRebootSlot(ctx context.Context, bn *bootcv1alpha1.BootcNode, node *corev1.Node) error { +func (r *BootcNodePoolReconciler) assignRebootSlot( + ctx context.Context, + bn *bootcv1alpha1.BootcNode, + node *corev1.Node, +) error { log := logf.FromContext(ctx) // Set annotations on the BootcNode if not already present. @@ -233,7 +247,11 @@ func (r *BootcNodePoolReconciler) freeCompletedSlots(ctx context.Context, rs *ro // freeRebootSlot releases a node's reboot slot by restoring its prior cordon // state and removing annotations from the BootcNode. -func (r *BootcNodePoolReconciler) freeRebootSlot(ctx context.Context, bn *bootcv1alpha1.BootcNode, node *corev1.Node) error { +func (r *BootcNodePoolReconciler) freeRebootSlot( + ctx context.Context, + bn *bootcv1alpha1.BootcNode, + node *corev1.Node, +) error { if err := r.restoreCordonState(ctx, bn, node); err != nil { return err } @@ -252,7 +270,11 @@ func (r *BootcNodePoolReconciler) freeRebootSlot(ctx context.Context, bn *bootcv // ensureDrain checks whether a drain goroutine is already running for // the given node and starts one if not. It is a no-op if a drain is // already in progress. -func (r *BootcNodePoolReconciler) ensureDrain(ctx context.Context, pool *bootcv1alpha1.BootcNodePool, bn *bootcv1alpha1.BootcNode) { +func (r *BootcNodePoolReconciler) ensureDrain( + ctx context.Context, + pool *bootcv1alpha1.BootcNodePool, + bn *bootcv1alpha1.BootcNode, +) { log := logf.FromContext(ctx) r.drainsMu.Lock() @@ -309,7 +331,10 @@ func (r *BootcNodePoolReconciler) ensureDrain(ctx context.Context, pool *bootcv1 // collectDrainResults checks all in-progress drains for completed // results. On success, it sets desiredImageState to Booted on the // BootcNode. -func (r *BootcNodePoolReconciler) collectDrainResults(ctx context.Context, ownedBootcNodes map[string]*bootcv1alpha1.BootcNode) error { +func (r *BootcNodePoolReconciler) collectDrainResults( + ctx context.Context, + ownedBootcNodes map[string]*bootcv1alpha1.BootcNode, +) error { log := logf.FromContext(ctx) r.drainsMu.Lock() @@ -375,7 +400,10 @@ func (r *BootcNodePoolReconciler) collectDrainResults(ctx context.Context, owned // buildRolloutState classifies all owned BootcNodes and counts occupied // reboot slots. -func buildRolloutState(log logr.Logger, ownedBootcNodes map[string]*bootcv1alpha1.BootcNode) *rolloutState { +func buildRolloutState( + log logr.Logger, + ownedBootcNodes map[string]*bootcv1alpha1.BootcNode, +) *rolloutState { rs := &rolloutState{} for _, bn := range ownedBootcNodes { // Count occupied reboot slots from the persistent annotation. @@ -404,7 +432,10 @@ func buildRolloutState(log logr.Logger, ownedBootcNodes map[string]*bootcv1alpha case nodeStateRebooting: rs.rebooting = append(rs.rebooting, bn) case nodeStateDegraded: - if cond := apimeta.FindStatusCondition(bn.Status.Conditions, bootcv1alpha1.NodeDegraded); cond != nil { + if cond := apimeta.FindStatusCondition( + bn.Status.Conditions, + bootcv1alpha1.NodeDegraded, + ); cond != nil { log.Info("Node is degraded", "node", bn.Name, "message", cond.Message) } rs.degraded = append(rs.degraded, bn) @@ -462,9 +493,19 @@ func resolveMaxUnavailable(pool *bootcv1alpha1.BootcNodePool, nodeCount int) (in } // We roundUp here; this matches Deployments maxUnavailable for example - v, err := intstr.GetScaledValueFromIntOrPercent(pool.Spec.Rollout.MaxUnavailable, nodeCount, true) + v, err := intstr.GetScaledValueFromIntOrPercent( + pool.Spec.Rollout.MaxUnavailable, + nodeCount, + true, + ) if err != nil { - return 0, newInvalidSpecError(fmt.Sprintf("invalid maxUnavailable %q: %v", pool.Spec.Rollout.MaxUnavailable.String(), err)) + return 0, newInvalidSpecError( + fmt.Sprintf( + "invalid maxUnavailable %q: %v", + pool.Spec.Rollout.MaxUnavailable.String(), + err, + ), + ) } return v, nil } @@ -475,7 +516,10 @@ func resolveMaxUnavailable(pool *bootcv1alpha1.BootcNodePool, nodeCount int) (in // drain restarted). These nodes are already counted in occupiedSlots so they // don't consume availableSlots. Beyond those, up to availableSlots unslotted // nodes are appended, sorted alphabetically. -func selectDrainCandidates(staged []*bootcv1alpha1.BootcNode, availableSlots int) []*bootcv1alpha1.BootcNode { +func selectDrainCandidates( + staged []*bootcv1alpha1.BootcNode, + availableSlots int, +) []*bootcv1alpha1.BootcNode { if len(staged) == 0 { return nil } diff --git a/internal/controller/rollout_envtest_test.go b/internal/controller/rollout_envtest_test.go index 0a056b7..dc4f2df 100644 --- a/internal/controller/rollout_envtest_test.go +++ b/internal/controller/rollout_envtest_test.go @@ -334,7 +334,11 @@ func TestUnhealthyNodesHaltRollout(t *testing.T) { // simulateDaemonStatus writes BootcNode status as if the daemon had // reported the given booted digest and Idle condition reason. -func simulateDaemonStatus(g Gomega, ctx context.Context, nodeName, bootedDigest, idleReason string) { +func simulateDaemonStatus( + g Gomega, + ctx context.Context, + nodeName, bootedDigest, idleReason string, +) { var bn bootcv1alpha1.BootcNode g.Expect(k8sClient.Get(ctx, client.ObjectKey{Name: nodeName}, &bn)).To(Succeed()) diff --git a/internal/controller/rollout_test.go b/internal/controller/rollout_test.go index cb7891a..9f0fcf7 100644 --- a/internal/controller/rollout_test.go +++ b/internal/controller/rollout_test.go @@ -26,23 +26,58 @@ func TestBuildRolloutState(t *testing.T) { // test focuses more on aggregation: bucketing, slot counting, and // nodeCount. nodes := map[string]*bootcv1alpha1.BootcNode{ - "uptodate": testutil.NewNode("uptodate", desiredImage, + "uptodate": testutil.NewNode( + "uptodate", + desiredImage, testutil.WithBootedDigest(testDigestA), - testutil.WithNodeCondition(bootcv1alpha1.NodeIdle, metav1.ConditionTrue, bootcv1alpha1.NodeReasonIdle)), - "pending": testutil.NewNode("pending", desiredImage, + testutil.WithNodeCondition( + bootcv1alpha1.NodeIdle, + metav1.ConditionTrue, + bootcv1alpha1.NodeReasonIdle, + ), + ), + "pending": testutil.NewNode( + "pending", + desiredImage, testutil.WithBootedDigest(otherDigest), - testutil.WithNodeCondition(bootcv1alpha1.NodeIdle, metav1.ConditionTrue, bootcv1alpha1.NodeReasonIdle)), - "staged": testutil.NewNode("staged", desiredImage, + testutil.WithNodeCondition( + bootcv1alpha1.NodeIdle, + metav1.ConditionTrue, + bootcv1alpha1.NodeReasonIdle, + ), + ), + "staged": testutil.NewNode( + "staged", + desiredImage, testutil.WithBootedDigest(otherDigest), - testutil.WithNodeCondition(bootcv1alpha1.NodeIdle, metav1.ConditionFalse, bootcv1alpha1.NodeReasonStaged)), - "rebooting-1": testutil.NewNode("rebooting-1", desiredImage, + testutil.WithNodeCondition( + bootcv1alpha1.NodeIdle, + metav1.ConditionFalse, + bootcv1alpha1.NodeReasonStaged, + ), + ), + "rebooting-1": testutil.NewNode( + "rebooting-1", + desiredImage, testutil.WithBootedDigest(otherDigest), - testutil.WithNodeCondition(bootcv1alpha1.NodeIdle, metav1.ConditionFalse, bootcv1alpha1.NodeReasonRebooting), - testutil.WithNodeAnnotation(bootcv1alpha1.AnnotationInRebootSlot, "")), - "rebooting-2": testutil.NewNode("rebooting-2", desiredImage, + testutil.WithNodeCondition( + bootcv1alpha1.NodeIdle, + metav1.ConditionFalse, + bootcv1alpha1.NodeReasonRebooting, + ), + testutil.WithNodeAnnotation(bootcv1alpha1.AnnotationInRebootSlot, ""), + ), + "rebooting-2": testutil.NewNode( + "rebooting-2", + desiredImage, testutil.WithBootedDigest(otherDigest), - testutil.WithNodeCondition(bootcv1alpha1.NodeIdle, metav1.ConditionFalse, bootcv1alpha1.NodeReasonRebooting), - testutil.WithNodeAnnotation(bootcv1alpha1.AnnotationInRebootSlot, "")), + testutil.WithNodeCondition( + bootcv1alpha1.NodeIdle, + metav1.ConditionFalse, + bootcv1alpha1.NodeReasonRebooting, + ), + testutil.WithNodeAnnotation(bootcv1alpha1.AnnotationInRebootSlot, ""), + ), } rs := buildRolloutState(logr.Discard(), nodes) @@ -186,14 +221,18 @@ func TestClassifyNode(t *testing.T) { { name: "UpToDate: image matches, Idle=True", bootedDigest: desiredDigest, - conditions: []metav1.Condition{idleCond(metav1.ConditionTrue, bootcv1alpha1.NodeReasonIdle)}, - want: nodeStateUpToDate, + conditions: []metav1.Condition{ + idleCond(metav1.ConditionTrue, bootcv1alpha1.NodeReasonIdle), + }, + want: nodeStateUpToDate, }, { name: "Pending: image differs, Idle=True (daemon hasn't reacted)", bootedDigest: otherDigest, - conditions: []metav1.Condition{idleCond(metav1.ConditionTrue, bootcv1alpha1.NodeReasonIdle)}, - want: nodeStatePending, + conditions: []metav1.Condition{ + idleCond(metav1.ConditionTrue, bootcv1alpha1.NodeReasonIdle), + }, + want: nodeStatePending, }, { name: "Pending: no booted status yet (daemon starting)", @@ -210,20 +249,26 @@ func TestClassifyNode(t *testing.T) { { name: "Staging: image differs, Idle=False reason=Staging", bootedDigest: otherDigest, - conditions: []metav1.Condition{idleCond(metav1.ConditionFalse, bootcv1alpha1.NodeReasonStaging)}, - want: nodeStateStaging, + conditions: []metav1.Condition{ + idleCond(metav1.ConditionFalse, bootcv1alpha1.NodeReasonStaging), + }, + want: nodeStateStaging, }, { name: "Staged: image differs, Idle=False reason=Staged", bootedDigest: otherDigest, - conditions: []metav1.Condition{idleCond(metav1.ConditionFalse, bootcv1alpha1.NodeReasonStaged)}, - want: nodeStateStaged, + conditions: []metav1.Condition{ + idleCond(metav1.ConditionFalse, bootcv1alpha1.NodeReasonStaged), + }, + want: nodeStateStaged, }, { name: "Rebooting: image differs, Idle=False reason=Rebooting", bootedDigest: otherDigest, - conditions: []metav1.Condition{idleCond(metav1.ConditionFalse, bootcv1alpha1.NodeReasonRebooting)}, - want: nodeStateRebooting, + conditions: []metav1.Condition{ + idleCond(metav1.ConditionFalse, bootcv1alpha1.NodeReasonRebooting), + }, + want: nodeStateRebooting, }, { name: "Staging: Degraded=False does not affect classification", diff --git a/internal/controller/status.go b/internal/controller/status.go index 62eec56..83fc2fe 100644 --- a/internal/controller/status.go +++ b/internal/controller/status.go @@ -21,7 +21,9 @@ func syncPoolStatus(pool *bootcv1alpha1.BootcNodePool, rs *rolloutState) { pool.Status.ObservedGeneration = pool.Generation pool.Status.NodeCount = int32(rs.nodeCount()) pool.Status.UpdatedCount = int32(len(rs.upToDate)) - pool.Status.UpdatingCount = int32(len(rs.pending) + len(rs.staging) + len(rs.staged) + len(rs.rebooting)) + pool.Status.UpdatingCount = int32( + len(rs.pending) + len(rs.staging) + len(rs.staged) + len(rs.rebooting), + ) pool.Status.DegradedCount = int32(len(rs.degraded)) if pool.Status.NodeCount == pool.Status.UpdatedCount { diff --git a/internal/controller/status_test.go b/internal/controller/status_test.go index 4402401..7568b51 100644 --- a/internal/controller/status_test.go +++ b/internal/controller/status_test.go @@ -79,7 +79,10 @@ func TestSyncPoolStatusRolloutInProgress(t *testing.T) { HaveField("Type", bootcv1alpha1.PoolUpToDate), HaveField("Status", metav1.ConditionFalse), HaveField("Reason", bootcv1alpha1.PoolRolloutInProgress), - HaveField("Message", Equal("1/5 updated; 1 pending, 1 staging, 1 staged, 1 rebooting, 0 degraded")), + HaveField( + "Message", + Equal("1/5 updated; 1 pending, 1 staging, 1 staged, 1 rebooting, 0 degraded"), + ), ))) } @@ -88,7 +91,12 @@ func TestSyncPoolStatusRolloutInProgress(t *testing.T) { func TestSyncPoolStatusPaused(t *testing.T) { g := NewWithT(t) - pool := testutil.NewPool("test", testImageDigestRefA, testutil.WithWorkerSelector(), testutil.WithPaused(true)) + pool := testutil.NewPool( + "test", + testImageDigestRefA, + testutil.WithWorkerSelector(), + testutil.WithPaused(true), + ) pool.Status.TargetDigest = testDigestA rs := &rolloutState{ @@ -110,7 +118,10 @@ func TestSyncPoolStatusPaused(t *testing.T) { HaveField("Type", bootcv1alpha1.PoolUpToDate), HaveField("Status", metav1.ConditionFalse), HaveField("Reason", bootcv1alpha1.PoolPaused), - HaveField("Message", Equal("1/3 updated; 2 pending, 0 staging, 0 staged, 0 rebooting, 0 degraded")), + HaveField( + "Message", + Equal("1/3 updated; 2 pending, 0 staging, 0 staged, 0 rebooting, 0 degraded"), + ), ))) } diff --git a/internal/daemon/reconciler.go b/internal/daemon/reconciler.go index d670e20..2f77c99 100644 --- a/internal/daemon/reconciler.go +++ b/internal/daemon/reconciler.go @@ -75,7 +75,10 @@ func (r *BootcNodeReconciler) SetupWithManager(mgr ctrl.Manager) error { Complete(r) } -func (r *BootcNodeReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { +func (r *BootcNodeReconciler) Reconcile( + ctx context.Context, + req ctrl.Request, +) (ctrl.Result, error) { log := logf.FromContext(ctx).WithValues("node", r.NodeName) if req.Name != r.NodeName { @@ -153,7 +156,10 @@ type reconcileResult struct { // reconcileBootcNode defines the result of the reconcile of the bootc nodes. It returns the results for the reconcile, // the degraded message and eventual errors. We distinguish the degraded message from a reconcile error since we want to // implement an exponential back-off if the staging failed. -func (r *BootcNodeReconciler) reconcileBootcNode(ctx context.Context, bn *bootcv1alpha1.BootcNode) (reconcileResult, error) { +func (r *BootcNodeReconciler) reconcileBootcNode( + ctx context.Context, + bn *bootcv1alpha1.BootcNode, +) (reconcileResult, error) { log := logf.FromContext(ctx).WithValues("node", r.NodeName) if err := r.populateBootcFields(ctx, bn); err != nil { @@ -284,7 +290,12 @@ func (s *stageOp) reset() { // there's multiple rapid image changes (e.g. B→C→D spawns three goroutines). // This is harmless: each cancelled goroutine checks ctx.Err() after acquiring // the lock and exits immediately without starting a process. -func (s *stageOp) run(ctx context.Context, nodeName, image string, executor bootc.Executor, done chan<- event.GenericEvent) { +func (s *stageOp) run( + ctx context.Context, + nodeName, image string, + executor bootc.Executor, + done chan<- event.GenericEvent, +) { s.runMu.Lock() defer s.runMu.Unlock() @@ -320,7 +331,10 @@ func (s *stageOp) run(ctx context.Context, nodeName, image string, executor boot } } -func (r *BootcNodeReconciler) populateBootcFields(ctx context.Context, bn *bootcv1alpha1.BootcNode) error { +func (r *BootcNodeReconciler) populateBootcFields( + ctx context.Context, + bn *bootcv1alpha1.BootcNode, +) error { status, err := r.StatusWatcher.GetStatus(ctx) if err != nil { return fmt.Errorf("getting bootc status: %w", err) @@ -344,7 +358,11 @@ const ( actionReboot // staged + approved, issue reboot ) -func (r *BootcNodeReconciler) classifyAction(bn *bootcv1alpha1.BootcNode, digested reference.Digested, desiredImage string) updateAction { +func (r *BootcNodeReconciler) classifyAction( + bn *bootcv1alpha1.BootcNode, + digested reference.Digested, + desiredImage string, +) updateAction { desiredDigest := digested.Digest().String() alreadyStaged := bn.Status.Staged != nil && bn.Status.Staged.ImageDigest == desiredDigest if !alreadyStaged { diff --git a/internal/daemon/reconciler_test.go b/internal/daemon/reconciler_test.go index 1827efb..81f654a 100644 --- a/internal/daemon/reconciler_test.go +++ b/internal/daemon/reconciler_test.go @@ -41,7 +41,10 @@ func TestReconcilePopulatesStatus(t *testing.T) { fake.status.Status.Booted.Image.Version = &v1 fake.status.Status.Staged = &bootc.BootEntry{ Image: &bootc.ImageStatus{ - Image: bootc.ImageReference{Image: testutil.ImageTaggedRef, Transport: "registry"}, + Image: bootc.ImageReference{ + Image: testutil.ImageTaggedRef, + Transport: "registry", + }, ImageDigest: testutil.DigestB, Version: &v2, Architecture: "amd64", @@ -50,7 +53,10 @@ func TestReconcilePopulatesStatus(t *testing.T) { } fake.status.Status.Rollback = &bootc.BootEntry{ Image: &bootc.ImageStatus{ - Image: bootc.ImageReference{Image: testutil.ImageTaggedRef, Transport: "registry"}, + Image: bootc.ImageReference{ + Image: testutil.ImageTaggedRef, + Transport: "registry", + }, ImageDigest: testutil.DigestC, Version: &v3, Architecture: "amd64", @@ -124,7 +130,12 @@ func TestReconcileBootcStatusError(t *testing.T) { HaveField("Type", bootcv1alpha1.NodeDegraded), HaveField("Status", metav1.ConditionTrue), HaveField("Reason", bootcv1alpha1.NodeReasonError), - HaveField("Message", Equal(fmt.Sprintf("populating bootc fields: getting bootc status: %s", bootcStatusErrMsg))), + HaveField( + "Message", + Equal( + fmt.Sprintf("populating bootc fields: getting bootc status: %s", bootcStatusErrMsg), + ), + ), ))) } @@ -243,7 +254,11 @@ func TestRebootingSet(t *testing.T) { fake.status = newBootcStatus(testutil.DigestA) fake.status.Status.Staged = newBootEntry(testutil.ImageDigestRefB, testutil.DigestB) - bn := testutil.NewNode(testNodeName, testutil.ImageDigestRefB, testutil.WithDesiredImageState(bootcv1alpha1.DesiredImageStateBooted)) + bn := testutil.NewNode( + testNodeName, + testutil.ImageDigestRefB, + testutil.WithDesiredImageState(bootcv1alpha1.DesiredImageStateBooted), + ) g.Expect(k8sClient.Create(ctx, bn)).To(Succeed()) t.Cleanup(func() { _ = k8sClient.Delete(ctx, bn) diff --git a/internal/daemon/watcher_test.go b/internal/daemon/watcher_test.go index b431c60..ccf59a6 100644 --- a/internal/daemon/watcher_test.go +++ b/internal/daemon/watcher_test.go @@ -124,7 +124,11 @@ func TestWatcherEvents(t *testing.T) { func TestWatcherCachesStatus(t *testing.T) { dir := t.TempDir() - w := newTestWatcher(filepath.Join(dir, "nonexistent"), filepath.Join(dir, "nonexistent2"), 200*time.Millisecond) + w := newTestWatcher( + filepath.Join(dir, "nonexistent"), + filepath.Join(dir, "nonexistent2"), + 200*time.Millisecond, + ) done, cancel := startWatcher(t, w) defer cancel() @@ -144,7 +148,11 @@ func TestWatcherCachesStatus(t *testing.T) { t.Fatal("expected booted entry in cached status") } if status.Status.Booted.Image.ImageDigest != testutil.DigestA { - t.Errorf("expected digest %s, got %s", testutil.DigestA, status.Status.Booted.Image.ImageDigest) + t.Errorf( + "expected digest %s, got %s", + testutil.DigestA, + status.Status.Booted.Image.ImageDigest, + ) } // Change the executor's data to simulate a stale cache where @@ -160,7 +168,11 @@ func TestWatcherCachesStatus(t *testing.T) { t.Fatalf("GetStatus returned error after executor change: %v", err) } if status.Status.Booted.Image.ImageDigest != testutil.DigestA { - t.Errorf("expected cached digest %s, got %s", testutil.DigestA, status.Status.Booted.Image.ImageDigest) + t.Errorf( + "expected cached digest %s, got %s", + testutil.DigestA, + status.Status.Booted.Image.ImageDigest, + ) } cancel() @@ -183,7 +195,8 @@ func TestWatcherGetStatusColdCache(t *testing.T) { if err != nil { t.Fatalf("GetStatus returned error: %v", err) } - if status.Status.Booted == nil || status.Status.Booted.Image == nil || status.Status.Booted.Image.ImageDigest != testutil.DigestA { + if status.Status.Booted == nil || status.Status.Booted.Image == nil || + status.Status.Booted.Image.ImageDigest != testutil.DigestA { t.Fatalf("expected booted digest %s", testutil.DigestA) } } diff --git a/test/e2e/bootcnode_test.go b/test/e2e/bootcnode_test.go index 120b755..5c2c330 100644 --- a/test/e2e/bootcnode_test.go +++ b/test/e2e/bootcnode_test.go @@ -95,7 +95,8 @@ func TestControllerMembership(t *testing.T) { )) // Verify pool status reflects steady state. - g.Eventually(fetchPoolStatus(ctx, env.Client, pool)).Should(poolAllUpdated(1, env.NodeImageDigest())) + g.Eventually(fetchPoolStatus(ctx, env.Client, pool)). + Should(poolAllUpdated(1, env.NodeImageDigest())) } // TestUpdateReboot provisions a worker node, creates a pool with the @@ -188,7 +189,8 @@ func TestUpdateReboot(t *testing.T) { t.Logf("Node %q is Idle with update image", nodeName) // Verify pool status after rollout completes. - g.Eventually(fetchPoolStatus(ctx, env.Client, pool)).Should(poolAllUpdated(1, env.NodeImageUpdateDigest())) + g.Eventually(fetchPoolStatus(ctx, env.Client, pool)). + Should(poolAllUpdated(1, env.NodeImageUpdateDigest())) // Phase 5: Verify node is schedulable (uncordoned after reboot). g.Eventually(func() (bool, error) { @@ -244,7 +246,8 @@ func TestUpdateReboot(t *testing.T) { "stat", "/proc/1/root/usr/share/update-marker") out, err := cmd.CombinedOutput() g.Expect(err).NotTo(HaveOccurred(), - fmt.Sprintf("expected update-marker to exist on host, kubectl exec output: %s", string(out))) + fmt.Sprintf("expected update-marker to exist on host, kubectl exec output: %s", string(out)), + ) t.Logf("Verified update-marker exists on host via daemon pod") @@ -278,7 +281,8 @@ func TestUpdateReboot(t *testing.T) { t.Logf("Node %q successfully rolled back to original image", nodeName) // Verify pool status after rollback completes. - g.Eventually(fetchPoolStatus(ctx, env.Client, pool)).Should(poolAllUpdated(1, env.NodeImageDigest())) + g.Eventually(fetchPoolStatus(ctx, env.Client, pool)). + Should(poolAllUpdated(1, env.NodeImageDigest())) } // TestTagResolution creates a pool with a tag-based image ref, verifies @@ -490,7 +494,8 @@ func TestPauseResume(t *testing.T) { t.Logf("Node %q completed update after resume", nodeName) // Verify pool status after resume completes. - g.Eventually(fetchPoolStatus(ctx, env.Client, pool)).Should(poolAllUpdated(1, env.NodeImageUpdateDigest())) + g.Eventually(fetchPoolStatus(ctx, env.Client, pool)). + Should(poolAllUpdated(1, env.NodeImageUpdateDigest())) } // TestNonExistingImage provisions a worker node, creates a pool with the @@ -577,7 +582,11 @@ func TestNonExistingImage(t *testing.T) { t.Logf("Verified node %q did not stage non-existing image", nodeName) } -func fetchPoolStatus(ctx context.Context, c client.Client, pool *bootcv1alpha1.BootcNodePool) func() (bootcv1alpha1.BootcNodePoolStatus, error) { +func fetchPoolStatus( + ctx context.Context, + c client.Client, + pool *bootcv1alpha1.BootcNodePool, +) func() (bootcv1alpha1.BootcNodePoolStatus, error) { return func() (bootcv1alpha1.BootcNodePoolStatus, error) { var p bootcv1alpha1.BootcNodePool err := c.Get(ctx, client.ObjectKeyFromObject(pool), &p) diff --git a/test/e2e/e2eutil/env.go b/test/e2e/e2eutil/env.go index aa55d83..330e7f3 100644 --- a/test/e2e/e2eutil/env.go +++ b/test/e2e/e2eutil/env.go @@ -164,7 +164,9 @@ func (e *Env) AddNode(t *testing.T, opts ...NodeOption) string { if cfg.targetImgRef == "" { if e.nodeImageRegistry == "" || e.nodeImageDigest == "" { - t.Fatal("BINK_LOCAL_REGISTRY_NODE_IMAGE and NODE_IMAGE_DIGEST must be set (or use WithTargetImgRef)") + t.Fatal( + "BINK_LOCAL_REGISTRY_NODE_IMAGE and NODE_IMAGE_DIGEST must be set (or use WithTargetImgRef)", + ) } cfg.targetImgRef = e.nodeImageRegistry + "@" + e.nodeImageDigest } @@ -201,7 +203,10 @@ func (e *Env) AddNode(t *testing.T, opts ...NodeOption) string { // The pool is labeled with LabelE2ETest for cleanup. If no // WithNodeSelector option is provided, it defaults to selecting nodes // with LabelE2ETest (i.e. all nodes belonging to this test). -func (e *Env) NewPool(suffix, imageRef string, opts ...testutil.PoolOption) *bootcv1alpha1.BootcNodePool { +func (e *Env) NewPool( + suffix, imageRef string, + opts ...testutil.PoolOption, +) *bootcv1alpha1.BootcNodePool { defaults := []testutil.PoolOption{ testutil.WithLabel(LabelE2ETest, e.testID), testutil.WithNodeSelector(e.TestLabels()), @@ -285,12 +290,24 @@ func (e *Env) cleanup(t *testing.T) { ctx := context.Background() t.Logf("Removing pools with label %s=%s...", LabelE2ETest, e.testID) - if err := e.Client.DeleteAllOf(ctx, &bootcv1alpha1.BootcNodePool{}, client.MatchingLabels(e.TestLabels())); err != nil { + if err := e.Client.DeleteAllOf( + ctx, + &bootcv1alpha1.BootcNodePool{}, + client.MatchingLabels(e.TestLabels()), + ); err != nil { t.Logf("WARNING: pool cleanup: %v", err) } for _, name := range e.nodes { t.Logf("Removing node %q...", name) - if err := runBink(t, "node", "remove", name, "--force", "--cluster-name", e.clusterName); err != nil { + if err := runBink( + t, + "node", + "remove", + name, + "--force", + "--cluster-name", + e.clusterName, + ); err != nil { t.Logf("WARNING: failed to remove node %q: %v", name, err) } } diff --git a/test/util/builders.go b/test/util/builders.go index 21d9c2e..1e291be 100644 --- a/test/util/builders.go +++ b/test/util/builders.go @@ -166,11 +166,10 @@ func WithBootedDigest(digest string) NodeOption { // WithNodeCondition appends a condition to the node's status. func WithNodeCondition(condType string, status metav1.ConditionStatus, reason string) NodeOption { return func(node *bootcv1alpha1.BootcNode) { - node.Status.Conditions = append(node.Status.Conditions, metav1.Condition{ - Type: condType, - Status: status, - Reason: reason, - }) + node.Status.Conditions = append( + node.Status.Conditions, + metav1.Condition{Type: condType, Status: status, Reason: reason}, + ) } }