Add limited config-map support to kube-dns #36775

bowei · 2016-11-14T21:49:49Z

This is an integration bugfix for #36194

kube-dns

Added --config-map and --config-map-namespace command line options. 
If --config-map is set, kube-dns will load dynamic configuration from the config map 
referenced by --config-map-namespace, --config-map. The config-map supports
the following properties: "federations".

--federations flag is now deprecated. Prefer to set federations via the config-map.
Federations can be configured by settings the "federations" field to the value currently 
set in the command line.

Example:

  kind: ConfigMap
  apiVersion: v1
  metadata:
    name: kube-dns
    namespace: kube-system
  data:
    federations: abc=def

This change is

k8s-ci-robot · 2016-11-14T22:08:57Z

Jenkins unit/integration failed for commit f3adc5d9c23f5d66484077f7df949938aa8af55f. Full PR test history.

The magic incantation to run this job again is @k8s-bot unit test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

bowei · 2016-11-15T01:50:53Z

@k8s-bot test this

bowei · 2016-11-15T05:02:26Z

@k8s-bot e2e test this

bowei · 2016-11-15T18:40:41Z

@k8s-bot e2e test this

k8s-ci-robot · 2016-11-15T19:12:35Z

Jenkins GKE smoke e2e failed for commit 79c8f51ac62bbaf399d8debd538d8d8102afd1f0. Full PR test history.

The magic incantation to run this job again is @k8s-bot cvm gke e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

k8s-ci-robot · 2016-11-15T19:15:58Z

Jenkins GCI GCE e2e failed for commit 79c8f51ac62bbaf399d8debd538d8d8102afd1f0. Full PR test history.

The magic incantation to run this job again is @k8s-bot gci gce e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

k8s-ci-robot · 2016-11-15T19:16:29Z

Jenkins GCE e2e failed for commit 79c8f51ac62bbaf399d8debd538d8d8102afd1f0. Full PR test history.

The magic incantation to run this job again is @k8s-bot cvm gce e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

k8s-ci-robot · 2016-11-15T19:38:02Z

Jenkins Kubemark GCE e2e failed for commit 79c8f51ac62bbaf399d8debd538d8d8102afd1f0. Full PR test history.

The magic incantation to run this job again is @k8s-bot kubemark e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

bowei · 2016-11-15T20:16:15Z

@thockin @nikhiljindal @MrHohn

This is the fix that needs to go into 1.5 for federations to work with config flags

nikhiljindal · 2016-11-15T21:04:09Z

Awesome, thanks!
I will review this later tonight.
cc @kubernetes/sig-cluster-federation

nikhiljindal · 2016-11-16T08:16:40Z

test/e2e/framework/exec_util.go

