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

Contour namespace #2389

Merged
merged 1 commit into from
Mar 30, 2020
Merged

Conversation

stevesloka
Copy link
Member

Fixes #2255 by using the downward api to give the default namespace for configuration variables. They are still able to be overridden/configured as before but are no longer hardcoded to projectcontour.

@codecov
Copy link

codecov bot commented Mar 26, 2020

Codecov Report

Merging #2389 into master will decrease coverage by 0.02%.
The diff coverage is 60.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2389      +/-   ##
==========================================
- Coverage   78.12%   78.09%   -0.03%     
==========================================
  Files          62       62              
  Lines        5288     5292       +4     
==========================================
+ Hits         4131     4133       +2     
- Misses       1069     1070       +1     
- Partials       88       89       +1     
Impacted Files Coverage Δ
cmd/contour/serve.go 0.00% <ø> (ø)
cmd/contour/servecontext.go 92.92% <60.00%> (-1.58%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7696eb2...e072107. Read the comment docs.

@stevesloka stevesloka added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Mar 26, 2020
@jpeach jpeach added this to the 1.4.0 milestone Mar 30, 2020
Copy link
Contributor

@jpeach jpeach left a comment

Choose a reason for hiding this comment

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

This looks good to me. I was a little concerned that CONTOUR_NAMESPACE is used differently in all three cases here, but I suppose that's unavoidable, and I didn't come up with a better alternative.

We should file a separate issue to document this, since there is no documentation for environment variable usages (not even in the --help output).

Finally, @stevesloka let me know if you want to squash and update the commit massage, otherwise I can do that when merging the PR.

@stevesloka
Copy link
Member Author

@jpeach update message? Because of the squash? Bah, this is why I have separate commits to make it clear the changes. I guess I can just rebase them together.

@jpeach jpeach merged commit d4fb68d into projectcontour:master Mar 30, 2020
@stevesloka stevesloka deleted the contourNamespace branch March 31, 2020 16:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note Denotes a PR that will be considered when it comes time to generate release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Contour should be better about running in alternate namespaces
2 participants