feat: Add Service field to Rollout Experiment to allow service creation

[daniddelrio]

Follow-up to: #2624

#2624 rolled back the change to allow rollout experiments with no weights to still create services. However, we still want the allow the users to choose to create services for their experiments even with no weight/traffic routing enabled. This PR makes it so that users can set the Service attribute for their rollout experiments (with an optional name field), giving them the ability to still create services without setting weight.

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.

[github-actions]

Go Published Test Results

1 877 tests   1 877 ✔️  2m 33s ⏱️
   118 suites         0 💤
       1 files           0 ❌

Results for commit 1d34b63.

♻️ This comment has been updated with latest results.

[codecov]

Codecov Report

Patch coverage: 85.71% and no project coverage change.

Comparison is base (558f2f1) 81.51% compared to head (1d34b63) 81.52%.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #2633   +/-   ##
=======================================
  Coverage   81.51%   81.52%           
=======================================
  Files         133      133           
  Lines       19859    19866    +7     
=======================================
+ Hits        16188    16195    +7     
  Misses       2836     2836           
  Partials      835      835           

Impacted Files	Coverage Δ
experiments/service.go	68.29% <33.33%> (ø)
experiments/experiment.go	90.01% <100.00%> (+0.08%)	⬆️
rollout/experiment.go	84.36% <100.00%> (+0.22%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[github-actions]

E2E Tests Published Test Results

    4 files      4 suites   3h 18m 9s ⏱️
  95 tests   80 ✔️   5 💤 10 ❌
394 runs  360 ✔️ 20 💤 14 ❌

For more details on these failures, see this check.

Results for commit 1d34b63.

♻️ This comment has been updated with latest results.

[alexef]

see comment

[alexef]

hmm, shouldn't you pass template.Service.Name in CreateService call? instead of rs.Name

https://github.com/daniddelrio/argo-rollouts/blob/service-in-experiment/experiments/experiment.go#L304

[zachaller]

Can we also add it to the spec here https://argoproj.github.io/argo-rollouts/features/specification/ with a comment about it like weight has #optional etc

[zachaller]

Can we also add an e2e test where we set the name, I just want to make sure things still get cleaned up etc and all the other functions work that look up service. I took a quick peek at it seems we mostly use an annotation on the service to find services but it would be nice to run an e2e with name set.

We have this already: https://github.com/argoproj/argo-rollouts/blob/master/test/e2e/functional/experiment-with-service.yaml

[daniddelrio]

hmm, shouldn't you pass template.Service.Name in CreateService call? instead of rs.Name

https://github.com/daniddelrio/argo-rollouts/blob/service-in-experiment/experiments/experiment.go#L304

@alexef That makes sense. That said, I can't find documentation on including the Service field in an Experiment. Doesn't seem to be on the Experiment page. Should we also add that?

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
1 Code Smell

No Coverage information
9.4% Duplication