Set "node.kubernetes.io/exclude-from-external-load-balancers" label at reboot and unset it at startup

pkg/daemon/daemon.go

@@ -23,6 +23,7 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	"k8s.io/apimachinery/pkg/util/wait"
 	coreinformersv1 "k8s.io/client-go/informers/core/v1"
@@ -164,6 +165,13 @@ const (
 	// in the face of errors.
 	maxUpdateBackoff = 60 * time.Second
 
+	// excludeFromLoadBalancerLabel is used by the cloud service controller in order to know
+	// which nodes to exclude from backend pools of cloud load balancers.
+	excludeFromLoadBalancerLabel = "node.kubernetes.io/exclude-from-external-load-balancers"
+	// excludeFromLoadBalancerLabelValue is a custom value to recognize that
+	// the label was added by MCO.
+	excludeFromLoadBalancerLabelValue = "added-by-mco"
+
 	// used for Hypershift daemon
 	mcsServedConfigPath         = "/etc/mcs-machine-config-content.json"
 	hypershiftCurrentConfigPath = "/etc/mcd-currentconfig.json"
@@ -456,6 +464,11 @@ func (dn *Daemon) initializeNode() error {
 	if dn.nodeInitialized {
 		return nil
 	}
+
+	if err := dn.addOrRemoveExcludeFromLoadBalancerLabel(false); err != nil {
+		glog.Warningf("Unable to remove label %s from node : %s", excludeFromLoadBalancerLabel, err)
+	}
+
 	// Some parts of the MCO dispatch on whether or not we're managing a control plane node
 	if _, isControlPlane := dn.node.Labels[ctrlcommon.MasterLabel]; isControlPlane {
 		glog.Infof("Node %s is part of the control plane", dn.node.Name)
@@ -2102,3 +2115,107 @@ func forceFileExists() bool {
 
 	return false
 }
+
+// patchLabelsOnNode takes a map of key/value string pairs and patches them as labels on the node
+func (dn *Daemon) patchLabelsOnNode(labels map[string]interface{}) error {
+	var err error
+	var patchData []byte
+	patch := struct {
+		Metadata map[string]interface{} `json:"metadata"`
+	}{
+		Metadata: map[string]interface{}{
+			"labels": labels,
+		},
+	}
+	dn.logSystem("Patching labels %v", labels)
+	patchData, err = json.Marshal(&patch)
+	if err != nil {
+		dn.logSystem("Error in patching labels: %v", err)
+		return err
+	}
+	_, err = dn.kubeClient.CoreV1().Nodes().Patch(
+		context.TODO(), dn.name, types.MergePatchType, patchData, metav1.PatchOptions{})
+	if err != nil {
+		dn.logSystem("Error patching labels: %v", err)
+		return err
+	}
+	return nil
+}
+
+// Add or remove excludeFromLoadBalancerLabel to the node. If addLabel=true, it
+// adds the label if the label doesn't already exist and sets it to a custom value
+// so as to know that it was added by MCO. If addLabel=false, it removes the label,
+// only if its value is the known custom value, otherwise it leaves it as is.
+func (dn *Daemon) addOrRemoveExcludeFromLoadBalancerLabel(addLabel bool) error {
+	var err error
+	var labelValue interface{}
+	if dn.node == nil {
+		return nil
+	}
+	// Add the label only if not present; remove the label only if it has our custom value
+	labelValue, labelExists := dn.node.Labels[excludeFromLoadBalancerLabel]
+	if addLabel {
+		if labelExists {
+			dn.logSystem("node %s already has label %s=%s (addLabel=%t), nothing to add",
+				dn.name, excludeFromLoadBalancerLabel, labelValue, addLabel)
+			return nil
+		}
+		// Add addOrRemoveExcludeFromLoadBalancerLabel to the node, as in:
+		// labels:
+		//   beta.kubernetes.io/arch: amd64
+		//   beta.kubernetes.io/instance-type: Standard_D4s_v3
+		//   beta.kubernetes.io/os: linux
+		//   failure-domain.beta.kubernetes.io/region: centralus
+		//   failure-domain.beta.kubernetes.io/zone: centralus-1
+		//   kubernetes.io/arch: amd64
+		//   kubernetes.io/hostname: ci-ln-0h2ssy2-1d09d-vpvjc-worker-centralus1-l2thg
+		//   kubernetes.io/os: linux
+		//   node-role.kubernetes.io/worker: ""
+		//   node.kubernetes.io/exclude-from-external-load-balancers: added-by-mco
+		//   node.kubernetes.io/instance-type: Standard_D4s_v3
+		//   node.openshift.io/os_id: rhcos
+		//   topology.disk.csi.azure.com/zone: centralus-1
+		//   topology.kubernetes.io/region: centralus
+		//   topology.kubernetes.io/zone: centralus-1
+		labelValue = excludeFromLoadBalancerLabelValue
+	} else {
+		if labelExists {
+			if labelValue != excludeFromLoadBalancerLabelValue {
+				dn.logSystem("label %s=%s was not added by MCO, not removing it",
+					excludeFromLoadBalancerLabel, labelValue)
+				return nil
+			}
+		} else {
+			dn.logSystem("label %s not present on node %s, nothing to remove",
+				excludeFromLoadBalancerLabel, dn.name)
+			return nil
+		}
+
+		// Removing the label by setting its value to nil.
+		// Continuing from the example above:
+		// labels:
+		//   beta.kubernetes.io/arch: amd64
+		//   beta.kubernetes.io/instance-type: Standard_D4s_v3
+		//   beta.kubernetes.io/os: linux
+		//   failure-domain.beta.kubernetes.io/region: centralus
+		//   failure-domain.beta.kubernetes.io/zone: centralus-1
+		//   kubernetes.io/arch: amd64
+		//   kubernetes.io/hostname: ci-ln-0h2ssy2-1d09d-vpvjc-worker-centralus1-l2thg
+		//   kubernetes.io/os: linux
+		//   node-role.kubernetes.io/worker: ""
+		//   node.kubernetes.io/instance-type: Standard_D4s_v3
+		//   node.openshift.io/os_id: rhcos
+		//   topology.disk.csi.azure.com/zone: centralus-1
+		//   topology.kubernetes.io/region: centralus
+		//   topology.kubernetes.io/zone: centralus-1
+
+		labelValue = nil
+	}
+	err = dn.patchLabelsOnNode(map[string]interface{}{excludeFromLoadBalancerLabel: labelValue})
+	if err != nil {
+		dn.logSystem("Error patching node %s with label %s=%s: %v",
+			dn.name, excludeFromLoadBalancerLabel, labelValue, err)
+		return err
+	}
+	return nil
+}

pkg/daemon/daemon_test.go

@@ -1,6 +1,7 @@
 package daemon
 
 import (
+	"context"
 	"encoding/json"
 	"os"
 	"reflect"
@@ -13,6 +14,7 @@ import (
 	"github.com/stretchr/testify/require"
 	"github.com/vincent-petithory/dataurl"
 	corev1 "k8s.io/api/core/v1"
+	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/equality"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -318,6 +320,14 @@ func newNode(annotations map[string]string) *corev1.Node {
 	}
 }
 
+func newNodeWithLabels(labels map[string]string) *corev1.Node {
+	return &corev1.Node{
+		ObjectMeta: metav1.ObjectMeta{
+			Labels: labels,
+		},
+	}
+}
+
 func TestPrepUpdateFromClusterOnDiskDrift(t *testing.T) {
 	tmpCurrentConfig, err := os.CreateTemp("", "currentconfig")
 	require.Nil(t, err)
@@ -395,3 +405,137 @@ func TestPrepUpdateFromClusterOnDiskDrift(t *testing.T) {
 	require.Equal(t, onDiskMC.GetName(), current.GetName())
 	require.Equal(t, desired.GetName(), "test2")
 }
+
+func TestAddOrRemoveExcludeFromLoadBalancerLabel(t *testing.T) {
+	otherExcludeValue := "not-set-by-mco"
+	otherLabel := "myLabel"
+	otherLabelValue := "myValue"
+	labelsWithNoExclude := map[string]string{
+		otherLabel: otherLabelValue,
+	}
+	labelsWithEmptyExclude := map[string]string{
+		otherLabel:                   otherLabelValue,
+		excludeFromLoadBalancerLabel: "",
+	}
+	labelsWithMCOExclude := map[string]string{
+		otherLabel:                   otherLabelValue,
+		excludeFromLoadBalancerLabel: excludeFromLoadBalancerLabelValue,
+	}
+	labelsWithOtherExclude := map[string]string{
+		otherLabel:                   otherLabelValue,
+		excludeFromLoadBalancerLabel: otherExcludeValue,
+	}
+
+	initializeEnvironment := func(labels map[string]string) *Daemon {
+		f := newFixture(t)
+		node := newNodeWithLabels(labels)
+		node.Name = "node_name_test"
+		f.nodeLister = append(f.nodeLister, node)
+		f.kubeobjects = append(f.kubeobjects, node)
+		dn := f.newController()
+		dn.node = node
+		return dn
+	}
+
+	readNode := func(dn *Daemon) *v1.Node {
+		newNode, err := dn.kubeClient.CoreV1().Nodes().Get(
+			context.TODO(), dn.node.Name, metav1.GetOptions{})
+		if err != nil {
+			t.Errorf("error reading node: %v", err)
+		}
+		return newNode
+	}
+
+	checkForOtherLabel := func(node *v1.Node) {
+		otherLabelValueRead, otherLabelExists := node.Labels[otherLabel]
+		assert.True(t, otherLabelExists)
+		assert.EqualValues(t, otherLabelValue, otherLabelValueRead)
+	}
+
+	// Add "exclude" label to a node that doesn't have it yet
+	dn := initializeEnvironment(labelsWithNoExclude)
+	err := dn.addOrRemoveExcludeFromLoadBalancerLabel(true)
+	assert.Nil(t, err)
+	newNode := readNode(dn)
+	labelValue, labelExists := newNode.Labels[excludeFromLoadBalancerLabel]
+	assert.True(t, labelExists)
+	assert.EqualValues(t, excludeFromLoadBalancerLabelValue, labelValue)
+	checkForOtherLabel(newNode)
+
+	// Try to remove the "exclude" label from a node that doesn't have it yet
+	dn = initializeEnvironment(labelsWithNoExclude)
+	err = dn.addOrRemoveExcludeFromLoadBalancerLabel(false)
+	assert.Nil(t, err)
+	newNode = readNode(dn)
+	labelValue, labelExists = newNode.Labels[excludeFromLoadBalancerLabel]
+	assert.False(t, labelExists)
+	checkForOtherLabel(newNode)
+
+	// Try to add the "exclude" label to a node that has it already, but whose value is empty.
+	// It should leave the node as is.
+	dn = initializeEnvironment(labelsWithEmptyExclude)
+	err = dn.addOrRemoveExcludeFromLoadBalancerLabel(true)
+	assert.Nil(t, err)
+	newNode = readNode(dn)
+	labelValue, labelExists = newNode.Labels[excludeFromLoadBalancerLabel]
+	assert.True(t, labelExists)
+	assert.EqualValues(t, "", labelValue)
+	checkForOtherLabel(newNode)
+
+	// Try to remove the "exclude" label from a node that has it already,
+	// but whose value is empty. Check that the label does NOT get removed.
+	dn = initializeEnvironment(labelsWithEmptyExclude)
+	err = dn.addOrRemoveExcludeFromLoadBalancerLabel(false)
+	assert.Nil(t, err)
+	newNode = readNode(dn)
+	labelValue, labelExists = newNode.Labels[excludeFromLoadBalancerLabel]
+	assert.True(t, labelExists)
+	assert.EqualValues(t, "", labelValue)
+	checkForOtherLabel(newNode)
+
+	// Try to add the "exclude" label to a node that has it already, and whose value is
+	// the one that MCO adds. Nothing should change.
+	dn = initializeEnvironment(labelsWithMCOExclude)
+	err = dn.addOrRemoveExcludeFromLoadBalancerLabel(true)
+	assert.Nil(t, err)
+	// check that the label is still there
+	newNode = readNode(dn)
+	labelValue, labelExists = newNode.Labels[excludeFromLoadBalancerLabel]
+	assert.True(t, labelExists)
+	assert.EqualValues(t, excludeFromLoadBalancerLabelValue, labelValue)
+	checkForOtherLabel(newNode)
+
+	// Remove the "exclude" label from a node that has it already, and whose value is
+	// the one MCO adds. The label should be removed
+	dn = initializeEnvironment(labelsWithMCOExclude)
+	err = dn.addOrRemoveExcludeFromLoadBalancerLabel(false)
+	assert.Nil(t, err)
+	newNode = readNode(dn)
+	labelValue, labelExists = newNode.Labels[excludeFromLoadBalancerLabel]
+	assert.False(t, labelExists)
+	checkForOtherLabel(newNode)
+
+	// Try to add the "exclude" label to a node that has it already, but whose value
+	// is arbitrary.
+	// Both "add" and "remove" should leave the node as is.
+	dn = initializeEnvironment(labelsWithOtherExclude)
+	err = dn.addOrRemoveExcludeFromLoadBalancerLabel(true)
+	assert.Nil(t, err)
+	newNode = readNode(dn)
+	labelValue, labelExists = newNode.Labels[excludeFromLoadBalancerLabel]
+	assert.True(t, labelExists)
+	assert.EqualValues(t, otherExcludeValue, labelValue)
+	checkForOtherLabel(newNode)
+
+	// Try to remove the "exclude" label from a node that has it already, but whose value
+	// is arbitrary. Check that the label does NOT get removed
+	dn = initializeEnvironment(labelsWithOtherExclude)
+	err = dn.addOrRemoveExcludeFromLoadBalancerLabel(false)
+	assert.Nil(t, err)
+	newNode = readNode(dn)
+	labelValue, labelExists = newNode.Labels[excludeFromLoadBalancerLabel]
+	assert.True(t, labelExists)
+	assert.EqualValues(t, otherExcludeValue, labelValue)
+	checkForOtherLabel(newNode)
+
+}

pkg/daemon/update.go

@@ -128,6 +128,10 @@ func (dn *Daemon) performPostConfigChangeAction(postConfigChangeActions []string
 // finalizeBeforeReboot is the last step in an update() and then we take appropriate postConfigChangeAction.
 // It can also be called as a special case for the "bootstrap pivot".
 func (dn *Daemon) finalizeBeforeReboot(newConfig *mcfgv1.MachineConfig) (retErr error) {
+	if err := dn.addOrRemoveExcludeFromLoadBalancerLabel(true); err != nil {
+		glog.Warningf("Unable to add label %s to node : %s", excludeFromLoadBalancerLabel, err)
+	}
+
 	if out, err := dn.storePendingState(newConfig, 1); err != nil {
 		return fmt.Errorf("failed to log pending config: %s: %w", string(out), err)
 	}