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

Fix docs about default namespaces. #1961

Merged
merged 1 commit into from
Aug 18, 2017

Conversation

timoreimann
Copy link
Contributor

It's not the default namespace that's watched if nothing is specified, but all namespaces.

Proof: Our Namespaces type is a string slice which we use in the Kubernetes provider, and we accept all Ingresses if the slice is empty.

An alternative approach to fixing this would be to really make the default namespace the default. I don't think that's a good solution though because we probably want users to be able to watch all namespaces if they like to without complete enumeration. In fact, I ran across the doc/code divergence when somebody on Slack asked if it was possible to watch all namespaces, which I was about to deny until I digged into the code.

@containous/kubernetes PTAL.

@timoreimann timoreimann added this to the 1.4 milestone Aug 16, 2017
@timoreimann timoreimann changed the title [kubernetes] Fix docs about default namespaces. Fix docs about default namespaces. Aug 16, 2017
@ldez ldez added the size/S label Aug 16, 2017
Copy link
Contributor

@ldez ldez left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@nmengin nmengin left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@errm errm left a comment

Choose a reason for hiding this comment

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

LGTM

It's not the "default" namespace that's watched if nothing is specified,
but "all namespaces".
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.

5 participants