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

Update traefik ingress docs #1855

Merged
merged 4 commits into from
Oct 23, 2024
Merged

Conversation

chicocvenancio
Copy link
Contributor

Fixes #1633

Copy link
Member

@kflynn kflynn left a comment

Choose a reason for hiding this comment

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

Thanks @chicocvenancio! There are a few tweaks that I'd like to see, but this looks way better than what we had before and I appreciate it! 🙂

linkerd.io/content/2.16/tasks/using-ingress.md Outdated Show resolved Hide resolved
```yaml
ingress.kubernetes.io/custom-request-headers: l5d-dst-override:web-svc.emojivoto.svc.cluster.local:80
```
Previous versions of Ttraefik needed to use [ingress mode](#ingress-mode),
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Previous versions of Ttraefik needed to use [ingress mode](#ingress-mode),
Versions of Traefik prior to 2.10 must use [ingress mode](#ingress-mode),


```yaml
apiVersion: traefik.containo.us/v1alpha1
apiVersion: traefik.io/v1alpha1
Copy link
Member

Choose a reason for hiding this comment

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

What version of Traefik is required for this API group change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Version 2.10 as well. traefik.containo.us is dropped in v3.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I left new api group for versions 2.10 and up example but kept old group for the legacy example.

Copy link
Member

@kflynn kflynn left a comment

Choose a reason for hiding this comment

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

Thanks @chicocvenancio! This looks great save for one small typo.

linkerd.io/content/2.16/tasks/using-ingress.md Outdated Show resolved Hide resolved
@kflynn
Copy link
Member

kflynn commented Oct 22, 2024

Also, it would be lovely if you could add the DCO signature to your commits (or just confirm here that you're OK with the DCO).

Fixes linkerd#1633

Signed-off-by: Chico Venancio <[email protected]>
Signed-off-by: Chico Venancio <[email protected]>
Signed-off-by: Chico Venancio <[email protected]>
@chicocvenancio
Copy link
Contributor Author

Rebased to sign off all commits and fixed the typo.

Copy link
Member

@kflynn kflynn left a comment

Choose a reason for hiding this comment

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

Many thanks!! 🙂

@kflynn
Copy link
Member

kflynn commented Oct 22, 2024

Looks like this needs to have main merged before we can land it? Sorry for the extra hassle!

@kflynn kflynn merged commit 042a9c9 into linkerd:main Oct 23, 2024
7 checks passed
@kflynn
Copy link
Member

kflynn commented Oct 23, 2024

Thanks for sticking with this one, @chicocvenancio! 🙂

@chicocvenancio chicocvenancio deleted the traefik-docs branch October 23, 2024 17:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update Linkerd Ingress Traffic doc
2 participants