Handle unknown kind returned by GuessObjectGroupVersionKind()

pkg/helper/helpers.go

@@ -7,15 +7,33 @@ import (
 
 	workv1client "github.com/open-cluster-management/api/client/work/clientset/versioned/typed/work/v1"
 	workapiv1 "github.com/open-cluster-management/api/work/v1"
+	"github.com/openshift/library-go/pkg/operator/resource/resourcehelper"
+	apiextensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
 	"k8s.io/apimachinery/pkg/api/equality"
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/dynamic"
 	"k8s.io/client-go/util/retry"
 	"k8s.io/klog"
 )
 
+const (
+	// unknownKind is returned by resourcehelper.GuessObjectGroupVersionKind() when it
+	// cannot tell the kind of the given object
+	unknownKind = "<unknown>"
+)
+
+var (
+	genericScheme = runtime.NewScheme()
+)
+
+func init() {
+	// add apiextensions v1beta1 to scheme to support CustomResourceDefinition v1beta1
+	_ = apiextensionsv1beta1.AddToScheme(genericScheme)
+}
+
 // MergeManifestConditions return a new ManifestCondition array which merges the existing manifest
 // conditions and the new manifest conditions. Rules to match ManifestCondition between two arrays:
 // 1. match the manifest condition with the whole ManifestResourceMeta;
@@ -328,3 +346,19 @@ func copyAppliedResource(resource workapiv1.AppliedManifestResourceMeta) workapi
 		UID:       resource.UID,
 	}
 }
+
+// GuessObjectGroupVersionKind returns GVK for the passed runtime object.
+func GuessObjectGroupVersionKind(object runtime.Object) (*schema.GroupVersionKind, error) {
+	gvk := resourcehelper.GuessObjectGroupVersionKind(object)
+	// return gvk if found
+	if gvk.Kind != unknownKind {
+		return &gvk, nil
+	}
+
+	// otherwise fall back to genericScheme
+	if kinds, _, _ := genericScheme.ObjectKinds(object); len(kinds) > 0 {
+		return &kinds[0], nil
+	}
+
+	return nil, fmt.Errorf("cannot get gvk of %v", object)
+}

pkg/spoke/controllers/manifestcontroller/manifestwork_controller.go

