diff --git a/pkg/helper/helpers.go b/pkg/helper/helpers.go index e48a5684c..cdaa9a8b3 100644 --- a/pkg/helper/helpers.go +++ b/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 = "" +) + +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) +} diff --git a/pkg/spoke/controllers/manifestcontroller/manifestwork_controller.go b/pkg/spoke/controllers/manifestcontroller/manifestwork_controller.go index 3a54de87e..884e6fb3d 100644 --- a/pkg/spoke/controllers/manifestcontroller/manifestwork_controller.go +++ b/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 } diff --git a/test/integration/suite_test.go b/test/integration/suite_test.go index cf3bcc079..eb79e6fc9 100644 --- a/test/integration/suite_test.go +++ b/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()) diff --git a/test/integration/work_test.go b/test/integration/work_test.go index ce485fe2b..a5ea2751c 100644 --- a/test/integration/work_test.go +++ b/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() {