Adding support for generating A records for headless services. #8643

vishh · 2015-05-21T23:02:57Z

Unit and e2e tests will be added.

thockin · 2015-05-21T23:22:59Z

cluster/addons/dns/kube2sky/Makefile

@@ -4,13 +4,13 @@

 .PHONY: all kube2sky container push clean test

-TAG = 1.6
+TAG = 1.7


We only need to bump it if you intend to cut a release - is that the intent?

Not yet. Reverted this change.

vishh · 2015-05-21T23:23:12Z

Sample output:

# kubectl get services elasticsearch-logging-h
NAME                      LABELS                                                                                                    SELECTOR                     IP(S)     PORT(S)
elasticsearch-logging-h   kubernetes.io/cluster-service=true,kubernetes.io/name=Elasticsearch-headless,name=elasticsearch-logging   name=elasticsearch-logging   None      9200/TCP

$ dig @10.244.1.10 elasticsearch-logging-h.default.cluster.local

;; QUESTION SECTION:
;elasticsearch-logging-h.default.cluster.local. IN A

;; ANSWER SECTION:
elasticsearch-logging-h.default.cluster.local. 30 IN A 10.244.0.4
elasticsearch-logging-h.default.cluster.local. 30 IN A 10.244.1.3

k8s-bot · 2015-05-21T23:24:16Z

Can one of the admins verify that this patch is reasonable to test? (reply "ok to test", or if you trust the user, reply "add to whitelist")

If this message is too spammy, please complain @ixdy.

thockin · 2015-05-21T23:36:36Z

cluster/addons/dns/kube2sky/kube2sky.go

+	if !kapi.IsServiceIPSet(service) {
+		return ks.newHeadlessService(subdomain, service)
+	}
+	return ks.generateRecordsForRegularService(subdomain, service)


maybe ks.newPortalService() to parallel newHeadlessService() ?

thockin · 2015-05-21T23:52:34Z

Overall good, a few minor points - could use some helpful comments for poor future me (or future you :)

googlebot · 2015-05-22T21:03:30Z

We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm.

vishh · 2015-05-22T21:04:20Z

@thockin: Unit tests and e2e tests added. The e2e test needs to be upgraded to verify the number of A records. But, this PR is safe to land as-is.

thockin · 2015-05-22T22:13:41Z

cluster/addons/dns/kube2sky/kube2sky.go

+		glog.V(2).Infof("Subdomain %q does not exist in etcd", subdomain)
+		return nil
+	}
+	// TODO: Ignore non-existent subdomains.


isn't this TODO done by the above?

Good catch. Had it in there while I was working on the PR. Removed.

thockin · 2015-05-22T22:18:36Z

cmd/kubelet/app/plugins.go

