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

Added Traefik as an available ingress controller #440

Merged
merged 45 commits into from
Nov 14, 2022
Merged

Conversation

MattLeach25
Copy link
Contributor

PR Summary

Added Traefik as an available ingress controller to postdeploy script and options in the GUI.

Fixed a bug with NGINX where the helm chart would deploy, however there was no pods or services deployed. This turned out to be a typo within the postdeploy script.

This PR covers feature #96

image

image

image

PR Checklist

  • PR has a meaningful title
  • Summarized changes
  • This PR is ready to merge and is not Work in Progress
  • Link to a filed issue
  • Screenshot of UI changes (if PR includes UI changes)

@Gordonby
Copy link
Collaborator

Gordonby commented Oct 25, 2022

@MattLeach25 Looks good to me.

Just make sure to add traefik to the cspell dictionary (cspell.json)
image

@khowling - Can you take a look?

Gordonby
Gordonby previously approved these changes Oct 25, 2022
Copy link
Collaborator

@Gordonby Gordonby left a comment

Choose a reason for hiding this comment

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

LGTM - But i'll let Keith provide the final approval for merge.

@Gordonby Gordonby added the enhancement New feature or request label Oct 26, 2022
@khowling
Copy link
Contributor

Hey Matt, looks great! thanks for catching the case bug for nginx, just a couple of questions for qa.

  1. ingress_controller_kind - this is used by nginx and contour, and now traefik. Does the change work with contour, or do we need to have a special logic for contour? (I think I did most of my testing originally with contour & so bit worried it will break)
  2. When adding a ingress controller, I like to test with the most complex options with certman / external-dns and grafana, to see if you get a ssl fqdn for grafana. If that all works, then we are in good shape

@Gordonby
Copy link
Collaborator

@MattLeach25 I can walk you through these advanced testing scenarios on Monday. Stick some time in.

@MattLeach25
Copy link
Contributor Author

@MattLeach25 I can walk you through these advanced testing scenarios on Monday. Stick some time in.

Keith has already talked me through the changes so I can get going. If I get stuck I’ll pop up next week :)

thanks both.

@Gordonby Gordonby temporarily deployed to csu November 12, 2022 11:07 Inactive
@Gordonby Gordonby temporarily deployed to csu November 12, 2022 11:13 Inactive
@Gordonby Gordonby temporarily deployed to csu November 12, 2022 11:18 Inactive
@Gordonby Gordonby temporarily deployed to csu November 12, 2022 11:18 Inactive
@Gordonby Gordonby temporarily deployed to csu November 12, 2022 11:26 Inactive
@Gordonby Gordonby temporarily deployed to csu November 12, 2022 11:26 Inactive
@Gordonby Gordonby temporarily deployed to csu November 12, 2022 11:29 Inactive
@Gordonby Gordonby temporarily deployed to csu November 12, 2022 11:52 Inactive
@Gordonby Gordonby temporarily deployed to csu November 12, 2022 11:52 Inactive
@Gordonby Gordonby temporarily deployed to csu November 12, 2022 11:59 Inactive
@Gordonby Gordonby temporarily deployed to csu November 12, 2022 11:59 Inactive
@Gordonby Gordonby temporarily deployed to csu November 12, 2022 12:03 Inactive
@Gordonby Gordonby temporarily deployed to csu November 12, 2022 12:03 Inactive
@Gordonby Gordonby temporarily deployed to csu November 12, 2022 12:10 Inactive
@Gordonby Gordonby temporarily deployed to csu November 12, 2022 12:12 Inactive
@Gordonby Gordonby temporarily deployed to csu November 12, 2022 12:12 Inactive
@Gordonby Gordonby temporarily deployed to csu November 12, 2022 12:18 Inactive
Copy link
Collaborator

@Gordonby Gordonby left a comment

Choose a reason for hiding this comment

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

LGTM

image

@MattLeach25 MattLeach25 merged commit 0f88fb9 into main Nov 14, 2022
@MattLeach25 MattLeach25 deleted the ml-traefik branch November 14, 2022 07:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request helper-ui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants