-
Notifications
You must be signed in to change notification settings - Fork 5.1k
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
Change code block of traefik-web-ui to match file #3542
Conversation
docs/user-guide/kubernetes.md
Outdated
@@ -323,6 +323,7 @@ For more information, check out [the documentation](https://github.com/kubernete | |||
Lets start by creating a Service and an Ingress that will expose the [Træfik Web UI](https://github.com/containous/traefik#web-ui). | |||
|
|||
```yaml | |||
--- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line is not needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can probably remove that line from the actual yaml too
docs/user-guide/kubernetes.md
Outdated
serviceName: traefik-web-ui | ||
servicePort: 80 | ||
servicePort: web |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indenting
I updated with those changes. It appears this document has the leading '---' on any yaml code block with multiple components. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
targetPort: 8080 | ||
--- | ||
apiVersion: extensions/v1beta1 | ||
kind: Ingress | ||
metadata: | ||
name: traefik-web-ui | ||
namespace: kube-system | ||
annotations: | ||
kubernetes.io/ingress.class: traefik | ||
spec: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The annotation makes sense to have in general. I'm thinking that we should rather add it to the YAML file than remove it from the inline spec.
@dtomcej WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The argument is that we don't specify an ingress class to watch on any of our examples. If we keep it, we should be adding the class to our deployment/daemonset...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gotcha. Even if we decided to do that, it should be done by a separate PR. No reason to block this one.
c8ebcb5
to
e41f2f4
Compare
@dtomcej Docs still show corrections necessary against Master
https://github.com/containous/traefik/blob/master/examples/k8s/ui.yaml