[node_allocator] Add stale annotation cleanup.

To only execute it once, run for default network NodeAllocator only,
since it is always created.

Signed-off-by: Nadia Pinaeva <[email protected]>

Author: npinaeva

go-controller/pkg/clustermanager/node/node_allocator.go

@@ -71,6 +71,7 @@ func (na *NodeAllocator) Init() error {
 	if !na.hasNodeSubnetAllocation() {
 		return nil
 	}
+	na.CleanupStaleAnnotation()
 
 	clusterSubnets := na.netInfo.Subnets()
 
@@ -96,6 +97,31 @@ func (na *NodeAllocator) Init() error {
 	return nil
 }
 
+// CleanupStaleAnnotation cleans up the stale annotations on all nodes.
+// If an error occurs, it logs the error and continues to the next node.
+func (na *NodeAllocator) CleanupStaleAnnotation() {
+	// only cleanup once with the default network NodeAllocator
+	if !na.netInfo.IsDefault() {
+		return
+	}
+	existingNodes, err := na.nodeLister.List(labels.Everything())
+	if err != nil {
+		klog.Errorf("Error in retrieving the nodes: %v", err)
+		return
+	}
+
+	for _, node := range existingNodes {
+		if _, ok := node.Annotations[util.OVNNodeGRLRPAddrs]; !ok {
+			continue
+		}
+		// to cleanup an annotation, set it to nil
+		if err = na.kube.SetAnnotationsOnNode(node.Name, map[string]interface{}{util.OVNNodeGRLRPAddrs: nil}); err != nil {
+			klog.Warningf("Failed to clear node %s annotation %s: %v",
+				node.Name, util.OVNNodeGRLRPAddrs, err)
+		}
+	}
+}
+
 func (na *NodeAllocator) hasHybridOverlayAllocation() bool {
 	// When config.HybridOverlay.ClusterSubnets is empty, assume the subnet allocation will be managed by an external component.
 	return config.HybridOverlay.Enabled && !na.netInfo.IsSecondary() && len(config.HybridOverlay.ClusterSubnets) > 0

go-controller/pkg/clustermanager/node/node_allocator_test.go

@@ -1,6 +1,7 @@
 package node
 
 import (
+	"context"
 	"fmt"
 	"net"
 	"reflect"
@@ -10,11 +11,13 @@ import (
 
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/client-go/kubernetes/fake"
 	"k8s.io/client-go/listers/core/v1"
 	"k8s.io/client-go/tools/cache"
 
 	ovncnitypes "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/cni/types"
 	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/config"
+	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/kube"
 	ovntest "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/testing"
 	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/types"
 	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/util"
@@ -288,6 +291,7 @@ func TestController_allocateNodeSubnets(t *testing.T) {
 			na := &NodeAllocator{
 				netInfo:                netInfo,
 				clusterSubnetAllocator: NewSubnetAllocator(),
+				nodeLister:             newFakeNodeLister([]*corev1.Node{}),
 			}
 
 			if err := na.Init(); err != nil {
@@ -403,3 +407,44 @@ func newFakeNodeLister(nodes []*corev1.Node) v1.NodeLister {
 	}
 	return v1.NewNodeLister(indexer)
 }
+
+func TestController_CleanupStaleAnnotation(t *testing.T) {
+	// create a node with an annotation that shouldn't be changed and one that should be cleaned up.
+	newNode := &corev1.Node{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:        "node1",
+			Annotations: map[string]string{"leave-me": "value", util.OVNNodeGRLRPAddrs: "remove-me"},
+		},
+	}
+	fakeClient := fake.NewClientset(newNode)
+	kube := &kube.Kube{
+		KClient: fakeClient,
+	}
+
+	netInfo, err := util.NewNetInfo(
+		&ovncnitypes.NetConf{
+			NetConf: cnitypes.NetConf{Name: types.DefaultNetworkName},
+		},
+	)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	na := &NodeAllocator{
+		nodeLister: newFakeNodeLister([]*corev1.Node{newNode}),
+		kube:       kube,
+		netInfo:    netInfo,
+	}
+	na.CleanupStaleAnnotation()
+	nodes, err := fakeClient.CoreV1().Nodes().List(context.TODO(), metav1.ListOptions{})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(nodes.Items) != 1 {
+		t.Fatalf("Expected 1 node, got %d", len(nodes.Items))
+	}
+	// check that unrelated annotation is not changed, and stale one is cleaned up
+	if !reflect.DeepEqual(nodes.Items[0].Annotations, map[string]string{"leave-me": "value"}) {
+		t.Fatalf("Expected annotation %s to be cleaned up, got %v", util.OVNNodeGRLRPAddrs, nodes.Items[0].Annotations)
+	}
+}

go-controller/pkg/util/node_annotations.go

@@ -85,7 +85,7 @@ const (
 	//		\"l2-network\":{\"ipv4\":\"100.65.0.4/16\",\"ipv6\":\"fd99::4/64\"},
 	//		\"l3-network\":{\"ipv4\":\"100.65.0.4/16\",\"ipv6\":\"fd99::4/64\"}
 	// }",
-	// TODO add cleanup code
+	// deprecated, only used for cleanup
 	OVNNodeGRLRPAddrs = "k8s.ovn.org/node-gateway-router-lrp-ifaddrs"
 
 	// OvnNodeMasqCIDR is the CIDR form representation of the masquerade subnet that is currently configured on this node (i.e. 169.254.169.0/29)