Federation apiobject cluster #23847

jianhuiz · 2016-04-04T23:39:22Z

add federation api group
add cluster api object and registry
~~generate cluster client~~ moved to #24117
update scripts to generate files for /federation
#19313 #23653 #23554

@nikhiljindal @quinton-hoole, @deepak-vij, @XiaoningDing, @alfred-huangjian @mfanjie @huangyuqi @colhom

k8s-bot · 2016-04-04T23:40:18Z

Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test".
(Note: "add to whitelist" is no longer supported. Please update configurations in hack/jenkins/job-configs/kubernetes-jenkins-pull/ instead.)

This message may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

Otherwise, if this message is too spammy, please complain to ixdy.

k8s-bot · 2016-04-04T23:40:35Z

Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test".
(Note: "add to whitelist" is no longer supported. Please update configurations in hack/jenkins/job-configs/kubernetes-jenkins-pull/ instead.)

This message may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

Otherwise, if this message is too spammy, please complain to ixdy.

k8s-bot · 2016-04-04T23:41:17Z

Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test".
(Note: "add to whitelist" is no longer supported. Please update configurations in hack/jenkins/job-configs/kubernetes-jenkins-pull/ instead.)

This message may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

Otherwise, if this message is too spammy, please complain to ixdy.

huangyuqi · 2016-04-11T02:50:24Z

federation/client/clientset_generated/internalclientset/clientset.go

