Fix: Webhook will take over Deployment even Rollout CR doesn't exist (#278) (#280)

(cherry picked from commit deaa5f38b5)

Signed-off-by: AiRanthem <zhongtianyun.zty@alibaba-inc.com>
This commit is contained in:
Ai Ranthem 2025-06-20 17:33:24 +08:00 committed by GitHub
parent 727e98ad32
commit 75dc72aa33
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 39 additions and 15 deletions

View File

@ -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{}

View File

@ -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)

View File

@ -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) {