wip

Signed-off-by: zhangzujian <[email protected]>

cmd/daemon/cniserver.go

@@ -197,7 +197,7 @@ func initChassisAnno(cfg *daemon.Configuration) error {
 		return err
 	}
 	annotations := map[string]any{util.ChassisAnnotation: chassesName}
-	if err = util.UpdateNodeAnnotations(cfg.KubeClient.CoreV1().Nodes(), cfg.NodeName, annotations); err != nil {
+	if err = util.PatchAnnotations(cfg.KubeClient.CoreV1().Nodes(), cfg.NodeName, annotations); err != nil {
 		klog.Errorf("failed to update chassis annotation of node %s: %v", cfg.NodeName, err)
 		return err
 	}

pkg/apis/kubeovn/v1/zz_generated.deepcopy_test.go

@@ -3,7 +3,7 @@ package v1
 import (
 	"testing"
 
-	runtime "k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime"
 
 	"github.com/brianvoe/gofakeit/v7"
 	"github.com/stretchr/testify/require"

pkg/controller/external_gw.go

@@ -84,7 +84,7 @@ func (c *Controller) removeExternalGateway() error {
 	}
 	for _, node := range nodes {
 		labels := map[string]any{util.ExGatewayLabel: "false"}
-		if err = util.UpdateNodeLabels(c.config.KubeClient.CoreV1().Nodes(), node.Name, labels); err != nil {
+		if err = util.PatchLabels(c.config.KubeClient.CoreV1().Nodes(), node.Name, labels); err != nil {
 			klog.Errorf("failed to patch external gw node %s: %v", node.Name, err)
 			return err
 		}
@@ -235,7 +235,7 @@ func (c *Controller) getGatewayChassis(config map[string]string) ([]string, erro
 			return nil, err
 		}
 		labels := map[string]any{util.ExGatewayLabel: "true"}
-		if err = util.UpdateNodeLabels(c.config.KubeClient.CoreV1().Nodes(), node.Name, labels); err != nil {
+		if err = util.PatchLabels(c.config.KubeClient.CoreV1().Nodes(), node.Name, labels); err != nil {
 			klog.Errorf("failed to update annotations of node %s: %v", node.Name, err)
 			return nil, err
 		}

pkg/controller/node.go

@@ -228,7 +228,7 @@ func (c *Controller) handleAddNode(key string) error {
 		util.AllocatedAnnotation:     "true",
 		util.PortNameAnnotation:      portName,
 	}
-	if err = util.UpdateNodeAnnotations(c.config.KubeClient.CoreV1().Nodes(), node.Name, annotations); err != nil {
+	if err = util.PatchAnnotations(c.config.KubeClient.CoreV1().Nodes(), node.Name, annotations); err != nil {
 		klog.Errorf("failed to update annotations of node %s: %v", node.Name, err)
 		return err
 	}

pkg/daemon/controller.go

@@ -309,7 +309,7 @@ func (c *Controller) initProviderNetwork(pn *kubeovnv1.ProviderNetwork, node *v1
 	klog.V(3).Infof("ovs init provider network %s", pn.Name)
 	if mtu, err = c.ovsInitProviderNetwork(pn.Name, nic, vlans.List(), pn.Spec.ExchangeLinkName, c.config.MacLearningFallback); err != nil {
 		delete(labels, fmt.Sprintf(util.ProviderNetworkExcludeTemplate, pn.Name))
-		if err1 := util.UpdateNodeLabels(c.config.KubeClient.CoreV1().Nodes(), node.Name, labels); err1 != nil {
+		if err1 := util.PatchLabels(c.config.KubeClient.CoreV1().Nodes(), node.Name, labels); err1 != nil {
 			klog.Errorf("failed to update annotations of node %s: %v", node.Name, err1)
 		}
 		c.recordProviderNetworkErr(pn.Name, err.Error())
@@ -319,7 +319,7 @@ func (c *Controller) initProviderNetwork(pn *kubeovnv1.ProviderNetwork, node *v1
 	labels[fmt.Sprintf(util.ProviderNetworkReadyTemplate, pn.Name)] = "true"
 	labels[fmt.Sprintf(util.ProviderNetworkInterfaceTemplate, pn.Name)] = nic
 	labels[fmt.Sprintf(util.ProviderNetworkMtuTemplate, pn.Name)] = strconv.Itoa(mtu)
-	if err = util.UpdateNodeLabels(c.config.KubeClient.CoreV1().Nodes(), node.Name, labels); err != nil {
+	if err = util.PatchLabels(c.config.KubeClient.CoreV1().Nodes(), node.Name, labels); err != nil {
 		klog.Errorf("failed to update labels of node %s: %v", node.Name, err)
 		return err
 	}
@@ -388,7 +388,7 @@ func (c *Controller) cleanProviderNetwork(pn *kubeovnv1.ProviderNetwork, node *v
 		fmt.Sprintf(util.ProviderNetworkMtuTemplate, pn.Name):       nil,
 		fmt.Sprintf(util.ProviderNetworkExcludeTemplate, pn.Name):   "true",
 	}
-	if err := util.UpdateNodeLabels(c.config.KubeClient.CoreV1().Nodes(), node.Name, labels); err != nil {
+	if err := util.PatchLabels(c.config.KubeClient.CoreV1().Nodes(), node.Name, labels); err != nil {
 		klog.Errorf("failed to update labels of node %s: %v", node.Name, err)
 		return err
 	}
@@ -417,7 +417,7 @@ func (c *Controller) handleDeleteProviderNetwork(pn *kubeovnv1.ProviderNetwork)
 		fmt.Sprintf(util.ProviderNetworkMtuTemplate, pn.Name):       nil,
 		fmt.Sprintf(util.ProviderNetworkExcludeTemplate, pn.Name):   nil,
 	}
-	if err = util.UpdateNodeLabels(c.config.KubeClient.CoreV1().Nodes(), node.Name, labels); err != nil {
+	if err = util.PatchLabels(c.config.KubeClient.CoreV1().Nodes(), node.Name, labels); err != nil {
 		klog.Errorf("failed to update labels of node %s: %v", node.Name, err)
 		return err
 	}

pkg/daemon/ovs_linux.go

@@ -1052,7 +1052,7 @@ func (c *Controller) patchNodeExternalGwLabel(enabled bool) error {
 	}
 
 	labels := map[string]any{util.NodeExtGwLabel: strconv.FormatBool(enabled)}
-	if err = util.UpdateNodeLabels(c.config.KubeClient.CoreV1().Nodes(), node.Name, labels); err != nil {
+	if err = util.PatchLabels(c.config.KubeClient.CoreV1().Nodes(), node.Name, labels); err != nil {
 		klog.Errorf("failed to update labels of node %s: %v", node.Name, err)
 		return err
 	}

pkg/ovn_ic_controller/ovn_ic_controller.go

@@ -243,7 +243,7 @@ func (c *Controller) removeInterConnection(azName string) error {
 	}
 	for _, node := range nodes {
 		labels := map[string]any{util.ICGatewayLabel: "false"}
-		if err = util.UpdateNodeLabels(c.config.KubeClient.CoreV1().Nodes(), node.Name, labels); err != nil {
+		if err = util.PatchLabels(c.config.KubeClient.CoreV1().Nodes(), node.Name, labels); err != nil {
 			klog.Errorf("failed to patch ic gw node %s: %v", node.Name, err)
 			return err
 		}
@@ -306,7 +306,7 @@ func (c *Controller) establishInterConnection(config map[string]string) error {
 				return err
 			}
 			labels := map[string]any{util.ICGatewayLabel: "true"}
-			if err = util.UpdateNodeLabels(c.config.KubeClient.CoreV1().Nodes(), node.Name, labels); err != nil {
+			if err = util.PatchLabels(c.config.KubeClient.CoreV1().Nodes(), node.Name, labels); err != nil {
 				klog.Errorf("failed to patch ic gw node %s: %v", node.Name, err)
 				return err
 			}

pkg/util/k8s.go

@@ -18,7 +18,6 @@ import (
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/selection"
 	"k8s.io/apimachinery/pkg/types"
-	clientv1 "k8s.io/client-go/kubernetes/typed/core/v1"
 	"k8s.io/klog/v2"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 
@@ -149,37 +148,34 @@ func GetTruncatedUID(uid string) string {
 	return uid[len(uid)-12:]
 }
 
-func UpdateNodeLabels(cs clientv1.NodeInterface, node string, labels map[string]any) error {
-	buf, err := json.Marshal(labels)
-	if err != nil {
-		klog.Errorf("failed to marshal labels: %v", err)
-		return err
-	}
-	patch := fmt.Sprintf(`{"metadata":{"labels":%s}}`, string(buf))
-	return nodeMergePatch(cs, node, patch)
+type Patcher[T metav1.Object] interface {
+	Patch(ctx context.Context, name string, patchType types.PatchType, patch []byte, opt metav1.PatchOptions, subresources ...string) (T, error)
 }
 
-func UpdateNodeAnnotations(cs clientv1.NodeInterface, node string, annotations map[string]any) error {
-	buf, err := json.Marshal(annotations)
+func patchMetaKVs[T metav1.Object](cs Patcher[T], name string, field string, kvs map[string]any) error {
+	m := map[string]map[string]map[string]any{"metadata": {field: kvs}}
+	patch, err := json.Marshal(m)
 	if err != nil {
-		klog.Errorf("failed to marshal annotations: %v", err)
+		klog.Errorf("failed to marshal patch %#v for field .metadata.%s: %v", kvs, field, err)
 		return err
 	}
-	patch := fmt.Sprintf(`{"metadata":{"annotations":%s}}`, string(buf))
-	return nodeMergePatch(cs, node, patch)
-}
 
-// we do not use GenerateMergePatchPayload/GenerateStrategicMergePatchPayload,
-// because we use a `null` value to delete a label/annotation
-func nodeMergePatch(cs clientv1.NodeInterface, node, patch string) error {
-	_, err := cs.Patch(context.Background(), node, types.MergePatchType, []byte(patch), metav1.PatchOptions{})
+	_, err = cs.Patch(context.Background(), name, types.MergePatchType, patch, metav1.PatchOptions{})
 	if err != nil {
-		klog.Errorf("failed to patch node %s with json merge patch %q: %v", node, patch, err)
+		klog.Errorf("failed to patch resource %s with json merge patch %q: %v", name, patch, err)
 		return err
 	}
 	return nil
 }
 
+func PatchLabels[T metav1.Object](cs Patcher[T], name string, labels map[string]any) error {
+	return patchMetaKVs(cs, name, "labels", labels)
+}
+
+func PatchAnnotations[T metav1.Object](cs Patcher[T], name string, annotations map[string]any) error {
+	return patchMetaKVs(cs, name, "annotations", annotations)
+}
+
 func SetOwnerReference(owner, object metav1.Object) error {
 	return controllerutil.SetOwnerReference(owner, object, scheme.Scheme)
 }

pkg/util/k8s_test.go

@@ -401,7 +401,7 @@ func TestUpdateNodeLabels(t *testing.T) {
 		require.NoError(t, err)
 		require.NotNil(t, node)
 		t.Run(tt.name, func(t *testing.T) {
-			err := UpdateNodeLabels(tt.cs, tt.node, tt.labels)
+			err := PatchLabels(tt.cs, tt.node, tt.labels)
 			if tt.exp == nil {
 				require.NoError(t, err)
 				return
@@ -459,47 +459,7 @@ func TestUpdateNodeAnnotations(t *testing.T) {
 		require.NoError(t, err)
 		require.NotNil(t, node)
 		t.Run(tt.name, func(t *testing.T) {
-			err := UpdateNodeAnnotations(tt.cs, tt.node, tt.annotations)
-			if tt.exp == nil {
-				require.NoError(t, err)
-				return
-			}
-			if errors.Is(err, tt.exp) {
-				t.Errorf("got %v, want %v", err, tt.exp)
-			}
-		})
-	}
-}
-
-func TestNodeMergePatch(t *testing.T) {
-	client := fake.NewSimpleClientset()
-	nodeClient := client.CoreV1().Nodes()
-	tests := []struct {
-		name  string
-		cs    clientv1.NodeInterface
-		node  string
-		patch string
-		exp   error
-	}{
-		{
-			name:  "node_with_patch",
-			cs:    nodeClient,
-			node:  "node",
-			patch: `{"metadata":{"labels":{"key1":"value1"}}}`,
-			exp:   nil,
-		},
-	}
-	for _, tt := range tests {
-		// create a node
-		node, err := client.CoreV1().Nodes().Create(context.Background(), &v1.Node{
-			ObjectMeta: metav1.ObjectMeta{
-				Name: tt.node,
-			},
-		}, metav1.CreateOptions{})
-		require.NoError(t, err)
-		require.NotNil(t, node)
-		t.Run(tt.name, func(t *testing.T) {
-			err := nodeMergePatch(tt.cs, tt.node, tt.patch)
+			err := PatchAnnotations(tt.cs, tt.node, tt.annotations)
 			if tt.exp == nil {
 				require.NoError(t, err)
 				return
@@ -830,41 +790,3 @@ func TestDeploymentIsReady(t *testing.T) {
 		})
 	}
 }
-
-func Test_nodeMergePatch(t *testing.T) {
-	tests := []struct {
-		name    string
-		patch   string
-		wantErr bool
-	}{
-		{
-			name:    "valid_merge_patch",
-			patch:   `{"metadata":{"labels":{"key1":"value1"}}}`,
-			wantErr: false,
-		},
-		{
-			name:    "invalid_merge_patch",
-			patch:   "invalid_merge_patch",
-			wantErr: true,
-		},
-	}
-
-	client := fake.NewClientset(&v1.Node{}).CoreV1().Nodes()
-	_, err := client.Create(context.TODO(), &v1.Node{
-		ObjectMeta: metav1.ObjectMeta{
-			Name: "node1",
-		},
-	}, metav1.CreateOptions{})
-	require.NoError(t, err)
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			err = nodeMergePatch(client, "node1", tt.patch)
-			if tt.wantErr {
-				require.Error(t, err)
-				return
-			}
-			require.NoError(t, err)
-		})
-	}
-}