Skip to content

Commit

Permalink
fix GVK diff to work for special kinds with multiple groups
Browse files Browse the repository at this point in the history
  • Loading branch information
alaypatel07 committed May 11, 2020
1 parent 4a92b21 commit be4ae12
Show file tree
Hide file tree
Showing 3 changed files with 159 additions and 13 deletions.
9 changes: 5 additions & 4 deletions pkg/controller/migplan/gvk.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
78 changes: 70 additions & 8 deletions pkg/gvk/gvk.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package gvk

import (
"sort"
"strings"

"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -239,12 +267,20 @@ func resourceExist(resource metav1.APIResource, resources []metav1.APIResource)
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 {
// TODO: need to add log entry
continue
}
cohabitator.seen = true
}
if !resourceExist(resource, findResourceList(srcList.GroupVersion, dst)) {
missing = append(missing, resource)
}
}
Expand All @@ -261,7 +297,7 @@ func compareResources(src []*metav1.APIResourceList, dst []*metav1.APIResourceLi
return missingResources
}

func fingResourceList(groupVersion string, list []*metav1.APIResourceList) []metav1.APIResource {
func findResourceList(groupVersion string, list []*metav1.APIResourceList) []metav1.APIResource {
for _, l := range list {
if l.GroupVersion == groupVersion {
return l.APIResources
Expand Down Expand Up @@ -290,3 +326,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
})
}
85 changes: 84 additions & 1 deletion pkg/gvk/gvk_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
})
Expand Down

0 comments on commit be4ae12

Please sign in to comment.