Repository: clusternet/clusternet
Pull request #173 (2021-11-02 08:17:07)

supports resources with skew versions
<!--  Thanks for sending a pull request!

-->

#### What type of PR is this?
kind/feature

#### What this PR does / why we need it:

#### Which issue(s) this PR fixes:
<!--
*Automatically closes linked issue when PR is merged.
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
-->
xref #137 

#### Special notes for your reviewer:


codecov-commenter: # [Codecov](https://codecov.io/gh/clusternet/clusternet/pull/173?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clusternet) Report
> Merging [#173](https://codecov.io/gh/clusternet/clusternet/pull/173?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clusternet) (442c949) into [main](https://codecov.io/gh/clusternet/clusternet/commit/62c70e5c55942a764b5bd47636fb3a17a2100a26?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clusternet) (62c70e5) will **decrease** coverage by `0.33%`.
> The diff coverage is `n/a`.

[![Impacted file tree graph](https://codecov.io/gh/clusternet/clusternet/pull/173/graphs/tree.svg?width=650&height=150&src=pr&token=4O6PTR0RJN&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clusternet)](https://codecov.io/gh/clusternet/clusternet/pull/173?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clusternet)

```diff
@@           Coverage Diff            @@
##            main    #173      +/-   ##
========================================
- Coverage   7.84%   7.51%   -0.34%     
========================================
  Files         26      29       +3     
  Lines       2970    3728     +758     
========================================
+ Hits         233     280      +47     
- Misses      2715    3425     +710     
- Partials      22      23       +1     
```


| [Impacted Files](https://codecov.io/gh/clusternet/clusternet/pull/173?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clusternet) | Coverage Δ | |
|---|---|---|
| [...ernet/clusternet/pkg/hub/apiserver/shadow/utils.go](https://codecov.io/gh/clusternet/clusternet/pull/173/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clusternet#diff-c3JjL2dpdGh1Yi5jb20vY2x1c3Rlcm5ldC9jbHVzdGVybmV0L3BrZy9odWIvYXBpc2VydmVyL3NoYWRvdy91dGlscy5nbw==) | `89.28% <0.00%> (ø)` | |
| [...pkg/hub/apiserver/shadow/customresource\_handler.go](https://codecov.io/gh/clusternet/clusternet/pull/173/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clusternet#diff-c3JjL2dpdGh1Yi5jb20vY2x1c3Rlcm5ldC9jbHVzdGVybmV0L3BrZy9odWIvYXBpc2VydmVyL3NoYWRvdy9jdXN0b21yZXNvdXJjZV9oYW5kbGVyLmdv) | `0.00% <0.00%> (ø)` | |
| [...t/clusternet/pkg/hub/apiserver/shadow/apiserver.go](https://codecov.io/gh/clusternet/clusternet/pull/173/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clusternet#diff-c3JjL2dpdGh1Yi5jb20vY2x1c3Rlcm5ldC9jbHVzdGVybmV0L3BrZy9odWIvYXBpc2VydmVyL3NoYWRvdy9hcGlzZXJ2ZXIuZ28=) | `11.16% <0.00%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/clusternet/clusternet/pull/173?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clusternet).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clusternet)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/clusternet/clusternet/pull/173?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clusternet). Last update [62c70e5...442c949](https://codecov.io/gh/clusternet/clusternet/pull/173?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clusternet). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clusternet).


Patch:
diff --git a/pkg/hub/apiserver/shadow/apiserver.go b/pkg/hub/apiserver/shadow/apiserver.go
index 158b2df..e8833d3 100644
--- a/pkg/hub/apiserver/shadow/apiserver.go
+++ b/pkg/hub/apiserver/shadow/apiserver.go
@@ -182,11 +182,10 @@ func (ss *ShadowAPIServer) InstallShadowAPIGroups(stopCh <-chan struct{}, cl dis
 			continue
 		}
 
-		preferredVersion := apiGroupResource.Group.PreferredVersion.Version
-		for _, apiresource := range apiGroupResource.VersionedResources[preferredVersion] {
+		for _, apiresource := range normalizeAPIGroupResources(apiGroupResource) {
 			ss.crdHandler.AddNonCRDAPIResource(apiresource)
 			// register scheme for original GVK
-			Scheme.AddKnownTypeWithName(schema.GroupVersion{Group: apiGroupResource.Group.Name, Version: preferredVersion}.WithKind(apiresource.Kind),
+			Scheme.AddKnownTypeWithName(schema.GroupVersion{Group: apiGroupResource.Group.Name, Version: apiresource.Version}.WithKind(apiresource.Kind),
 				&unstructured.Unstructured{},
 			)
 			resourceRest := template.NewREST(ss.kubeRESTClient, ss.clusternetclient, ParameterCodec, ss.manifestLister)
@@ -194,8 +193,8 @@ func (ss *ShadowAPIServer) InstallShadowAPIGroups(stopCh <-chan struct{}, cl dis
 			resourceRest.SetName(apiresource.Name)
 			resourceRest.SetShortNames(apiresource.ShortNames)
 			resourceRest.SetKind(apiresource.Kind)
-			resourceRest.SetGroup(apiGroupResource.Group.Name)
-			resourceRest.SetVersion(preferredVersion)
+			resourceRest.SetGroup(apiresource.Group)
+			resourceRest.SetVersion(apiresource.Version)
 			switch {
 			case strings.HasSuffix(apiresource.Name, "/scale"):
 				shadowv1alpha1storage[apiresource.Name] = template.NewScaleREST(resourceRest)
diff --git a/pkg/hub/apiserver/shadow/utils.go b/pkg/hub/apiserver/shadow/utils.go
new file mode 100644
index 0000000..6aaca6f
--- /dev/null
+++ b/pkg/hub/apiserver/shadow/utils.go
@@ -0,0 +1,70 @@
+/*
+Copyright 2021 The Clusternet Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package apiserver
+
+import (
+	"sort"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/version"
+	"k8s.io/client-go/restmapper"
+)
+
+// normalizeAPIGroupResources returns resources with preferred version or highest semantic version
+func normalizeAPIGroupResources(apiGroupResource *restmapper.APIGroupResources) []metav1.APIResource {
+	var versionedResources []metav1.APIResource
+	for version, vr := range apiGroupResource.VersionedResources {
+		for _, resource := range vr {
+			resource.Group = apiGroupResource.Group.Name
+			resource.Version = version
+			versionedResources = append(versionedResources, resource)
+		}
+	}
+
+	// Ensure deterministic output.
+	preferredVersion := apiGroupResource.Group.PreferredVersion.Version
+	sort.SliceStable(versionedResources, func(i, j int) bool {
+		if versionedResources[i].Version == versionedResources[j].Version {
+			return versionedResources[i].Name < versionedResources[j].Name
+		}
+
+		// preferred version
+		if versionedResources[i].Version == preferredVersion {
+			return true
+		}
+		if versionedResources[j].Version == preferredVersion {
+			return false
+		}
+
+		// compare kube-like version
+		// Versions will be sorted based on GA/alpha/beta first and then major and minor versions.
+		// e.g. v2, v1, v1beta2, v1beta1, v1alpha1.
+		return version.CompareKubeAwareVersionStrings(versionedResources[i].Version, versionedResources[j].Version) > 0
+	})
+
+	// pick out preferred version or highest semantic version
+	registered := make(map[string]bool)
+	var normalizedVersionResources []metav1.APIResource
+	for _, vr := range versionedResources {
+		if registered[vr.Name] {
+			continue
+		}
+		normalizedVersionResources = append(normalizedVersionResources, vr)
+		registered[vr.Name] = true
+	}
+	return normalizedVersionResources
+}


Test patch:
diff --git a/pkg/hub/apiserver/shadow/utils_test.go b/pkg/hub/apiserver/shadow/utils_test.go
new file mode 100644
index 0000000..dcecf1f
--- /dev/null
+++ b/pkg/hub/apiserver/shadow/utils_test.go
@@ -0,0 +1,252 @@
+/*
+Copyright 2021 The Clusternet Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package apiserver
+
+import (
+	"reflect"
+	"testing"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/client-go/restmapper"
+)
+
+func TestNormalizeAPIGroupResources(t *testing.T) {
+	tests := []struct {
+		name             string
+		apiGroupResource *restmapper.APIGroupResources
+		want             []metav1.APIResource
+	}{
+		{
+			name: "batch",
+			apiGroupResource: &restmapper.APIGroupResources{
+				Group: metav1.APIGroup{
+					Name: "batch",
+					Versions: []metav1.GroupVersionForDiscovery{
+						{
+							GroupVersion: "batch/v2alpha1",
+							Version:      "v2alpha1",
+						},
+						{
+							GroupVersion: "batch/v1",
+							Version:      "v1",
+						},
+						{
+							GroupVersion: "batch/v1beta1",
+							Version:      "v1beta1",
+						},
+						{
+							GroupVersion: "batch/v1alpha1",
+							Version:      "v1alpha1",
+						},
+					},
+					PreferredVersion: metav1.GroupVersionForDiscovery{
+						GroupVersion: "batch/v1",
+						Version:      "v1",
+					},
+				},
+				VersionedResources: map[string][]metav1.APIResource{
+					"v2alpha1": {
+						metav1.APIResource{
+							Name:               "xyz",
+							SingularName:       "",
+							Namespaced:         true,
+							Group:              "",
+							Version:            "",
+							Kind:               "Xyz",
+							Verbs:              metav1.Verbs{"foo", "bar", "far"},
+							ShortNames:         nil,
+							Categories:         []string{"all"},
+							StorageVersionHash: "",
+						},
+					},
+					"v1": {
+						metav1.APIResource{
+							Name:               "xyz",
+							SingularName:       "",
+							Namespaced:         true,
+							Group:              "",
+							Version:            "",
+							Kind:               "Xyz",
+							Verbs:              metav1.Verbs{"foo", "bar"},
+							ShortNames:         nil,
+							Categories:         []string{"all"},
+							StorageVersionHash: "",
+						},
+						metav1.APIResource{
+							Name:               "jobs",
+							SingularName:       "",
+							Namespaced:         true,
+							Group:              "",
+							Version:            "",
+							Kind:               "Job",
+							Verbs:              metav1.Verbs{"foo", "bar"},
+							ShortNames:         nil,
+							Categories:         []string{"all"},
+							StorageVersionHash: "mudhfqk/qZY=",
+						},
+						metav1.APIResource{
+							Name:               "jobs/status",
+							SingularName:       "",
+							Namespaced:         true,
+							Group:              "",
+							Version:            "",
+							Kind:               "Job",
+							Verbs:              metav1.Verbs{"foo", "bar"},
+							ShortNames:         nil,
+							Categories:         nil,
+							StorageVersionHash: "",
+						},
+					},
+					"v1beta1": {
+						metav1.APIResource{
+							Name:               "cronjobs",
+							SingularName:       "",
+							Namespaced:         true,
+							Group:              "",
+							Version:            "",
+							Kind:               "CronJob",
+							Verbs:              metav1.Verbs{"foo", "bar"},
+							ShortNames:         []string{"cj"},
+							Categories:         []string{"all"},
+							StorageVersionHash: "h/JlFAZkyyY=",
+						},
+						metav1.APIResource{
+							Name:               "cronjobs/status",
+							SingularName:       "",
+							Namespaced:         true,
+							Group:              "",
+							Version:            "",
+							Kind:               "CronJob",
+							Verbs:              metav1.Verbs{"foo", "bar"},
+							ShortNames:         nil,
+							Categories:         nil,
+							StorageVersionHash: "",
+						},
+						metav1.APIResource{
+							Name:               "scenarios",
+							SingularName:       "",
+							Namespaced:         true,
+							Group:              "",
+							Version:            "",
+							Kind:               "Scenario",
+							Verbs:              metav1.Verbs{"foo", "bar", "far"},
+							ShortNames:         []string{"sc"},
+							Categories:         []string{"all"},
+							StorageVersionHash: "",
+						},
+					},
+					"v1alpha1": {
+						metav1.APIResource{
+							Name:               "scenarios",
+							SingularName:       "",
+							Namespaced:         true,
+							Group:              "",
+							Version:            "",
+							Kind:               "Scenario",
+							Verbs:              metav1.Verbs{"foo", "bar"},
+							ShortNames:         []string{"sc"},
+							Categories:         []string{"all"},
+							StorageVersionHash: "",
+						},
+					},
+				},
+			},
+			want: []metav1.APIResource{
+				{
+					Name:               "jobs",
+					SingularName:       "",
+					Namespaced:         true,
+					Group:              "batch",
+					Version:            "v1",
+					Kind:               "Job",
+					Verbs:              metav1.Verbs{"foo", "bar"},
+					ShortNames:         nil,
+					Categories:         []string{"all"},
+					StorageVersionHash: "mudhfqk/qZY=",
+				},
+				{
+					Name:               "jobs/status",
+					SingularName:       "",
+					Namespaced:         true,
+					Group:              "batch",
+					Version:            "v1",
+					Kind:               "Job",
+					Verbs:              metav1.Verbs{"foo", "bar"},
+					ShortNames:         nil,
+					Categories:         nil,
+					StorageVersionHash: "",
+				},
+				{
+					Name:               "xyz",
+					SingularName:       "",
+					Namespaced:         true,
+					Group:              "batch",
+					Version:            "v1",
+					Kind:               "Xyz",
+					Verbs:              metav1.Verbs{"foo", "bar"},
+					ShortNames:         nil,
+					Categories:         []string{"all"},
+					StorageVersionHash: "",
+				},
+				{
+					Name:               "cronjobs",
+					SingularName:       "",
+					Namespaced:         true,
+					Group:              "batch",
+					Version:            "v1beta1",
+					Kind:               "CronJob",
+					Verbs:              metav1.Verbs{"foo", "bar"},
+					ShortNames:         []string{"cj"},
+					Categories:         []string{"all"},
+					StorageVersionHash: "h/JlFAZkyyY=",
+				},
+				{
+					Name:               "cronjobs/status",
+					SingularName:       "",
+					Namespaced:         true,
+					Group:              "batch",
+					Version:            "v1beta1",
+					Kind:               "CronJob",
+					Verbs:              metav1.Verbs{"foo", "bar"},
+					ShortNames:         nil,
+					Categories:         nil,
+					StorageVersionHash: "",
+				},
+				{
+					Name:               "scenarios",
+					SingularName:       "",
+					Namespaced:         true,
+					Group:              "batch",
+					Version:            "v1beta1",
+					Kind:               "Scenario",
+					Verbs:              metav1.Verbs{"foo", "bar", "far"},
+					ShortNames:         []string{"sc"},
+					Categories:         []string{"all"},
+					StorageVersionHash: "",
+				},
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := normalizeAPIGroupResources(tt.apiGroupResource); !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("got: %v\n, want %v\n", got, tt.want)
+			}
+		})
+	}
+}