Refactor node annotations

daemon/cmd/daemon.go

@@ -157,7 +157,8 @@ type Daemon struct {
 	datapath datapath.Datapath
 
 	// nodeDiscovery defines the node discovery logic of the agent
-	nodeDiscovery *nodediscovery.NodeDiscovery
+	nodeDiscovery  *nodediscovery.NodeDiscovery
+	nodeLocalStore node.LocalNodeStore
 
 	// ipam is the IP address manager of the agent
 	ipam *ipam.IPAM
@@ -410,6 +411,7 @@ func newDaemon(ctx context.Context, cleaner *daemonCleanup,
 	sharedResources k8s.SharedResources,
 	certManager certificatemanager.CertificateManager,
 	secretManager certificatemanager.SecretManager,
+	nodeLocalStore node.LocalNodeStore,
 ) (*Daemon, *endpointRestoreState, error) {
 
 	var (
@@ -550,6 +552,7 @@ func newDaemon(ctx context.Context, cleaner *daemonCleanup,
 		datapath:          dp,
 		deviceManager:     devMngr,
 		nodeDiscovery:     nd,
+		nodeLocalStore:    nodeLocalStore,
 		endpointCreations: newEndpointCreationManager(clientset),
 		apiLimiterSet:     apiLimiterSet,
 		controllers:       controller.NewManager(),
@@ -1185,17 +1188,15 @@ func newDaemon(ctx context.Context, cleaner *daemonCleanup,
 			logfields.V6CiliumHostIP: node.GetIPv6Router(),
 		}).Info("Annotating k8s node")
 
-		err := k8s.AnnotateNode(
+		_, err := k8s.AnnotateNode(
 			clientset,
 			nodeTypes.GetName(),
-			encryptKeyID,
-			node.GetIPv4AllocRange(), node.GetIPv6AllocRange(),
-			node.GetEndpointHealthIPv4(), node.GetEndpointHealthIPv6(),
-			node.GetIngressIPv4(), node.GetIngressIPv6(),
-			node.GetInternalIPv4Router(), node.GetIPv6Router())
+			d.nodeLocalStore.Get().Node,
+			encryptKeyID)
 		if err != nil {
 			log.WithError(err).Warning("Cannot annotate k8s node with CIDR range")
 		}
+
 		bootstrapStats.k8sInit.End(true)
 	} else if !option.Config.AnnotateK8sNode {
 		log.Debug("Annotate k8s node is disabled.")

daemon/cmd/daemon_main.go

@@ -1692,7 +1692,8 @@ func newDaemonPromise(params daemonParams) promise.Promise[*Daemon] {
 				params.Clientset,
 				params.SharedResources,
 				params.CertManager,
-				params.SecretManager)
+				params.SecretManager,
+				params.LocalNodeStore)
 			if err != nil {
 				return fmt.Errorf("daemon creation failed: %w", err)
 			}

pkg/defaults/defaults.go

@@ -345,7 +345,7 @@ const (
 	EnableEndpointRoutes = false
 
 	// AnnotateK8sNode is the default value for option.AnnotateK8sNode. It is
-	// enabled by default to annotate kubernetes node and can be disabled using
+	// disabled by default to annotate kubernetes node and can be enabled using
 	// the provided option.
 	AnnotateK8sNode = false
 

pkg/k8s/annotate.go

@@ -7,61 +7,54 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
-	"net"
+	"reflect"
 	"strconv"
+	"strings"
 
 	"github.com/sirupsen/logrus"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
+	k8sTypes "k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/kubernetes"
 
 	"github.com/cilium/cilium/pkg/annotation"
-	"github.com/cilium/cilium/pkg/cidr"
 	"github.com/cilium/cilium/pkg/controller"
 	"github.com/cilium/cilium/pkg/logging/logfields"
+	nodeTypes "github.com/cilium/cilium/pkg/node/types"
 )
 
-func updateNodeAnnotation(c kubernetes.Interface, nodeName string, encryptKey uint8, v4CIDR, v6CIDR *cidr.CIDR, v4HealthIP, v6HealthIP, v4IngressIP, v6IngressIP, v4CiliumHostIP, v6CiliumHostIP net.IP) error {
-	annotations := map[string]string{}
-
-	if v4CIDR != nil {
-		annotations[annotation.V4CIDRName] = v4CIDR.String()
-	}
-	if v6CIDR != nil {
-		annotations[annotation.V6CIDRName] = v6CIDR.String()
-	}
-
-	if v4HealthIP != nil {
-		annotations[annotation.V4HealthName] = v4HealthIP.String()
-	}
-	if v6HealthIP != nil {
-		annotations[annotation.V6HealthName] = v6HealthIP.String()
+type nodeAnnotation = map[string]string
+
+func prepareNodeAnnotation(nd nodeTypes.Node, encryptKey uint8) nodeAnnotation {
+	annotationMap := map[string]fmt.Stringer{
+		annotation.V4CIDRName:     nd.IPv4AllocCIDR,
+		annotation.V6CIDRName:     nd.IPv6AllocCIDR,
+		annotation.V4HealthName:   nd.IPv4HealthIP,
+		annotation.V6HealthName:   nd.IPv6HealthIP,
+		annotation.V4IngressName:  nd.IPv4IngressIP,
+		annotation.V6IngressName:  nd.IPv6IngressIP,
+		annotation.CiliumHostIP:   nd.GetCiliumInternalIP(false),
+		annotation.CiliumHostIPv6: nd.GetCiliumInternalIP(true),
 	}
 
-	if v4IngressIP != nil {
-		annotations[annotation.V4IngressName] = v4IngressIP.String()
-	}
-	if v6IngressIP != nil {
-		annotations[annotation.V6IngressName] = v6IngressIP.String()
-	}
-
-	if v4CiliumHostIP != nil {
-		annotations[annotation.CiliumHostIP] = v4CiliumHostIP.String()
-	}
-
-	if v6CiliumHostIP != nil {
-		annotations[annotation.CiliumHostIPv6] = v6CiliumHostIP.String()
+	annotations := map[string]string{}
+	for k, v := range annotationMap {
+		if !reflect.ValueOf(v).IsNil() {
+			annotations[k] = v.String()
+		}
 	}
-
 	if encryptKey != 0 {
 		annotations[annotation.CiliumEncryptionKey] = strconv.FormatUint(uint64(encryptKey), 10)
 	}
+	return annotations
+}
 
-	if len(annotations) == 0 {
+func updateNodeAnnotation(c kubernetes.Interface, nodeName string, annotation nodeAnnotation) error {
+	if len(annotation) == 0 {
 		return nil
 	}
 
-	raw, err := json.Marshal(annotations)
+	raw, err := json.Marshal(annotation)
 	if err != nil {
 		return err
 	}
@@ -75,31 +68,57 @@ func updateNodeAnnotation(c kubernetes.Interface, nodeName string, encryptKey ui
 // AnnotateNode writes v4 and v6 CIDRs and health IPs in the given k8s node name.
 // In case of failure while updating the node, this function while spawn a go
 // routine to retry the node update indefinitely.
-func AnnotateNode(cs kubernetes.Interface, nodeName string, encryptKey uint8, v4CIDR, v6CIDR *cidr.CIDR, v4HealthIP, v6HealthIP, v4IngressIP, v6IngressIP, v4CiliumHostIP, v6CiliumHostIP net.IP) error {
+func AnnotateNode(cs kubernetes.Interface, nodeName string, nd nodeTypes.Node, encryptKey uint8) (nodeAnnotation, error) {
 	scopedLog := log.WithFields(logrus.Fields{
 		logfields.NodeName:       nodeName,
-		logfields.V4Prefix:       v4CIDR,
-		logfields.V6Prefix:       v6CIDR,
-		logfields.V4HealthIP:     v4HealthIP,
-		logfields.V6HealthIP:     v6HealthIP,
-		logfields.V4IngressIP:    v4IngressIP,
-		logfields.V6IngressIP:    v6IngressIP,
-		logfields.V4CiliumHostIP: v4CiliumHostIP,
-		logfields.V6CiliumHostIP: v6CiliumHostIP,
+		logfields.V4Prefix:       nd.IPv4AllocCIDR,
+		logfields.V6Prefix:       nd.IPv6AllocCIDR,
+		logfields.V4HealthIP:     nd.IPv4HealthIP,
+		logfields.V6HealthIP:     nd.IPv6HealthIP,
+		logfields.V4IngressIP:    nd.IPv4IngressIP,
+		logfields.V6IngressIP:    nd.IPv6IngressIP,
+		logfields.V4CiliumHostIP: nd.GetCiliumInternalIP(false),
+		logfields.V6CiliumHostIP: nd.GetCiliumInternalIP(true),
 		logfields.Key:            encryptKey,
 	})
 	scopedLog.Debug("Updating node annotations with node CIDRs")
-
+	annotation := prepareNodeAnnotation(nd, encryptKey)
 	controller.NewManager().UpdateController("update-k8s-node-annotations",
 		controller.ControllerParams{
 			DoFunc: func(_ context.Context) error {
-				err := updateNodeAnnotation(cs, nodeName, encryptKey, v4CIDR, v6CIDR, v4HealthIP, v6HealthIP, v4IngressIP, v6IngressIP, v4CiliumHostIP, v6CiliumHostIP)
+				err := updateNodeAnnotation(cs, nodeName, annotation)
 				if err != nil {
 					scopedLog.WithFields(logrus.Fields{}).WithError(err).Warn("Unable to patch node resource with annotation")
 				}
 				return err
 			},
 		})
 
-	return nil
+	return annotation, nil
+}
+
+func prepareRemoveNodeAnnotationsPayload(annotation nodeAnnotation) ([]byte, error) {
+	deleteAnnotations := []JSONPatch{}
+
+	for key := range annotation {
+		deleteAnnotations = append(deleteAnnotations, JSONPatch{
+			OP:   "remove",
+			Path: "/metadata/annotations/" + encodeJsonElement(key),
+		})
+	}
+
+	return json.Marshal(deleteAnnotations)
+}
+
+func RemoveNodeAnnotations(c kubernetes.Interface, nodeName string, annotation nodeAnnotation) error {
+	patch, err := prepareRemoveNodeAnnotationsPayload(annotation)
+	if err != nil {
+		return err
+	}
+	_, err = c.CoreV1().Nodes().Patch(context.TODO(), nodeName, k8sTypes.JSONPatchType, patch, metav1.PatchOptions{}, "status")
+	return err
+}
+
+func encodeJsonElement(element string) string {
+	return strings.Replace(element, "/", "~1", -1)
 }

pkg/k8s/annotate_test.go

@@ -0,0 +1,37 @@
+// SPDX-License-Identifier: Apache-2.0
+// Copyright Authors of Cilium
+
+package k8s
+
+import (
+	. "gopkg.in/check.v1"
+
+	"github.com/cilium/cilium/pkg/annotation"
+)
+
+func (s *K8sSuite) TestPrepareRemoveNodeAnnotationsPayload(c *C) {
+	tests := []struct {
+		name       string
+		annotation nodeAnnotation
+		wantJson   string
+	}{
+		{
+			name: "Test remove one annotation",
+			annotation: nodeAnnotation{
+				annotation.V4CIDRName: "cidr",
+			},
+			wantJson: "[{\"op\":\"remove\",\"path\":\"/metadata/annotations/network.cilium.io~1ipv4-pod-cidr\",\"value\":null}]",
+		},
+		{
+			name:       "Test remove zero annotations",
+			annotation: nodeAnnotation{},
+			wantJson:   "[]",
+		},
+	}
+
+	for _, tt := range tests {
+		got, err := prepareRemoveNodeAnnotationsPayload(tt.annotation)
+		c.Assert(err, IsNil)
+		c.Assert(string(got), Equals, tt.wantJson, Commentf("Test Name: %s", tt.name))
+	}
+}

pkg/k8s/init_test.go

@@ -67,19 +67,12 @@ func (s *K8sSuite) TestUseNodeCIDR(c *C) {
 
 	node1Slim := ConvertToNode(node1.DeepCopy()).(*slim_corev1.Node)
 	node1Cilium := ParseNode(node1Slim, source.Unspec)
-
+	node1Cilium.SetCiliumInternalIP(net.ParseIP("10.254.0.1"))
 	useNodeCIDR(node1Cilium)
 	c.Assert(node.GetIPv4AllocRange().String(), Equals, "10.2.0.0/16")
 	// IPv6 Node range is not checked because it shouldn't be changed.
 
-	err := AnnotateNode(fakeK8sClient, "node1",
-		0,
-		node.GetIPv4AllocRange(),
-		node.GetIPv6AllocRange(),
-		nil, nil,
-		nil, nil,
-		net.ParseIP("10.254.0.1"),
-		net.ParseIP(""))
+	_, err := AnnotateNode(fakeK8sClient, "node1", *node1Cilium, 0)
 
 	c.Assert(err, IsNil)
 
@@ -131,22 +124,50 @@ func (s *K8sSuite) TestUseNodeCIDR(c *C) {
 
 	node2Slim := ConvertToNode(node2.DeepCopy()).(*slim_corev1.Node)
 	node2Cilium := ParseNode(node2Slim, source.Unspec)
+	node2Cilium.SetCiliumInternalIP(net.ParseIP("10.254.0.1"))
 	useNodeCIDR(node2Cilium)
 
 	// We use the node's annotation for the IPv4 and the PodCIDR for the
 	// IPv6.
 	c.Assert(node.GetIPv4AllocRange().String(), Equals, "10.254.0.0/16")
 	c.Assert(node.GetIPv6AllocRange().String(), Equals, "aaaa:aaaa:aaaa:aaaa:beef:beef::/96")
 
-	err = AnnotateNode(fakeK8sClient, "node2",
-		0,
-		node.GetIPv4AllocRange(),
-		node.GetIPv6AllocRange(),
-		nil, nil,
-		nil, nil,
-		net.ParseIP("10.254.0.1"),
-		net.ParseIP(""))
+	_, err = AnnotateNode(fakeK8sClient, "node2", *node2Cilium, 0)
+
+	c.Assert(err, IsNil)
+
+	select {
+	case <-patchChan:
+	case <-time.Tick(10 * time.Second):
+		c.Errorf("d.fakeK8sClient.CoreV1().Nodes().Update() was not called")
+		c.FailNow()
+	}
+}
+
+func (s *K8sSuite) TestRemovalOfNodeAnnotations(c *C) {
+	node1 := v1.Node{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "node1",
+			Annotations: map[string]string{
+				annotation.V4CIDRName: "10.254.0.0/16",
+			},
+		},
+	}
+
+	patchChan := make(chan bool, 1)
+	fakeK8sClient := &fake.Clientset{}
+	fakeK8sClient.AddReactor("patch", "nodes",
+		func(action testing.Action) (bool, runtime.Object, error) {
+			n1copy := node1.DeepCopy()
+			delete(n1copy.Annotations, annotation.V4CIDRName)
+			patchWanted := []byte("[{\"op\":\"remove\",\"path\":\"/metadata/annotations/network.cilium.io~1ipv4-pod-cidr\",\"value\":null}]")
+			patchReceived := action.(testing.PatchAction).GetPatch()
+			c.Assert(string(patchReceived), checker.DeepEquals, string(patchWanted))
+			patchChan <- true
+			return true, n1copy, nil
+		})
 
+	err := RemoveNodeAnnotations(fakeK8sClient, "node1", map[string]string{annotation.V4CIDRName: "10.254.0.0/16"})
 	c.Assert(err, IsNil)
 
 	select {