fix security groups changed when vm is shut down (#4976)

* fix security groups changed when vm is closed * add unit test for UnionStringSlice Signed-off-by: zhaocongqi <[email protected]> --------- Signed-off-by: zhaocongqi <[email protected]>
kubeovn · Feb 12, 2025 · 7d7d2dd · 7d7d2dd
1 parent 501231a
commit 7d7d2dd
Show file tree

Hide file tree

Showing 3 changed files with 74 additions and 3 deletions.
diff --git a/pkg/controller/pod.go b/pkg/controller/pod.go
@@ -571,8 +571,19 @@ func (c *Controller) reconcileAllocateSubnets(pod *v1.Pod, needAllocatePodNets [
 				DHCPv6OptionsUUID: subnet.Status.DHCPv6OptionsUUID,
 			}
 
+			var oldSgList []string
+			if vmKey != "" {
+				existingLsp, err := c.OVNNbClient.GetLogicalSwitchPort(portName, true)
+				if err != nil {
+					klog.Errorf("failed to get logical switch port %s: %v", portName, err)
+					return nil, err
+				}
+				if existingLsp != nil {
+					oldSgList, _ = c.getPortSg(existingLsp)
+				}
+			}
+
 			securityGroupAnnotation := pod.Annotations[fmt.Sprintf(util.SecurityGroupAnnotationTemplate, podNet.ProviderName)]
-			securityGroups := strings.ReplaceAll(securityGroupAnnotation, " ", "")
 			if err := c.OVNNbClient.CreateLogicalSwitchPort(subnet.Name, portName, ipStr, mac, podName, pod.Namespace,
 				portSecurity, securityGroupAnnotation, vips, podNet.Subnet.Spec.EnableDHCP, dhcpOptions, subnet.Spec.Vpc); err != nil {
 				c.recorder.Eventf(pod, v1.EventTypeWarning, "CreateOVNPortFailed", err.Error())
@@ -588,8 +599,10 @@ func (c *Controller) reconcileAllocateSubnets(pod *v1.Pod, needAllocatePodNets [
 				}
 			}
 
-			if securityGroupAnnotation != "" {
-				sgNames := strings.Split(securityGroups, ",")
+			if securityGroupAnnotation != "" || oldSgList != nil {
+				securityGroups := strings.ReplaceAll(securityGroupAnnotation, " ", "")
+				newSgList := strings.Split(securityGroups, ",")
+				sgNames := util.UnionStringSlice(oldSgList, newSgList)
 				for _, sgName := range sgNames {
 					if sgName != "" {
 						c.syncSgPortsQueue.Add(sgName)

diff --git a/pkg/util/slice.go b/pkg/util/slice.go
@@ -1,5 +1,7 @@
 package util
 
+import "k8s.io/utils/set"
+
 func DiffStringSlice(slice1, slice2 []string) []string {
 	var diff []string
 
@@ -27,6 +29,14 @@ func DiffStringSlice(slice1, slice2 []string) []string {
 	return diff
 }
 
+func UnionStringSlice(slices ...[]string) []string {
+	union := set.New[string]()
+	for _, s := range slices {
+		union.Insert(s...)
+	}
+	return union.UnsortedList()
+}
+
 // IsStringsOverlap check if two string slices are overlapped
 func IsStringsOverlap(a, b []string) bool {
 	for _, sa := range a {

diff --git a/pkg/util/slice_test.go b/pkg/util/slice_test.go
@@ -37,6 +37,54 @@ func TestDiffStringSlice(t *testing.T) {
 	}
 }
 
+func TestUnionStringSlice(t *testing.T) {
+	t.Parallel()
+	tests := []struct {
+		desc   string
+		slice1 []string
+		slice2 []string
+		want   []string
+	}{
+		{
+			desc:   "both slices nil",
+			slice1: nil,
+			slice2: nil,
+			want:   []string{},
+		},
+		{
+			desc:   "first slice nil",
+			slice1: nil,
+			slice2: []string{"a", "b", "c"},
+			want:   []string{"a", "b", "c"},
+		},
+		{
+			desc:   "second slice nil",
+			slice1: []string{"x", "y", "z"},
+			slice2: nil,
+			want:   []string{"x", "y", "z"},
+		},
+		{
+			desc:   "duplicate elements",
+			slice1: []string{"a", "b", "a", "c"},
+			slice2: []string{"b", "c", "c", "d"},
+			want:   []string{"a", "b", "c", "d"},
+		},
+		{
+			desc:   "empty slices",
+			slice1: []string{},
+			slice2: []string{},
+			want:   []string{},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.desc, func(t *testing.T) {
+			result := UnionStringSlice(tt.slice1, tt.slice2)
+			require.ElementsMatch(t, tt.want, result)
+		})
+	}
+}
+
 func TestIsStringsOverlap(t *testing.T) {
 	tests := []struct {
 		name string