[Federation][(Un)join-00] Implement federation/cluster resource generator. #35153

madhusudancs · 2016-10-19T20:24:56Z

Design Doc: PR #34484

cc @kubernetes/sig-cluster-federation @nikhiljindal

This change is

k8s-ci-robot · 2016-10-27T07:56:10Z

Jenkins verification failed for commit 69f4b8f. 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.

nikhiljindal · 2016-10-27T18:16:29Z

pkg/kubectl/cluster.go

+	// Name of the cluster context (required)
+	Name string
+	// ServerAddress is the APIServer address of the Kubernetes cluster
+	// that is being registered (optional)


Why optional? What happens when this is not specified? Is there a default value?

There is no default value, but it could be an empty string right?

Anyway, made it "required" here and added validation.

nikhiljindal · 2016-10-27T18:16:45Z

pkg/kubectl/cluster.go

+	// that is being registered (optional)
+	ServerAddress string
+	// SecretName is the name of the secret that stores the credentials
+	// for the Kubernetes cluster that is being registered (optional)


Same question for what happens when not specified

Same as above.

nikhiljindal · 2016-10-27T18:21:29Z

pkg/kubectl/cluster.go

+		Spec: federationapi.ClusterSpec{
+			ServerAddressByClientCIDRs: []federationapi.ServerAddressByClientCIDR{
+				{
+					ServerAddress: s.ServerAddress,


I dont think ClientCIDR is optional

nikhiljindal · 2016-10-27T18:23:10Z

pkg/kubectl/cluster_test.go

+			params: map[string]interface{}{
+				"name": "foo",
+			},
+			expected: &federationapi.Cluster{


I am not sure if this is a valid cluster spec, that the apiserver will accept

nikhiljindal · 2016-10-27T18:24:07Z

pkg/kubectl/cluster.go

+func (s ClusterGeneratorV1Beta1) ParamNames() []GeneratorParam {
+	return []GeneratorParam{
+		{"name", true},
+		{"serverAddress", false},


Is this a command line flag? Should it be server-address in that case?

It isn't a flag. There is no direct kubectl create user for this generator right now. But there could be one in the future and they might define a flag with that name. So making it look like a flag to be safe.

madhusudancs

@nikhiljindal addressed the comments. PTAL.

madhusudancs · 2016-10-27T19:53:48Z

pkg/kubectl/cluster.go

+func (s ClusterGeneratorV1Beta1) ParamNames() []GeneratorParam {
+	return []GeneratorParam{
+		{"name", true},
+		{"serverAddress", false},


It isn't a flag. There is no direct kubectl create user for this generator right now. But there could be one in the future and they might define a flag with that name. So making it look like a flag to be safe.

madhusudancs · 2016-10-27T19:54:57Z

pkg/kubectl/cluster.go

+	// Name of the cluster context (required)
+	Name string
+	// ServerAddress is the APIServer address of the Kubernetes cluster
+	// that is being registered (optional)


There is no default value, but it could be an empty string right?

Anyway, made it "required" here and added validation.

madhusudancs · 2016-10-27T19:55:01Z

pkg/kubectl/cluster.go

+	// that is being registered (optional)
+	ServerAddress string
+	// SecretName is the name of the secret that stores the credentials
+	// for the Kubernetes cluster that is being registered (optional)


Same as above.

madhusudancs · 2016-10-27T20:06:45Z

pkg/kubectl/cluster.go

+		Spec: federationapi.ClusterSpec{
+			ServerAddressByClientCIDRs: []federationapi.ServerAddressByClientCIDR{
+				{
+					ServerAddress: s.ServerAddress,


madhusudancs · 2016-10-27T20:07:18Z

pkg/kubectl/cluster_test.go

+			params: map[string]interface{}{
+				"name": "foo",
+			},
+			expected: &federationapi.Cluster{


nikhiljindal · 2016-10-28T05:55:56Z

pkg/kubectl/cluster.go

+		return fmt.Errorf("name must be specified")
+	}
+	if len(s.ClientCIDR) == 0 {
+		return fmt.Errorf("client CIDR must be specified")


We can use "0.0.0.0/0" as the default CIDR (matches everything)

My original plan was to do this defaulting in the caller. It doesn't hurt to default at both the places. So done.

nikhiljindal · 2016-10-28T05:56:10Z

pkg/kubectl/cluster.go

+		return fmt.Errorf("server address must be specified")
+	}
+	if len(s.SecretName) == 0 {
+		return fmt.Errorf("secret name must be specified")


We can use cluster name as default secret name

Same as above. Done.

nikhiljindal · 2016-10-28T05:57:18Z

pkg/kubectl/cluster.go

+	return []GeneratorParam{
+		{"name", true},
+		{"client-cidr", false},
+		{"server-address", false},


Shouldnt the second parameter be true here?

nikhiljindal · 2016-10-28T05:58:38Z

I think cluster name and server-address should only be required. secretName and clientCIDR can be optional. Looks good otherwise

madhusudancs

@nikhiljindal addressed the comments. PTAL.

madhusudancs · 2016-10-28T07:30:17Z

pkg/kubectl/cluster.go

+	return []GeneratorParam{
+		{"name", true},
+		{"client-cidr", false},
+		{"server-address", false},


madhusudancs · 2016-10-28T07:33:36Z

pkg/kubectl/cluster.go

+		return fmt.Errorf("name must be specified")
+	}
+	if len(s.ClientCIDR) == 0 {
+		return fmt.Errorf("client CIDR must be specified")


My original plan was to do this defaulting in the caller. It doesn't hurt to default at both the places. So done.

madhusudancs · 2016-10-28T07:33:52Z

pkg/kubectl/cluster.go

+		return fmt.Errorf("server address must be specified")
+	}
+	if len(s.SecretName) == 0 {
+		return fmt.Errorf("secret name must be specified")


Same as above. Done.

k8s-ci-robot · 2016-10-28T07:50:48Z

Jenkins GKE smoke e2e failed for commit b9c4a8142b26d324447c1e388ee283ad20b41f13. 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-10-28T09:15:10Z

Jenkins GCI GCE e2e failed for commit b9c4a8142b26d324447c1e388ee283ad20b41f13. 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-10-28T09:22:17Z

Jenkins Kubemark GCE e2e failed for commit b9c4a8142b26d324447c1e388ee283ad20b41f13. 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.

nikhiljindal · 2016-11-01T07:18:30Z

Thx for addressing the comments.
LGTM.
Please squash commits.

…ator.

madhusudancs · 2016-11-01T08:00:15Z

@nikhiljindal squashed the commits and rebased. Thank you very much for a thorough review.

k8s-ci-robot · 2016-11-01T10:24:11Z

Jenkins GCE Node e2e failed for commit 1712a19. Full PR test history.

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

madhusudancs · 2016-11-01T16:59:41Z

@k8s-bot node e2e test this

madhusudancs · 2016-11-01T17:38:51Z

@k8s-bot node e2e test this

k8s-github-robot · 2016-11-01T18:08:52Z

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

k8s-ci-robot · 2016-11-01T18:30:11Z

Jenkins unit/integration failed for commit 1712a19. 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.

madhusudancs · 2016-11-01T18:47:27Z

@k8s-bot unit test this

madhusudancs · 2016-11-01T18:52:09Z

@k8s-bot unit test this

madhusudancs · 2016-11-01T18:59:44Z

@k8s-bot unit test this

madhusudancs · 2016-11-01T19:06:02Z

@k8s-bot unit test this

nikhiljindal · 2016-11-01T19:09:56Z

@k8s-bot unit test this

madhusudancs · 2016-11-01T19:11:23Z

@k8s-oncall pinging k8s-bot doesn't seem to work. Any idea what's going on?

saad-ali · 2016-11-01T19:16:10Z

@k8s-bot unit test this

saad-ali · 2016-11-01T19:22:22Z

@kubernetes/test-infra-admins Any ideas why k8s-bot is not responding?

saad-ali · 2016-11-01T19:23:16Z

@k8s-bot unit test this, Issue #35898

saad-ali · 2016-11-01T19:40:39Z

Manually merging to unblock--failing unit test is due to #35898. All other tests are green.

madhusudancs · 2016-11-01T19:43:24Z

@saad-ali thanks!

ixdy · 2016-11-01T20:33:28Z

it looks like the tests were run several times (per https://k8s-gubernator.appspot.com/pr/35153), but I don't know why it didn't update the commit status here. @spxtr any ideas?

madhusudancs added area/cluster-federation release-note-none Denotes a PR that doesn't merit a release note. labels Oct 19, 2016

madhusudancs added this to the v1.5 milestone Oct 19, 2016

madhusudancs assigned ghost Oct 19, 2016

googlebot added the cla: yes label Oct 19, 2016

This was referenced Oct 19, 2016

[Federation][(Un)join-01] Add a shell join command to kubectl. #35154

Closed

[Federation][(Un)join-02] Implement kubectl join federation command. #35155

Closed

k8s-github-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Oct 19, 2016

madhusudancs mentioned this pull request Oct 25, 2016

Make the fake RESTClient usable by all the API groups, not just core. #35492

Merged

madhusudancs force-pushed the federation-kubectl-register-00 branch from 4c1eb90 to 69f4b8f Compare October 27, 2016 07:29

madhusudancs assigned nikhiljindal and unassigned ghost Oct 27, 2016

nikhiljindal reviewed Oct 27, 2016

View reviewed changes

madhusudancs commented Oct 27, 2016

View reviewed changes

nikhiljindal reviewed Oct 28, 2016

View reviewed changes

madhusudancs commented Oct 28, 2016

View reviewed changes

madhusudancs force-pushed the federation-kubectl-register-00 branch from b9c4a81 to aa22ed6 Compare October 30, 2016 02:34

ghost added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 31, 2016

[Federation][(Un)join-00] Implement federation/cluster resource gener…

1712a19

…ator.

madhusudancs force-pushed the federation-kubectl-register-00 branch from aa22ed6 to 1712a19 Compare November 1, 2016 07:59

k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 1, 2016

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

saad-ali merged commit edab99b into kubernetes:master Nov 1, 2016

madhusudancs deleted the federation-kubectl-register-00 branch November 2, 2016 21:56

madhusudancs mentioned this pull request Nov 17, 2016

It should be fast and painless to deploy a Federation of Kubernetes clusters kubernetes/enhancements#97

Closed

22 tasks

[Federation][(Un)join-00] Implement federation/cluster resource generator. #35153

[Federation][(Un)join-00] Implement federation/cluster resource generator. #35153

Conversation

madhusudancs commented Oct 19, 2016 • edited Loading

k8s-ci-robot commented Oct 27, 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

madhusudancs left a 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 Oct 28, 2016

madhusudancs left a 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

k8s-ci-robot commented Oct 28, 2016

k8s-ci-robot commented Oct 28, 2016

k8s-ci-robot commented Oct 28, 2016

nikhiljindal commented Nov 1, 2016

madhusudancs commented Nov 1, 2016

k8s-ci-robot commented Nov 1, 2016

madhusudancs commented Nov 1, 2016

madhusudancs commented Nov 1, 2016

k8s-github-robot commented Nov 1, 2016

k8s-ci-robot commented Nov 1, 2016

madhusudancs commented Nov 1, 2016

madhusudancs commented Nov 1, 2016

madhusudancs commented Nov 1, 2016

madhusudancs commented Nov 1, 2016

nikhiljindal commented Nov 1, 2016

madhusudancs commented Nov 1, 2016

saad-ali commented Nov 1, 2016

saad-ali commented Nov 1, 2016

saad-ali commented Nov 1, 2016

saad-ali commented Nov 1, 2016

madhusudancs commented Nov 1, 2016

ixdy commented Nov 1, 2016

madhusudancs commented Oct 19, 2016 •

edited

Loading