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

🐛 Set dashboard ingressroute as an Helm resource #668

Closed
wants to merge 1 commit into from
Closed

🐛 Set dashboard ingressroute as an Helm resource #668

wants to merge 1 commit into from

Conversation

mloiseleur
Copy link
Member

@mloiseleur mloiseleur commented Oct 17, 2022

What does this PR do?

Set dashboard IngressRoute as a regular helm resource instead of a hook.

Motivation

It's not needed anymore to install it with a hook.
Fixes #115

More

  • Yes, I updated the tests accordingly
  • Yes, I ran make test and all the tests passed
  • Is there a way to avoid break during upgrade between hook version and regular version ?
Error: UPGRADE FAILED: rendered manifests contain a resource that already exists. 
Unable to continue with update: IngressRoute "traefik-dashboard" in namespace "default" exists and cannot be imported into the current release: invalid ownership metadata;
annotation validation error: missing key "meta.helm.sh/release-name": must be set to "traefik"; 
annotation validation error: missing key "meta.helm.sh/release-namespace": must be set to "default"

@mloiseleur mloiseleur added the kind/bug/confirmed a confirmed bug (reproducible). label Oct 17, 2022
@ldez ldez added kind/bug/fix kind/enhancement New feature or request and removed kind/bug/confirmed a confirmed bug (reproducible). kind/enhancement New feature or request labels Oct 17, 2022
@mloiseleur mloiseleur added the kind/breaking This PR introduce a breaking change label Oct 18, 2022
@mloiseleur mloiseleur closed this by deleting the head repository Nov 15, 2022
@mloiseleur mloiseleur reopened this Nov 21, 2022
@jnoordsij
Copy link
Collaborator

#763 (comment)

Thanks for this report, @jnoordsij. For #668, that's right, but it may break upgrade path for users who haven't yet upgraded their Helm Chart to a recent versions. Maybe #668 can be merged when this Helm Chart will switch to Traefik Proxy v3.0.

Merging this with a Traefik Proxy v3.0 major chart upgrade sounds like a good idea. Maybe add an upgrade note in that case? I think manually deleting the old ingressroute, using something like kubectl delete ingressroute traefik-ingress-controller-dashboard, then upgrading the chart should work. This would cause minimal downtime on the dashboard, but I think that's not a huge issue for most people.

@mloiseleur
Copy link
Member Author

@jnoordsij This command will be needed only when upgrading from v15.3.0 or below. Since v15.3.1 (and PR #674), there is no command needed since all needed annotations are provided for the switch.

@maxnitze
Copy link

Really looking forward to this change! It currently kind-of breaks my setup with ArgoCD (see https://community.traefik.io/t/why-dashboard-ingress-in-helm-chart-with-post-install-and-post-upgrade-hooks/17235)

Is there any chance this will be backported to Traefik v2?

@mloiseleur mloiseleur marked this pull request as ready for review February 10, 2023 09:40
@mloiseleur mloiseleur closed this Feb 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/breaking This PR introduce a breaking change kind/bug/fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Disabling dashboard ingressroute does not delete it
4 participants