Merge pull request alibaba#402 from hhyasdf/feature/introduce-update-ipinstance-status-for-daemon

feat: introduce update-ipinstance-status for daemon

Author: mars1024

charts/hybridnet/crds/multicluster.alibaba.com_remoteclusters.yaml

@@ -83,13 +83,14 @@ spec:
                   description: "Condition contains details for one aspect of the current
                     state of this API Resource. --- This struct is intended for direct
                     use as an array at the field path .status.conditions.  For example,
-                    type FooStatus struct{     // Represents the observations of a
-                    foo's current state.     // Known .status.conditions.type are:
-                    \"Available\", \"Progressing\", and \"Degraded\"     // +patchMergeKey=type
-                    \    // +patchStrategy=merge     // +listType=map     // +listMapKey=type
-                    \    Conditions []metav1.Condition `json:\"conditions,omitempty\"
-                    patchStrategy:\"merge\" patchMergeKey:\"type\" protobuf:\"bytes,1,rep,name=conditions\"`
-                    \n     // other fields }"
+                    \n \ttype FooStatus struct{ \t    // Represents the observations
+                    of a foo's current state. \t    // Known .status.conditions.type
+                    are: \"Available\", \"Progressing\", and \"Degraded\" \t    //
+                    +patchMergeKey=type \t    // +patchStrategy=merge \t    // +listType=map
+                    \t    // +listMapKey=type \t    Conditions []metav1.Condition
+                    `json:\"conditions,omitempty\" patchStrategy:\"merge\" patchMergeKey:\"type\"
+                    protobuf:\"bytes,1,rep,name=conditions\"` \n \t    // other fields
+                    \t}"
                   properties:
                     lastTransitionTime:
                       description: lastTransitionTime is the last time the condition

charts/hybridnet/crds/multicluster.alibaba.com_remoteendpointslice.yaml

@@ -67,11 +67,13 @@ spec:
                     a service.
                   properties:
                     addresses:
-                      description: addresses of this endpoint. The contents of this
+                      description: 'addresses of this endpoint. The contents of this
                         field are interpreted according to the corresponding EndpointSlice
                         addressType field. Consumers must handle different types of
                         addresses in the context of their own capabilities. This must
-                        contain at least one address but no more than 100.
+                        contain at least one address but no more than 100. These are
+                        all assumed to be fungible and clients may choose to only
+                        use the first element. Refer to: https://issue.k8s.io/106267'
                       items:
                         type: string
                       type: array
@@ -136,8 +138,7 @@ spec:
                     nodeName:
                       description: nodeName represents the name of the Node hosting
                         this endpoint. This can be used to determine endpoints local
-                        to a Node. This field can be enabled with the EndpointSliceNodeName
-                        feature gate.
+                        to a Node.
                       type: string
                     targetRef:
                       description: targetRef is a reference to a Kubernetes object
@@ -212,7 +213,7 @@ spec:
                       description: The application protocol for this port. This field
                         follows standard Kubernetes label syntax. Un-prefixed names
                         are reserved for IANA standard service names (as per RFC-6335
-                        and http://www.iana.org/assignments/service-names). Non-standard
+                        and https://www.iana.org/assignments/service-names). Non-standard
                         protocols should use prefixed names such as mycompany.com/my-custom-protocol.
                       type: string
                     name:

charts/hybridnet/crds/networking.alibaba.com_ipinstances.yaml

@@ -125,9 +125,6 @@ spec:
             properties:
               nodeName:
                 type: string
-              phase:
-                description: DEPRECATED. Planned to remove in v0.6
-                type: string
               podName:
                 type: string
               podNamespace:

charts/hybridnet/templates/daemonsets.yaml

@@ -82,6 +82,7 @@ spec:
             - --check-pod-connectivity-from-host={{ .Values.daemon.checkPodConnectivityFromHost }}
             - --enable-vlan-arp-enhancement={{ .Values.daemon.enableVlanARPEnhancement }}
             - --feature-gates=MultiCluster={{ .Values.multiCluster }}
+            - --update-ipinstance-status={{ .Values.daemon.updateIPInstanceStatus }}
           securityContext:
             runAsUser: 0
             privileged: true

charts/hybridnet/values.yaml

@@ -90,7 +90,7 @@ daemon:
 
   ## If it is empty, daemon on each node will take one of the valid address of the vxlan interface's parent
   ## (try ipv4 addresses first and then ipv6 addresses if no valid ipv4 address exists) as node's VTEP address
-  ## randomly. If is is not empty, the first result matches any of the CIDRs will be chose as VTEP address.
+  ## randomly. If it is not empty, the first result matches any of the CIDRs will be chosen as VTEP address.
   vtepAddressCIDRs: "0.0.0.0/0,::/0"
 
   # -- The community CNI plugins needed to be copied by hybridnet from inside container to the /opt/cni/bin/ directory of host
@@ -102,6 +102,9 @@ daemon:
   # -- Whether will daemon check the connectivity of local pod before staring it
   checkPodConnectivityFromHost: true
 
+  # -- Whether will daemon update the status of IPInstance while create pod sandbox
+  updateIPInstanceStatus: true
+
   # -- Specifies the resources for the cni-daemon containers
   resources: {}
     # limits:

pkg/apis/networking/v1/ipinstance_types.go

@@ -70,9 +70,6 @@ type StatefulInfo struct {
 type IPInstanceStatus struct {
 	// +kubebuilder:validation:Optional
 	NodeName string `json:"nodeName,omitempty"`
-	// DEPRECATED. Planned to remove in v0.6
-	// +kubebuilder:validation:Optional
-	Phase IPPhase `json:"phase,omitempty"`
 	// +kubebuilder:validation:Optional
 	PodName string `json:"podName,omitempty"`
 	// +kubebuilder:validation:Optional

pkg/apis/networking/v1/types.go

@@ -124,8 +124,6 @@ type VTEPInfo struct {
 	LocalIPs []string `json:"localIPs,omitempty"`
 }
 
-type IPPhase string
-
 // The conversion process from IPInstance v1.1 to v1.2 has been removed after hybridnet v0.6.0.
 const (
 	IPInstanceV12           = "v1.2"

pkg/controllers/multicluster/remotevtep_controller.go

@@ -285,9 +285,6 @@ func (r *RemoteVtepReconciler) SetupWithManager(mgr ctrl.Manager) (err error) {
 							constants.LabelNode,
 						},
 					},
-					// TODO: phase change means nothing in new IPInstance model,
-					// to be removed in next major version
-					&utils.IPInstancePhaseChangePredicate{},
 				),
 			),
 		).

pkg/controllers/networking/pod_controller.go

@@ -692,7 +692,7 @@ func (r *PodReconciler) checkMACAddressCollision(pod *corev1.Pod, networkName st
 		if !ipInstance.DeletionTimestamp.IsZero() {
 			continue
 		}
-		if ipInstance.Status.PodNamespace != pod.GetNamespace() || ipInstance.Status.PodName != pod.GetName() {
+		if ipInstance.Namespace != pod.GetNamespace() || ipInstance.Spec.Binding.PodName != pod.GetName() {
 			return fmt.Errorf("specified mac address %s is in conflict with existing ip instance %s/%s", macAddr, ipInstance.Namespace, ipInstance.Name)
 		}
 	}

pkg/controllers/utils/predicates.go

@@ -186,24 +186,6 @@ func (s SpecifiedLabelChangedPredicate) Update(e event.UpdateEvent) bool {
 	return false
 }
 
-type IPInstancePhaseChangePredicate struct {
-	predicate.Funcs
-}
-
-// Update implements default UpdateEvent filter for checking whether IPInstance phase change
-func (IPInstancePhaseChangePredicate) Update(e event.UpdateEvent) bool {
-	oldIPInstance, ok := e.ObjectOld.(*networkingv1.IPInstance)
-	if !ok {
-		return false
-	}
-	newIPInstance, ok := e.ObjectNew.(*networkingv1.IPInstance)
-	if !ok {
-		return false
-	}
-
-	return oldIPInstance.Status.Phase != newIPInstance.Status.Phase
-}
-
 type RemoteClusterUUIDChangePredicate struct {
 	predicate.Funcs
 }

pkg/daemon/config/config.go

@@ -103,6 +103,7 @@ type Configuration struct {
 	EnableVlanArpEnhancement     bool
 	PatchCalicoPodIPsAnnotation  bool
 	CheckPodConnectivityFromHost bool
+	UpdateIPInstanceStatus       bool
 }
 
 // ParseFlags will parse cmd args then init kubeClient and configuration
@@ -134,6 +135,7 @@ func ParseFlags() (*Configuration, error) {
 		argIPv6RouteCacheGCThresh               = pflag.Int("ipv6-route-cache-gc-thresh", DefaultIPv6RouteCacheGCThresh, "Value to set net.ipv6.route.gc_thresh")
 		argPatchCalicoPodIPsAnnotation          = pflag.Bool("patch-calico-pod-ips-annotation", true, "Patch \"cni.projectcalico.org/podIPs\" annotations to pod")
 		argCheckPodConnectivityFromHost         = pflag.Bool("check-pod-connectivity-from-host", true, "Check pod's connectivity from host before start it")
+		argUpdateIPInstanceStatus               = pflag.Bool("update-ipinstance-status", true, "Update ipinstance status while creating pod sandbox")
 	)
 
 	// mute info log for ipset lib
@@ -172,6 +174,7 @@ func ParseFlags() (*Configuration, error) {
 		IPv6RouteCacheGCThresh:               *argIPv6RouteCacheGCThresh,
 		PatchCalicoPodIPsAnnotation:          *argPatchCalicoPodIPsAnnotation,
 		CheckPodConnectivityFromHost:         *argCheckPodConnectivityFromHost,
+		UpdateIPInstanceStatus:               *argUpdateIPInstanceStatus,
 	}
 
 	if *argPreferVlanInterfaces == "" {

pkg/daemon/server/handle.go

@@ -281,22 +281,24 @@ func (cdh *cniDaemonHandler) handleAdd(req *restful.Request, resp *restful.Respo
 		"macAddr", macAddr)
 
 	// update IPInstance crd status
-	for _, ip := range affectedIPInstances {
-		if err := retry.RetryOnConflict(retry.DefaultRetry, func() error {
-			var updateTimestamp string
-			updateTimestamp, err = metav1.Now().MarshalQueryParameter()
-			if err != nil {
-				return fmt.Errorf("failed to generate update timestamp: %v", err)
+	if cdh.config.UpdateIPInstanceStatus {
+		for _, ip := range affectedIPInstances {
+			if err := retry.RetryOnConflict(retry.DefaultRetry, func() error {
+				var updateTimestamp string
+				updateTimestamp, err = metav1.Now().MarshalQueryParameter()
+				if err != nil {
+					return fmt.Errorf("failed to generate update timestamp: %v", err)
+				}
+
+				return cdh.mgrClient.Status().Patch(context.TODO(), ip,
+					client.RawPatch(types.MergePatchType,
+						[]byte(fmt.Sprintf(`{"status":{"sandboxID":%q,"nodeName":%q,"podNamespace":%q,"podName":%q,"updateTimestamp":%q}}`,
+							podRequest.ContainerID, cdh.config.NodeName, podRequest.PodNamespace, podRequest.PodName, updateTimestamp))))
+			}); err != nil {
+				errMsg := fmt.Errorf("failed to update IPInstance crd for %s, %v", ip.Name, err)
+				cdh.errorWrapper(errMsg, http.StatusInternalServerError, resp)
+				return
 			}
-
-			return cdh.mgrClient.Status().Patch(context.TODO(), ip,
-				client.RawPatch(types.MergePatchType,
-					[]byte(fmt.Sprintf(`{"status":{"sandboxID":%q,"nodeName":%q,"podNamespace":%q,"podName":%q,"phase":null,"updateTimestamp":%q}}`,
-						podRequest.ContainerID, cdh.config.NodeName, podRequest.PodNamespace, podRequest.PodName, updateTimestamp))))
-		}); err != nil {
-			errMsg := fmt.Errorf("failed to update IPInstance crd for %s, %v", ip.Name, err)
-			cdh.errorWrapper(errMsg, http.StatusInternalServerError, resp)
-			return
 		}
 	}