@@ -34,6 +34,7 @@ import (
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/volume/iscsi"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/volume/nfs"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/volume/persistent_claim"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/volume/rbd"


why is the RBD PR merged into this one?

Rebase issue. Fixed it.

thockin · 2015-05-22T22:20:02Z

LGTM except you have another PR merged in here (ceph RBD)

googlebot · 2015-05-22T23:04:07Z

CLAs look good, thanks!

vishh · 2015-05-22T23:05:34Z

@thockin: Rebase issue fixed and unwanted TODO removed.

thockin · 2015-05-22T23:09:10Z

LGTM

vishh · 2015-05-23T06:01:55Z

Rebased to head and updated e2e tests to use the e2e framework.

lavalamp · 2015-05-24T03:28:16Z

Maybe I am dense but I can't tell from the PR-- what tag did you build kube2sky with, and where is the configuration change to make the cluster use the new build?

vishh · 2015-05-24T16:01:00Z

I am not making a new kube2sky as part of this release.

On Sat, May 23, 2015 at 8:28 PM, Daniel Smith notifications@github.com
wrote:

Maybe I am dense but I can't tell from the PR-- what tag did you build
kube2sky with, and where is the configuration change to make the cluster
use the new build?

—
Reply to this email directly or view it on GitHub
#8643 (comment)
.

lavalamp · 2015-05-24T20:51:38Z

I don't understand-- how can the new e2e test pass then?

vishh · 2015-05-25T01:36:05Z

I ran the e2e manually. If it is imperative to make a new release and use
jenkins e2e to verify this PR, I can make a new release.

On Sun, May 24, 2015 at 1:52 PM, Daniel Smith notifications@github.com
wrote:

I don't understand-- how can the new e2e test pass then?

—
Reply to this email directly or view it on GitHub
#8643 (comment)
.

thockin · 2015-05-25T23:17:22Z

Jenkins will run the e2e tests - if we don't have a kube2sky container
pushed it can't pass, right?

On Sun, May 24, 2015 at 6:36 PM, Vish Kannan notifications@github.com
wrote:

I ran the e2e manually. If it is imperative to make a new release and use
jenkins e2e to verify this PR, I can make a new release.

On Sun, May 24, 2015 at 1:52 PM, Daniel Smith notifications@github.com
wrote:

I don't understand-- how can the new e2e test pass then?

—
Reply to this email directly or view it on GitHub
<
#8643 (comment)

.

—
Reply to this email directly or view it on GitHub
#8643 (comment)
.

vishh · 2015-05-26T02:46:39Z

Added a new release of kube2sky.

ixdy · 2015-05-26T21:11:43Z

@thockin Jenkins will run e2e tests post-merge, but it isn't running any tests pre-merge yet - only building them.

Adding support for generating A records for headless services.

vishh · 2015-05-26T21:44:20Z

I ran the e2e again and it passed! @saad-ali

thockin · 2015-05-26T21:47:44Z

Thanks!

On Tue, May 26, 2015 at 2:44 PM, Vish Kannan notifications@github.com
wrote:

I ran the e2e again and it passed! @saad-ali https://github.com/saad-ali

—
Reply to this email directly or view it on GitHub
#8643 (comment)
.

saad-ali · 2015-05-26T21:49:53Z

Thanks. Merged. Will keep an eye on Jenkins

googlebot added the cla: yes label May 21, 2015

thockin reviewed May 21, 2015
View reviewed changes

vishh force-pushed the headless branch 4 times, most recently from e442421 to f912ddf Compare May 22, 2015 21:03

googlebot added cla: no and removed cla: yes labels May 22, 2015

vishh changed the title ~~WIP: Adding support for generating A records for headless services.~~ Adding support for generating A records for headless services. May 22, 2015

thockin reviewed May 22, 2015
View reviewed changes

thockin added lgtm "Looks good to me", indicates that a PR is ready to be merged. cla: yes and removed cla: no lgtm "Looks good to me", indicates that a PR is ready to be merged. labels May 22, 2015

thockin reviewed May 22, 2015
View reviewed changes

dchen1107 assigned thockin May 22, 2015

vishh force-pushed the headless branch from f912ddf to 28b54aa Compare May 22, 2015 23:04

thockin added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 22, 2015

vishh added 2 commits May 22, 2015 23:00

Adding support for generating A records for headless services.

f0c20e1

Adding an e2e test for headless services.

c2fcdec

vishh force-pushed the headless branch from 28b54aa to c2fcdec Compare May 23, 2015 06:00

vishh force-pushed the headless branch from 80cc22c to 50a062a Compare May 26, 2015 02:34

New release of kube2sky (v1.7)

d3c7edb

vishh force-pushed the headless branch from 50a062a to d3c7edb Compare May 26, 2015 02:44

saad-ali added a commit that referenced this pull request May 26, 2015

Merge pull request #8643 from vishh/headless

abaa278

Adding support for generating A records for headless services.

saad-ali merged commit abaa278 into kubernetes:master May 26, 2015

thockin mentioned this pull request May 29, 2015

DNS for headless services #7476

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding support for generating A records for headless services. #8643

Adding support for generating A records for headless services. #8643

vishh commented May 21, 2015

thockin May 21, 2015

vishh May 22, 2015

vishh commented May 21, 2015

k8s-bot commented May 21, 2015

thockin May 21, 2015

vishh May 22, 2015

thockin commented May 21, 2015

googlebot commented May 22, 2015

vishh commented May 22, 2015

thockin May 22, 2015

vishh May 22, 2015

thockin May 22, 2015

vishh May 22, 2015

thockin commented May 22, 2015

googlebot commented May 22, 2015

vishh commented May 22, 2015

thockin commented May 22, 2015

vishh commented May 23, 2015

lavalamp commented May 24, 2015

vishh commented May 24, 2015

lavalamp commented May 24, 2015

vishh commented May 25, 2015

thockin commented May 25, 2015

vishh commented May 26, 2015

ixdy commented May 26, 2015

vishh commented May 26, 2015

thockin commented May 26, 2015

saad-ali commented May 26, 2015

Adding support for generating A records for headless services. #8643

Adding support for generating A records for headless services. #8643

Conversation

vishh commented May 21, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vishh commented May 21, 2015

k8s-bot commented May 21, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thockin commented May 21, 2015

googlebot commented May 22, 2015

vishh commented May 22, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thockin commented May 22, 2015

googlebot commented May 22, 2015

vishh commented May 22, 2015

thockin commented May 22, 2015

vishh commented May 23, 2015

lavalamp commented May 24, 2015

vishh commented May 24, 2015

lavalamp commented May 24, 2015

vishh commented May 25, 2015

thockin commented May 25, 2015

vishh commented May 26, 2015

ixdy commented May 26, 2015

vishh commented May 26, 2015

thockin commented May 26, 2015

saad-ali commented May 26, 2015