diff --git a/operators/constellation-node-operator/controllers/nodeversion_controller.go b/operators/constellation-node-operator/controllers/nodeversion_controller.go index fc185b1e2f..f9526c3cf9 100644 --- a/operators/constellation-node-operator/controllers/nodeversion_controller.go +++ b/operators/constellation-node-operator/controllers/nodeversion_controller.go @@ -138,7 +138,7 @@ func (r *NodeVersionReconciler) Reconcile(ctx context.Context, req ctrl.Request) logr.Error(err, "Unable to list scaling groups") return ctrl.Result{}, err } - scalingGroupByID := make(map[string]updatev1alpha1.ScalingGroup, len(scalingGroupList.Items)) + scalingGroupByID := make(scalingGroupByID, len(scalingGroupList.Items)) for _, scalingGroup := range scalingGroupList.Items { scalingGroupByID[strings.ToLower(scalingGroup.Spec.GroupID)] = scalingGroup } @@ -214,7 +214,7 @@ func (r *NodeVersionReconciler) Reconcile(ctx context.Context, req ctrl.Request) return ctrl.Result{Requeue: shouldRequeue}, nil } - newNodeConfig := newNodeConfig{desiredNodeVersion, groups.Outdated, pendingNodeList.Items, scalingGroupByID, newNodesBudget} + newNodeConfig := newNodeConfig{desiredNodeVersion, groups.Outdated, groups.Donors, pendingNodeList.Items, scalingGroupByID, newNodesBudget} if err := r.createNewNodes(ctx, newNodeConfig); err != nil { logr.Error(err, "Creating new nodes") return ctrl.Result{Requeue: shouldRequeue}, nil @@ -614,6 +614,14 @@ func (r *NodeVersionReconciler) createNewNodes(ctx context.Context, config newNo if config.newNodesBudget < 1 || len(config.outdatedNodes) == 0 { return nil } + hasOutdatedControlPlanes := func() bool { + for _, entry := range append(config.outdatedNodes, config.donors...) { + if nodeutil.IsControlPlaneNode(&entry) { + return true + } + } + return false + }() outdatedNodesPerScalingGroup := make(map[string]int) for _, node := range config.outdatedNodes { // skip outdated nodes that got assigned an heir in this Reconcile call @@ -637,7 +645,7 @@ func (r *NodeVersionReconciler) createNewNodes(ctx context.Context, config newNo requiredNodesPerScalingGroup[scalingGroupID] = outdatedNodesPerScalingGroup[scalingGroupID] - pendingJoiningNodesPerScalingGroup[scalingGroupID] } } - for scalingGroupID := range requiredNodesPerScalingGroup { + for _, scalingGroupID := range config.scalingGroupByID.getScalingGroupIDsSorted() { scalingGroup, ok := config.scalingGroupByID[scalingGroupID] if !ok { logr.Info("Scaling group does not have matching resource", "scalingGroup", scalingGroupID, "scalingGroups", config.scalingGroupByID) @@ -648,6 +656,12 @@ func (r *NodeVersionReconciler) createNewNodes(ctx context.Context, config newNo continue } if requiredNodesPerScalingGroup[scalingGroupID] == 0 { + logr.Info("No new nodes needed for scaling group", "scalingGroup", scalingGroupID) + continue + } + // if we are a worker group and still have outdated control planes, we must wait for them to be upgraded. + if hasOutdatedControlPlanes && scalingGroup.Spec.Role != updatev1alpha1.ControlPlaneRole { + logr.Info("There are still outdated control plane nodes which must be replaced first before this worker scaling group is upgraded", "scalingGroup", scalingGroupID) continue } for { @@ -679,7 +693,7 @@ func (r *NodeVersionReconciler) createNewNodes(ctx context.Context, config newNo if err := r.Create(ctx, pendingNode); err != nil { return err } - logr.Info("Created new node", "createdNode", nodeName, "scalingGroup", scalingGroupID) + logr.Info("Created new node", "createdNode", nodeName, "scalingGroup", scalingGroupID, "requiredNodes", requiredNodesPerScalingGroup[scalingGroupID]) requiredNodesPerScalingGroup[scalingGroupID]-- config.newNodesBudget-- } @@ -939,7 +953,25 @@ type kubernetesServerVersionGetter interface { type newNodeConfig struct { desiredNodeVersion updatev1alpha1.NodeVersion outdatedNodes []corev1.Node + donors []corev1.Node pendingNodes []updatev1alpha1.PendingNode - scalingGroupByID map[string]updatev1alpha1.ScalingGroup + scalingGroupByID scalingGroupByID newNodesBudget int } + +// scalingGroupByID maps scaling group IDs to their configuration. +type scalingGroupByID map[string]updatev1alpha1.ScalingGroup + +// getScalingGroupIDsSorted returns the group IDs of all scaling groups with +// scaling groups with the role "control-plane" first. +func (s scalingGroupByID) getScalingGroupIDsSorted() []string { + var controlPlaneGroupIDs, otherGroupIDs []string + for id := range s { + if s[id].Spec.Role == updatev1alpha1.ControlPlaneRole { + controlPlaneGroupIDs = append(controlPlaneGroupIDs, id) + } else { + otherGroupIDs = append(otherGroupIDs, id) + } + } + return append(controlPlaneGroupIDs, otherGroupIDs...) +} diff --git a/operators/constellation-node-operator/controllers/nodeversion_controller_test.go b/operators/constellation-node-operator/controllers/nodeversion_controller_test.go index c9ae880421..3817d88107 100644 --- a/operators/constellation-node-operator/controllers/nodeversion_controller_test.go +++ b/operators/constellation-node-operator/controllers/nodeversion_controller_test.go @@ -330,13 +330,14 @@ func TestMatchDonorsAndHeirs(t *testing.T) { func TestCreateNewNodes(t *testing.T) { testCases := map[string]struct { outdatedNodes []corev1.Node + donors []corev1.Node pendingNodes []updatev1alpha1.PendingNode - scalingGroupByID map[string]updatev1alpha1.ScalingGroup + scalingGroupByID scalingGroupByID budget int wantCreateCalls []string }{ "no outdated nodes": { - scalingGroupByID: map[string]updatev1alpha1.ScalingGroup{ + scalingGroupByID: scalingGroupByID{ "scaling-group": { Spec: updatev1alpha1.ScalingGroupSpec{ GroupID: "scaling-group", @@ -359,7 +360,7 @@ func TestCreateNewNodes(t *testing.T) { }, }, }, - scalingGroupByID: map[string]updatev1alpha1.ScalingGroup{ + scalingGroupByID: scalingGroupByID{ "scaling-group": { Spec: updatev1alpha1.ScalingGroupSpec{ GroupID: "scaling-group", @@ -383,7 +384,7 @@ func TestCreateNewNodes(t *testing.T) { }, }, }, - scalingGroupByID: map[string]updatev1alpha1.ScalingGroup{ + scalingGroupByID: scalingGroupByID{ "scaling-group": { Spec: updatev1alpha1.ScalingGroupSpec{ GroupID: "scaling-group", @@ -407,7 +408,7 @@ func TestCreateNewNodes(t *testing.T) { }, }, }, - scalingGroupByID: map[string]updatev1alpha1.ScalingGroup{ + scalingGroupByID: scalingGroupByID{ "scaling-group": { Spec: updatev1alpha1.ScalingGroupSpec{ GroupID: "scaling-group", @@ -429,7 +430,7 @@ func TestCreateNewNodes(t *testing.T) { }, }, }, - scalingGroupByID: map[string]updatev1alpha1.ScalingGroup{ + scalingGroupByID: scalingGroupByID{ "scaling-group": { Spec: updatev1alpha1.ScalingGroupSpec{ GroupID: "scaling-group", @@ -460,7 +461,7 @@ func TestCreateNewNodes(t *testing.T) { }, }, }, - scalingGroupByID: map[string]updatev1alpha1.ScalingGroup{ + scalingGroupByID: scalingGroupByID{ "scaling-group": { Spec: updatev1alpha1.ScalingGroupSpec{ GroupID: "scaling-group", @@ -491,7 +492,7 @@ func TestCreateNewNodes(t *testing.T) { }, }, }, - scalingGroupByID: map[string]updatev1alpha1.ScalingGroup{ + scalingGroupByID: scalingGroupByID{ "scaling-group": { Spec: updatev1alpha1.ScalingGroupSpec{ GroupID: "scaling-group", @@ -516,7 +517,7 @@ func TestCreateNewNodes(t *testing.T) { }, }, }, - scalingGroupByID: map[string]updatev1alpha1.ScalingGroup{ + scalingGroupByID: scalingGroupByID{ "scaling-group": { Spec: updatev1alpha1.ScalingGroupSpec{ GroupID: "scaling-group", @@ -539,7 +540,7 @@ func TestCreateNewNodes(t *testing.T) { }, }, }, - scalingGroupByID: map[string]updatev1alpha1.ScalingGroup{ + scalingGroupByID: scalingGroupByID{ "scaling-group": { Spec: updatev1alpha1.ScalingGroupSpec{ GroupID: "scaling-group", @@ -573,6 +574,105 @@ func TestCreateNewNodes(t *testing.T) { }, budget: 1, }, + "control plane node upgraded first": { + outdatedNodes: []corev1.Node{ + // CP node + { + ObjectMeta: metav1.ObjectMeta{ + Name: "control-plane-node", + Annotations: map[string]string{ + scalingGroupAnnotation: "control-plane-scaling-group", + }, + Labels: map[string]string{ + // Mark this as a CP node as per + // https://kubernetes.io/docs/reference/labels-annotations-taints/#node-role-kubernetes-io-control-plane + "node-role.kubernetes.io/control-plane": "", + }, + }, + }, + // Worker node + { + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + Annotations: map[string]string{ + scalingGroupAnnotation: "scaling-group", + }, + }, + }, + }, + scalingGroupByID: scalingGroupByID{ + "scaling-group": { + Spec: updatev1alpha1.ScalingGroupSpec{ + GroupID: "scaling-group", + Role: updatev1alpha1.WorkerRole, + }, + Status: updatev1alpha1.ScalingGroupStatus{ + ImageReference: "image", + }, + }, + "control-plane-scaling-group": { + Spec: updatev1alpha1.ScalingGroupSpec{ + GroupID: "control-plane-scaling-group", + Role: updatev1alpha1.ControlPlaneRole, + }, + Status: updatev1alpha1.ScalingGroupStatus{ + ImageReference: "image", + }, + }, + }, + budget: 2, + wantCreateCalls: []string{"control-plane-scaling-group"}, + }, + "worker not upgraded while cp is in donors": { + donors: []corev1.Node{ + // CP node + { + ObjectMeta: metav1.ObjectMeta{ + Name: "control-plane-node", + Annotations: map[string]string{ + scalingGroupAnnotation: "control-plane-scaling-group", + }, + Labels: map[string]string{ + // Mark this as a CP node as per + // https://kubernetes.io/docs/reference/labels-annotations-taints/#node-role-kubernetes-io-control-plane + "node-role.kubernetes.io/control-plane": "", + }, + }, + }, + }, + outdatedNodes: []corev1.Node{ + // Worker node + { + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + Annotations: map[string]string{ + scalingGroupAnnotation: "scaling-group", + }, + }, + }, + }, + scalingGroupByID: scalingGroupByID{ + "scaling-group": { + Spec: updatev1alpha1.ScalingGroupSpec{ + GroupID: "scaling-group", + Role: updatev1alpha1.WorkerRole, + }, + Status: updatev1alpha1.ScalingGroupStatus{ + ImageReference: "image", + }, + }, + "control-plane-scaling-group": { + Spec: updatev1alpha1.ScalingGroupSpec{ + GroupID: "control-plane-scaling-group", + Role: updatev1alpha1.ControlPlaneRole, + }, + Status: updatev1alpha1.ScalingGroupStatus{ + ImageReference: "image", + }, + }, + }, + budget: 1, + }, } for name, tc := range testCases { @@ -592,7 +692,7 @@ func TestCreateNewNodes(t *testing.T) { }, Scheme: getScheme(t), } - newNodeConfig := newNodeConfig{desiredNodeImage, tc.outdatedNodes, tc.pendingNodes, tc.scalingGroupByID, tc.budget} + newNodeConfig := newNodeConfig{desiredNodeImage, tc.outdatedNodes, tc.donors, tc.pendingNodes, tc.scalingGroupByID, tc.budget} err := reconciler.createNewNodes(context.Background(), newNodeConfig) require.NoError(err) assert.Equal(tc.wantCreateCalls, reconciler.nodeReplacer.(*stubNodeReplacerWriter).createCalls)