feat: Implement OpenShift Routes as a Traffic Manager

[mbhatip]

Addressing #1258

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, (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.

[huikang]

Awesome work to expand the scope of Argo-rollouts.

[codecov]

Codecov Report

Base: 81.69% // Head: 81.73% // Increases project coverage by +0.03% 🎉

Coverage data is based on head (34e7e3c) compared to base (3c2cef9).
Patch coverage: 86.06% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1301      +/-   ##
==========================================
+ Coverage   81.69%   81.73%   +0.03%     
==========================================
  Files         126      127       +1     
  Lines       19136    19298     +162     
==========================================
+ Hits        15634    15774     +140     
- Misses       2709     2727      +18     
- Partials      793      797       +4     

Impacted Files	Coverage Δ
rollout/experiment.go	83.98% <0.00%> (ø)
metricproviders/prometheus/prometheus.go	89.43% <30.76%> (-7.00%)	⬇️
rollout/controller.go	80.33% <71.42%> (-0.44%)	⬇️
rollout/trafficrouting/openshift/openshift.go	94.94% <94.94%> (ø)
controller/controller.go	91.44% <100.00%> (+0.03%)	⬆️
.../apis/rollouts/validation/validation_references.go	91.05% <100.00%> (+0.40%)	⬆️
rollout/trafficrouting.go	76.83% <100.00%> (+0.61%)	⬆️

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 at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

[andyfeller]

@huikang : outside of getting this pull request to pass all status checks, can you confirm whether this feature is something that would be accepted into the argo-rollouts project?

I know Red Hat has adopted Argo CD as their preferred GitOps solution, so I can imagine that the Argo Project is going to see an influx of OpenShift users.

[huikang]

Hi, @andyfeller , thanks for reaching out.

Yeah, definitely OpenShift Routes is an option in argo-rollouts and shall be included in the next v1.0.x release. Sorry about the delayed review process due to some other work having to finish. I will take another pass on the PR soon.

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

[fabricepipart1a]

Hello there !
I am part of the people that would be very interested to see the PR progressing and being released. By looking at the history, I did not get what is preventing progress. By any chance, @mbhatip and @jessesuen , can you give us an update of where we stand today?

[mbhatip]

Hello there ! I am part of the people that would be very interested to see the PR progressing and being released. By looking at the history, I did not get what is preventing progress. By any chance, @mbhatip and @jessesuen , can you give us an update of where we stand today?

The feature works but is failing the E2E tests, perhaps if those are fixed it can be merged, although running the controller on a personal cluster shows that you are able to have rollouts without problems.

[fabricepipart1a]

@jessesuen or @huikang Do we have a clear understanding of what prevents the E2E tests from passing? We can't see the logs anymore. Could one rerun them to identify what needs to be fixed / improved ?

[iam-veeramalla]

Hi @mbhatip , I am from Red Hat and looking forward for getting this change into the controller. Can you please rebase the PR and push the rebase changes ? This will also re-run the E2E tests.

If you are busy with something else, Please let me know. I can help you in rebasing the PR and getting the E2E tests passed.

P.S: If you want me to rebase the PR, please grant me the permissions on your branch :)

[mbhatip]

Apologies, I don't have as much time to dedicate to the project

Added the folks here as collaborators to the fork

[iam-veeramalla]

o dedicate to the proj

No worries, Thanks @mbhatip . I will take this forward from here.

[fabricepipart1a]

@iam-veeramalla Keep me updated if I can help you with anything here! I can help you with anything that is not coding in go ;-)

[iam-veeramalla]

@iam-veeramalla Keep me updated if I can help you with anything here! I can help you with anything that is not coding in go ;-)

Thanks, I have rebased the changes and fixed conflicts. Some unit tests are failing. Looking into it.

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

[github-actions]

Go Published Test Results

1 855 tests   1 855 ✔️  2m 32s ⏱️
   106 suites         0 💤
       1 files           0 ❌

Results for commit 34e7e3c.

♻️ This comment has been updated with latest results.

[github-actions]

E2E Tests Published Test Results

    2 files      2 suites   1h 33m 59s ⏱️
  95 tests   90 ✔️ 3 💤 2 ❌
192 runs  184 ✔️ 6 💤 2 ❌

For more details on these failures, see this check.

Results for commit 34e7e3c.

♻️ This comment has been updated with latest results.

[christianh814]

Thanks for this!

A couple of thoughts. (as pointed out in the PR documentation). OpenShift routes have "stickyness" and "cookies" turned on by default. Which makes seeing the canary on a browser difficult.

You need to annotate the route with the following

oc annotate route/myapp haproxy.router.openshift.io/balance=roundrobin
oc annotate route/myapp haproxy.router.openshift.io/disable_cookies=true

See the official doc for other configurations that can be done.

Would it make sense to have the Rollouts controller annotate the route for you during a canary rollout?

@iam-veeramalla @mbhatip @fabricepipart1a @jessesuen

[kimfiedler]

Hi,

This would be a very useful addition to Argo Rollouts! Are you still working on it @iam-veeramalla? If not I would be happy to take over if you would help me in identifying exactly what is missing. Let me know if you want me to help out.

Regarding the stickiness as commented by @christianh814 I believe there are use cases where it makes sense to have stickyness turned on. A typical example would be a web application where you want to have the assets (JS/CSS/images) that are referenced from a HTML document to be available.

That being said there are definitely also use cases where you'd want to have the stickyness turned off.

My suggestion would be to keep it simple and update the documentation with instructions on how to disable stickyness if you want to do that. My assumption is that if stickyness is not important to you during rollout it would not be important to you once you've completed the rollout. The benefit would be to be aligned with OpenShift defaults and not add extra complexity or magic to Argo Rollouts. What do you think @christianh814 ?

[kimfiedler]

Fixed commits to make DCO pass. All checks are green now 👍

[iam-veeramalla]

Fixed commits to make DCO pass. All checks are green now 👍

Do you need any help on testing this on OpenShift ?

[kimfiedler]

Do you need any help on testing this on OpenShift ?

That would be great. Looks like it's been a while since it was manually tested by @mbhatip.

[iam-veeramalla]

better use latest version of the docs
https://docs.openshift.com/container-platform/4.11/networking/routes/route-configuration.html

[kimfiedler]

Thanks 👍

[iam-veeramalla]

better use latest version of the docs
https://docs.openshift.com/container-platform/4.11/networking/routes/route-configuration.html

[kimfiedler]

Thanks 👍

[iam-veeramalla]

some changes requires to docs

[iam-veeramalla]

Do you need any help on testing this on OpenShift ?

That would be great. Looks like it's been a while since it was manually tested by @mbhatip.

Testing it locally

[kimfiedler]

Updated links in documentation and resolved merge conflicts

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

[kimfiedler]

Testing it locally

I've done a test locally using OpenShift 4.11 on CRC and everything looks good!

[zachaller]

This is great to see I am hoping this become one of rollouts first plugins for traffic routers? The conversion process to a plugin should not be to difficult and am willing to help and let it shape any changes needed to the plugin system. I should be able to wrap up the plugin system here really soon next few days. https://cloud-native.slack.com/archives/C020XM04CUW/p1675065984845389

[kimfiedler]

I had a talk with @zachaller and I will be porting the OpenShift Traffic Manager into the new plugin together with him. This PR will be superseeded by a plugin when #2548 lands.

This means we will get the same capabilities as this PR give us but in a slightly more flexible architecture.

[michael-mulder]

Could someone help me understand what is remaining on this? I see 2548 was closed, not merged.

Is help needed?

[zachaller]

@michael-mulder
The plugin stuff is still the way this will still go I just created a new PR here #2573