From 75dc72aa33840bf3d0a5d7c4c04bb7a5ccf261bd Mon Sep 17 00:00:00 2001
From: Ai Ranthem <zhongtianyun.zty@alibaba-inc.com>
Date: Fri, 20 Jun 2025 17:33:24 +0800
Subject: [PATCH] Fix: Webhook will take over Deployment even Rollout CR
 doesn't exist (#278) (#280)

(cherry picked from commit deaa5f38b579b4c1be2db8ed8b42cb7dbfcb52ce)

Signed-off-by: AiRanthem <zhongtianyun.zty@alibaba-inc.com>
---
 pkg/controller/rollout/rollout_controller.go  |  9 ------
 .../mutating/workload_update_handler.go       | 14 +++++----
 .../mutating/workload_update_handler_test.go  | 31 +++++++++++++++++++
 3 files changed, 39 insertions(+), 15 deletions(-)

diff --git a/pkg/controller/rollout/rollout_controller.go b/pkg/controller/rollout/rollout_controller.go
index a1fe560..5efc2f2 100755
--- a/pkg/controller/rollout/rollout_controller.go
+++ b/pkg/controller/rollout/rollout_controller.go
@@ -85,15 +85,6 @@ type RolloutReconciler struct {
 // +kubebuilder:rbac:groups=apps.kruise.io,resources=daemonsets/status,verbs=get;update;patch
 //+kubebuilder:rbac:groups=networking.istio.io,resources=virtualservices;destinationrules,verbs=get;list;watch;update;patch
 
-// Reconcile is part of the main kubernetes reconciliation loop which aims to
-// move the current state of the cluster closer to the desired state.
-// TODO(user): Modify the Reconcile function to compare the state specified by
-// the Rollout object against the actual cluster state, and then
-// perform operations to make the cluster state reflect the state specified by
-// the user.
-//
-// For more details, check Reconcile and its Result here:
-// - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.8.3/pkg/reconcile
 func (r *RolloutReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
 	// Fetch the Rollout instance
 	rollout := &v1beta1.Rollout{}
diff --git a/pkg/webhook/workload/mutating/workload_update_handler.go b/pkg/webhook/workload/mutating/workload_update_handler.go
index 60a443d..390ce95 100644
--- a/pkg/webhook/workload/mutating/workload_update_handler.go
+++ b/pkg/webhook/workload/mutating/workload_update_handler.go
@@ -260,6 +260,14 @@ func (h *WorkloadHandler) handleStatefulSetLikeWorkload(newObj, oldObj *unstruct
 }
 
 func (h *WorkloadHandler) handleDeployment(newObj, oldObj *apps.Deployment) (bool, error) {
+	// make sure matched Rollout CR always exists
+	rollout, err := h.fetchMatchedRollout(newObj)
+	if err != nil {
+		return false, err
+	} else if rollout == nil || rollout.Spec.Strategy.Canary == nil {
+		return false, nil
+	}
+
 	// in rollout progressing
 	if newObj.Annotations[util.InRolloutProgressingAnnotation] != "" {
 		modified := false
@@ -307,12 +315,6 @@ func (h *WorkloadHandler) handleDeployment(newObj, oldObj *apps.Deployment) (boo
 		return false, nil
 	}
 
-	rollout, err := h.fetchMatchedRollout(newObj)
-	if err != nil {
-		return false, err
-	} else if rollout == nil || rollout.Spec.Strategy.Canary == nil {
-		return false, nil
-	}
 	rss, err := h.Finder.GetReplicaSetsForDeployment(newObj)
 	if err != nil || len(rss) == 0 {
 		klog.Warningf("Cannot find any activate replicaset for deployment %s/%s, no need to rolling", newObj.Namespace, newObj.Name)
diff --git a/pkg/webhook/workload/mutating/workload_update_handler_test.go b/pkg/webhook/workload/mutating/workload_update_handler_test.go
index 117d32b..f18c5dc 100644
--- a/pkg/webhook/workload/mutating/workload_update_handler_test.go
+++ b/pkg/webhook/workload/mutating/workload_update_handler_test.go
@@ -351,6 +351,37 @@ func TestHandlerDeployment(t *testing.T) {
 				return obj
 			},
 		},
+		{
+			name: "deployment image v1->v2, no matched rollout, but has in-progress annotation",
+			getObjs: func() (*apps.Deployment, *apps.Deployment) {
+				oldObj := deploymentDemo.DeepCopy()
+				newObj := deploymentDemo.DeepCopy()
+				newObj.Spec.Template.Spec.Containers[0].Image = "echoserver:v2"
+				newObj.Annotations[util.InRolloutProgressingAnnotation] = "{\"rolloutName\":\"rollouts-demo\"}"
+				newObj.Annotations[appsv1alpha1.DeploymentStrategyAnnotation] = "{\"rollingStyle\":\"Partition\",\"rollingUpdate\":{\"maxUnavailable\":\"25%\",\"maxSurge\":\"25%\"},\"paused\":true,\"partition\":1}"
+				return oldObj, newObj
+			},
+			expectObj: func() *apps.Deployment {
+				obj := deploymentDemo.DeepCopy()
+				obj.Spec.Template.Spec.Containers[0].Image = "echoserver:v2"
+				obj.Annotations[util.InRolloutProgressingAnnotation] = "{\"rolloutName\":\"rollouts-demo\"}"
+				obj.Annotations[appsv1alpha1.DeploymentStrategyAnnotation] = "{\"rollingStyle\":\"Partition\",\"rollingUpdate\":{\"maxUnavailable\":\"25%\",\"maxSurge\":\"25%\"},\"paused\":true,\"partition\":1}"
+				return obj
+			},
+			getRs: func() []*apps.ReplicaSet {
+				rs := rsDemo.DeepCopy()
+				return []*apps.ReplicaSet{rs}
+			},
+			getRollout: func() *appsv1beta1.Rollout {
+				obj := rolloutDemo.DeepCopy()
+				obj.Spec.WorkloadRef = appsv1beta1.ObjectRef{
+					APIVersion: "apps/v1",
+					Kind:       "Deployment",
+					Name:       "other",
+				}
+				return obj
+			},
+		},
 		{
 			name: "deployment image v2->v3, matched rollout, but multiple rss",
 			getObjs: func() (*apps.Deployment, *apps.Deployment) {