Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add websocket configuration for contour ingress #1320

Merged

Conversation

lumarel
Copy link
Contributor

@lumarel lumarel commented Apr 3, 2023

SUMMARY

As described in this issue, the websocket is not available through contour, as this needs special configuration.
Here is also the help page from contour.

ISSUE TYPE
  • Bug, Docs Fix or other nominal change
ADDITIONAL INFORMATION

So this change basically implements a check if a contour ingress is installed on the cluster, and if yes, adds the websocket route, as well as the seperate path for that.

Unfortunately I wasn't able to spin up the tests, as it looks for be that these are broken somehow, if I can get these working somehow I will gladly run through them!
As well as, if any changes are wanted I will definitely try to improve the contribution.

Copy link
Member

@TheRealHaoLiu TheRealHaoLiu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of adding is_ingress_contour i think we can add contour as a ingress_type and use that to trigger the setting

@lumarel
Copy link
Contributor Author

lumarel commented Apr 5, 2023

@TheRealHaoLiu I thought about that in the first place, but refrained from that, because I thought it would be better if it autodetects the ingress controller, but yes sure I am going to rebuild the change to an upfront parameter.

One thing I'm just thinking about is in this case if it would be better, to create a dependent ingress_controller variable instead of a completely new ingress_type, as the latter one will complicate the documentation a lot more.

In case this is also not really the desired outcome, I will of course rewrite it to its seperate ingress_type and add the new type to the docs

@lumarel lumarel force-pushed the feature/contour-ingress-websocket branch from 91ca772 to d8da285 Compare April 5, 2023 21:27
@TheRealHaoLiu TheRealHaoLiu self-requested a review April 12, 2023 18:24
@rooftopcellist rooftopcellist enabled auto-merge (squash) April 12, 2023 18:25
@rooftopcellist rooftopcellist merged commit faf51c8 into ansible:devel Apr 12, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants