Merge pull request #24006 from caesarxuchao/thirdparty-preferredversion

Automatic merge from submit-queue

Use the first version as thirdparty resource preferredVersion

First commit is a one-liner, which implements the server-half of #23985.

The other two commits rearrange the test code, and add back a commented out test of thirdparty resource.

@lavalamp @nikhiljindal

pkg/master/master.go

@@ -643,8 +643,9 @@ func (m *Master) InstallThirdPartyResource(rsrc *extensions.ThirdPartyResource)
 		Version:      rsrc.Versions[0].Name,
 	}
 	apiGroup := unversioned.APIGroup{
-		Name:     group,
+		Name:             group,
-		Versions: []unversioned.GroupVersionForDiscovery{groupVersion},
+		Versions:         []unversioned.GroupVersionForDiscovery{groupVersion},
+		PreferredVersion: groupVersion,
 	}
 	apiserver.AddGroupWebService(api.Codecs, m.HandlerContainer, path, apiGroup)
 	m.addThirdPartyResourceStorage(path, thirdparty.Storage[strings.ToLower(kind)+"s"].(*thirdpartyresourcedataetcd.REST), apiGroup)

pkg/master/master_test.go

@@ -36,9 +36,14 @@ import (
 	utilnet "k8s.io/kubernetes/pkg/util/net"
 	"k8s.io/kubernetes/pkg/util/sets"
 
+	apiutil "k8s.io/kubernetes/pkg/api/util"
+	apiv1 "k8s.io/kubernetes/pkg/api/v1"
 	"k8s.io/kubernetes/pkg/apis/autoscaling"
+	autoscalingapiv1 "k8s.io/kubernetes/pkg/apis/autoscaling/v1"
 	"k8s.io/kubernetes/pkg/apis/batch"
+	batchapiv1 "k8s.io/kubernetes/pkg/apis/batch/v1"
 	"k8s.io/kubernetes/pkg/apis/extensions"
+	extensionsapiv1beta1 "k8s.io/kubernetes/pkg/apis/extensions/v1beta1"
 	"k8s.io/kubernetes/pkg/genericapiserver"
 	"k8s.io/kubernetes/pkg/kubelet/client"
 	"k8s.io/kubernetes/pkg/registry/endpoint"
@@ -83,20 +88,11 @@ func setUp(t *testing.T) (*Master, *etcdtesting.EtcdTestServer, Config, *assert.
 	storageVersions[extensions.GroupName] = testapi.Extensions.GroupVersion().String()
 	config.StorageVersions = storageVersions
 	config.PublicAddress = net.ParseIP("192.168.10.4")
-	master.nodeRegistry = registrytest.NewNodeRegistry([]string{"node1", "node2"}, api.NodeResources{})
-
-	return master, server, config, assert.New(t)
-}
-
-func newMaster(t *testing.T) (*Master, *etcdtesting.EtcdTestServer, Config, *assert.Assertions) {
-	_, etcdserver, config, assert := setUp(t)
-
 	config.Serializer = api.Codecs
 	config.KubeletClient = client.FakeKubeletClient{}
 	config.APIPrefix = "/api"
 	config.APIGroupPrefix = "/apis"
 	config.APIResourceConfigSource = DefaultAPIResourceConfigSource()
-
 	config.ProxyDialer = func(network, addr string) (net.Conn, error) { return nil, nil }
 	config.ProxyTLSClientConfig = &tls.Config{}
 
@@ -108,6 +104,33 @@ func newMaster(t *testing.T) (*Master, *etcdtesting.EtcdTestServer, Config, *ass
 	// run the sync routine and register types manually.
 	config.disableThirdPartyControllerForTesting = true
 
+	master.nodeRegistry = registrytest.NewNodeRegistry([]string{"node1", "node2"}, api.NodeResources{})
+
+	return master, server, config, assert.New(t)
+}
+
+func newMaster(t *testing.T) (*Master, *etcdtesting.EtcdTestServer, Config, *assert.Assertions) {
+	_, etcdserver, config, assert := setUp(t)
+
+	master, err := New(&config)
+	if err != nil {
+		t.Fatalf("Error in bringing up the master: %v", err)
+	}
+
+	return master, etcdserver, config, assert
+}
+
+// limitedAPIResourceConfigSource only enables the core group, the extensions group, the batch group, and the autoscaling group.
+func limitedAPIResourceConfigSource() *genericapiserver.ResourceConfig {
+	ret := genericapiserver.NewResourceConfig()
+	ret.EnableVersions(apiv1.SchemeGroupVersion, extensionsapiv1beta1.SchemeGroupVersion, batchapiv1.SchemeGroupVersion, autoscalingapiv1.SchemeGroupVersion)
+	return ret
+}
+
+// newLimitedMaster only enables the core group, the extensions group, the batch group, and the autoscaling group.
+func newLimitedMaster(t *testing.T) (*Master, *etcdtesting.EtcdTestServer, Config, *assert.Assertions) {
+	_, etcdserver, config, assert := setUp(t)
+	config.APIResourceConfigSource = limitedAPIResourceConfigSource()
 	master, err := New(&config)
 	if err != nil {
 		t.Fatalf("Error in bringing up the master: %v", err)
@@ -370,114 +393,98 @@ func TestAPIVersionOfDiscoveryEndpoints(t *testing.T) {
 }
 
 func TestDiscoveryAtAPIS(t *testing.T) {
-	// TODO(caesarxuchao): make this pass now that batch is added,
+	master, etcdserver, config, assert := newLimitedMaster(t)
-	// and rewrite it so that the indexes do not need to change each time a new api group is added.
+	defer etcdserver.Terminate(t)
-	/*
-		master, etcdserver, config, assert := newMaster(t)
-		defer etcdserver.Terminate(t)
 
-		server := httptest.NewServer(master.HandlerContainer.ServeMux)
+	server := httptest.NewServer(master.HandlerContainer.ServeMux)
-		resp, err := http.Get(server.URL + "/apis")
+	resp, err := http.Get(server.URL + "/apis")
-		if !assert.NoError(err) {
+	if !assert.NoError(err) {
-			t.Errorf("unexpected error: %v", err)
+		t.Errorf("unexpected error: %v", err)
-		}
+	}
 
-		assert.Equal(http.StatusOK, resp.StatusCode)
+	assert.Equal(http.StatusOK, resp.StatusCode)
 
-		groupList := unversioned.APIGroupList{}
+	groupList := unversioned.APIGroupList{}
-		assert.NoError(decodeResponse(resp, &groupList))
+	assert.NoError(decodeResponse(resp, &groupList))
-		if err != nil {
+	if err != nil {
-			t.Fatalf("unexpected error: %v", err)
+		t.Fatalf("unexpected error: %v", err)
-		}
+	}
 
-		expectGroupNames := []string{autoscaling.GroupName, batch.GroupName, extensions.GroupName}
+	expectGroupNames := sets.NewString(autoscaling.GroupName, batch.GroupName, extensions.GroupName)
-		expectVersions := [][]unversioned.GroupVersionForDiscovery{
+	expectVersions := map[string][]unversioned.GroupVersionForDiscovery{
+		autoscaling.GroupName: {
 			{
-				{
+				GroupVersion: testapi.Autoscaling.GroupVersion().String(),
-					GroupVersion: testapi.Autoscaling.GroupVersion().String(),
+				Version:      testapi.Autoscaling.GroupVersion().Version,
-					Version:      testapi.Autoscaling.GroupVersion().Version,
-				},
 			},
+		},
+		batch.GroupName: {
 			{
-				{
+				GroupVersion: testapi.Batch.GroupVersion().String(),
-					GroupVersion: testapi.Batch.GroupVersion().String(),
+				Version:      testapi.Batch.GroupVersion().Version,
-					Version:      testapi.Batch.GroupVersion().Version,
-				},
 			},
+		},
+		extensions.GroupName: {
 			{
-				{
+				GroupVersion: testapi.Extensions.GroupVersion().String(),
-					GroupVersion: testapi.Extensions.GroupVersion().String(),
+				Version:      testapi.Extensions.GroupVersion().Version,
-					Version:      testapi.Extensions.GroupVersion().Version,
-				},
 			},
+		},
+	}
+	expectPreferredVersion := map[string]unversioned.GroupVersionForDiscovery{
+		autoscaling.GroupName: {
+			GroupVersion: config.StorageVersions[autoscaling.GroupName],
+			Version:      apiutil.GetVersion(config.StorageVersions[autoscaling.GroupName]),
+		},
+		batch.GroupName: {
+			GroupVersion: config.StorageVersions[batch.GroupName],
+			Version:      apiutil.GetVersion(config.StorageVersions[batch.GroupName]),
+		},
+		extensions.GroupName: {
+			GroupVersion: config.StorageVersions[extensions.GroupName],
+			Version:      apiutil.GetVersion(config.StorageVersions[extensions.GroupName]),
+		},
+	}
+
+	assert.Equal(3, len(groupList.Groups))
+	for _, group := range groupList.Groups {
+		if !expectGroupNames.Has(group.Name) {
+			t.Errorf("got unexpected group %s", group.Name)
 		}
-		expectPreferredVersion := []unversioned.GroupVersionForDiscovery{
+		assert.Equal(expectVersions[group.Name], group.Versions)
-			{
+		assert.Equal(expectPreferredVersion[group.Name], group.PreferredVersion)
-				GroupVersion: config.StorageVersions[autoscaling.GroupName],
+	}
-				Version:      apiutil.GetVersion(config.StorageVersions[autoscaling.GroupName]),
+
-			},
+	thirdPartyGV := unversioned.GroupVersionForDiscovery{GroupVersion: "company.com/v1", Version: "v1"}
-			{
+	master.addThirdPartyResourceStorage("/apis/company.com/v1", nil,
-				GroupVersion: config.StorageVersions[batch.GroupName],
+		unversioned.APIGroup{
-				Version:      apiutil.GetVersion(config.StorageVersions[batch.GroupName]),
+			Name:             "company.com",
-			},
+			Versions:         []unversioned.GroupVersionForDiscovery{thirdPartyGV},
-			{
+			PreferredVersion: thirdPartyGV,
-				GroupVersion: config.StorageVersions[extensions.GroupName],
+		})
-				Version:      apiutil.GetVersion(config.StorageVersions[extensions.GroupName]),
+
-			},
+	resp, err = http.Get(server.URL + "/apis")
+	if !assert.NoError(err) {
+		t.Errorf("unexpected error: %v", err)
+	}
+	assert.Equal(http.StatusOK, resp.StatusCode)
+	assert.NoError(decodeResponse(resp, &groupList))
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	assert.Equal(4, len(groupList.Groups))
+
+	expectGroupNames.Insert("company.com")
+	expectVersions["company.com"] = []unversioned.GroupVersionForDiscovery{thirdPartyGV}
+	expectPreferredVersion["company.com"] = thirdPartyGV
+	for _, group := range groupList.Groups {
+		if !expectGroupNames.Has(group.Name) {
+			t.Errorf("got unexpected group %s", group.Name)
 		}
-
+		assert.Equal(expectVersions[group.Name], group.Versions)
-
+		assert.Equal(expectPreferredVersion[group.Name], group.PreferredVersion)
-		assert.Equal(2, len(groupList.Groups))
+	}
-		assert.Equal(expectGroupNames[0], groupList.Groups[0].Name)
-		assert.Equal(expectGroupNames[1], groupList.Groups[1].Name)
-
-		assert.Equal(expectVersions[0], groupList.Groups[0].Versions)
-		assert.Equal(expectVersions[1], groupList.Groups[1].Versions)
-
-		assert.Equal(expectPreferredVersion[0], groupList.Groups[0].PreferredVersion)
-		assert.Equal(expectPreferredVersion[1], groupList.Groups[1].PreferredVersion)
-
-		thirdPartyGV := unversioned.GroupVersionForDiscovery{GroupVersion: "company.com/v1", Version: "v1"}
-		master.addThirdPartyResourceStorage("/apis/company.com/v1", nil,
-			unversioned.APIGroup{
-				Name:             "company.com",
-				Versions:         []unversioned.GroupVersionForDiscovery{thirdPartyGV},
-				PreferredVersion: thirdPartyGV,
-			})
-
-		resp, err = http.Get(server.URL + "/apis")
-		if !assert.NoError(err) {
-			t.Errorf("unexpected error: %v", err)
-		}
-
-		assert.Equal(http.StatusOK, resp.StatusCode)
-
-		assert.NoError(decodeResponse(resp, &groupList))
-		if err != nil {
-			t.Fatalf("unexpected error: %v", err)
-		}
-
-		thirdPartyGroupName := "company.com"
-		thirdPartyExpectVersions := []unversioned.GroupVersionForDiscovery{thirdPartyGV}
-
-		assert.Equal(4, len(groupList.Groups))
-		// autoscaling group
-		assert.Equal(expectGroupNames[0], groupList.Groups[0].Name)
-		assert.Equal(expectVersions[0], groupList.Groups[0].Versions)
-		assert.Equal(expectPreferredVersion[0], groupList.Groups[0].PreferredVersion)
-		// batch group
-		assert.Equal(expectGroupNames[1], groupList.Groups[1].Name)
-		assert.Equal(expectVersions[1], groupList.Groups[1].Versions)
-		assert.Equal(expectPreferredVersion[1], groupList.Groups[1].PreferredVersion)
-		// third party
-		assert.Equal(thirdPartyGroupName, groupList.Groups[2].Name)
-		assert.Equal(thirdPartyExpectVersions, groupList.Groups[2].Versions)
-		assert.Equal(thirdPartyGV, groupList.Groups[2].PreferredVersion)
-		// extensions group
-		assert.Equal(expectGroupNames[2], groupList.Groups[3].Name)
-		assert.Equal(expectVersions[2], groupList.Groups[3].Versions)
-		assert.Equal(expectPreferredVersion[2], groupList.Groups[3].PreferredVersion)
-	*/
 }
 
 var versionsToTest = []string{"v1", "v3"}
@@ -1004,13 +1011,13 @@ func testThirdPartyDiscovery(t *testing.T, version string) {
 	assert.Equal(group.APIVersion, "v1")
 	assert.Equal(group.Kind, "APIGroup")
 	assert.Equal(group.Name, "company.com")
-	assert.Equal(group.Versions, []unversioned.GroupVersionForDiscovery{
+	expectedVersion := unversioned.GroupVersionForDiscovery{
-		{
+		GroupVersion: "company.com/" + version,
-			GroupVersion: "company.com/" + version,
+		Version:      version,
-			Version:      version,
+	}
-		},
+
-	})
+	assert.Equal(group.Versions, []unversioned.GroupVersionForDiscovery{expectedVersion})
-	assert.Equal(group.PreferredVersion, unversioned.GroupVersionForDiscovery{})
+	assert.Equal(group.PreferredVersion, expectedVersion)
 
 	resp, err = http.Get(server.URL + "/apis/company.com/" + version)
 	if !assert.NoError(err) {