Merge pull request #634 from cybozu-go/fix-to-apply-resources-with-sa…

…me-rank fix to apply resources that has same rank at the same time
cybozu-go · Jun 20, 2023 · a4606eb · a4606eb
2 parents cbe93e0 + 5a9ce76
commit a4606eb
Show file tree

Hide file tree

Showing 3 changed files with 389 additions and 26 deletions.
diff --git a/mtest/kubernetes_test.go b/mtest/kubernetes_test.go
@@ -1035,6 +1035,195 @@ spec:
 
 	})
 
+	It("should apply resources that has same rank at the same time", func() {
+		resources := `apiVersion: apps/v1
+kind: Deployment
+metadata:
+  namespace: foo
+  name: test-deployment4
+  labels:
+    cke.cybozu.com/appname: test-deployment4
+  annotations:
+    cke.cybozu.com/rank: "2500"
+spec:
+  replicas: 1
+  selector:
+    matchLabels:
+      cke.cybozu.com/appname: test-deployment4
+  template:
+    metadata:
+      labels:
+        cke.cybozu.com/appname: test-deployment4
+    spec:
+      containers:
+      - name: ubuntu
+        image: quay.io/cybozu/ubuntu:20.04
+        args:
+        - pause
+---
+kind: DaemonSet
+apiVersion: apps/v1
+metadata:
+  name: test-daemonset2
+  namespace: foo
+  annotations:
+    cke.cybozu.com/rank: "2500"
+spec:
+  selector:
+    matchLabels:
+      cke.cybozu.com/appname: test-daemonset2
+  template:
+    metadata:
+      labels:
+        cke.cybozu.com/appname: test-daemonset2
+    spec:
+      initContainers:
+      - name: wait-10sec
+        image: quay.io/cybozu/ubuntu:20.04
+        args:
+        - sleep
+        - "10"
+      containers:
+      - name: ubuntu
+        image: quay.io/cybozu/ubuntu:20.04
+        args:
+        - pause
+`
+		By("creating resources")
+		ckecliWithInput([]byte(resources), "resource", "set", "-")
+		defer ckecliWithInput([]byte(resources), "resource", "delete", "-")
+		ts := time.Now()
+
+		By("getting test-deployment4 and test-daemonset2")
+		Eventually(func() error {
+			dp := &appsv1.Deployment{}
+			stdout, _, err := kubectl("-n", "foo", "get", "deployment", "test-deployment4", "-o", "json")
+			if err != nil {
+				return err
+			}
+			if err := json.Unmarshal(stdout, dp); err != nil {
+				return err
+			}
+			ds := &appsv1.DaemonSet{}
+			stdout, _, err = kubectl("-n", "foo", "get", "daemonset", "test-daemonset2", "-o", "json")
+			if err != nil {
+				return err
+			}
+			if err := json.Unmarshal(stdout, ds); err != nil {
+				return err
+			}
+			return nil
+		}).Should(Succeed())
+
+		cluster := getCluster()
+		for i := 0; i < 3; i++ {
+			cluster.Nodes[i].ControlPlane = true
+		}
+		Eventually(func() error {
+			return checkCluster(cluster, ts)
+		}).Should(Succeed())
+	})
+
+	It("should update resources that has same rank at the same time", func() {
+		resources := `apiVersion: apps/v1
+kind: Deployment
+metadata:
+  namespace: foo
+  name: test-deployment4
+  labels:
+    cke.cybozu.com/appname: test-deployment4
+    updated: "true"
+  annotations:
+    cke.cybozu.com/rank: "2500"
+spec:
+  replicas: 1
+  selector:
+    matchLabels:
+      cke.cybozu.com/appname: test-deployment4
+  template:
+    metadata:
+      labels:
+        cke.cybozu.com/appname: test-deployment4
+    spec:
+      containers:
+      - name: ubuntu
+        image: quay.io/cybozu/ubuntu:20.04
+        args:
+        - pause
+---
+kind: DaemonSet
+apiVersion: apps/v1
+metadata:
+  name: test-daemonset2
+  namespace: foo
+  annotations:
+    cke.cybozu.com/rank: "2500"
+  labels:
+    updated: "true"
+spec:
+  selector:
+    matchLabels:
+      cke.cybozu.com/appname: test-daemonset2
+  template:
+    metadata:
+      labels:
+        cke.cybozu.com/appname: test-daemonset2
+    spec:
+      initContainers:
+      - name: wait-10sec
+        image: quay.io/cybozu/ubuntu:20.04
+        args:
+        - sleep
+        - "10"
+      containers:
+      - name: ubuntu
+        image: quay.io/cybozu/ubuntu:20.04
+        args:
+        - pause
+`
+		By("creating resources")
+		ckecliWithInput([]byte(resources), "resource", "set", "-")
+		defer ckecliWithInput([]byte(resources), "resource", "delete", "-")
+		ts := time.Now()
+
+		By("getting updated test-deployment4 and test-daemonset2")
+		Eventually(func() error {
+			dp := &appsv1.Deployment{}
+			stdout, _, err := kubectl("-n", "foo", "get", "deployment", "test-deployment4", "-o", "json")
+			if err != nil {
+				return err
+			}
+			if err := json.Unmarshal(stdout, dp); err != nil {
+				return err
+			}
+			l, ok := dp.Labels["updated"]
+			if !ok || l != "true" {
+				return fmt.Errorf("test-deployment4 must has the label named update")
+			}
+			ds := &appsv1.DaemonSet{}
+			stdout, _, err = kubectl("-n", "foo", "get", "daemonset", "test-daemonset2", "-o", "json")
+			if err != nil {
+				return err
+			}
+			if err := json.Unmarshal(stdout, ds); err != nil {
+				return err
+			}
+			l, ok = ds.Labels["updated"]
+			if !ok || l != "true" {
+				return fmt.Errorf("test-daemonset2 must has the label named update")
+			}
+			return nil
+		}).Should(Succeed())
+
+		cluster := getCluster()
+		for i := 0; i < 3; i++ {
+			cluster.Nodes[i].ControlPlane = true
+		}
+		Eventually(func() error {
+			return checkCluster(cluster, ts)
+		}).Should(Succeed())
+	})
+
 	It("embed certificates for webhooks", func() {
 		By("set user-defined resource")
 		_, _, err := ckecliWithInput(webhookYAML, "resource", "set", "-")

diff --git a/server/strategy.go b/server/strategy.go
@@ -625,20 +625,39 @@ func decideResourceOps(apiServer *cke.Node, ks cke.KubernetesClusterStatus, reso
 			return ops
 		}
 	}
+
+	// decide to apply user defined resources
+	if len(resources) == 0 {
+		return ops
+	}
+	rank := resources[0].Rank
+
+	// resources are sorted by rank in advance
 	for _, res := range resources {
+		if res.Rank != rank && len(ops) > 0 {
+			return ops
+		}
 		if res.Kind == cke.KindDeployment && !isReady {
 			continue
 		}
 		status, ok := ks.ResourceStatuses[res.Key]
 		if !ok {
 			ops = append(ops, op.ResourceApplyOp(apiServer, res, !status.HasBeenSSA))
-			// To wait for the completion to create or update, we avoid applying subsequent resources.
-			return ops
+			log.Info("need to create", map[string]interface{}{
+				"resource_name":      res.Name,
+				"resource_namespace": res.Namespace,
+				"kind":               res.Kind,
+				"completed":          status.Completed,
+			})
 		} else {
 			if res.NeedUpdate(&status) {
+				log.Info("need to update", map[string]interface{}{
+					"resource_name":      res.Name,
+					"resource_namespace": res.Namespace,
+					"kind":               res.Kind,
+					"completed":          status.Completed,
+				})
 				ops = append(ops, op.ResourceApplyOp(apiServer, res, !status.HasBeenSSA))
-				// To avoid applying subsequent resources not to wait the completion the update or creation
-				return ops
 			} else {
 				if !status.Completed {
 					log.Info("need to wait", map[string]interface{}{
@@ -648,11 +667,10 @@ func decideResourceOps(apiServer *cke.Node, ks cke.KubernetesClusterStatus, reso
 						"completed":          status.Completed,
 					})
 					ops = append(ops, op.NopOp())
-					// To avoid applying subsequent resources not to wait the completion the update or creation
-					return ops
 				}
 			}
 		}
+		rank = res.Rank
 	}
 	return ops
 }