node-controller: Support an annotation to hold/prioritize updates

pkg/controller/node/node_controller.go

@@ -5,6 +5,8 @@ import (
 	"encoding/json"
 	"fmt"
 	"reflect"
+	"sort"
+	"strconv"
 	"time"
 
 	"github.com/golang/glog"
@@ -866,7 +868,7 @@ func getAllCandidateMachines(pool *mcfgv1.MachineConfigPool, nodesInPool []*core
 	}
 	capacity -= failingThisConfig
 
-	return nodes, uint(capacity)
+	return prioritizeCandidateNodes(nodes), uint(capacity)
 }
 
 // getCandidateMachines returns the maximum subset of nodes which can be updated to the target config given availability constraints.
@@ -878,46 +880,45 @@ func getCandidateMachines(pool *mcfgv1.MachineConfigPool, nodesInPool []*corev1.
 	return nodes[:capacity]
 }
 
-// getCurrentEtcdLeader is not yet implemented
-func (ctrl *Controller) getCurrentEtcdLeader(candidates []*corev1.Node) (*corev1.Node, error) {
-	return nil, nil
+type nodeOrdering struct {
+	nodes []*corev1.Node
+	order map[*corev1.Node]int
 }
 
-// filterControlPlaneCandidateNodes adjusts the candidates and capacity specifically
-// for the control plane, e.g. based on which node is the etcd leader at the time.
-// nolint:unparam
-func (ctrl *Controller) filterControlPlaneCandidateNodes(pool *mcfgv1.MachineConfigPool, candidates []*corev1.Node, capacity uint) ([]*corev1.Node, uint, error) {
-	if len(candidates) <= 1 {
-		return candidates, capacity, nil
-	}
-	etcdLeader, err := ctrl.getCurrentEtcdLeader(candidates)
-	if err != nil {
-		glog.Warningf("Failed to find current etcd leader (continuing anyways): %v", err)
-	}
-	var newCandidates []*corev1.Node
+func (p nodeOrdering) Len() int           { return len(p.nodes) }
+func (p nodeOrdering) Less(i, j int) bool { return p.order[p.nodes[i]] > p.order[p.nodes[j]] }
+func (p nodeOrdering) Swap(i, j int)      { p.nodes[i], p.nodes[j] = p.nodes[j], p.nodes[i] }
+
+// prioritizeCandidateNodes chooses the update ordering
+func prioritizeCandidateNodes(candidates []*corev1.Node) []*corev1.Node {
+	var newCandidates nodeOrdering
+	newCandidates.order = make(map[*corev1.Node]int)
 	for _, node := range candidates {
-		if node == etcdLeader {
-			// For now make this an event so we know it's working, even though it's more of a non-event
-			ctrl.eventRecorder.Eventf(pool, corev1.EventTypeNormal, "DeferringEtcdLeaderUpdate", "Deferring update of etcd leader %s", node.Name)
-			glog.Infof("Deferring update of etcd leader: %s", node.Name)
-			continue
+		orderVal, ok := node.Annotations[daemonconsts.MachineUpdateOrderingAnnotationKey]
+		if ok {
+			order, err := strconv.Atoi(orderVal)
+			if err != nil {
+				glog.Warningf("Failed to parse %s %s: %v", node.Name, daemonconsts.MachineUpdateOrderingAnnotationKey, err)
+				continue
+			}
+			// order 0 means "skip this node"
+			if order == 0 {
+				continue
+			}
+			if order < 0 {
+				glog.Warningf("Invalid negative ordering %s for node %s", orderVal, node.Name)
+				continue
+			}
+			newCandidates.order[node] = order
 		}
-		newCandidates = append(newCandidates, node)
+		newCandidates.nodes = append(newCandidates.nodes, node)
 	}
-	return newCandidates, capacity, nil
+	sort.Sort(newCandidates)
+	return newCandidates.nodes
 }
 
 // updateCandidateMachines sets the desiredConfig annotation the candidate machines
 func (ctrl *Controller) updateCandidateMachines(pool *mcfgv1.MachineConfigPool, candidates []*corev1.Node, capacity uint) error {
-	if pool.Name == masterPoolName {
-		var err error
-		candidates, capacity, err = ctrl.filterControlPlaneCandidateNodes(pool, candidates, capacity)
-		if err != nil {
-			return err
-		}
-		// In practice right now these counts will be 1 but let's stay general to support 5 etcd nodes in the future
-		ctrl.logPool(pool, "filtered to %d candidate nodes for update, capacity: %d", len(candidates), capacity)
-	}
 	if capacity < uint(len(candidates)) {
 		// Arbitrarily pick the first N candidates; no attempt at sorting.
 		// Perhaps later we allow admins to weight somehow, or do something more intelligent.

pkg/controller/node/node_controller_test.go

@@ -632,6 +632,23 @@ func TestGetCandidateMachines(t *testing.T) {
 		expected:        []string{"node-3", "node-4"},
 		otherCandidates: []string{"node-5", "node-6"},
 		capacity:        2,
+	}, {
+		// A test with more nodes in mixed order
+		progress: 4,
+		nodes: []*corev1.Node{
+			newNodeWithReady("node-0", "v1", "v1", corev1.ConditionTrue),
+			newNodeWithReady("node-1", "v1", "v1", corev1.ConditionFalse),
+			newNodeWithReady("node-2", "v0", "v1", corev1.ConditionTrue),
+			newNodeWithReadyAndOrder("node-3", "v0", "v0", 0, corev1.ConditionTrue),
+			newNodeWithReadyAndOrder("node-4", "v0", "v0", 5, corev1.ConditionTrue),
+			newNodeWithReadyAndOrder("node-5", "v0", "v0", 10, corev1.ConditionTrue),
+			newNodeWithReady("node-6", "v0", "v0", corev1.ConditionTrue),
+			newNodeWithReady("node-7", "v1", "v1", corev1.ConditionTrue),
+			newNodeWithReady("node-8", "v1", "v1", corev1.ConditionTrue),
+		},
+		expected:        []string{"node-5", "node-4"},
+		otherCandidates: []string{"node-6"},
+		capacity:        2,
 	}}
 
 	for idx, test := range tests {

pkg/controller/node/status_test.go

@@ -108,6 +108,16 @@ func newNodeWithReady(name string, currentConfig, desiredConfig string, status c
 	return node
 }
 
+func newNodeWithReadyAndOrder(name string, currentConfig, desiredConfig string, order int, status corev1.ConditionStatus) *corev1.Node {
+	node := newNode(name, currentConfig, desiredConfig)
+	node.Status = corev1.NodeStatus{Conditions: []corev1.NodeCondition{{Type: corev1.NodeReady, Status: status}}}
+	if node.Annotations == nil {
+		node.Annotations = map[string]string{}
+	}
+	node.Annotations[daemonconsts.MachineUpdateOrderingAnnotationKey] = fmt.Sprintf("%d", order)
+	return node
+}
+
 func newNodeWithReadyAndDaemonState(name string, currentConfig, desiredConfig string, status corev1.ConditionStatus, dstate string) *corev1.Node {
 	node := newNode(name, currentConfig, desiredConfig)
 	node.Status = corev1.NodeStatus{Conditions: []corev1.NodeCondition{{Type: corev1.NodeReady, Status: status}}}

pkg/daemon/constants/constants.go

@@ -14,6 +14,8 @@ const (
 	DesiredMachineConfigAnnotationKey = "machineconfiguration.openshift.io/desiredConfig"
 	// MachineConfigDaemonStateAnnotationKey is used to fetch the state of the daemon on the machine.
 	MachineConfigDaemonStateAnnotationKey = "machineconfiguration.openshift.io/state"
+	// MachineUpdateOrderingAnnotationKey is used to specify the desired ordering for an update
+	MachineUpdateOrderingAnnotationKey = "machineconfiguration.openshift.io/update-order"
 	// OpenShiftOperatorManagedLabel is used to filter out kube objects that don't need to be synced by the MCO
 	OpenShiftOperatorManagedLabel = "openshift.io/operator-managed"
 	// MachineConfigDaemonStateWorking is set by daemon when it is applying an update.