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

cluster up: set DNS bind and IP address for newer server versions #13539

Merged
merged 1 commit into from
Apr 1, 2017

Conversation

csrwng
Copy link
Contributor

@csrwng csrwng commented Mar 24, 2017

For origin versions >= 1.5, explicitly set the DNS bind and IP address in the node config.

@csrwng
Copy link
Contributor Author

csrwng commented Mar 24, 2017

@smarterclayton ptal
tested on newer and older versions and we do the right thing

@csrwng
Copy link
Contributor Author

csrwng commented Mar 24, 2017

[test]

}
if useBindAddress(version) {
nodeCfg.DNSBindAddress = "127.0.0.1:53"
nodeCfg.DNSIP = "172.30.0.1"
Copy link
Contributor

Choose a reason for hiding this comment

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

this is really highlighting how problematic the "don't embed an IP" code is. The dns ip should point to the bind address, which then ensures that the code in the node dns is used. That is the long term flow and avoids having to query etcd for endpoints. If we aren't going to use that, we shouldn't start it (don't pass bind address). What happened when only dns ip was set to the service ip?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So just tried... if I leave the bind address blank and only set the dns ip, then I get the right behavior:

oc rsh simple-ruby-1-48pcf cat /etc/resolv.conf
search default.svc.cluster.local svc.cluster.local cluster.local
nameserver 172.30.0.1
nameserver 192.168.65.1
options ndots:5

I'll change the code to not set the bind address.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

and I don't have to check the version, since the previous versions just ignore the DNSIP

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nm, the previous versions ignore DNSBindAddress ... not DNSIP

@csrwng csrwng force-pushed the fix_clusterup_dns_bind_ip branch from 3fe9b9a to 030fb36 Compare March 25, 2017 02:46
@csrwng
Copy link
Contributor Author

csrwng commented Mar 25, 2017

updated the code to not set a bind address

@openshift-bot
Copy link
Contributor

Evaluated for origin test up to 030fb36

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_origin/458/) (Base Commit: ec459b4)

@csrwng
Copy link
Contributor Author

csrwng commented Apr 1, 2017

bump

@smarterclayton
Copy link
Contributor

smarterclayton commented Apr 1, 2017 via email

@csrwng
Copy link
Contributor Author

csrwng commented Apr 1, 2017

[merge]

@openshift-bot
Copy link
Contributor

Evaluated for origin merge up to 030fb36

@openshift-bot
Copy link
Contributor

openshift-bot commented Apr 1, 2017

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_request_origin/232/) (Base Commit: 645cac4) (Image: devenv-rhel7_6112)

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.

3 participants