Merge pull request spidernet-io#3812 from cyclinder/0.9/multus_uninstall

pod launched by unexpected CNI when the health checking of the agent fails and multus.conf is lost
ty-dc · Aug 1, 2024 · d9c5d5b · d9c5d5b
2 parents 35e9c89 + 8458374
commit d9c5d5b
Show file tree

Hide file tree

Showing 3 changed files with 64 additions and 43 deletions.
diff --git a/charts/spiderpool/templates/daemonset.yaml b/charts/spiderpool/templates/daemonset.yaml
@@ -104,36 +104,6 @@ spec:
             - name: cni-bin-path
               mountPath: /host/opt/cni/bin
         {{- end }}
-        {{- if .Values.multus.multusCNI.install }}
-        - name: multus-cni
-          imagePullPolicy: {{ .Values.multus.multusCNI.image.pullPolicy }}
-          image: {{ include "spiderpool.multus.image" . | quote }}
-          command:
-            - "/bin/sh"
-            - "-c"
-            - |
-              ITEM="multus"
-              rm -f /host/opt/cni/bin/${ITEM}.old || true
-              ( [ -f "/host/opt/cni/bin/${ITEM}" ] && mv /host/opt/cni/bin/${ITEM} /host/opt/cni/bin/${ITEM}.old ) || true
-              cp /usr/src/multus-cni/bin/${ITEM} /host/opt/cni/bin/${ITEM}
-              rm -f /host/opt/cni/bin/${ITEM}.old &>/dev/null  || true
-              sed -i 's/sleep infinity/echo \"exit...\"/g'  entrypoint.sh
-              ./entrypoint.sh --multus-conf-file=/tmp/multus-conf/00-multus.conf \
-                --cni-version=0.3.1
-          securityContext:
-            privileged: true
-          volumeMounts:
-            - name: cni
-              mountPath: /host/etc/cni/net.d
-            - name: cni-bin-path
-              mountPath: /host/opt/cni/bin
-              mountPropagation: Bidirectional
-            - name: multus-cfg
-              mountPath: /tmp/multus-conf
-            {{- if .Values.multus.multusCNI.extraVolumes }}
-            {{- include "tplvalues.render" ( dict "value" .Values.multus.multusCNI.extraVolumeMounts "context" $ ) | nindent 12 }}
-            {{- end }}
-        {{- end }}
       containers:
       - name: {{ .Values.spiderpoolAgent.name | trunc 63 | trimSuffix "-" }}
         image: {{ include "spiderpool.spiderpoolAgent.image" . | quote }}
@@ -203,18 +173,6 @@ spec:
                 	cp /usr/bin/${ITEM} /host/opt/cni/bin/${ITEM}
                 	rm -f /host/opt/cni/bin/${ITEM}.old &>/dev/null  || true
                 done
-          preStop:
-            exec:
-              command:
-              - "/bin/sh"
-              - "-c"
-              - | 
-                {{- if .Values.multus.multusCNI.uninstall }}
-                rm -f /host/opt/cni/bin/multus || true
-                rm -rf /host/etc/cni/net.d/multus.d || true
-                rm -f /host/etc/cni/net.d/00-multus.conf || true
-                {{- end }}
-                {{ .Values.spiderpoolAgent.binName }} shutdown
         env:
         - name: SPIDERPOOL_POD_NAME
           valueFrom:
@@ -262,6 +220,47 @@ spec:
         {{- if .Values.spiderpoolAgent.extraVolumes }}
         {{- include "tplvalues.render" ( dict "value" .Values.spiderpoolAgent.extraVolumeMounts "context" $ ) | nindent 8 }}
         {{- end }}
+     {{- if .Values.multus.multusCNI.install }}
+      - name: multus-cni
+        imagePullPolicy: {{ .Values.multus.multusCNI.image.pullPolicy }}
+        image: {{ include "spiderpool.multus.image" . | quote }}
+        command:
+          - "/bin/sh"
+          - "-c"
+          - |
+            ITEM="multus"
+            rm -f /host/opt/cni/bin/${ITEM}.old || true
+            ( [ -f "/host/opt/cni/bin/${ITEM}" ] && mv /host/opt/cni/bin/${ITEM} /host/opt/cni/bin/${ITEM}.old ) || true
+            cp /usr/src/multus-cni/bin/${ITEM} /host/opt/cni/bin/${ITEM}
+            rm -f /host/opt/cni/bin/${ITEM}.old &>/dev/null  || true
+            ./entrypoint.sh --multus-conf-file=/tmp/multus-conf/00-multus.conf \
+              --cni-version=0.3.1
+        securityContext:
+          privileged: true
+        {{- if .Values.multus.multusCNI.uninstall }}        
+        lifecycle:
+          preStop:
+            exec:
+              command:
+              - "/bin/sh"
+              - "-c"
+              - | 
+                rm -f /host/opt/cni/bin/multus || true
+                rm -rf /host/etc/cni/net.d/multus.d || true
+                rm -f /host/etc/cni/net.d/00-multus.conf || true
+        {{- end }}
+        volumeMounts:
+          - name: cni
+            mountPath: /host/etc/cni/net.d
+          - name: cni-bin-path
+            mountPath: /host/opt/cni/bin
+            mountPropagation: Bidirectional
+          - name: multus-cfg
+            mountPath: /tmp/multus-conf
+          {{- if .Values.multus.multusCNI.extraVolumes }}
+          {{- include "tplvalues.render" ( dict "value" .Values.multus.multusCNI.extraVolumeMounts "context" $ ) | nindent 12 }}
+          {{- end }}
+      {{- end }}
       volumes:
         # To read the configuration from the config map
       - name: config-path