+// ExecCommandInContainer execute a command in the specified container.
+func (f *Framework) ExecCommandInContainer(podName, containerName string, cmd ...string) string {
+	stdout, stderr, err := f.ExecCommandInContainerWithFullOutput(podName, containerName, cmd...)
+	Logf("Exec stderr: %q", stderr)


Add some context to this error message like "Exec stderr while executing cmd %s in container %s in pod %s"?

the Exec* family of command already dumps this information into the log (ExecWithOptions logs its arguments). I don't really want to duplicate it.

nikhiljindal · 2016-11-16T08:16:57Z

test/e2e/framework/exec_util.go

+func (f *Framework) ExecCommandInContainer(podName, containerName string, cmd ...string) string {
+	stdout, stderr, err := f.ExecCommandInContainerWithFullOutput(podName, containerName, cmd...)
+	Logf("Exec stderr: %q", stderr)
+	Expect(err).NotTo(HaveOccurred(), "fail to execute command")


Also print the command, container and pod for which this failed

nikhiljindal · 2016-11-16T08:17:57Z

cmd/kube-dns/app/options/options.go

 	HealthzPort    int
 	DNSBindAddress string
 	DNSPort        int
-	// Federations maps federation names to their registered domain names.


Why remove this comment?

It duplicates the usage in the flags description below and increases the maintenance cost.

nikhiljindal · 2016-11-16T08:21:29Z

cmd/kube-dns/app/options/options.go

+
+		Federations: make(map[string]string),
+
+		ConfigMapNs: "kube-system",


Use the NamespaceSystem const:

kubernetes/pkg/api/types.go

Line 190 in 00458a1

NamespaceSystem string = "kube-system"

?

nikhiljindal · 2016-11-16T08:23:38Z

cmd/kube-dns/app/options/options.go

+			" domain names to which this cluster belongs. Example:"+
+			" \"myfederation1=example.com,myfederation2=example2.com,myfederation3=example.com\"."+
+			" It is an error to set both the federations and config-map flags.")
+	fs.MarkDeprecated("federations", "use ConfigMap instead. Will be removed in future version")


use ConfigMap -> use config-map

nikhiljindal · 2016-11-16T09:07:07Z

test/e2e/dns_configmap.go

+}
+
+func (t *dnsConfigMapTest) validateFederation() {
+	const timeout = time.Duration(10) * time.Second


We recommend using wait.ForeverTestTimeout in all tests

nikhiljindal · 2016-11-16T09:08:48Z

test/e2e/dns_configmap.go

+	ok := false
+	var actual []string
+
+	for time.Since(startTime) < timeout {


Use wait.PollImmediate

nikhiljindal · 2016-11-16T09:12:19Z

test/e2e/dns_configmap.go

+		},
+		ObjectMeta: api.ObjectMeta{
+			Namespace: t.f.Namespace.Name,
+			Name:      "e2e-dns-configmap-" + string(uuid.NewUUID()),


You can set GenerateName. Example:

kubernetes/test/e2e/service_latency.go

Line 320 in 33ebe1f

GenerateName: "latency-svc-",

nikhiljindal · 2016-11-16T09:13:30Z

test/e2e/dns_configmap.go

+			Containers: []api.Container{
+				{
+					Name:    "util",
+					Image:   "gcr.io/google_containers/dnsutils:e2e",


Is this tag documented somewhere?

you mean :e2e? Not sure, but this is already used by the other dns related tests

nikhiljindal · 2016-11-16T09:14:48Z

test/e2e/dns_configmap.go

+		Spec: api.ServiceSpec{
+			Selector: map[string]string{"app": "e2e-dns-configmap"},
+			Ports: []api.ServicePort{
+				{Protocol: "TCP", Port: 10101, TargetPort: intstr.FromInt(10101)},


Is 10101 a random port you picked or is it specific?

random port

optional nit: you can make it a const at top and add a comment saying its random.

nikhiljindal · 2016-11-16T09:21:19Z

Mostly minor suggestions and some questions.
Thanks @bowei for including great tests as well!

liggitt · 2016-11-16T17:59:19Z

this seems like a big change... is it a bug fix or a feature?

bowei · 2016-11-17T00:04:02Z

@liggitt yes, this is a bug fix (see #36194)

k8s-ci-robot · 2016-11-17T00:25:05Z

Jenkins verification failed for commit c043981b5ce0e77d14aedb9d0f269dfdee49589b. Full PR test history.

The magic incantation to run this job again is @k8s-bot verify test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

k8s-ci-robot · 2016-11-19T00:15:00Z

Jenkins GCE etcd3 e2e failed for commit d94b2b7. Full PR test history.

The magic incantation to run this job again is @k8s-bot gce etcd3 e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

bowei · 2016-11-19T00:15:08Z

ok -- images pushed to google_containers, version 1.9

bowei · 2016-11-19T00:35:15Z

k8s-bot test this

saad-ali · 2016-11-19T01:09:12Z

@kubernetes/test-infra-admins Any idea why only 4 pre-submit tests are running against this PR and not the usual 10-15?

ixdy · 2016-11-19T01:11:40Z

I don't see any test results for 19be1d2. @spxtr did we miss a webhook?

ixdy · 2016-11-19T01:13:38Z

Actually, it looks like it was triggered on https://prow.k8s.io/, but Jenkins might be down. @fejta

bowei · 2016-11-19T02:53:12Z

@dims @saad-ali lgtm?

bowei · 2016-11-19T02:56:15Z

@smarterclayton -- Is it not possible for the pod author to restrict access to a specific config-map? For example, in this case, the pod yaml would grant kubedns access to kube-system:kube-dns config map only. Requests to other config-maps by API would be rejected.

dims · 2016-11-19T03:01:38Z

@bowei 👍 LGTM

bowei · 2016-11-19T04:31:04Z

/lgtm

k8s-ci-robot · 2016-11-19T04:31:05Z

@bowei: you can't LGTM your own PR.

In response to [this comment](https://github.com//pull/36775#issuecomment-261692810):

/lgtm

If you have questions or suggestions related to this bot's behavior, please file an issue against the kubernetes/test-infra repository.

bowei · 2016-11-19T04:31:24Z

@dims @saad-ali it needs to be /lgtm

k8s-github-robot · 2016-11-19T06:29:10Z

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

k8s-github-robot · 2016-11-19T07:03:54Z

Automatic merge from submit-queue

luxas · 2016-11-25T17:10:01Z

@bowei You should push the image for arm, arm64 and ppc64le as well.

Please do it as soon as possible, kubeadm needs it.
Thanks

MrHohn · 2016-11-25T18:17:34Z

@luxas Sorry about the inconvenience. I just built and pushed the other three images.

luxas · 2016-11-25T18:20:13Z

Thank you @MrHohn!

luxas · 2016-11-26T08:48:25Z

@bowei @MrHohn Sorry to disturb once more, but I noticed dnsmasq-metrics-ARCH:1.0 isn't pushed for the other architectures...
Please push them as well

MrHohn · 2016-11-27T19:25:25Z

@luxas Built and pushed. Sorry for the delay.

bowei · 2016-11-27T19:44:50Z

@MrHohn -- thanks for taking care of it for me :-)

googlebot added the cla: yes label Nov 14, 2016

bowei force-pushed the kube-dns-config-map branch from f0d5abd to f3adc5d Compare November 14, 2016 21:51

k8s-github-robot assigned ixdy Nov 14, 2016

k8s-github-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. release-note-label-needed labels Nov 14, 2016

ixdy assigned nikhiljindal and unassigned ixdy Nov 14, 2016

bowei force-pushed the kube-dns-config-map branch 5 times, most recently from d151eeb to 79c8f51 Compare November 15, 2016 01:12

bowei force-pushed the kube-dns-config-map branch from 79c8f51 to a1800d2 Compare November 15, 2016 19:21

nikhiljindal reviewed Nov 16, 2016

View reviewed changes

bowei force-pushed the kube-dns-config-map branch from a1800d2 to c043981 Compare November 16, 2016 23:56

bowei force-pushed the kube-dns-config-map branch from c043981 to a340294 Compare November 17, 2016 00:36

bowei force-pushed the kube-dns-config-map branch from d94b2b7 to 19be1d2 Compare November 19, 2016 00:12

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Nov 19, 2016

thockin added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 19, 2016

k8s-github-robot merged commit d725b3e into kubernetes:master Nov 19, 2016

madhusudancs mentioned this pull request Nov 19, 2016

kube-dns --federations flag needs to be user modifiable (integration between federation and the new addon manager) #36194

Closed

MrHohn mentioned this pull request Nov 22, 2016

Adds changelog for kubedns v1.9 #37300

Merged

chentao1596 mentioned this pull request Dec 5, 2016

WIP:kubelet: support multi-headers when getting pod from HTTP source #38089

Closed

MrHohn mentioned this pull request Dec 15, 2016

move kube-dns to a separate service account #38816

Merged

liggitt mentioned this pull request Jan 24, 2017

Add support for pulling config from a directory kubernetes/dns#39

Merged

bowei deleted the kube-dns-config-map branch February 21, 2017 19:11


		Federations: make(map[string]string),

		ConfigMapNs: "kube-system",

Add limited config-map support to kube-dns #36775

Add limited config-map support to kube-dns #36775

Conversation

bowei commented Nov 14, 2016 • edited

k8s-ci-robot commented Nov 14, 2016

bowei commented Nov 15, 2016

bowei commented Nov 15, 2016

bowei commented Nov 15, 2016

k8s-ci-robot commented Nov 15, 2016

k8s-ci-robot commented Nov 15, 2016

k8s-ci-robot commented Nov 15, 2016

k8s-ci-robot commented Nov 15, 2016

bowei commented Nov 15, 2016

nikhiljindal commented Nov 15, 2016

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

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

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

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

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

Choose a reason for hiding this comment

nikhiljindal commented Nov 16, 2016

liggitt commented Nov 16, 2016

bowei commented Nov 17, 2016

k8s-ci-robot commented Nov 17, 2016

k8s-ci-robot commented Nov 19, 2016

bowei commented Nov 19, 2016

bowei commented Nov 19, 2016

saad-ali commented Nov 19, 2016

ixdy commented Nov 19, 2016

ixdy commented Nov 19, 2016

bowei commented Nov 19, 2016

bowei commented Nov 19, 2016

dims commented Nov 19, 2016

bowei commented Nov 19, 2016

k8s-ci-robot commented Nov 19, 2016

bowei commented Nov 19, 2016

k8s-github-robot commented Nov 19, 2016

k8s-github-robot commented Nov 19, 2016

luxas commented Nov 25, 2016

MrHohn commented Nov 25, 2016

luxas commented Nov 25, 2016

luxas commented Nov 26, 2016

MrHohn commented Nov 27, 2016

bowei commented Nov 27, 2016

bowei commented Nov 14, 2016 •

edited