From 973f1d60771fc382caebdd8937533e0a1ec7b6ee Mon Sep 17 00:00:00 2001 From: Jan Safranek Date: Mon, 8 Jun 2020 14:45:00 +0200 Subject: [PATCH] Add extra check for v1alpha CRD The operator should print a human friendly message when v1alpha1 CRDs are detected. Such cluster can't support OCP v1beta1 version of snapshots and users should know why. --- pkg/operator/operator.go | 11 ++- pkg/operator/operator_test.go | 134 +++++++++++++++++++++++++++++++++- pkg/operator/sync.go | 37 ++++++++++ 3 files changed, 179 insertions(+), 3 deletions(-) diff --git a/pkg/operator/operator.go b/pkg/operator/operator.go index 778db2d73..7d7622d5d 100644 --- a/pkg/operator/operator.go +++ b/pkg/operator/operator.go @@ -1,6 +1,7 @@ package operator import ( + "errors" "fmt" "time" @@ -177,11 +178,16 @@ func (c *csiSnapshotOperator) sync() error { func (c *csiSnapshotOperator) updateSyncError(status *operatorv1.OperatorStatus, err error) { if err != nil { + degradedReason := "OperatorSync" + var errAlpha *AlphaCRDError + if errors.As(err, &errAlpha) { + degradedReason = "AlphaCRDsExist" + } v1helpers.SetOperatorCondition(&status.Conditions, operatorv1.OperatorCondition{ Type: operatorv1.OperatorStatusTypeDegraded, Status: operatorv1.ConditionTrue, - Reason: "OperatorSync", + Reason: degradedReason, Message: err.Error(), }) } else { @@ -195,7 +201,8 @@ func (c *csiSnapshotOperator) updateSyncError(status *operatorv1.OperatorStatus, func (c *csiSnapshotOperator) handleSync(instance *operatorv1.CSISnapshotController) error { if err := c.syncCustomResourceDefinitions(); err != nil { - return fmt.Errorf("failed to sync CRDs: %s", err) + // Pass through AlphaCRDError via %w + return fmt.Errorf("failed to sync CRDs: %w", err) } deployment, err := c.syncDeployment(instance) diff --git a/pkg/operator/operator_test.go b/pkg/operator/operator_test.go index 50058dfb1..d344de165 100644 --- a/pkg/operator/operator_test.go +++ b/pkg/operator/operator_test.go @@ -87,7 +87,7 @@ func newOperator(test operatorTest) *testContext { extAPIInformerFactory := apiextinformers.NewSharedInformerFactory(extAPIClient, 0) // Fill the informer for i := range test.initialObjects.crds { - extAPIInformerFactory.Apiextensions().V1().CustomResourceDefinitions().Informer().GetIndexer().Add(test.initialObjects.crds[i]) + extAPIInformerFactory.Apiextensions().V1beta1().CustomResourceDefinitions().Informer().GetIndexer().Add(test.initialObjects.crds[i]) } if test.reactors.crds != nil { test.reactors.crds(extAPIClient, extAPIInformerFactory) @@ -317,6 +317,86 @@ func withEstablishedConditions(instance *apiextv1beta1.CustomResourceDefinition) return instance } +func getAlphaCRD(crdName string) *apiextv1beta1.CustomResourceDefinition { + var crdFile string + switch crdName { + case "VolumeSnapshot": + crdFile = ` +apiVersion: apiextensions.k8s.io/v1beta1 +kind: CustomResourceDefinition +metadata: + name: volumesnapshots.snapshot.storage.k8s.io +spec: + conversion: + strategy: None + group: snapshot.storage.k8s.io + names: + kind: VolumeSnapshot + listKind: VolumeSnapshotList + plural: volumesnapshots + singular: volumesnapshot + preserveUnknownFields: true + scope: Namespaced + versions: + - name: v1alpha1 + served: true + storage: true + subresources: + status: {} +` + + case "VolumeSnapshotContent": + crdFile = ` +apiVersion: apiextensions.k8s.io/v1beta1 +kind: CustomResourceDefinition +metadata: + name: volumesnapshotcontents.snapshot.storage.k8s.io +spec: + conversion: + strategy: None + group: snapshot.storage.k8s.io + names: + kind: VolumeSnapshotContent + listKind: VolumeSnapshotContentList + plural: volumesnapshotcontents + singular: volumesnapshotcontent + preserveUnknownFields: true + scope: Cluster + versions: + - name: v1alpha1 + served: true + storage: true +` + case "VolumeSnapshotClass": + crdFile = ` +apiVersion: apiextensions.k8s.io/v1beta1 +kind: CustomResourceDefinition +metadata: + name: volumesnapshotclasses.snapshot.storage.k8s.io +spec: + conversion: + strategy: None + group: snapshot.storage.k8s.io + names: + kind: VolumeSnapshotClass + listKind: VolumeSnapshotClassList + plural: volumesnapshotclasses + singular: volumesnapshotclass + preserveUnknownFields: true + scope: Cluster + versions: + - name: v1alpha1 + served: true + storage: true + subresources: + status: +` + default: + panic(crdName) + } + return resourceread.ReadCustomResourceDefinitionV1Beta1OrDie([]byte(crdFile)) +} + // Optional reactor that sets Established condition to True. It's needed by the operator that polls for CRDs until they get the condition func addCRDEstablishedRector(client *fakeextapi.Clientset, informer apiextinformers.SharedInformerFactory) { client.PrependReactor("*", "customresourcedefinitions", func(action core.Action) (handled bool, ret runtime.Object, err error) { @@ -536,6 +616,58 @@ func TestSync(t *testing.T) { }, expectErr: true, }, + { + // error: v1alpha1 VolumeSnapshot already exists + name: "v1alpha1 VolumeSnapshot", + image: defaultImage, + initialObjects: testObjects{ + csiSnapshotController: csiSnapshotController(), + crds: []*apiextv1beta1.CustomResourceDefinition{getAlphaCRD("VolumeSnapshot")}, + }, + expectedObjects: testObjects{ + crds: []*apiextv1beta1.CustomResourceDefinition{getAlphaCRD("VolumeSnapshot")}, + csiSnapshotController: csiSnapshotController(withTrueConditions(opv1.OperatorStatusTypeDegraded)), + }, + expectErr: true, + reactors: testReactors{ + crds: addCRDEstablishedRector, + }, + }, + { + // error: v1alpha1 VolumeSnapshotContent already exists + name: "v1alpha1 VolumeSnapshotContent", + image: defaultImage, + initialObjects: testObjects{ + csiSnapshotController: csiSnapshotController(), + crds: []*apiextv1beta1.CustomResourceDefinition{getAlphaCRD("VolumeSnapshotContent")}, + }, + expectedObjects: testObjects{ + crds: []*apiextv1beta1.CustomResourceDefinition{getAlphaCRD("VolumeSnapshotContent")}, + csiSnapshotController: csiSnapshotController(withTrueConditions(opv1.OperatorStatusTypeDegraded)), + }, + expectErr: true, + reactors: testReactors{ + crds: addCRDEstablishedRector, + }, + }, + { + // error: v1alpha1 VolumeSnapshotClass already exists + name: "v1alpha1 VolumeSnapshotClass", + image: defaultImage, + initialObjects: testObjects{ + csiSnapshotController: csiSnapshotController(), + crds: []*apiextv1beta1.CustomResourceDefinition{getAlphaCRD("VolumeSnapshotClass")}, + }, + expectedObjects: testObjects{ + crds: []*apiextv1beta1.CustomResourceDefinition{getAlphaCRD("VolumeSnapshotClass")}, + csiSnapshotController: csiSnapshotController(withTrueConditions(opv1.OperatorStatusTypeDegraded)), + }, + expectErr: true, + reactors: testReactors{ + crds: addCRDEstablishedRector, + }, + }, + // TODO: more error cases? Deployment creation fails and things like that? } diff --git a/pkg/operator/sync.go b/pkg/operator/sync.go index 8d6745a11..76e6ff7ad 100644 --- a/pkg/operator/sync.go +++ b/pkg/operator/sync.go @@ -7,6 +7,7 @@ import ( appsv1 "k8s.io/api/apps/v1" apiextv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" + "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/util/wait" operatorv1 "github.com/openshift/api/operator/v1" @@ -29,7 +30,18 @@ var ( customResourceReadyTimeout = 10 * time.Minute ) +type AlphaCRDError struct { + alphaCRDs []string +} + +func (a *AlphaCRDError) Error() string { + return fmt.Sprintf("cluster-csi-snapshot-controller-operator does not support v1alpha1 version of snapshot CRDs %s installed by user or 3rd party controller", strings.Join(a.alphaCRDs, ", ")) +} + func (c *csiSnapshotOperator) syncCustomResourceDefinitions() error { + if err := c.checkAlphaCRDs(); err != nil { + return err + } for _, file := range crds { crd := resourceread.ReadCustomResourceDefinitionV1Beta1OrDie(generated.MustAsset(file)) _, updated, err := resourceapply.ApplyCustomResourceDefinition(c.crdClient.ApiextensionsV1beta1(), c.eventRecorder, crd) @@ -70,6 +82,31 @@ func (c *csiSnapshotOperator) waitForCustomResourceDefinition(resource *apiextv1 return nil } +// checkCRDAlpha checks if v1alpha1 version of the CRD exists and returns human-friendly error if so. +// This happens during update from 4.3 cluster that may use v1alpha1 CRD version installed by cluster admin. +func (c *csiSnapshotOperator) checkAlphaCRDs() error { + var alphas []string + for _, file := range crds { + crd := resourceread.ReadCustomResourceDefinitionV1Beta1OrDie(generated.MustAsset(file)) + oldCRD, err := c.crdLister.Get(crd.Name) + if err != nil { + if errors.IsNotFound(err) { + continue + } + return fmt.Errorf("error getting CustomResourceDefinition %s: %v", crd.Name, err) + } + for _, version := range oldCRD.Spec.Versions { + if version.Name == "v1alpha1" { + alphas = append(alphas, oldCRD.Name) + } + } + } + if len(alphas) != 0 { + return &AlphaCRDError{alphas} + } + return nil +} + func (c *csiSnapshotOperator) syncDeployment(instance *operatorv1.CSISnapshotController) (*appsv1.Deployment, error) { deploy := c.getExpectedDeployment(instance)