Fixing Rollout restart, Previously would trigger rollout deployment strategy

[jangeja]

The current rollout update method triggers a rollout strategy. This is what I believe to not be the desired outcome.

This issue clearly defines the problem.

Below is the old rollout update method that clearly shows the rollout deployment strategy triggered instead of restart all of the pods.

After updating the dependent secret, a canary rollout is triggered, which imo is not desired.

Name:            canary-demo
Namespace:       default
Status:          ॥ Paused
Message:         CanaryPauseStep
Strategy:        Canary
  Step:          1/3
  SetWeight:     20
  ActualWeight:  20
Images:          nginx:latest (canary, stable)
Replicas:
  Desired:       5
  Current:       5
  Updated:       1
  Ready:         5
  Available:     5

NAME                                     KIND        STATUS        AGE    INFO
⟳ canary-demo                            Rollout     ॥ Paused      62m
├──# revision:16
│  └──⧉ canary-demo-5df7ffbff            ReplicaSet  ✔ Healthy     8s     canary
│     └──□ canary-demo-5df7ffbff-4b7bd   Pod         ✔ Running     8s     ready:1/1
├──# revision:15
│  └──⧉ canary-demo-698b5f88f7           ReplicaSet  ✔ Healthy     27m    stable
│     ├──□ canary-demo-698b5f88f7-hqct9  Pod         ✔ Running     2m52s  ready:1/1
│     ├──□ canary-demo-698b5f88f7-qnrkd  Pod         ✔ Running     2m50s  ready:1/1
│     ├──□ canary-demo-698b5f88f7-ntvph  Pod         ✔ Running     2m46s  ready:1/1
│     └──□ canary-demo-698b5f88f7-p4wdd  Pod         ✔ Running     2m44s  ready:1/1

After my change, I tested changing a secret while we were mid rollout, and during no rollout. The functionality remained the same, with all pods being restarted without triggering a rollout.

After changing a secret, not during a rollout:

Name:            canary-demo
Namespace:       default
Status:          �[32m✔�[0m Healthy
Strategy:        Canary
  Step:          3/3
  SetWeight:     100
  ActualWeight:  100
Images:          nginx:latest (�[32mstable�[0m)
Replicas:
  Desired:       5
  Current:       5
  Updated:       5
  Ready:         5
  Available:     5

NAME                                    KIND        STATUS        AGE    INFO
⟳ canary-demo                           Rollout     �[32m✔�[0m Healthy     68m    
├──# revision:16                                                         
│  └──⧉ �[32mcanary-demo-5df7ffbff�[0m           ReplicaSet  �[32m✔�[0m Healthy     6m38s  �[32mstable�[0m
│     ├──□ canary-demo-5df7ffbff-xpxz9  Pod         �[32m✔�[0m Running     15s    ready:1/1
│     ├──□ canary-demo-5df7ffbff-tv2mx  Pod         �[32m✔�[0m Running     13s    ready:1/1
│     ├──□ canary-demo-5df7ffbff-v9mvr  Pod         �[32m✔�[0m Running     11s    ready:1/1
│     ├──□ canary-demo-5df7ffbff-ssdwr  Pod         �[32m✔�[0m Running     9s     ready:1/1
│     └──□ canary-demo-5df7ffbff-7w6fv  Pod         �[32m✔�[0m Running     7s     ready:1/1
└──# revision:15                                                         
   └──⧉ canary-demo-698b5f88f7          ReplicaSet  �[0m•�[0m ScaledDown  33m    

After changing the secret, during canary rollout

Name:            canary-demo
Namespace:       default
Status:          �[0m॥�[0m Paused
Message:         CanaryPauseStep
Strategy:        Canary
  Step:          1/3
  SetWeight:     20
  ActualWeight:  20
Images:          nginx:latest (�[33mcanary�[0m, �[32mstable�[0m)
Replicas:
  Desired:       5
  Current:       5
  Updated:       1
  Ready:         5
  Available:     5

NAME                                     KIND        STATUS        AGE    INFO
⟳ canary-demo                            Rollout     �[0m॥�[0m Paused      70m    
├──# revision:17                                                          
│  └──⧉ �[33mcanary-demo-5b4548699b�[0m           ReplicaSet  �[32m✔�[0m Healthy     44s    �[33mcanary�[0m
│     └──□ canary-demo-5b4548699b-6vqd4  Pod         �[32m✔�[0m Running     20s    ready:1/1
├──# revision:16                                                          
│  └──⧉ �[32mcanary-demo-5df7ffbff�[0m            ReplicaSet  �[32m✔�[0m Healthy     8m30s  �[32mstable�[0m
│     ├──□ canary-demo-5df7ffbff-r82xb   Pod         �[32m✔�[0m Running     19s    ready:1/1
│     ├──□ canary-demo-5df7ffbff-l7ss8   Pod         �[32m✔�[0m Running     17s    ready:1/1
│     ├──□ canary-demo-5df7ffbff-l9vtq   Pod         �[32m✔�[0m Running     14s    ready:1/1
│     └──□ canary-demo-5df7ffbff-wnhlg   Pod         �[32m✔�[0m Running     12s    ready:1/1
└──# revision:15                                                          
   └──⧉ canary-demo-698b5f88f7           ReplicaSet  �[0m•�[0m ScaledDown  35m    

[jangeja]

@IdanAdar Can I get a set of 👀 on this? We had an issue related to the argo rollouts issue described in this ticket. Was wondering your thoughts on this simple change.

[IdanAdar]

Wish I could help, I'm not a maintainer.

[bnallapeta]

@jangeja could you please fix the issues with golint?

[bnallapeta]

looks like the linting issue is on another file unrelated to this change.

[jangeja]

@bnallapeta I've updated my branch with the latest master. Can you rerun the CI, and re-review 🥺

[jangeja]

@bnallapeta Looks like the CI passed! Can we get this merged and released?

[bnallapeta]

@MuneebAijaz PTAL.