diff --git a/test/Makefile b/test/Makefile
@@ -226,6 +226,7 @@ setup_spiderpool:
 		  	HELM_OPTION+=" --set coordinator.enabled=false " ; \
 		fi ; \
 		HELM_OPTION+=" --set multus.multusCNI.install=true " ; \
+		HELM_OPTION+=" --set multus.multusCNI.uninstall=true " ; \
 		HELM_OPTION+=" --set multus.multusCNI.image.registry= " ; \
 		HELM_OPTION+=" --set multus.multusCNI.image.repository=$(E2E_MULTUS_IMAGE_NAME) " ; \
 		if [ "$(INSTALL_OVERLAY_CNI)" == "true" ]; then \
@@ -382,6 +383,27 @@ helm_uninstall_spiderpool:
 	@echo -e "\033[35m [helm uninstall spiderpool] \033[0m"
 	helm uninstall $(RELEASE_NAME) --wait --debug -n $(RELEASE_NAMESPACE) \
 	     --kubeconfig $(E2E_KUBECONFIG)  || { KIND_CLUSTER_NAME=$(E2E_CLUSTER_NAME) ./scripts/debugEnv.sh $(E2E_KUBECONFIG) "detail"   ; exit 1 ; } ; \
+	echo "Multus config has been cleanup successfully." ; \
+	for ((i=0; i<100; i++)); do \
+    	if ! kubectl --kubeconfig=$(E2E_KUBECONFIG) get all --all-namespaces | grep -q "spiderpool" ; then \
+			echo "All resources successfully cleared." ; \
+			exit 0; \
+		fi ;\
+		echo "found spiderpool resources, waiting..." ; \
+		sleep 1 ; \
+	done; \
+	echo "error: found spiderpool resources" ; exit 1 ;
+    @echo "check cni conf removed from nodes: $(POST_UNINSTALL_CHECK_CNI_CONF) " ; \
+		NODE_LIST=` docker ps | egrep "kindest/node.* $(E2E_CLUSTER_NAME)-(control|worker)" | awk '{print $$1 }' ` ; \
+		[ -n "$$NODE_LIST" ] || { echo "error, failed to find any kind nodes, please setup kind cluster $(E2E_CLUSTER_NAME) first" ; exit 1 ; } ; \
+		for NODE in $${NODE_LIST} ; do \
+			ALL_CONF=`docker exec $${NODE} ls /etc/cni/net.d ` || { echo "error, failed to get cni conf /etc/cni/net.d on node $${NODE} " ; exit 1 ; }; \
+			ALL_CONF=` echo "$${ALL_CONF}" | tr '\n' ' ' ` ; \
+			echo "all cni conf on node $${NODE}: $${ALL_CONF} " ; \
+			for ITEM in $(POST_UNINSTALL_CHECK_CNI_CONF) ; do \
+			     grep " $${ITEM} " <<< " $${ALL_CONF} " &>/dev/null && { echo "error, found $${ITEM} under /etc/cni/net.d on node $${NODE} " ; exit 1 ; } ; \
+			done ; \
+		done
 
 .PHONY: upgrade_spiderpool
 upgrade_spiderpool:

diff --git a/test/e2e/coordinator/macvlan-overlay-one/macvlan_overlay_one_test.go b/test/e2e/coordinator/macvlan-overlay-one/macvlan_overlay_one_test.go
@@ -663,7 +663,7 @@ var _ = Describe("MacvlanOverlayOne", Label("overlay", "one-nic", "coordinator")
 			const SPIDERPOOL_ENABLED_RELEASE_CONFLICT_IPS = "SPIDERPOOL_ENABLED_RELEASE_CONFLICT_IPS"
 			spiderpoolAgentDS, err := frame.GetDaemonSet(constant.SpiderpoolAgent, "kube-system")
 			Expect(err).NotTo(HaveOccurred())
-			Expect(spiderpoolAgentDS.Spec.Template.Spec.Containers).To(HaveLen(1))
+			Expect(spiderpoolAgentDS.Spec.Template.Spec.Containers).To(HaveLen(2))
 
 			// the release conflicted IPs feature is default to be true if we do not set the ENV
 			isReleaseConflictIPs := true