Bug 1831711: Fix GVK diff to work for special kinds with multiple groups

pkg/controller/migplan/gvk.go

@@ -60,10 +60,11 @@ func (r ReconcileMigPlan) newGVKCompare(plan *migapi.MigPlan) (*gvk.Compare, err
 	}
 
 	return &gvk.Compare{
-		Plan:         plan,
-		SrcClient:    dynamicClient,
-		DstDiscovery: dstClient,
-		SrcDiscovery: srcClient,
+		Plan:                  plan,
+		SrcClient:             dynamicClient,
+		DstDiscovery:          dstClient,
+		SrcDiscovery:          srcClient,
+		CohabitatingResources: gvk.NewCohabitatingResources(),
 	}, nil
 }
 

pkg/gvk/gvk.go

@@ -1,6 +1,7 @@
 package gvk
 
 import (
+	"sort"
 	"strings"
 
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -19,12 +20,39 @@ var crdGVR = schema.GroupVersionResource{
 	Resource: "customresourcedefinitions",
 }
 
+type cohabitatingResource struct {
+	resource       string
+	groupResource1 schema.GroupResource
+	groupResource2 schema.GroupResource
+	seen           bool
+}
+
+func newCohabitatingResource(resource, group1, group2 string) *cohabitatingResource {
+	return &cohabitatingResource{
+		resource:       resource,
+		groupResource1: schema.GroupResource{Group: group1, Resource: resource},
+		groupResource2: schema.GroupResource{Group: group2, Resource: resource},
+		seen:           false,
+	}
+}
+
+func NewCohabitatingResources() map[string]*cohabitatingResource {
+	return map[string]*cohabitatingResource{
+		"deployments":     newCohabitatingResource("deployments", "extensions", "apps"),
+		"daemonsets":      newCohabitatingResource("daemonsets", "extensions", "apps"),
+		"replicasets":     newCohabitatingResource("replicasets", "extensions", "apps"),
+		"networkpolicies": newCohabitatingResource("networkpolicies", "extensions", "networking.k8s.io"),
+		"events":          newCohabitatingResource("events", "", "events.k8s.io"),
+	}
+}
+
 // Compare is a store for discovery and dynamic clients to do GVK compare
 type Compare struct {
-	Plan         *migapi.MigPlan
-	SrcDiscovery discovery.DiscoveryInterface
-	DstDiscovery discovery.DiscoveryInterface
-	SrcClient    dynamic.Interface
+	Plan                  *migapi.MigPlan
+	SrcDiscovery          discovery.DiscoveryInterface
+	DstDiscovery          discovery.DiscoveryInterface
+	SrcClient             dynamic.Interface
+	CohabitatingResources map[string]*cohabitatingResource
 }
 
 // Compare GVKs on both clusters, find incompatible GVKs
@@ -45,7 +73,7 @@ func (r *Compare) Compare() (map[string][]schema.GroupVersionResource, error) {
 		return nil, err
 	}
 
-	resourcesDiff := compareResources(preferredSrcResourceList, dstResourceList)
+	resourcesDiff := r.compareResources(preferredSrcResourceList, dstResourceList)
 	incompatibleGVKs, err := convertToGVRList(resourcesDiff)
 	if err != nil {
 		return nil, err
@@ -229,22 +257,19 @@ func (r *Compare) excludeCRDs(resources []*metav1.APIResourceList) ([]*metav1.AP
 	return updatedLists, nil
 }
 
-func resourceExist(resource metav1.APIResource, resources []metav1.APIResource) bool {
-	for _, resourceItem := range resources {
-		if resource.Name == resourceItem.Name {
-			return true
-		}
-	}
-
-	return false
-}
-
-func compareResources(src []*metav1.APIResourceList, dst []*metav1.APIResourceList) []*metav1.APIResourceList {
+func (r *Compare) compareResources(src []*metav1.APIResourceList, dst []*metav1.APIResourceList) []*metav1.APIResourceList {
 	missingResources := []*metav1.APIResourceList{}
+	sortResources(src)
 	for _, srcList := range src {
 		missing := []metav1.APIResource{}
 		for _, resource := range srcList.APIResources {
-			if !resourceExist(resource, fingResourceList(srcList.GroupVersion, dst)) {
+			if cohabitator, found := r.CohabitatingResources[resource.Name]; found {
+				if cohabitator.seen {
+					continue
+				}
+				cohabitator.seen = true
+			}
+			if !resourceExist(resource, findResourceList(srcList.GroupVersion, dst)) {
 				missing = append(missing, resource)
 			}
 		}
@@ -261,7 +286,17 @@ func compareResources(src []*metav1.APIResourceList, dst []*metav1.APIResourceLi
 	return missingResources
 }
 
-func fingResourceList(groupVersion string, list []*metav1.APIResourceList) []metav1.APIResource {
+func resourceExist(resource metav1.APIResource, resources []metav1.APIResource) bool {
+	for _, resourceItem := range resources {
+		if resource.Name == resourceItem.Name {
+			return true
+		}
+	}
+
+	return false
+}
+
+func findResourceList(groupVersion string, list []*metav1.APIResourceList) []metav1.APIResource {
 	for _, l := range list {
 		if l.GroupVersion == groupVersion {
 			return l.APIResources
@@ -290,3 +325,29 @@ func isCRDGroup(group string, crdGroups []string) bool {
 
 	return false
 }
+
+// sortResources sources resources by moving extensions to the end of the slice. The order of all
+// the other resources is preserved.
+func sortResources(resources []*metav1.APIResourceList) {
+	sort.SliceStable(resources, func(i, j int) bool {
+		left := resources[i]
+		leftGV, _ := schema.ParseGroupVersion(left.GroupVersion)
+		// not checking error because it should be impossible to fail to parse data coming from the
+		// apiserver
+		if leftGV.Group == "extensions" {
+			// always sort extensions at the bottom by saying left is "greater"
+			return false
+		}
+
+		right := resources[j]
+		rightGV, _ := schema.ParseGroupVersion(right.GroupVersion)
+		// not checking error because it should be impossible to fail to parse data coming from the
+		// apiserver
+		if rightGV.Group == "extensions" {
+			// always sort extensions at the bottom by saying left is "less"
+			return true
+		}
+
+		return i < j
+	})
+}

pkg/gvk/gvk_test.go

@@ -125,10 +125,93 @@ func Test_compareResources(t *testing.T) {
 			},
 			want: []*metav1.APIResourceList{},
 		},
+		{
+			name: "test deployment with both apps and extensions groups",
+			args: args{
+				src: []*metav1.APIResourceList{
+					{
+						GroupVersion: "apps/v1",
+						APIResources: []metav1.APIResource{
+							{
+								Name:       "deployments",
+								Namespaced: true,
+								Group:      "apps",
+								Version:    "v1",
+								Kind:       "Deployment",
+								ShortNames: []string{"deploy"},
+								Verbs:      []string{"create", "update", "list", "delete"},
+							},
+						},
+					},
+					{
+						GroupVersion: "extensions/v1beta1",
+						APIResources: []metav1.APIResource{
+							{
+								Name:       "deployments",
+								Namespaced: true,
+								Group:      "apps",
+								Version:    "v1beta1",
+								Kind:       "Deployment",
+								ShortNames: []string{"deploy"},
+								Verbs:      []string{"create", "update", "list", "delete"},
+							},
+						},
+					},
+				},
+				dst: []*metav1.APIResourceList{
+					{
+						GroupVersion: "apps/v1",
+						APIResources: []metav1.APIResource{
+							{
+								Name:       "deployments",
+								Namespaced: true,
+								Group:      "apps",
+								Version:    "v1",
+								Kind:       "Deployment",
+								ShortNames: []string{"deploy"},
+								Verbs:      []string{"create", "update", "list", "delete"},
+							},
+						},
+					},
+					{
+						GroupVersion: "apps/v1beta1",
+						APIResources: []metav1.APIResource{
+							{
+								Name:       "deployments",
+								Namespaced: true,
+								Group:      "apps",
+								Version:    "v1beta1",
+								Kind:       "Deployment",
+								ShortNames: []string{"deploy"},
+								Verbs:      []string{"create", "update", "list", "delete"},
+							},
+						},
+					},
+					{
+						GroupVersion: "apps/v1beta2",
+						APIResources: []metav1.APIResource{
+							{
+								Name:       "deployments",
+								Namespaced: true,
+								Group:      "apps",
+								Version:    "v1",
+								Kind:       "Deployment",
+								ShortNames: []string{"deploy"},
+								Verbs:      []string{"create", "update", "list", "delete"},
+							},
+						},
+					},
+				},
+			},
+			want: []*metav1.APIResourceList{},
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			if got := compareResources(tt.args.src, tt.args.dst); !reflect.DeepEqual(got, tt.want) {
+			c := Compare{
+				CohabitatingResources: NewCohabitatingResources(),
+			}
+			if got := c.compareResources(tt.args.src, tt.args.dst); !reflect.DeepEqual(got, tt.want) {
 				t.Errorf("compareResources() = %v, want %v", got, tt.want)
 			}
 		})