-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
CODENVY-1749: add possibility to set dns resolvers #4129
Conversation
Signed-off-by: Alexander Garagatyi <[email protected]>
@bmicklea @eivantsov @JamesDrummond Please review. |
Let's change it to be CHE_DNS_RESOLVERS inside of che.env. If you can provide an explanation of what this does (or when an admin would need to configure this) then I can write release notes and identify docs changes that are needed. Is there a matching codenvy PR? |
@garagatyi - I will review this - this will not need @bmicklea or @JamesDrummond review. |
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 from configuration point of view
Build success. https://ci.codenvycorp.com/job/che-pullrequests-build/1960/ |
I think this needs docs. I created PR for docs that is in review. |
@JamesDrummond - the dock PR links to a che PR - it's not a docs PR. I am not sure this requires docs - we don't document every configuration property in or docs. |
Signed-off-by: Alexander Garagatyi <[email protected]>
@TylerJewell I updated env variable name. Also I added my comments for docs in bold, please take a look. |
Also not all the parts of the system inherit DNS resolution rules from host. Nginx container doesn't respect that and use 8.8.8.8 for dns resolution. So user might be surprised if by default we inherit from host but WSs don't start because of Nginx. |
@JamesDrummond If you still need docs please use my comments from this PR to change explanation in your PR. |
Build success. https://ci.codenvycorp.com/job/che-pullrequests-build/1962/ |
Signed-off-by: Alexander Garagatyi <[email protected]>
Signed-off-by: Alexander Garagatyi <[email protected]>
@riuvshin I had to rework PR, so now empty string is used instead of NULL. It is needed because Java code can't interpret NULL as empty array |
@TylerJewell please take a look at release notes. I added my comments into it. |
Ok for you to merge. I can fix release notes after merge when have time to review. |
@TylerJewell I updated the docs PR link. I somehow copied the wrong url. I think documentation on this would be helpful to those evaluating the product features. @garagatyi What is |
Build success. https://ci.codenvycorp.com/job/che-pullrequests-build/1966/ |
@JamesDrummond I meant ws-master aka che-server |
@garagatyi I rewrote the docs to include che server. https://github.com/eclipse/che-docs/pull/139/files#diff-ba9f2804ca5dc20e010ba41f64ef1cac . Please review. |
I updated the release notes based upon @garagatyi comments |
Signed-off-by: Alexander Garagatyi <[email protected]>
What does this PR do?
Add possibility to set DNS resolving servers.
If Che is set to use custom DNS that is resolvable using custom DNS resolving server admin should set env var: CHE_DNS_RESOLVERS
It may contain several values in comma-separated format, e.g.
What issues does this PR fix or reference?
Related to codenvy/codenvy#1749
Changelog
Add possibility to set custom DNS resolvers for workspace (master too) container runtimes.
Release Notes
We now let you override the default DNS resolvers used by Che and you workspace runtimes. The default behavior is for Che and workspaces to inherit DNS resolution from the host. You can add
CHE_DNS_RESOLVERS=<ip-list>
toche.env
to customize how DNS resolution will occur within the Che server and workspace containers. Services such as agents and servers running within Che workspaces will usually require custom DNS resolution when there are internal providers behind a proxy or firewall.Docs PR
eclipse-che/che-docs#139