argoproj / argo-rollouts

Progressive Delivery for Kubernetes
https://argo-rollouts.readthedocs.io/
Apache License 2.0
2.73k stars 851 forks source link

Add validation to avoid changes to the Rollout selector field #2105

Open mvgmb opened 2 years ago

mvgmb commented 2 years ago

Summary

Rollout's selector field should be immutable. However, Argo Rollout's validation allows changing this field, resulting in orphan ReplicaSets.

How to Reproduce

Create a rollout by running kubectl apply -f rollout.yaml:

# rollout.yaml
apiVersion: argoproj.io/v1alpha1
kind: Rollout
metadata:
  name: dnsutils
spec:
  selector:
    matchLabels:
      app.kubernetes.io/name: dnsutils
  strategy:
    canary:
      maxSurge: 1
      maxUnavailable: 1
  template:
    metadata:
      labels:
        app.kubernetes.io/name: dnsutils
    spec:
      containers:
        - name: dnsutils
          image: k8s.gcr.io/e2e-test-images/jessie-dnsutils:1.3
          command: ['sleep', '3600']

Update the Rollout's selector field by adding a new label and then run kubectl apply -f rollout.yaml:

# rollout.yaml
apiVersion: argoproj.io/v1alpha1
kind: Rollout
metadata:
  name: dnsutils
spec:
  selector:
    matchLabels:
      app.kubernetes.io/name: dnsutils
      app.kubernetes.io/version: '1.3'
  strategy:
    canary:
      maxSurge: 1
      maxUnavailable: 1
  template:
    metadata:
      labels:
        app.kubernetes.io/name: dnsutils
        app.kubernetes.io/version: '1.3'
    spec:
      containers:
        - name: dnsutils
          image: k8s.gcr.io/e2e-test-images/jessie-dnsutils:1.3
          command: ['sleep', '3600']

Then run kubectl get pods. It should have only one ReplicaSet, however, the first ReplicaSet is still there:

$ kubectl get replicasets.apps 
NAME                  DESIRED   CURRENT   READY   AGE
dnsutils-6658dc5fc    1         1         1       35s
dnsutils-7c87d7d6ff   1         1         1       7m15s

$ kubectl get rollouts.argoproj.io 
NAME       DESIRED   CURRENT   UP-TO-DATE   AVAILABLE   AGE
dnsutils   1         1         1            1           37s

Solution

To deal with this scenario, Argo Rollouts validation should block any changes to the selector field, similar to Deployment validation. When trying to change the selector field on a Deployment, it returns the following:

$ kubectl apply -f deployment.yaml
The Deployment "dnsutils" is invalid: spec.selector: Invalid value: v1.LabelSelector{MatchLabels:map[string]string{"app.kubernetes.io/name":"dnsutils", "app.kubernetes.io/version":"1.3"}, MatchExpressions:[]v1.LabelSelectorRequirement(nil)}: field is immutable

Use Cases

This validation is important to avoid having orphan ReplicaSets.


Message from the maintainers:

Impacted by this bug? Give it a 👍. We prioritize the issues with the most 👍.

github-actions[bot] commented 1 year ago

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 7 days.

zachaller commented 1 year ago

THis should be able to be done via https://kubernetes.io/blog/2022/09/29/enforce-immutability-using-cel/ now

github-actions[bot] commented 1 year ago

This issue is stale because it has been open 60 days with no activity.

eytanhanig commented 1 year ago

This is highly unexpected behavior and should be fixed if we wish to claim parity with deployments. Personally, if I hadn't caught all these orphaned resources from Rollouts our cluster costs would have nearly doubled.

MoseDien commented 4 months ago

@zachaller Has this issue been fixed? Or is there any solution? Thanks.

PauloLeaoMove commented 4 months ago

I am on the same boat, I have multiple deployments with orphaned replicasets. I am looking forward for a solution.

ajax-bychenok-y commented 3 weeks ago

I don't think that it's only reason of staled working replica sets. For example we do not change selector field in our rollout. https://github.com/argoproj/argo-rollouts/issues/1761#issuecomment-2331739689