@@ -16,7 +16,6 @@ import (
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
-	"k8s.io/apimachinery/pkg/runtime/serializer"
 	utilerrors "k8s.io/apimachinery/pkg/util/errors"
 	"k8s.io/client-go/dynamic"
 	"k8s.io/client-go/kubernetes"
@@ -29,7 +28,6 @@ import (
 	"github.com/openshift/library-go/pkg/controller/factory"
 	"github.com/openshift/library-go/pkg/operator/events"
 	"github.com/openshift/library-go/pkg/operator/resource/resourceapply"
-	"github.com/openshift/library-go/pkg/operator/resource/resourcehelper"
 
 	"github.com/open-cluster-management/work/pkg/helper"
 	"github.com/open-cluster-management/work/pkg/spoke/resource"
@@ -47,12 +45,6 @@ type ManifestWorkController struct {
 	restMapper *resource.Mapper
 }
 
-var (
-	genericScheme = runtime.NewScheme()
-	genericCodecs = serializer.NewCodecFactory(genericScheme)
-	genericCodec  = genericCodecs.UniversalDeserializer()
-)
-
 const (
 	manifestWorkFinalizer = "cluster.open-cluster-management.io/manifest-work-cleanup"
 )
@@ -135,8 +127,6 @@ func (m *ManifestWorkController) sync(ctx context.Context, controllerContext fac
 	// merge the new manifest conditions with the existing manifest conditions
 	manifestWork.Status.ResourceStatus.Manifests = helper.MergeManifestConditions(manifestWork.Status.ResourceStatus.Manifests, newManifestConditions)
 
-	// TODO find resources to be deleted if it is not owned by manifestwork any longer
-
 	// Update work status
 	_, _, err = helper.UpdateManifestWorkStatus(
 		ctx, m.manifestWorkClient, manifestWork.Name, m.generateUpdateStatusFunc(manifestWork.Status.ResourceStatus))
@@ -184,7 +174,7 @@ func (m *ManifestWorkController) decodeUnstructured(data []byte) (schema.GroupVe
 	}
 	mapping, err := m.restMapper.MappingForGVK(unstructuredObj.GroupVersionKind())
 	if err != nil {
-		return schema.GroupVersionResource{}, nil, fmt.Errorf("Failed to find grv from restmapping: %w", err)
+		return schema.GroupVersionResource{}, nil, fmt.Errorf("Failed to find gvr from restmapping: %w", err)
 	}
 
 	return mapping.Resource, unstructuredObj, nil
@@ -348,7 +338,10 @@ func buildManifestResourceMeta(index int, object runtime.Object, restMapper *res
 	}
 
 	// set gvk
-	gvk := resourcehelper.GuessObjectGroupVersionKind(object)
+	gvk, err := helper.GuessObjectGroupVersionKind(object)
+	if err != nil {
+		return resourceMeta, err
+	}
 	resourceMeta.Group = gvk.Group
 	resourceMeta.Version = gvk.Version
 	resourceMeta.Kind = gvk.Kind
@@ -365,7 +358,7 @@ func buildManifestResourceMeta(index int, object runtime.Object, restMapper *res
 	if restMapper == nil {
 		return resourceMeta, err
 	}
-	if mapping, e := restMapper.MappingForGVK(gvk); e == nil {
+	if mapping, e := restMapper.MappingForGVK(*gvk); e == nil {
 		resourceMeta.Resource = mapping.Resource.Resource
 	}
 

test/integration/suite_test.go

@@ -10,7 +10,6 @@ import (
 	"github.com/onsi/ginkgo"
 	"github.com/onsi/gomega"
 
-	apiextensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/kubernetes/scheme"
 	"k8s.io/client-go/rest"
@@ -64,9 +63,6 @@ var _ = ginkgo.BeforeSuite(func(done ginkgo.Done) {
 	err = util.CreateKubeconfigFile(cfg, hubKubeconfigFileName)
 	gomega.Expect(err).ToNot(gomega.HaveOccurred())
 
-	err = apiextensionsv1beta1.AddToScheme(scheme.Scheme)
-	gomega.Expect(err).NotTo(gomega.HaveOccurred())
-
 	err = workapiv1.AddToScheme(scheme.Scheme)
 	gomega.Expect(err).NotTo(gomega.HaveOccurred())
 

test/integration/work_test.go

@@ -237,6 +237,40 @@ var _ = ginkgo.Describe("ManifestWork", func() {
 			util.AssertExistenceOfResources(gvrs, namespaces, names, spokeDynamicClient, eventuallyTimeout, eventuallyInterval)
 			util.AssertAppliedResources(work.Namespace, work.Name, gvrs, namespaces, names, hubWorkClient, eventuallyTimeout, eventuallyInterval)
 		})
+
+		ginkgo.It("should delete CRD and CR successfully", func() {
+			util.AssertWorkCondition(work.Namespace, work.Name, hubWorkClient, string(workapiv1.WorkApplied), metav1.ConditionTrue,
+				[]metav1.ConditionStatus{metav1.ConditionTrue, metav1.ConditionTrue}, eventuallyTimeout, eventuallyInterval)
+
+			var namespaces, names []string
+			for _, obj := range objects {
+				namespaces = append(namespaces, obj.GetNamespace())
+				names = append(names, obj.GetName())
+			}
+
+			util.AssertExistenceOfResources(gvrs, namespaces, names, spokeDynamicClient, eventuallyTimeout, eventuallyInterval)
+			util.AssertAppliedResources(work.Namespace, work.Name, gvrs, namespaces, names, hubWorkClient, eventuallyTimeout, eventuallyInterval)
+
+			// delete manifest work
+			err = hubWorkClient.WorkV1().ManifestWorks(o.SpokeClusterName).Delete(context.Background(), work.Name, metav1.DeleteOptions{})
+			gomega.Expect(err).ToNot(gomega.HaveOccurred())
+
+			// wait for deletion of manifest work
+			gomega.Eventually(func() bool {
+				_, err := hubWorkClient.WorkV1().ManifestWorks(o.SpokeClusterName).Get(context.Background(), work.Name, metav1.GetOptions{})
+				if !errors.IsNotFound(err) {
+					return false
+				}
+
+				return true
+			}, eventuallyTimeout, eventuallyInterval).Should(gomega.BeTrue())
+
+			// Once manifest work is deleted, all CRs/CRD should have already been deleted too
+			for i := range gvrs {
+				_, err := util.GetResource(namespaces[i], names[i], gvrs[i], spokeDynamicClient)
+				gomega.Expect(errors.IsNotFound(err)).To(gomega.BeTrue())
+			}
+		})
 	})
 
 	ginkgo.Context("With Service Account, Role, RoleBinding and Deployment in manifests", func() {