1.11 distinguish portSecurity with security group (#3863)

* distinguish portSecurity with security group --------- Signed-off-by: bobz965 <[email protected]>
kubeovn · Apr 17, 2024 · da69a41 · da69a41
1 parent 9c45f46
commit da69a41
Show file tree

Hide file tree

Showing 4 changed files with 43 additions and 35 deletions.
diff --git a/pkg/controller/pod.go b/pkg/controller/pod.go
@@ -672,7 +672,6 @@ func (c *Controller) handleAddPod(key string) error {
 				portSecurity = true
 			}
 
-			securityGroupAnnotation := pod.Annotations[fmt.Sprintf(util.SecurityGroupAnnotationTemplate, podNet.ProviderName)]
 			vips := pod.Annotations[fmt.Sprintf(util.PortVipAnnotationTemplate, podNet.ProviderName)]
 			for _, ip := range strings.Split(vips, ",") {
 				if ip != "" && net.ParseIP(ip) == nil {
@@ -689,18 +688,20 @@ func (c *Controller) handleAddPod(key string) error {
 			}
 
 			hasUnknown := pod.Annotations[fmt.Sprintf(util.Layer2ForwardAnnotationTemplate, podNet.ProviderName)] == "true"
-			if err := c.ovnLegacyClient.CreatePort(subnet.Name, portName, ipStr, mac, podName, pod.Namespace, portSecurity, securityGroupAnnotation, vips, podNet.AllowLiveMigration, podNet.Subnet.Spec.EnableDHCP, dhcpOptions, hasUnknown); err != nil {
+			securityGroupAnnotation := pod.Annotations[fmt.Sprintf(util.SecurityGroupAnnotationTemplate, podNet.ProviderName)]
+			var securityGroups string
+			if securityGroupAnnotation != "" {
+				securityGroups = strings.ReplaceAll(securityGroupAnnotation, " ", "")
+			}
+			if err := c.ovnLegacyClient.CreatePort(subnet.Name, portName, ipStr, mac, podName, pod.Namespace, portSecurity,
+				securityGroups, vips, podNet.AllowLiveMigration, podNet.Subnet.Spec.EnableDHCP, dhcpOptions, hasUnknown); err != nil {
 				c.recorder.Eventf(pod, v1.EventTypeWarning, "CreateOVNPortFailed", err.Error())
 				klog.Error(err)
 				return err
 			}
-
-			if portSecurity {
-				sgNames := strings.Split(securityGroupAnnotation, ",")
-				for _, sgName := range sgNames {
-					if sgName == "" {
-						continue
-					}
+			sgNames := strings.Split(securityGroups, ",")
+			for _, sgName := range sgNames {
+				if sgName != "" {
 					c.syncSgPortsQueue.Add(sgName)
 				}
 			}
@@ -916,10 +917,10 @@ func (c *Controller) handleUpdatePodSecurity(key string) error {
 		}
 		c.syncVirtualPortsQueue.Add(podNet.Subnet.Name)
 
+		securityGroupAnnotation := pod.Annotations[fmt.Sprintf(util.SecurityGroupAnnotationTemplate, podNet.ProviderName)]
 		var securityGroups string
-		if portSecurity {
-			securityGroups = pod.Annotations[fmt.Sprintf(util.SecurityGroupAnnotationTemplate, podNet.ProviderName)]
-			securityGroups = strings.ReplaceAll(securityGroups, " ", "")
+		if securityGroupAnnotation != "" {
+			securityGroups = strings.ReplaceAll(securityGroupAnnotation, " ", "")
 		}
 		if err = c.reconcilePortSg(ovs.PodNameToPortName(podName, namespace, podNet.ProviderName), securityGroups); err != nil {
 			klog.Errorf("reconcilePortSg failed. %v", err)

diff --git a/pkg/controller/security_group.go b/pkg/controller/security_group.go
@@ -359,16 +359,6 @@ func (c *Controller) syncSgLogicalPort(key string) error {
 	c.sgKeyMutex.Lock(key)
 	defer c.sgKeyMutex.Unlock(key)
 
-	sg, err := c.sgsLister.Get(key)
-	if err != nil {
-		if k8serrors.IsNotFound(err) {
-			klog.Errorf("sg '%s' not found.", key)
-			return nil
-		}
-		klog.Errorf("failed to get sg '%s'. %v", key, err)
-		return err
-	}
-
 	results, err := c.ovnLegacyClient.CustomFindEntity("logical_switch_port", []string{"_uuid", "name", "port_security"}, fmt.Sprintf("external_ids:associated_sg_%s=true", key))
 	if err != nil {
 		klog.Errorf("failed to find logical port, %v", err)
@@ -378,10 +368,10 @@ func (c *Controller) syncSgLogicalPort(key string) error {
 	var v4s, v6s []string
 	var ports []string
 	for _, ret := range results {
+		ports = append(ports, ret["name"][0])
 		if len(ret["port_security"]) < 2 {
 			continue
 		}
-		ports = append(ports, ret["name"][0])
 		for _, address := range ret["port_security"][1:] {
 			if strings.Contains(address, ":") {
 				v6s = append(v6s, address)
@@ -391,6 +381,15 @@ func (c *Controller) syncSgLogicalPort(key string) error {
 		}
 	}
 
+	sg, err := c.sgsLister.Get(key)
+	if err != nil {
+		if k8serrors.IsNotFound(err) {
+			klog.Warningf("no security group %s ", key)
+			return nil
+		}
+		klog.Errorf("failed to get security group %s: %v", key, err)
+		return err
+	}
 	if err = c.ovnLegacyClient.SetPortsToPortGroup(sg.Status.PortGroup, ports); err != nil {
 		klog.Errorf("failed to set port to sg, %v", err)
 		return err

diff --git a/pkg/controller/service.go b/pkg/controller/service.go
@@ -89,7 +89,7 @@ func (c *Controller) enqueueDeleteService(obj interface{}) {
 			for _, ip := range ips {
 				vpcSvc.Vips = append(vpcSvc.Vips, util.JoinHostPort(ip, port.Port))
 			}
-			klog.Infof("delete vpc service %v", vpcSvc)
+			klog.V(3).Infof("delete vpc service: %v", vpcSvc)
 			c.deleteServiceQueue.Add(vpcSvc)
 		}
 	}

diff --git a/pkg/ovs/ovn-nbctl-legacy.go b/pkg/ovs/ovn-nbctl-legacy.go
@@ -348,17 +348,16 @@ func (c LegacyClient) CreatePort(ls, port, ip, mac, pod, namespace string, portS
 		if vips != "" {
 			addresses = append(addresses, strings.Split(vips, ",")...)
 		}
-		ovnCommand = append(ovnCommand,
-			"--", "lsp-set-port-security", port, strings.Join(addresses, " "))
+		ovnCommand = append(ovnCommand, "--", "lsp-set-port-security", port, strings.Join(addresses, " "))
+	}
 
-		if securityGroups != "" {
-			sgList := strings.Split(securityGroups, ",")
+	if securityGroups != "" {
+		sgList := strings.Split(securityGroups, ",")
+		ovnCommand = append(ovnCommand,
+			"--", "set", "logical_switch_port", port, fmt.Sprintf("external_ids:security_groups=%s", strings.ReplaceAll(securityGroups, ",", "/")))
+		for _, sg := range sgList {
 			ovnCommand = append(ovnCommand,
-				"--", "set", "logical_switch_port", port, fmt.Sprintf("external_ids:security_groups=%s", strings.ReplaceAll(securityGroups, ",", "/")))
-			for _, sg := range sgList {
-				ovnCommand = append(ovnCommand,
-					"--", "set", "logical_switch_port", port, fmt.Sprintf("external_ids:associated_sg_%s=true", sg))
-			}
+				"--", "set", "logical_switch_port", port, fmt.Sprintf("external_ids:associated_sg_%s=true", sg))
 		}
 	}
 
@@ -2113,13 +2112,22 @@ func (c LegacyClient) ListPgPortsForNodePortgroup() (map[string][]string, error)
 }
 
 func (c LegacyClient) SetPortsToPortGroup(portGroup string, portNames []string) error {
+	if portGroup == "" {
+		err := fmt.Errorf("port group name is empty")
+		klog.Error(err)
+		return err
+	}
 	ovnArgs := []string{"clear", "port_group", portGroup, "ports"}
 	if len(portNames) > 0 {
 		ovnArgs = []string{"pg-set-ports", portGroup}
 		ovnArgs = append(ovnArgs, portNames...)
 	}
-	_, err := c.ovnNbCommand(ovnArgs...)
-	return err
+	if _, err := c.ovnNbCommand(ovnArgs...); err != nil {
+		err = fmt.Errorf("failed to set ports to port group %s: %v", portGroup, err)
+		klog.Error(err)
+		return err
+	}
+	return nil
 }
 
 func (c LegacyClient) SetAddressesToAddressSet(addresses []string, as string) error {