fix: fix the rollout stuck issue when pod/replicas changed together for canary rollout.

[andyliuliming]

when the rollout is in the "Pause" status,
and replica/poc spec changed together, then the rollout will stuck there forever.
this is because the newRS will be nil,
and there's one short circut return in reconcileNewReplicaSet().

issue reported in:
#3256

the change will follow the "BlueGreen" style to handle this.
pick the replica set which the canary svc targeted to. and reconcile it.

Checklist:

• Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this is a chore.
• The title of the PR is (a) conventional with a list of types and scopes found here, (b) states what changed, and (c) suffixes the related issues number. E.g. "fix(controller): Updates such and such. Fixes #1234".
• I've signed my commits with DCO
• I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
• My builds are green. Try syncing with master if they are not.
• My organization is added to USERS.md.

[andyliuliming]

@zachaller could you please help take a look when you have time? thanks.

[github-actions]

Go Published Test Results

2 099 tests   2 098 ✔️  2m 50s ⏱️
   118 suites         0 💤
       1 files           1 ❌

For more details on these failures, see this check.

Results for commit 6cffabe.

♻️ This comment has been updated with latest results.

[codecov]

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (1ecd394) 81.81% compared to head (6cffabe) 81.83%.
Report is 1 commits behind head on master.

Files	Patch %	Lines
rollout/canary.go	50.00%	2 Missing and 1 partial ⚠️
rollout/service.go	93.75%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3257      +/-   ##
==========================================
+ Coverage   81.81%   81.83%   +0.01%     
==========================================
  Files         135      135              
  Lines       20629    20651      +22     
==========================================
+ Hits        16878    16899      +21     
  Misses       2880     2880              
- Partials      871      872       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[github-actions]

E2E Tests Published Test Results

    4 files      4 suites   3h 27m 55s ⏱️
106 tests   98 ✔️   6 💤   2 ❌
434 runs  400 ✔️ 24 💤 10 ❌

For more details on these failures, see this check.

Results for commit 6cffabe.

♻️ This comment has been updated with latest results.

[sonarcloud]

Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
2.4% Duplication on New Code

See analysis details on SonarCloud

[zachaller]

Linking to the BlueGreen issue for reference: #3060

[zachaller]

@andyliuliming I could still be doing something wrong so still digging but this PR does not seem to fix the issue for me.

[zachaller]

I also think the issue is around this code:

func (c *rolloutContext) rolloutCanary() error {
	var err error
	if replicasetutil.PodTemplateOrStepsChanged(c.rollout, c.newRS) {
		c.newRS, err = c.getAllReplicaSetsAndSyncRevision(false)
		if err != nil {
			return err
		}
		return c.syncRolloutStatusCanary()
	}

	c.newRS, err = c.getAllReplicaSetsAndSyncRevision(true)
	if err != nil {
		return err
	}

[zachaller]

This also happens in Rollouts 1.5

[andyliuliming]

@zachaller

@andyliuliming I could still be doing something wrong so still digging but this PR does not seem to fix the issue for me.

this only works when there's canaryservice defined in rollout.
the fix works as expected in our rollout.

[zachaller]

You are right I don't know how I missed that in my test rollout. Still going to spend some time on it though because I feel there is a deeper underlying issue, I want to investigate that a bit more but this does seem to fix the issue as long as you have canary service defined.

[zachaller]

I think this would also fix the issue but also work for rollouts without a canary service: https://github.com/argoproj/argo-rollouts/pull/3272/files

[andyliuliming]

I think this would also fix the issue but also work for rollouts without a canary service: https://github.com/argoproj/argo-rollouts/pull/3272/files

tested. it works! abandoned mine.