+)
+
+type Interface interface {
+	Discovery() discovery.DiscoveryInterface


Hi, @jianhuiz , in cluster-controller and scheduler, we will use this client interface to operate resource. Except cluster and sub-replicaset , we also need relicaset operation. So would you like to define the interface like this?

type Interface interface { Discovery() discovery.DiscoveryInterface Core() unversionedcore.CoreInterface Federation() unversionedfederation.FederationInterface }

@mfanjie

huangyuqi · 2016-04-11T03:24:50Z

@jianhuiz ,
In kubernetes, client always has a cache interface https://github.com/kubernetes/kubernetes/tree/master/pkg/client/cache, used to storing resource in kubernetes controller and scheduler.
So, for unity, a cache interface need be added in federated client, right?
@mfanjie

mfanjie · 2016-04-11T03:30:50Z

I agreed on both comments from @huangyuqi, same request here from scheduler, in addition, I had an email to @jianhuiz, we need to define the annotation somewhere in apiobject component, most likely in types.go

var ClusterSelectorKey = "kubernetes.io/cluster-names"
var TargetClusterKey = "kubernetes.io/target-cluster"
var FederationReplicaSetKey = "kubernetes.io/created-by”

nikhiljindal · 2016-04-11T23:13:39Z

We should move the client changes to a separate PR. @caesarxuchao will be the best reviewer for that.

Lets get the APIserver parts correct in this PR.

@k8s-bot ok to test.

cc @kubernetes/sig-api-machinery @kubernetes/sig-cluster-federation

nikhiljindal · 2016-04-11T23:50:33Z

cmd/genconversion/conversion.go

@@ -45,10 +46,11 @@ const pkgBase = "k8s.io/kubernetes/pkg"
 var (
 	functionDest = flag.StringP("funcDest", "f", "-", "Output for conversion functions; '-' means stdout")
 	groupVersion = flag.StringP("version", "v", "api/v1", "groupPath/version for conversion.")
+	gvPath       = flag.StringP("path", "p", "", "path to the api group version directory, relative path to pkg/apis")


How about naming it apiTypesPath?

nikhiljindal · 2016-04-11T23:58:50Z

Most of the api machinery changes look good (have added a few comments)

Have added a few comments on the API as well.
@quinton-hoole and @bgrant0607 might have more comments on the API.

nikhiljindal · 2016-04-27T00:14:01Z

unit tests failed on Jenkins with the following error:

Verifying ./hack/../hack/verify-govet.sh
federation/registry/cluster/etcd/etcd_test.go:35: k8s.io/kubernetes/pkg/registry/generic.RESTOptions composite literal uses unkeyed fields
FAILED   ./hack/../hack/verify-govet.sh 128s

nikhiljindal · 2016-04-27T00:15:36Z

federation/registry/cluster/etcd/etcd_test.go

+
+func newStorage(t *testing.T) (*REST, *etcdtesting.EtcdTestServer) {
+	etcdStorage, server := registrytest.NewEtcdStorage(t, federation.GroupName)
+	restOptions := generic.RESTOptions{etcdStorage, generic.UndecoratedStorage, 1}


Change this to:
RestOptions{Storage: etcdStorage, Decorator: generic.UndecoratedStorage, DeleteCollectionWorkers: 1}.
hack/verify-govet.sh is failing because of this

nikhiljindal · 2016-04-27T00:20:22Z

Code changes LGTM.
Ready to merge as soon as tests pass.

nikhiljindal · 2016-04-27T02:00:46Z

Great. tests pass now!
Can you squash your commits? You dont need to squash all commits. Its fine to keep the big ones. Probably squash the fixup ones?

k8s-bot · 2016-04-27T05:25:52Z

GCE e2e build/test passed for commit 3bdd413.

nikhiljindal · 2016-04-27T06:04:44Z

LGTM, thanks!

k8s-github-robot · 2016-04-27T06:37:38Z

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

k8s-bot · 2016-04-27T07:26:18Z

GCE e2e build/test passed for commit 3bdd413.

k8s-github-robot · 2016-04-27T07:26:22Z

Automatic merge from submit-queue

bgrant0607 · 2016-04-28T05:32:16Z

federation/apis/federation/types.go

+	// ClusterReady means the cluster is ready to accept workloads.
+	ClusterReady ClusterConditionType = "Ready"
+	// ClusterOffline means the cluster is temporarily down or not reachable
+	ClusterOffline ClusterConditionType = "Offline"


What's the difference between Ready and Offline? And why do they have have the opposite sense? For instance, this isn't this "Online"?

If Offline is about reachability, it should be called Reachable.

I believe when a cluster is ready, then other conditions should be generally false or absent.

When Ready becomes false, there could be one or more conditions that indicate the reason for that.

Offline is about reachability (#19313 Cluster which I can change it to Unreachable if you prefer.

There may be more conditions (reason field also maybe) in the future to indicate the status of an unready cluster (like NodeOutOfDisk for NodeCondition)

@nikhiljindal

Automatic merge from submit-queue Federation client for cluster generate v1alpha1 and unversioned client for federation/clusters #23653, requires #23847, #23998 @nikhiljindal @quinton-hoole @caesarxuchao

Automatic merge from submit-queue Federation kubectl for clusters add federation/clusters resource to kubectl #23653, requires #23847  --- This change is [<img src="http://reviewable.k8s.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](http://reviewable.k8s.io/reviews/kubernetes/kubernetes/24016)

googlebot added the cla: yes label Apr 4, 2016

k8s-github-robot assigned zmerlynn Apr 4, 2016

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

ghost assigned ghost and nikhiljindal and unassigned zmerlynn and ghost Apr 4, 2016

This was referenced Apr 7, 2016

Federation apiobject replicaset #23998

Closed

Federation kubectl for clusters #24016

Merged

huangyuqi reviewed Apr 11, 2016
View reviewed changes

jianhuiz mentioned this pull request Apr 11, 2016

Federation client for cluster #24117

Merged

nikhiljindal reviewed Apr 11, 2016
View reviewed changes

ghost added area/cluster-federation labels Apr 12, 2016

nikhiljindal removed the release-note-label-needed label Apr 25, 2016

jianhuiz added 5 commits April 26, 2016 02:05

add api group 'federation' and 'cluster' object

fcb241d

add cluster registry

1c1ad1a

register federation/clusters to federated-apiserver

b195732

replace cluster phase with conditions

384f801

add protobuf generation

3bdd413

nikhiljindal mentioned this pull request Apr 27, 2016

Move install of version handler to genericapiserver #24774

Merged

nikhiljindal reviewed Apr 27, 2016
View reviewed changes

k8s-github-robot added the needs-ok-to-merge label Apr 27, 2016

nikhiljindal added lgtm "Looks good to me", indicates that a PR is ready to be merged. ok-to-merge and removed needs-ok-to-merge labels Apr 27, 2016

k8s-github-robot merged commit b7e4672 into kubernetes:master Apr 27, 2016

nikhiljindal mentioned this pull request Apr 27, 2016

e2e test for federated-apiserver #24880

Closed

bgrant0607 reviewed Apr 28, 2016
View reviewed changes

nikhiljindal mentioned this pull request May 5, 2016

Cluster Federation: task list #23653

Closed

20 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Federation apiobject cluster #23847

Federation apiobject cluster #23847

jianhuiz commented Apr 4, 2016

k8s-bot commented Apr 4, 2016

k8s-bot commented Apr 4, 2016

k8s-bot commented Apr 4, 2016

huangyuqi Apr 11, 2016

huangyuqi commented Apr 11, 2016

mfanjie commented Apr 11, 2016

nikhiljindal commented Apr 11, 2016

nikhiljindal Apr 11, 2016

nikhiljindal commented Apr 11, 2016

nikhiljindal commented Apr 27, 2016

nikhiljindal Apr 27, 2016 •

edited

nikhiljindal commented Apr 27, 2016

nikhiljindal commented Apr 27, 2016

k8s-bot commented Apr 27, 2016

nikhiljindal commented Apr 27, 2016

k8s-github-robot commented Apr 27, 2016

k8s-bot commented Apr 27, 2016

k8s-github-robot commented Apr 27, 2016

bgrant0607 Apr 28, 2016

jianhuiz Apr 28, 2016

Federation apiobject cluster #23847

Federation apiobject cluster #23847

Conversation

jianhuiz commented Apr 4, 2016

k8s-bot commented Apr 4, 2016

k8s-bot commented Apr 4, 2016

k8s-bot commented Apr 4, 2016

huangyuqi Apr 11, 2016

Choose a reason for hiding this comment

huangyuqi commented Apr 11, 2016

mfanjie commented Apr 11, 2016

nikhiljindal commented Apr 11, 2016

nikhiljindal Apr 11, 2016

Choose a reason for hiding this comment

nikhiljindal commented Apr 11, 2016

nikhiljindal commented Apr 27, 2016

nikhiljindal Apr 27, 2016 • edited

Choose a reason for hiding this comment

nikhiljindal commented Apr 27, 2016

nikhiljindal commented Apr 27, 2016

k8s-bot commented Apr 27, 2016

nikhiljindal commented Apr 27, 2016

k8s-github-robot commented Apr 27, 2016

k8s-bot commented Apr 27, 2016

k8s-github-robot commented Apr 27, 2016

bgrant0607 Apr 28, 2016

Choose a reason for hiding this comment

jianhuiz Apr 28, 2016

Choose a reason for hiding this comment

nikhiljindal Apr 27, 2016 •

edited