From 8b6343374bfd0d398c4ea9a1cfbd3f99a23e23ad Mon Sep 17 00:00:00 2001 From: Harish Date: Fri, 13 Mar 2020 19:06:48 -0400 Subject: [PATCH] feat: support v1 CRDS in OLM. Simply InstallPlan Execution via new Stepper interface. --- pkg/controller/operators/catalog/operator.go | 233 +++++++-------- .../operators/catalog/operator_test.go | 265 +++++++++++++++++- pkg/controller/operators/catalog/step.go | 264 +++++++++++++++++ pkg/controller/operators/olm/operator.go | 11 +- pkg/lib/crd/version.go | 68 +++++ pkg/lib/index/api.go | 33 ++- pkg/lib/operatorclient/client.go | 6 +- .../operatorclientmocks/mock_client.go | 10 +- .../customresourcedefinition.go | 59 +++- pkg/lib/operatorlister/lister.go | 38 ++- test/e2e/crd_upgrade_e2e_test.go | 132 +++++++++ test/e2e/csv_e2e_test.go | 6 +- test/e2e/gc_e2e_test.go | 6 +- test/e2e/installplan_e2e_test.go | 8 +- vendor/github.com/onsi/gomega/go.mod | 2 - 15 files changed, 981 insertions(+), 160 deletions(-) create mode 100644 pkg/controller/operators/catalog/step.go create mode 100644 pkg/lib/crd/version.go create mode 100644 test/e2e/crd_upgrade_e2e_test.go diff --git a/pkg/controller/operators/catalog/operator.go b/pkg/controller/operators/catalog/operator.go index 33097785089..7ceb7168268 100644 --- a/pkg/controller/operators/catalog/operator.go +++ b/pkg/controller/operators/catalog/operator.go @@ -16,10 +16,10 @@ import ( corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" + v1ext "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" v1beta1ext "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" "k8s.io/apiextensions-apiserver/pkg/apiserver/validation" extinf "k8s.io/apiextensions-apiserver/pkg/client/informers/externalversions" - k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" @@ -53,7 +53,6 @@ import ( "github.com/operator-framework/operator-lifecycle-manager/pkg/lib/scoped" sharedtime "github.com/operator-framework/operator-lifecycle-manager/pkg/lib/time" "github.com/operator-framework/operator-lifecycle-manager/pkg/metrics" - "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" ) const ( @@ -322,7 +321,7 @@ func NewOperator(ctx context.Context, kubeconfigPath string, clock utilclock.Clo } // Register CustomResourceDefinition QueueInformer - crdInformer := extinf.NewSharedInformerFactory(op.opClient.ApiextensionsV1beta1Interface(), resyncPeriod()).Apiextensions().V1beta1().CustomResourceDefinitions() + crdInformer := extinf.NewSharedInformerFactory(op.opClient.ApiextensionsInterface(), resyncPeriod()).Apiextensions().V1beta1().CustomResourceDefinitions() op.lister.APIExtensionsV1beta1().RegisterCustomResourceDefinitionLister(crdInformer.Lister()) crdQueueInformer, err := queueinformer.NewQueueInformer( ctx, @@ -1299,7 +1298,7 @@ func (o *Operator) ResolvePlan(plan *v1alpha1.InstallPlan) error { return nil } -func getCRDVersionsMap(crd *v1beta1ext.CustomResourceDefinition) map[string]struct{} { +func getCRDV1Beta1VersionsMap(crd *v1beta1ext.CustomResourceDefinition) map[string]struct{} { versionsMap := map[string]struct{}{} for _, version := range crd.Spec.Versions { @@ -1312,9 +1311,18 @@ func getCRDVersionsMap(crd *v1beta1ext.CustomResourceDefinition) map[string]stru return versionsMap } +func getCRDV1VersionsMap(crd *v1ext.CustomResourceDefinition) map[string]struct{} { + versionsMap := map[string]struct{}{} + + for _, version := range crd.Spec.Versions { + versionsMap[version.Name] = struct{}{} + } + return versionsMap +} + // Ensure all existing served versions are present in new CRD -func ensureCRDVersions(oldCRD *v1beta1ext.CustomResourceDefinition, newCRD *v1beta1ext.CustomResourceDefinition) error { - newCRDVersions := getCRDVersionsMap(newCRD) +func ensureCRDV1Beta1Versions(oldCRD *v1beta1ext.CustomResourceDefinition, newCRD *v1beta1ext.CustomResourceDefinition) error { + newCRDVersions := getCRDV1Beta1VersionsMap(newCRD) for _, oldVersion := range oldCRD.Spec.Versions { if oldVersion.Served { @@ -1333,9 +1341,24 @@ func ensureCRDVersions(oldCRD *v1beta1ext.CustomResourceDefinition, newCRD *v1be return nil } -// Validate all existing served versions against new CRD's validation (if changed) -func (o *Operator) validateCustomResourceDefinition(oldCRD *v1beta1ext.CustomResourceDefinition, newCRD *v1beta1ext.CustomResourceDefinition) error { - o.logger.Debugf("Comparing %#v to %#v", oldCRD.Spec.Validation, newCRD.Spec.Validation) +// Ensure all existing served versions are present in new CRD +func ensureCRDV1Versions(oldCRD *v1ext.CustomResourceDefinition, newCRD *v1ext.CustomResourceDefinition) error { + newCRDVersions := getCRDV1VersionsMap(newCRD) + + for _, oldVersion := range oldCRD.Spec.Versions { + if oldVersion.Served { + _, ok := newCRDVersions[oldVersion.Name] + if !ok { + return fmt.Errorf("New CRD (%s) must contain existing served versions (%s)", oldCRD.Name, oldVersion.Name) + } + } + } + return nil +} + +// Validate all existing served versions against new v1beta CRD's validation (if changed) +func validateV1BetaCustomResourceDefinition(dynamicClient dynamic.Interface, oldCRD *v1beta1ext.CustomResourceDefinition, newCRD *v1beta1ext.CustomResourceDefinition) error { + logrus.Debugf("Comparing %#v to %#v", oldCRD.Spec.Validation, newCRD.Spec.Validation) // If validation schema is unchanged, return right away if reflect.DeepEqual(oldCRD.Spec.Validation, newCRD.Spec.Validation) { return nil @@ -1347,7 +1370,7 @@ func (o *Operator) validateCustomResourceDefinition(oldCRD *v1beta1ext.CustomRes for _, version := range oldCRD.Spec.Versions { if !version.Served { gvr := schema.GroupVersionResource{Group: oldCRD.Spec.Group, Version: version.Name, Resource: oldCRD.Spec.Names.Plural} - err := o.validateExistingCRs(gvr, convertedCRD) + err := validateExistingCRs(dynamicClient, gvr, convertedCRD) if err != nil { return err } @@ -1356,17 +1379,53 @@ func (o *Operator) validateCustomResourceDefinition(oldCRD *v1beta1ext.CustomRes if oldCRD.Spec.Version != "" { gvr := schema.GroupVersionResource{Group: oldCRD.Spec.Group, Version: oldCRD.Spec.Version, Resource: oldCRD.Spec.Names.Plural} - err := o.validateExistingCRs(gvr, convertedCRD) + err := validateExistingCRs(dynamicClient, gvr, convertedCRD) if err != nil { return err } } - o.logger.Debugf("Successfully validated CRD %s\n", newCRD.Name) + logrus.Debugf("Successfully validated CRD %s\n", newCRD.Name) + return nil +} + +// Validate all existing served versions against new CRD's validation (if changed) +func validateV1CustomResourceDefinition(dynamicClient dynamic.Interface, oldCRD *v1ext.CustomResourceDefinition, newCRD *v1ext.CustomResourceDefinition) error { + logrus.Debugf("Comparing %#v to %#v", oldCRD.Spec.Versions, newCRD.Spec.Versions) + + // If validation schema is unchanged, return right away + newestSchema := newCRD.Spec.Versions[len(newCRD.Spec.Versions)-1].Schema + for i, oldVersion := range oldCRD.Spec.Versions { + if !reflect.DeepEqual(oldVersion.Schema, newestSchema) { + break + } + if i == len(oldCRD.Spec.Versions)-1 { + // we are on the last iteration + // schema has not changed between versions at this point. + return nil + } + } + + convertedCRD := &apiextensions.CustomResourceDefinition{} + if err := v1ext.Convert_v1_CustomResourceDefinition_To_apiextensions_CustomResourceDefinition(newCRD, convertedCRD, nil); err != nil { + return err + } + for _, version := range oldCRD.Spec.Versions { + if !version.Served { + gvr := schema.GroupVersionResource{Group: oldCRD.Spec.Group, Version: version.Name, Resource: oldCRD.Spec.Names.Plural} + err := validateExistingCRs(dynamicClient, gvr, convertedCRD) + if err != nil { + return err + } + } + } + + logrus.Debugf("Successfully validated CRD %s\n", newCRD.Name) return nil } -func (o *Operator) validateExistingCRs(gvr schema.GroupVersionResource, newCRD *apiextensions.CustomResourceDefinition) error { - crList, err := o.dynamicClient.Resource(gvr).List(metav1.ListOptions{}) +func validateExistingCRs(dynamicClient dynamic.Interface, gvr schema.GroupVersionResource, newCRD *apiextensions.CustomResourceDefinition) error { + // make dynamic client + crList, err := dynamicClient.Resource(gvr).List(metav1.ListOptions{}) if err != nil { return fmt.Errorf("error listing resources in GroupVersionResource %#v: %s", gvr, err) } @@ -1388,14 +1447,36 @@ func (o *Operator) validateExistingCRs(gvr schema.GroupVersionResource, newCRD * // The function may not always succeed as storedVersions requires at least one // version. If there is only stored version, it won't be removed until a new // stored version is added. -func removeDeprecatedStoredVersions(oldCRD *v1beta1ext.CustomResourceDefinition, newCRD *v1beta1ext.CustomResourceDefinition) []string { +func removeDeprecatedV1Beta1StoredVersions(oldCRD *v1beta1ext.CustomResourceDefinition, newCRD *v1beta1ext.CustomResourceDefinition) []string { + // StoredVersions requires to have at least one version. + if len(oldCRD.Status.StoredVersions) <= 1 { + return nil + } + + newStoredVersions := []string{} + newCRDVersions := getCRDV1Beta1VersionsMap(newCRD) + for _, v := range oldCRD.Status.StoredVersions { + _, ok := newCRDVersions[v] + if ok { + newStoredVersions = append(newStoredVersions, v) + } + } + + if len(newStoredVersions) < 1 { + return nil + } else { + return newStoredVersions + } +} + +func removeDeprecatedV1StoredVersions(oldCRD *v1ext.CustomResourceDefinition, newCRD *v1ext.CustomResourceDefinition) []string { // StoredVersions requires to have at least one version. if len(oldCRD.Status.StoredVersions) <= 1 { return nil } newStoredVersions := []string{} - newCRDVersions := getCRDVersionsMap(newCRD) + newCRDVersions := getCRDV1VersionsMap(newCRD) for _, v := range oldCRD.Status.StoredVersions { _, ok := newCRDVersions[v] if ok { @@ -1438,111 +1519,35 @@ func (o *Operator) ExecutePlan(plan *v1alpha1.InstallPlan) error { ensurer := newStepEnsurer(kubeclient, crclient, dynamicClient) for i, step := range plan.Status.Plan { + b := newBuilder(kubeclient, dynamicClient, o.csvProvidedAPIsIndexer) + doStep := true + s, err := b.step(step.Status, step.Resource.Manifest, step.Resource.Name) + if err != nil { + if _, ok := err.(*notSupportedStepperErr); ok { + // stepper not implemented for this type yet + // stepper currently only implemented for CRD types + doStep = false + } else { + return err + } + } + if doStep { + status, err := s.Status() + if err != nil { + return err + } + plan.Status.Plan[i].Status = status + return nil + } + switch step.Status { case v1alpha1.StepStatusPresent, v1alpha1.StepStatusCreated: continue case v1alpha1.StepStatusWaitingForAPI: - switch step.Resource.Kind { - case crdKind: - crd, err := o.opClient.ApiextensionsV1beta1Interface().ApiextensionsV1beta1().CustomResourceDefinitions().Get(step.Resource.Name, metav1.GetOptions{}) - if err != nil { - if k8serrors.IsNotFound(err) { - plan.Status.Plan[i].Status = v1alpha1.StepStatusNotPresent - } else { - return errorwrap.Wrapf(err, "error finding the %s CRD", crd.Name) - } - continue - } - - established, namesAccepted := false, false - for _, cdt := range crd.Status.Conditions { - switch cdt.Type { - case v1beta1.Established: - if cdt.Status == v1beta1.ConditionTrue { - established = true - } - case v1beta1.NamesAccepted: - if cdt.Status == v1beta1.ConditionTrue { - namesAccepted = true - } - } - } - - if established && namesAccepted { - plan.Status.Plan[i].Status = v1alpha1.StepStatusCreated - } - continue - } + continue case v1alpha1.StepStatusUnknown, v1alpha1.StepStatusNotPresent: o.logger.WithFields(logrus.Fields{"kind": step.Resource.Kind, "name": step.Resource.Name}).Debug("execute resource") switch step.Resource.Kind { - case crdKind: - // Marshal the manifest into a CRD instance. - var crd v1beta1ext.CustomResourceDefinition - err := json.Unmarshal([]byte(step.Resource.Manifest), &crd) - if err != nil { - return errorwrap.Wrapf(err, "error parsing step manifest: %s", step.Resource.Name) - } - - // TODO: check that names are accepted - // Attempt to create the CRD. - _, err = o.opClient.ApiextensionsV1beta1Interface().ApiextensionsV1beta1().CustomResourceDefinitions().Create(&crd) - if k8serrors.IsAlreadyExists(err) { - currentCRD, _ := o.lister.APIExtensionsV1beta1().CustomResourceDefinitionLister().Get(crd.GetName()) - // Compare 2 CRDs to see if it needs to be updatetd - if !(reflect.DeepEqual(crd.Spec.Version, currentCRD.Spec.Version) && - reflect.DeepEqual(crd.Spec.Versions, currentCRD.Spec.Versions) && - reflect.DeepEqual(crd.Spec.Validation, currentCRD.Spec.Validation)) { - // Verify CRD ownership, only attempt to update if - // CRD has only one owner - // Example: provided=database.coreos.com/v1alpha1/EtcdCluster - matchedCSV, err := index.CRDProviderNames(o.csvProvidedAPIsIndexer, crd) - if err != nil { - return errorwrap.Wrapf(err, "error find matched CSV: %s", step.Resource.Name) - } - crd.SetResourceVersion(currentCRD.GetResourceVersion()) - if len(matchedCSV) == 1 { - o.logger.Debugf("Found one owner for CRD %v", crd) - } else if len(matchedCSV) > 1 { - o.logger.Debugf("Found multiple owners for CRD %v", crd) - - err := ensureCRDVersions(currentCRD, &crd) - if err != nil { - return errorwrap.Wrapf(err, "error missing existing CRD version(s) in new CRD: %s", step.Resource.Name) - } - - if err = o.validateCustomResourceDefinition(currentCRD, &crd); err != nil { - return errorwrap.Wrapf(err, "error validating existing CRs agains new CRD's schema: %s", step.Resource.Name) - } - } - // Remove deprecated version in CRD storedVersions - storeVersions := removeDeprecatedStoredVersions(currentCRD, &crd) - if storeVersions != nil { - currentCRD.Status.StoredVersions = storeVersions - resultCRD, err := o.opClient.ApiextensionsV1beta1Interface().ApiextensionsV1beta1().CustomResourceDefinitions().UpdateStatus(currentCRD) - if err != nil { - return errorwrap.Wrapf(err, "error updating CRD's status: %s", step.Resource.Name) - } - crd.SetResourceVersion(resultCRD.GetResourceVersion()) - } - // Update CRD to new version - _, err = o.opClient.ApiextensionsV1beta1Interface().ApiextensionsV1beta1().CustomResourceDefinitions().Update(&crd) - if err != nil { - return errorwrap.Wrapf(err, "error updating CRD: %s", step.Resource.Name) - } - } - // If it already existed, mark the step as Present. - plan.Status.Plan[i].Status = v1alpha1.StepStatusPresent - continue - } else if err != nil { - // Unexpected error creating the CRD. - return err - } else { - // If no error occured, make sure to wait for the API to become available. - plan.Status.Plan[i].Status = v1alpha1.StepStatusWaitingForAPI - continue - } - case v1alpha1.ClusterServiceVersionKind: // Marshal the manifest into a CSV instance. var csv v1alpha1.ClusterServiceVersion diff --git a/pkg/controller/operators/catalog/operator_test.go b/pkg/controller/operators/catalog/operator_test.go index 4c9f221497e..3f8260fb7ac 100644 --- a/pkg/controller/operators/catalog/operator_test.go +++ b/pkg/controller/operators/catalog/operator_test.go @@ -17,6 +17,7 @@ import ( appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" + apiexv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" apiextensionsfake "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/fake" "k8s.io/apimachinery/pkg/api/meta" @@ -134,7 +135,7 @@ func TestTransitionInstallPlan(t *testing.T) { } } -func TestEnsureCRDVersions(t *testing.T) { +func TestEnsureV1Beta1CRDVersions(t *testing.T) { mainCRDPlural := "ins-main-abcde" currentVersions := []v1beta1.CustomResourceDefinitionVersion{ @@ -236,14 +237,118 @@ func TestEnsureCRDVersions(t *testing.T) { } for _, tt := range tests { - err := ensureCRDVersions(&tt.oldCRD, &tt.newCRD) + err := ensureCRDV1Beta1Versions(&tt.oldCRD, &tt.newCRD) if tt.expectedFailure { require.Error(t, err) } } } -func TestRemoveDeprecatedStoredVersions(t *testing.T) { +func TestEnsureV1CRDVersions(t *testing.T) { + mainCRDPlural := "ins-main-abcde" + + currentVersions := []apiexv1.CustomResourceDefinitionVersion{ + { + Name: "v1alpha1", + Served: true, + Storage: true, + }, + } + + addedVersions := []apiexv1.CustomResourceDefinitionVersion{ + { + Name: "v1alpha1", + Served: true, + Storage: false, + }, + { + Name: "v1alpha2", + Served: true, + Storage: true, + }, + } + + missingVersions := []apiexv1.CustomResourceDefinitionVersion{ + { + Name: "v1alpha2", + Served: true, + Storage: true, + }, + } + + tests := []struct { + name string + oldCRD apiexv1.CustomResourceDefinition + newCRD apiexv1.CustomResourceDefinition + expectedFailure bool + }{ + { + name: "existing versions are present", + oldCRD: func() apiexv1.CustomResourceDefinition { + oldCRD := v1crd(mainCRDPlural) + oldCRD.Spec.Versions = currentVersions + return oldCRD + }(), + newCRD: func() apiexv1.CustomResourceDefinition { + newCRD := v1crd(mainCRDPlural) + newCRD.Spec.Versions = addedVersions + return newCRD + }(), + expectedFailure: false, + }, + { + name: "missing versions in new CRD 1", + oldCRD: func() apiexv1.CustomResourceDefinition { + oldCRD := v1crd(mainCRDPlural) + oldCRD.Spec.Versions = currentVersions + return oldCRD + }(), + newCRD: func() apiexv1.CustomResourceDefinition { + newCRD := v1crd(mainCRDPlural) + newCRD.Spec.Versions = missingVersions + return newCRD + }(), + expectedFailure: true, + }, + { + name: "missing version in new CRD 2", + oldCRD: func() apiexv1.CustomResourceDefinition { + oldCRD := v1crd(mainCRDPlural) + oldCRD.Spec.Versions[0].Name = "v1alpha1" + return oldCRD + }(), + newCRD: func() apiexv1.CustomResourceDefinition { + newCRD := v1crd(mainCRDPlural) + newCRD.Spec.Versions[0].Name = "v1alpha2" + return newCRD + }(), + expectedFailure: true, + }, + { + name: "existing version is present in new CRD's versions", + oldCRD: func() apiexv1.CustomResourceDefinition { + oldCRD := v1crd(mainCRDPlural) + oldCRD.Spec.Versions[0].Name = "v1alpha1" + return oldCRD + }(), + newCRD: func() apiexv1.CustomResourceDefinition { + newCRD := v1crd(mainCRDPlural) + newCRD.Spec.Versions = addedVersions + return newCRD + }(), + expectedFailure: false, + }, + } + + for _, tt := range tests { + err := ensureCRDV1Versions(&tt.oldCRD, &tt.newCRD) + if tt.expectedFailure { + require.Error(t, err) + } + } +} + +func TestRemoveDeprecatedV1Beta1StoredVersions(t *testing.T) { mainCRDPlural := "ins-main-test" currentVersions := []v1beta1.CustomResourceDefinitionVersion{ @@ -337,7 +442,100 @@ func TestRemoveDeprecatedStoredVersions(t *testing.T) { } for _, tt := range tests { - resultCRD := removeDeprecatedStoredVersions(&tt.oldCRD, &tt.newCRD) + resultCRD := removeDeprecatedV1Beta1StoredVersions(&tt.oldCRD, &tt.newCRD) + require.Equal(t, tt.expectedResult, resultCRD) + } +} + +func TestRemoveDeprecatedV1StoredVersions(t *testing.T) { + mainCRDPlural := "ins-main-test" + + currentVersions := []apiexv1.CustomResourceDefinitionVersion{ + { + Name: "v1alpha1", + Served: true, + Storage: false, + }, + { + Name: "v1alpha2", + Served: true, + Storage: true, + }, + } + + newVersions := []apiexv1.CustomResourceDefinitionVersion{ + { + Name: "v1alpha2", + Served: true, + Storage: false, + }, + { + Name: "v1beta1", + Served: true, + Storage: true, + }, + } + + crdStatusStoredVersions := apiexv1.CustomResourceDefinitionStatus{ + StoredVersions: []string{}, + } + + tests := []struct { + name string + oldCRD apiexv1.CustomResourceDefinition + newCRD apiexv1.CustomResourceDefinition + expectedResult []string + }{ + { + name: "only one stored version exists", + oldCRD: func() apiexv1.CustomResourceDefinition { + oldCRD := v1crd(mainCRDPlural) + oldCRD.Spec.Versions = currentVersions + oldCRD.Status = crdStatusStoredVersions + oldCRD.Status.StoredVersions = []string{"v1alpha1"} + return oldCRD + }(), + newCRD: func() apiexv1.CustomResourceDefinition { + newCRD := v1crd(mainCRDPlural) + newCRD.Spec.Versions = newVersions + return newCRD + }(), + expectedResult: nil, + }, + { + name: "multiple stored versions with one deprecated version", + oldCRD: func() apiexv1.CustomResourceDefinition { + oldCRD := v1crd(mainCRDPlural) + oldCRD.Spec.Versions = currentVersions + oldCRD.Status.StoredVersions = []string{"v1alpha1", "v1alpha2"} + return oldCRD + }(), + newCRD: func() apiexv1.CustomResourceDefinition { + newCRD := v1crd(mainCRDPlural) + newCRD.Spec.Versions = newVersions + return newCRD + }(), + expectedResult: []string{"v1alpha2"}, + }, + { + name: "multiple stored versions with all deprecated version", + oldCRD: func() apiexv1.CustomResourceDefinition { + oldCRD := v1crd(mainCRDPlural) + oldCRD.Spec.Versions = currentVersions + oldCRD.Status.StoredVersions = []string{"v1alpha1", "v1alpha3"} + return oldCRD + }(), + newCRD: func() apiexv1.CustomResourceDefinition { + newCRD := v1crd(mainCRDPlural) + newCRD.Spec.Versions = newVersions + return newCRD + }(), + expectedResult: nil, + }, + } + + for _, tt := range tests { + resultCRD := removeDeprecatedV1StoredVersions(&tt.oldCRD, &tt.newCRD) require.Equal(t, tt.expectedResult, resultCRD) } } @@ -523,6 +721,41 @@ func TestExecutePlan(t *testing.T) { }, err: nil, }, + { + testName: "V1CRDResourceIsCreated", + in: withSteps(installPlan("p", namespace, v1alpha1.InstallPlanPhaseInstalling, "crdv1"), + []*v1alpha1.Step{ + { + Resource: v1alpha1.StepResource{ + CatalogSource: "catalog", + CatalogSourceNamespace: namespace, + Group: "", + Version: "v1", + Kind: "CustomResourceDefinition", + Name: "crd", + Manifest: toManifest(t, + &apiexv1.CustomResourceDefinition{ + TypeMeta: metav1.TypeMeta{ + Kind: "CustomResourceDefinition", + APIVersion: "v1", // v1 CRD version of API + }, + ObjectMeta: metav1.ObjectMeta{Name: "test"}, + Spec: apiexv1.CustomResourceDefinitionSpec{}, + }), + }, + Status: v1alpha1.StepStatusUnknown, + }, + }), + want: []runtime.Object{ + &apiexv1.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{Name: "test"}, + TypeMeta: metav1.TypeMeta{ + Kind: "CustomResourceDefinition", + APIVersion: "v1", // v1 CRD version of API + }, + }, + }, + }, } for _, tt := range tests { @@ -558,7 +791,9 @@ func TestExecutePlan(t *testing.T) { case *corev1.Service: fetched, err = op.opClient.GetService(namespace, o.GetName()) case *v1beta1.CustomResourceDefinition: - fetched, err = op.opClient.ApiextensionsV1beta1Interface().ApiextensionsV1beta1().CustomResourceDefinitions().Get(o.GetName(), getOpts) + fetched, err = op.opClient.ApiextensionsInterface().ApiextensionsV1beta1().CustomResourceDefinitions().Get(o.GetName(), getOpts) + case *apiexv1.CustomResourceDefinition: + fetched, err = op.opClient.ApiextensionsInterface().ApiextensionsV1().CustomResourceDefinitions().Get(o.GetName(), getOpts) case *v1alpha1.ClusterServiceVersion: fetched, err = op.client.OperatorsV1alpha1().ClusterServiceVersions(namespace).Get(o.GetName(), getOpts) case *unstructured.Unstructured: @@ -1301,6 +1536,26 @@ func crd(name string) v1beta1.CustomResourceDefinition { } } +func v1crd(name string) apiexv1.CustomResourceDefinition { + return apiexv1.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + Spec: apiexv1.CustomResourceDefinitionSpec{ + Group: name + "group", + Versions: []apiexv1.CustomResourceDefinitionVersion{ + { + Name: "v1", + Served: true, + }, + }, + Names: apiexv1.CustomResourceDefinitionNames{ + Kind: name, + }, + }, + } +} + func service(name, namespace string) *corev1.Service { return &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ diff --git a/pkg/controller/operators/catalog/step.go b/pkg/controller/operators/catalog/step.go new file mode 100644 index 00000000000..127f40e297e --- /dev/null +++ b/pkg/controller/operators/catalog/step.go @@ -0,0 +1,264 @@ +package catalog + +import ( + "encoding/json" + "fmt" + "k8s.io/client-go/dynamic" + + "k8s.io/client-go/tools/cache" + + "github.com/operator-framework/operator-lifecycle-manager/pkg/api/apis/operators/v1alpha1" + crdlib "github.com/operator-framework/operator-lifecycle-manager/pkg/lib/crd" + index "github.com/operator-framework/operator-lifecycle-manager/pkg/lib/index" + "github.com/operator-framework/operator-lifecycle-manager/pkg/lib/operatorclient" + errorwrap "github.com/pkg/errors" + logger "github.com/sirupsen/logrus" + v1ext "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + v1beta1ext "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" + "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset" + k8serrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// Stepper manages cluster interactions based on the step. +type Stepper interface { + Status() (v1alpha1.StepStatus, error) +} + +// StepperFunc fulfills the Stepper interface. +type StepperFunc func() (v1alpha1.StepStatus, error) + +func (s StepperFunc) Status() (v1alpha1.StepStatus, error) { + return s() +} + +// Builder holds clients and data structures required for the StepBuilder to work +// Builder attributes are not to meant to be accessed outside the StepBuilder method +type builder struct { + opclient operatorclient.ClientInterface + dynamicClient dynamic.Interface + csvToProvidedAPIs map[string]cache.Indexer +} + +func newBuilder(opclient operatorclient.ClientInterface, dynamicClient dynamic.Interface, csvToProvidedAPIs map[string]cache.Indexer) *builder { + return &builder{ + opclient: opclient, + dynamicClient: dynamicClient, + csvToProvidedAPIs: csvToProvidedAPIs, + } +} + +type notSupportedStepperErr struct { + message string +} + +func (n notSupportedStepperErr) Error() string { + return n.message +} + +// step is a factory that creates StepperFuncs based on the Kind provided and the install plan step. +func (b *builder) step(stepStatus v1alpha1.StepStatus, manifest, name string) (Stepper, error) { + gvk := metav1.GroupVersionKind{} + err := json.Unmarshal([]byte(manifest), &gvk) + if err != nil { + return nil, errorwrap.Wrapf(err, "error finding the %s GVK", name) + } + + switch gvk.Kind { + case crdKind: + switch gvk.Version { + case crdlib.V1Beta1Version: + return b.NewCRDV1Beta1Step(manifest, b.opclient.ApiextensionsInterface(), stepStatus, name), nil + case crdlib.V1Version: + return b.NewCRDV1Step(manifest, b.opclient.ApiextensionsInterface(), stepStatus, name), nil + } + } + + return nil, notSupportedStepperErr{fmt.Sprintf("stepper interface does not support %s", gvk.Kind)} +} + +// NewCRDV1Beta1Step returns a StepperFunc based on the status of the provided step +func (b *builder) NewCRDV1Beta1Step(manifest string, client clientset.Interface, status v1alpha1.StepStatus, name string) StepperFunc { + return func() (v1alpha1.StepStatus, error) { + switch status { + case v1alpha1.StepStatusWaitingForAPI: + crd, err := client.ApiextensionsV1().CustomResourceDefinitions().Get(name, metav1.GetOptions{}) + if err != nil { + if k8serrors.IsNotFound(err) { + return v1alpha1.StepStatusNotPresent, nil + } else { + return v1alpha1.StepStatusNotPresent, errorwrap.Wrapf(err, "error finding the %s CRD", crd.Name) + } + } + established, namesAccepted := false, false + for _, cdt := range crd.Status.Conditions { + switch cdt.Type { + case v1ext.Established: + if cdt.Status == v1ext.ConditionTrue { + established = true + } + case v1ext.NamesAccepted: + if cdt.Status == v1ext.ConditionTrue { + namesAccepted = true + } + } + } + if established && namesAccepted { + return v1alpha1.StepStatusCreated, nil + } + case v1alpha1.StepStatusUnknown, v1alpha1.StepStatusNotPresent: + var crd v1beta1ext.CustomResourceDefinition + err := json.Unmarshal([]byte(manifest), &crd) + if err != nil { + return v1alpha1.StepStatusUnknown, errorwrap.Wrapf(err, "error parsing step manifest: %s", name) + } + + _, err = client.ApiextensionsV1beta1().CustomResourceDefinitions().Create(&crd) + if k8serrors.IsAlreadyExists(err) { + currentCRD, _ := client.ApiextensionsV1beta1().CustomResourceDefinitions().Get(crd.GetName(), metav1.GetOptions{}) + if crdlib.NotEqualV1Beta1(currentCRD, &crd) { + // Verify CRD ownership, only attempt to update if CRD has only one owner + // Example: provided=database.coreos.com/v1alpha1/EtcdCluster + matchedCSV, err := index.CRDV1Beta1ProviderNames(b.csvToProvidedAPIs, crd) + if err != nil { + return v1alpha1.StepStatusUnknown, errorwrap.Wrapf(err, "error find matched CSV: %s", name) + } + crd.SetResourceVersion(currentCRD.GetResourceVersion()) + if len(matchedCSV) == 1 { + logger.Debugf("Found one owner for CRD %v", crd) + } else if len(matchedCSV) > 1 { + logger.Debugf("Found multiple owners for CRD %v", crd) + + err := ensureCRDV1Beta1Versions(currentCRD, &crd) + if err != nil { + return v1alpha1.StepStatusUnknown, errorwrap.Wrapf(err, "error missing existing CRD version(s) in new CRD: %s", name) + } + // TODO make this function accessible from here + if err = validateV1BetaCustomResourceDefinition(b.dynamicClient, currentCRD, &crd); err != nil { + return v1alpha1.StepStatusUnknown, errorwrap.Wrapf(err, "error validating existing CRs agains new CRD's schema: %s", name) + } + } + // Remove deprecated version in CRD storedVersions + storeVersions := removeDeprecatedV1Beta1StoredVersions(currentCRD, &crd) + if storeVersions != nil { + currentCRD.Status.StoredVersions = storeVersions + resultCRD, err := client.ApiextensionsV1beta1().CustomResourceDefinitions().UpdateStatus(currentCRD) + if err != nil { + return v1alpha1.StepStatusUnknown, errorwrap.Wrapf(err, "error updating CRD's status: %s", name) + } + crd.SetResourceVersion(resultCRD.GetResourceVersion()) + } + // Update CRD to new version + _, err = client.ApiextensionsV1beta1().CustomResourceDefinitions().Update(&crd) + if err != nil { + return v1alpha1.StepStatusUnknown, errorwrap.Wrapf(err, "error updating CRD: %s", name) + } + } + // they were equal - mark CRD as present + return v1alpha1.StepStatusPresent, nil + } else if err != nil { + // unknown error when getting CRD object + return v1alpha1.StepStatusUnknown, err + } else { + return v1alpha1.StepStatusWaitingForAPI, nil + } + } + + return v1alpha1.StepStatusUnknown, nil + } +} + +func (b *builder) NewCRDV1Step(manifest string, client clientset.Interface, status v1alpha1.StepStatus, name string) StepperFunc { + return func() (v1alpha1.StepStatus, error) { + switch status { + case v1alpha1.StepStatusWaitingForAPI: + crd, err := client.ApiextensionsV1().CustomResourceDefinitions().Get(name, metav1.GetOptions{}) + if err != nil { + if k8serrors.IsNotFound(err) { + return v1alpha1.StepStatusNotPresent, nil + } else { + return v1alpha1.StepStatusNotPresent, errorwrap.Wrapf(err, "error finding the %s CRD", crd.Name) + } + } + established, namesAccepted := false, false + for _, cdt := range crd.Status.Conditions { + switch cdt.Type { + case v1ext.Established: + if cdt.Status == v1ext.ConditionTrue { + established = true + } + case v1ext.NamesAccepted: + if cdt.Status == v1ext.ConditionTrue { + namesAccepted = true + } + } + } + if established && namesAccepted { + return v1alpha1.StepStatusCreated, nil + } + case v1alpha1.StepStatusUnknown, v1alpha1.StepStatusNotPresent: + var crd v1ext.CustomResourceDefinition + err := json.Unmarshal([]byte(manifest), &crd) + if err != nil { + return v1alpha1.StepStatusUnknown, errorwrap.Wrapf(err, "error parsing step manifest: %s", name) + } + + // Attempt to create the CRD. + _, err = client.ApiextensionsV1().CustomResourceDefinitions().Create(&crd) + if k8serrors.IsAlreadyExists(err) { + currentCRD, _ := client.ApiextensionsV1().CustomResourceDefinitions().Get(crd.GetName(), metav1.GetOptions{}) + // Compare 2 CRDs to see if it needs to be updatetd + if crdlib.NotEqual(currentCRD, &crd) { + // Verify CRD ownership, only attempt to update if + // CRD has only one owner + // Example: provided=database.coreos.com/v1alpha1/EtcdCluster + matchedCSV, err := index.CRDV1ProviderNames(b.csvToProvidedAPIs, &crd) + if err != nil { + return v1alpha1.StepStatusUnknown, errorwrap.Wrapf(err, "error find matched CSV: %s", name) + } + crd.SetResourceVersion(currentCRD.GetResourceVersion()) + if len(matchedCSV) == 1 { + logger.Debugf("Found one owner for CRD %v", crd) + } else if len(matchedCSV) > 1 { + logger.Debugf("Found multiple owners for CRD %v", crd) + + err := ensureCRDV1Versions(currentCRD, &crd) + if err != nil { + return v1alpha1.StepStatusUnknown, errorwrap.Wrapf(err, "error missing existing CRD version(s) in new CRD: %s", name) + } + + if err = validateV1CustomResourceDefinition(b.dynamicClient, currentCRD, &crd); err != nil { + return v1alpha1.StepStatusUnknown, errorwrap.Wrapf(err, "error validating existing CRs agains new CRD's schema: %s", name) + } + } + // Remove deprecated version in CRD storedVersions + storeVersions := removeDeprecatedV1StoredVersions(currentCRD, &crd) + if storeVersions != nil { + currentCRD.Status.StoredVersions = storeVersions + resultCRD, err := client.ApiextensionsV1().CustomResourceDefinitions().UpdateStatus(currentCRD) + if err != nil { + return v1alpha1.StepStatusUnknown, errorwrap.Wrapf(err, "error updating CRD's status: %s", name) + } + crd.SetResourceVersion(resultCRD.GetResourceVersion()) + } + // Update CRD to new version + _, err = client.ApiextensionsV1().CustomResourceDefinitions().Update(&crd) + if err != nil { + return v1alpha1.StepStatusUnknown, errorwrap.Wrapf(err, "error updating CRD: %s", name) + } + } + // If it already existed, mark the step as Present. + // they were equal - mark CRD as present + return v1alpha1.StepStatusPresent, nil + } else if err != nil { + // Unexpected error creating the CRD. + return v1alpha1.StepStatusUnknown, err + } else { + // If no error occured, make sure to wait for the API to become available. + return v1alpha1.StepStatusWaitingForAPI, nil + } + } + + return v1alpha1.StepStatusUnknown, nil + } +} diff --git a/pkg/controller/operators/olm/operator.go b/pkg/controller/operators/olm/operator.go index fe70679430b..dd93914ac96 100644 --- a/pkg/controller/operators/olm/operator.go +++ b/pkg/controller/operators/olm/operator.go @@ -434,18 +434,19 @@ func newOperatorWithConfig(ctx context.Context, config *operatorConfig) (*Operat } // Register CustomResourceDefinition QueueInformer - crdInformer := extinf.NewSharedInformerFactory(op.opClient.ApiextensionsV1beta1Interface(), config.resyncPeriod()).Apiextensions().V1beta1().CustomResourceDefinitions() - op.lister.APIExtensionsV1beta1().RegisterCustomResourceDefinitionLister(crdInformer.Lister()) - crdQueueInformer, err := queueinformer.NewQueueInformer( + // This informer will inform on both v1 and v1beta1 APIVersions + crdV1Informer := extinf.NewSharedInformerFactory(op.opClient.ApiextensionsInterface(), config.resyncPeriod()).Apiextensions().V1().CustomResourceDefinitions() + op.lister.APIExtensionsV1().RegisterCustomResourceDefinitionLister(crdV1Informer.Lister()) + crdV1QueueInformer, err := queueinformer.NewQueueInformer( ctx, queueinformer.WithLogger(op.logger), - queueinformer.WithInformer(crdInformer.Informer()), + queueinformer.WithInformer(crdV1Informer.Informer()), queueinformer.WithSyncer(k8sSyncer), ) if err != nil { return nil, err } - if err := op.RegisterQueueInformer(crdQueueInformer); err != nil { + if err := op.RegisterQueueInformer(crdV1QueueInformer); err != nil { return nil, err } diff --git a/pkg/lib/crd/version.go b/pkg/lib/crd/version.go new file mode 100644 index 00000000000..95dea515883 --- /dev/null +++ b/pkg/lib/crd/version.go @@ -0,0 +1,68 @@ +package crd + +import ( + "fmt" + "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" + "reflect" + "strings" + + v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/util/yaml" +) + +// V1Beta1 refers to the deprecated v1beta1 APIVersion of CRD objects +const V1Beta1Version string = "v1beta1" + +// V1 refers to the new v1 APIVersion of CRD objects +const V1Version string = "v1" + +// Version takes a CRD manifest and determines whether it is v1 or v1beta1 type based on the APIVersion. +func Version(manifest *string) (string, error) { + dec := yaml.NewYAMLOrJSONDecoder(strings.NewReader(*manifest), 10) + unst := &unstructured.Unstructured{} + if err := dec.Decode(unst); err != nil { + return "", err + } + + if unst.GetObjectKind().GroupVersionKind().Version == V1Beta1Version { + return V1Beta1Version, nil + } + + if unst.GetObjectKind().GroupVersionKind().Version == V1Version { + return V1Version, nil + } + + return "", fmt.Errorf("could not determine CRD version from manifest") +} + +// NotEqual determines whether two CRDs are equal based on the versions and validations of both. +// If false, then we know we need to update the CRD on cluster. +func NotEqual(currentCRD *v1.CustomResourceDefinition, oldCRD *v1.CustomResourceDefinition) bool { + var equalVersions bool + var equalValidation bool + var oldRange = len(oldCRD.Spec.Versions) - 1 + + equalVersions = reflect.DeepEqual(currentCRD.Spec.Versions, oldCRD.Spec.Versions) + if !equalVersions { + return true + } + + for i := range currentCRD.Spec.Versions { + if i > oldRange { + return true + } + equalValidation = reflect.DeepEqual(currentCRD.Spec.Versions[i].Schema, oldCRD.Spec.Versions[i].Schema) + if equalValidation == false { + return true + } + } + + return false +} + +func NotEqualV1Beta1(currentCRD *v1beta1.CustomResourceDefinition, oldCRD *v1beta1.CustomResourceDefinition) bool { + return !(reflect.DeepEqual(oldCRD.Spec.Version, currentCRD.Spec.Version) && + reflect.DeepEqual(oldCRD.Spec.Versions, currentCRD.Spec.Versions) && + reflect.DeepEqual(oldCRD.Spec.Validation, currentCRD.Spec.Validation)) +} diff --git a/pkg/lib/index/api.go b/pkg/lib/index/api.go index 1213766f172..140279d99d8 100644 --- a/pkg/lib/index/api.go +++ b/pkg/lib/index/api.go @@ -5,7 +5,9 @@ import ( "strings" "github.com/operator-framework/operator-lifecycle-manager/pkg/api/apis/operators/v1alpha1" + v1ext "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" v1beta1ext "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" + "k8s.io/client-go/tools/cache" ) @@ -37,8 +39,8 @@ func ProvidedAPIsIndexFunc(obj interface{}) ([]string, error) { return indicies, nil } -// CRDProviderNames returns the names of CSVs that own the given CRD -func CRDProviderNames(indexers map[string]cache.Indexer, crd v1beta1ext.CustomResourceDefinition) (map[string]struct{}, error) { +// CRDV1Beta1ProviderNames returns the names of CSVs that own the given CRD +func CRDV1Beta1ProviderNames(indexers map[string]cache.Indexer, crd v1beta1ext.CustomResourceDefinition) (map[string]struct{}, error) { csvSet := map[string]struct{}{} crdSpec := map[string]struct{}{} for _, v := range crd.Spec.Versions { @@ -65,3 +67,30 @@ func CRDProviderNames(indexers map[string]cache.Indexer, crd v1beta1ext.CustomRe } return csvSet, nil } + +// CRDV1ProviderNames returns the names of CSVs that own the given CRD +func CRDV1ProviderNames(indexers map[string]cache.Indexer, crd *v1ext.CustomResourceDefinition) (map[string]struct{}, error) { + csvSet := map[string]struct{}{} + crdSpec := map[string]struct{}{} + for _, v := range crd.Spec.Versions { + crdSpec[fmt.Sprintf("%s/%s/%s", crd.Spec.Group, v.Name, crd.Spec.Names.Kind)] = struct{}{} + } + + for _, indexer := range indexers { + for key := range crdSpec { + csvs, err := indexer.ByIndex(ProvidedAPIsIndexFuncKey, key) + if err != nil { + return nil, err + } + for _, item := range csvs { + csv, ok := item.(*v1alpha1.ClusterServiceVersion) + if !ok { + continue + } + // Add to set + csvSet[csv.GetName()] = struct{}{} + } + } + } + return csvSet, nil +} diff --git a/pkg/lib/operatorclient/client.go b/pkg/lib/operatorclient/client.go index e6cc56c51a9..ee9c664b9c5 100644 --- a/pkg/lib/operatorclient/client.go +++ b/pkg/lib/operatorclient/client.go @@ -19,7 +19,7 @@ import ( type ClientInterface interface { KubernetesInterface() kubernetes.Interface - ApiextensionsV1beta1Interface() apiextensions.Interface + ApiextensionsInterface() apiextensions.Interface ApiregistrationV1Interface() apiregistration.Interface APIServiceClient CustomResourceClient @@ -192,8 +192,8 @@ func (c *Client) KubernetesInterface() kubernetes.Interface { return c.Interface } -// ApiextensionsV1beta1Interface returns the API extension interface. -func (c *Client) ApiextensionsV1beta1Interface() apiextensions.Interface { +// ApiextensionsInterface returns the API extension interface. +func (c *Client) ApiextensionsInterface() apiextensions.Interface { return c.extInterface } diff --git a/pkg/lib/operatorclient/operatorclientmocks/mock_client.go b/pkg/lib/operatorclient/operatorclientmocks/mock_client.go index 9144643a869..8cd22dd7db6 100644 --- a/pkg/lib/operatorclient/operatorclientmocks/mock_client.go +++ b/pkg/lib/operatorclient/operatorclientmocks/mock_client.go @@ -57,18 +57,18 @@ func (mr *MockClientInterfaceMockRecorder) KubernetesInterface() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "KubernetesInterface", reflect.TypeOf((*MockClientInterface)(nil).KubernetesInterface)) } -// ApiextensionsV1beta1Interface mocks base method -func (m *MockClientInterface) ApiextensionsV1beta1Interface() clientset.Interface { +// ApiextensionsInterface mocks base method +func (m *MockClientInterface) ApiextensionsInterface() clientset.Interface { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "ApiextensionsV1beta1Interface") + ret := m.ctrl.Call(m, "ApiextensionsInterface") ret0, _ := ret[0].(clientset.Interface) return ret0 } -// ApiextensionsV1beta1Interface indicates an expected call of ApiextensionsV1beta1Interface +// ApiextensionsInterface indicates an expected call of ApiextensionsInterface func (mr *MockClientInterfaceMockRecorder) ApiextensionsV1beta1Interface() *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ApiextensionsV1beta1Interface", reflect.TypeOf((*MockClientInterface)(nil).ApiextensionsV1beta1Interface)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ApiextensionsInterface", reflect.TypeOf((*MockClientInterface)(nil).ApiextensionsInterface)) } // ApiregistrationV1Interface mocks base method diff --git a/pkg/lib/operatorlister/customresourcedefinition.go b/pkg/lib/operatorlister/customresourcedefinition.go index 9bbc6ce192b..270c532a5cb 100644 --- a/pkg/lib/operatorlister/customresourcedefinition.go +++ b/pkg/lib/operatorlister/customresourcedefinition.go @@ -4,20 +4,22 @@ import ( "fmt" "sync" + v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" v1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" + aextv1 "k8s.io/apiextensions-apiserver/pkg/client/listers/apiextensions/v1" aextv1beta1 "k8s.io/apiextensions-apiserver/pkg/client/listers/apiextensions/v1beta1" "k8s.io/apimachinery/pkg/labels" ) -// UnionCustomResourceDefinitionLister is a custom implementation of an CustomResourceDefinition lister that allows a new -// Lister to be registered on the fly -type UnionCustomResourceDefinitionLister struct { +// UnionCustomResourceDefinitionV1Beta1Lister is a custom implementation of an CustomResourceDefinition lister that allows a new +// Lister to be registered on the fly. For v1beta1 CRD versions only. +type UnionCustomResourceDefinitionV1Beta1Lister struct { CustomResourceDefinitionLister aextv1beta1.CustomResourceDefinitionLister CustomResourceDefinitionLock sync.RWMutex } // List lists all CustomResourceDefinitions in the indexer. -func (ucl *UnionCustomResourceDefinitionLister) List(selector labels.Selector) (ret []*v1beta1.CustomResourceDefinition, err error) { +func (ucl *UnionCustomResourceDefinitionV1Beta1Lister) List(selector labels.Selector) (ret []*v1beta1.CustomResourceDefinition, err error) { ucl.CustomResourceDefinitionLock.RLock() defer ucl.CustomResourceDefinitionLock.RUnlock() @@ -28,7 +30,7 @@ func (ucl *UnionCustomResourceDefinitionLister) List(selector labels.Selector) ( } // Get retrieves the CustomResourceDefinition with the given name -func (ucl *UnionCustomResourceDefinitionLister) Get(name string) (*v1beta1.CustomResourceDefinition, error) { +func (ucl *UnionCustomResourceDefinitionV1Beta1Lister) Get(name string) (*v1beta1.CustomResourceDefinition, error) { ucl.CustomResourceDefinitionLock.RLock() defer ucl.CustomResourceDefinitionLock.RUnlock() @@ -39,7 +41,7 @@ func (ucl *UnionCustomResourceDefinitionLister) Get(name string) (*v1beta1.Custo } // RegisterCustomResourceDefinitionLister registers a new CustomResourceDefinitionLister -func (ucl *UnionCustomResourceDefinitionLister) RegisterCustomResourceDefinitionLister(lister aextv1beta1.CustomResourceDefinitionLister) { +func (ucl *UnionCustomResourceDefinitionV1Beta1Lister) RegisterCustomResourceDefinitionLister(lister aextv1beta1.CustomResourceDefinitionLister) { ucl.CustomResourceDefinitionLock.Lock() defer ucl.CustomResourceDefinitionLock.Unlock() @@ -53,3 +55,48 @@ func (l *apiExtensionsV1beta1Lister) RegisterCustomResourceDefinitionLister(list func (l *apiExtensionsV1beta1Lister) CustomResourceDefinitionLister() aextv1beta1.CustomResourceDefinitionLister { return l.customResourceDefinitionLister } + +// UnionCustomResourceDefinitionV1Lister is a custom implementation of an CustomResourceDefinition lister that allows a new +// Lister to be registered on the fly. For v1 CRD versions only. +type UnionCustomResourceDefinitionV1Lister struct { + CustomResourceDefinitionLister aextv1.CustomResourceDefinitionLister + CustomResourceDefinitionLock sync.RWMutex +} + +// List lists all CustomResourceDefinitions in the indexer. +func (ucl *UnionCustomResourceDefinitionV1Lister) List(selector labels.Selector) (ret []*v1.CustomResourceDefinition, err error) { + ucl.CustomResourceDefinitionLock.RLock() + defer ucl.CustomResourceDefinitionLock.RUnlock() + + if ucl.CustomResourceDefinitionLister == nil { + return nil, fmt.Errorf("no CustomResourceDefinition lister registered") + } + return ucl.CustomResourceDefinitionLister.List(selector) +} + +// Get retrieves the CustomResourceDefinition with the given name +func (ucl *UnionCustomResourceDefinitionV1Lister) Get(name string) (*v1.CustomResourceDefinition, error) { + ucl.CustomResourceDefinitionLock.RLock() + defer ucl.CustomResourceDefinitionLock.RUnlock() + + if ucl.CustomResourceDefinitionLister == nil { + return nil, fmt.Errorf("no CustomResourceDefinition lister registered") + } + return ucl.CustomResourceDefinitionLister.Get(name) +} + +// RegisterCustomResourceDefinitionLister registers a new CustomResourceDefinitionLister +func (ucl *UnionCustomResourceDefinitionV1Lister) RegisterCustomResourceDefinitionLister(lister aextv1.CustomResourceDefinitionLister) { + ucl.CustomResourceDefinitionLock.Lock() + defer ucl.CustomResourceDefinitionLock.Unlock() + + ucl.CustomResourceDefinitionLister = lister +} + +func (l *apiExtensionsV1Lister) RegisterCustomResourceDefinitionLister(lister aextv1.CustomResourceDefinitionLister) { + l.customResourceDefinitionLister.RegisterCustomResourceDefinitionLister(lister) +} + +func (l *apiExtensionsV1Lister) CustomResourceDefinitionLister() aextv1.CustomResourceDefinitionLister { + return l.customResourceDefinitionLister +} diff --git a/pkg/lib/operatorlister/lister.go b/pkg/lib/operatorlister/lister.go index 41c409a401e..1cefe680be9 100644 --- a/pkg/lib/operatorlister/lister.go +++ b/pkg/lib/operatorlister/lister.go @@ -1,6 +1,7 @@ package operatorlister import ( + aextv1 "k8s.io/apiextensions-apiserver/pkg/client/listers/apiextensions/v1" aextv1beta1 "k8s.io/apiextensions-apiserver/pkg/client/listers/apiextensions/v1beta1" appsv1 "k8s.io/client-go/listers/apps/v1" corev1 "k8s.io/client-go/listers/core/v1" @@ -19,6 +20,7 @@ type OperatorLister interface { RbacV1() RbacV1Lister APIRegistrationV1() APIRegistrationV1Lister APIExtensionsV1beta1() APIExtensionsV1beta1Lister + APIExtensionsV1() APIExtensionsV1Lister OperatorsV1alpha1() OperatorsV1alpha1Lister OperatorsV1() OperatorsV1Lister @@ -75,6 +77,12 @@ type APIExtensionsV1beta1Lister interface { CustomResourceDefinitionLister() aextv1beta1.CustomResourceDefinitionLister } +//go:generate counterfeiter . APIExtensionsV1Lister +type APIExtensionsV1Lister interface { + RegisterCustomResourceDefinitionLister(lister aextv1.CustomResourceDefinitionLister) + CustomResourceDefinitionLister() aextv1.CustomResourceDefinitionLister +} + //go:generate counterfeiter . OperatorsV1alpha1Lister type OperatorsV1alpha1Lister interface { RegisterClusterServiceVersionLister(namespace string, lister v1alpha1.ClusterServiceVersionLister) @@ -152,12 +160,22 @@ func newAPIRegistrationV1Lister() *apiRegistrationV1Lister { } type apiExtensionsV1beta1Lister struct { - customResourceDefinitionLister *UnionCustomResourceDefinitionLister + customResourceDefinitionLister *UnionCustomResourceDefinitionV1Beta1Lister +} + +type apiExtensionsV1Lister struct { + customResourceDefinitionLister *UnionCustomResourceDefinitionV1Lister } func newAPIExtensionsV1beta1Lister() *apiExtensionsV1beta1Lister { return &apiExtensionsV1beta1Lister{ - customResourceDefinitionLister: &UnionCustomResourceDefinitionLister{}, + customResourceDefinitionLister: &UnionCustomResourceDefinitionV1Beta1Lister{}, + } +} + +func newAPIExtensionsV1Lister() *apiExtensionsV1Lister { + return &apiExtensionsV1Lister{ + customResourceDefinitionLister: &UnionCustomResourceDefinitionV1Lister{}, } } @@ -196,9 +214,9 @@ type lister struct { rbacV1Lister *rbacV1Lister apiRegistrationV1Lister *apiRegistrationV1Lister apiExtensionsV1beta1Lister *apiExtensionsV1beta1Lister - - operatorsV1alpha1Lister *operatorsV1alpha1Lister - operatorsV1Lister *operatorsV1Lister + apiExtensionsV1Lister *apiExtensionsV1Lister + operatorsV1alpha1Lister *operatorsV1alpha1Lister + operatorsV1Lister *operatorsV1Lister } func (l *lister) AppsV1() AppsV1Lister { @@ -221,6 +239,10 @@ func (l *lister) APIExtensionsV1beta1() APIExtensionsV1beta1Lister { return l.apiExtensionsV1beta1Lister } +func (l *lister) APIExtensionsV1() APIExtensionsV1Lister { + return l.apiExtensionsV1Lister +} + func (l *lister) OperatorsV1alpha1() OperatorsV1alpha1Lister { return l.operatorsV1alpha1Lister } @@ -237,8 +259,8 @@ func NewLister() OperatorLister { rbacV1Lister: newRbacV1Lister(), apiRegistrationV1Lister: newAPIRegistrationV1Lister(), apiExtensionsV1beta1Lister: newAPIExtensionsV1beta1Lister(), - - operatorsV1alpha1Lister: newOperatorsV1alpha1Lister(), - operatorsV1Lister: newOperatorsV1Lister(), + apiExtensionsV1Lister: newAPIExtensionsV1Lister(), + operatorsV1alpha1Lister: newOperatorsV1alpha1Lister(), + operatorsV1Lister: newOperatorsV1Lister(), } } diff --git a/test/e2e/crd_upgrade_e2e_test.go b/test/e2e/crd_upgrade_e2e_test.go new file mode 100644 index 00000000000..d224ff95c24 --- /dev/null +++ b/test/e2e/crd_upgrade_e2e_test.go @@ -0,0 +1,132 @@ +package e2e + +import ( + "fmt" + "github.com/operator-framework/operator-lifecycle-manager/pkg/lib/operatorclient" + "k8s.io/apimachinery/pkg/util/wait" + "time" + + . "github.com/onsi/ginkgo" + "github.com/stretchr/testify/require" + apiexv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + apiexv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +var _ = Describe("CRD APIVersion upgrades", func() { + // This test specifies that the CRDs installed as part of an install plan change between APIVersions as expected. + // CRDs changed APIVersions from v1beta1 to v1 and OLM must support both versions. + // Upgrading from a v1beta1 to a v1 version of the same CRD should be seamless because the client always returns the latest version. + It("Handles CRD versioning changes as expected", func() { + defer cleaner.NotifyTestComplete(GinkgoT(), true) + log := func(s string) { + GinkgoT().Logf("%s: %s", time.Now().Format("15:04:05.9999"), s) + } + + c := newKubeClient(GinkgoT()) + //crc := newCRClient(GinkgoT()) + + crdPlural := genName("crd-") + oldv1beta1CRD := v1beta1crd(crdPlural) + require.NotEqual(GinkgoT(), "", testNamespace) + + + // create v1beta1 CRD on server + oldcrd, err := c.ApiextensionsInterface().ApiextensionsV1beta1().CustomResourceDefinitions().Create(&oldv1beta1CRD) + require.NoError(GinkgoT(), err) + log("created CRD") + + // poll for CRD to be ready (using the v1 client) + oldCRDConvertedtoV1, err := fetchCRD(GinkgoT(), c, oldcrd.Name, oldcrd.Namespace, checkCRD) + require.NoError(GinkgoT(), err) + + // confirm the v1 crd as is as expected + // run ensureCRDV1Versions on results + newCRD := v1crd(crdPlural) + err = ensureCRDV1Versions(oldCRDConvertedtoV1, &newCRD) + require.NoError(GinkgoT(), err) + }) +}) + +func v1beta1crd(name string) apiexv1beta1.CustomResourceDefinition { + return apiexv1beta1.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + Spec: apiexv1beta1.CustomResourceDefinitionSpec{ + Group: name + "group", + Version: "v1", + Names: apiexv1beta1.CustomResourceDefinitionNames{ + Kind: name, + }, + }, + } +} + +func v1crd(name string) apiexv1.CustomResourceDefinition { + return apiexv1.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + Spec: apiexv1.CustomResourceDefinitionSpec{ + Group: name + "group", + Versions: []apiexv1.CustomResourceDefinitionVersion{ + { + Name: "v1", + Served: true, + }, + }, + Names: apiexv1.CustomResourceDefinitionNames{ + Kind: name, + }, + }, + } +} + +func fetchCRD(t GinkgoTInterface, c operatorclient.ClientInterface, name string, namespace string, checkCRD checkCRDfunc) (*apiexv1.CustomResourceDefinition, error) { + var fetchedCRD *apiexv1.CustomResourceDefinition + var err error + + err = wait.Poll(pollInterval, pollDuration, func() (bool, error) { + fetchedCRD, err = c.ApiextensionsInterface().ApiextensionsV1().CustomResourceDefinitions().Get(name, metav1.GetOptions{}) + if err != nil || fetchedCRD == nil { + return false, err + } + + return checkCRD(fetchedCRD), nil + }) + return fetchedCRD, err +} + +type checkCRDfunc func(v1crd *apiexv1.CustomResourceDefinition) bool + +func checkCRD(v1crd *apiexv1.CustomResourceDefinition) bool { + if v1crd.Status.Conditions[0].Type == apiexv1.Established { + return true + } + return false +} + +// Ensure all existing served versions are present in new CRD +func ensureCRDV1Versions(oldCRD *apiexv1.CustomResourceDefinition, newCRD *apiexv1.CustomResourceDefinition) error { + newCRDVersions := getCRDV1VersionsMap(newCRD) + + for _, oldVersion := range oldCRD.Spec.Versions { + if oldVersion.Served { + _, ok := newCRDVersions[oldVersion.Name] + if !ok { + return fmt.Errorf("New CRD (%s) must contain existing served versions (%s)", oldCRD.Name, oldVersion.Name) + } + } + } + return nil +} + +func getCRDV1VersionsMap(crd *apiexv1.CustomResourceDefinition) map[string]struct{} { + versionsMap := map[string]struct{}{} + + for _, version := range crd.Spec.Versions { + versionsMap[version.Name] = struct{}{} + } + return versionsMap +} \ No newline at end of file diff --git a/test/e2e/csv_e2e_test.go b/test/e2e/csv_e2e_test.go index 755ddcd4473..80559e26225 100644 --- a/test/e2e/csv_e2e_test.go +++ b/test/e2e/csv_e2e_test.go @@ -3372,13 +3372,13 @@ func createCSV(t GinkgoTInterface, c operatorclient.ClientInterface, crc version func buildCRDCleanupFunc(c operatorclient.ClientInterface, crdName string) cleanupFunc { return func() { - err := c.ApiextensionsV1beta1Interface().ApiextensionsV1beta1().CustomResourceDefinitions().Delete(crdName, &metav1.DeleteOptions{GracePeriodSeconds: &immediateDeleteGracePeriod}) + err := c.ApiextensionsInterface().ApiextensionsV1beta1().CustomResourceDefinitions().Delete(crdName, &metav1.DeleteOptions{GracePeriodSeconds: &immediateDeleteGracePeriod}) if err != nil { fmt.Println(err) } waitForDelete(func() error { - _, err := c.ApiextensionsV1beta1Interface().ApiextensionsV1beta1().CustomResourceDefinitions().Get(crdName, metav1.GetOptions{}) + _, err := c.ApiextensionsInterface().ApiextensionsV1beta1().CustomResourceDefinitions().Get(crdName, metav1.GetOptions{}) return err }) } @@ -3410,7 +3410,7 @@ func createCRD(c operatorclient.ClientInterface, crd apiextensions.CustomResourc if err := scheme.Convert(&crd, out, nil); err != nil { return nil, err } - _, err := c.ApiextensionsV1beta1Interface().ApiextensionsV1beta1().CustomResourceDefinitions().Create(out) + _, err := c.ApiextensionsInterface().ApiextensionsV1beta1().CustomResourceDefinitions().Create(out) if err != nil { return nil, err } diff --git a/test/e2e/gc_e2e_test.go b/test/e2e/gc_e2e_test.go index 55e2ac44235..4481f0b3167 100644 --- a/test/e2e/gc_e2e_test.go +++ b/test/e2e/gc_e2e_test.go @@ -35,7 +35,7 @@ var _ = Describe("Garbage collector", func() { It("should delete a ClusterRole owned by a CustomResourceDefinition when the owner is deleted", func() { group := fmt.Sprintf("%s.com", rand.String(16)) - crd, err := kubeClient.ApiextensionsV1beta1Interface().ApiextensionsV1().CustomResourceDefinitions().Create(&apiextensionsv1.CustomResourceDefinition{ + crd, err := kubeClient.ApiextensionsInterface().ApiextensionsV1().CustomResourceDefinitions().Create(&apiextensionsv1.CustomResourceDefinition{ ObjectMeta: metav1.ObjectMeta{ Name: fmt.Sprintf("plural.%s", group), }, @@ -62,7 +62,7 @@ var _ = Describe("Garbage collector", func() { }) Expect(err).NotTo(HaveOccurred()) defer func() { - IgnoreError(kubeClient.ApiextensionsV1beta1Interface().ApiextensionsV1().CustomResourceDefinitions().Delete(crd.GetName(), &metav1.DeleteOptions{})) + IgnoreError(kubeClient.ApiextensionsInterface().ApiextensionsV1().CustomResourceDefinitions().Delete(crd.GetName(), &metav1.DeleteOptions{})) }() cr, err := kubeClient.CreateClusterRole(&rbacv1.ClusterRole{ @@ -76,7 +76,7 @@ var _ = Describe("Garbage collector", func() { IgnoreError(kubeClient.DeleteClusterRole(cr.GetName(), &metav1.DeleteOptions{})) }() - Expect(kubeClient.ApiextensionsV1beta1Interface().ApiextensionsV1().CustomResourceDefinitions().Delete(crd.GetName(), &metav1.DeleteOptions{})).To(Succeed()) + Expect(kubeClient.ApiextensionsInterface().ApiextensionsV1().CustomResourceDefinitions().Delete(crd.GetName(), &metav1.DeleteOptions{})).To(Succeed()) Eventually(func() bool { _, err := kubeClient.GetClusterRole(cr.GetName()) return k8serrors.IsNotFound(err) diff --git a/test/e2e/installplan_e2e_test.go b/test/e2e/installplan_e2e_test.go index 9757fd50db8..10159ab9952 100644 --- a/test/e2e/installplan_e2e_test.go +++ b/test/e2e/installplan_e2e_test.go @@ -1820,7 +1820,7 @@ var _ = Describe("Install Plan", func() { require.NoError(GinkgoT(), err) // Get the CRD to see if it is updated - fetchedCRD, err := c.ApiextensionsV1beta1Interface().ApiextensionsV1beta1().CustomResourceDefinitions().Get(crdName, metav1.GetOptions{}) + fetchedCRD, err := c.ApiextensionsInterface().ApiextensionsV1beta1().CustomResourceDefinitions().Get(crdName, metav1.GetOptions{}) require.NoError(GinkgoT(), err) require.Equal(GinkgoT(), len(fetchedCRD.Spec.Versions), len(updatedCRD.Spec.Versions), "The CRD versions counts don't match") @@ -1981,7 +1981,7 @@ var _ = Describe("Install Plan", func() { require.NoError(GinkgoT(), err) // Get the CRD to see if it is updated - fetchedCRD, err := c.ApiextensionsV1beta1Interface().ApiextensionsV1beta1().CustomResourceDefinitions().Get(crdName, metav1.GetOptions{}) + fetchedCRD, err := c.ApiextensionsInterface().ApiextensionsV1beta1().CustomResourceDefinitions().Get(crdName, metav1.GetOptions{}) require.NoError(GinkgoT(), err) require.Equal(GinkgoT(), len(fetchedCRD.Spec.Versions), len(mainCRD.Spec.Versions), "The CRD versions counts don't match") @@ -2575,7 +2575,7 @@ var _ = Describe("Install Plan", func() { // Make sure to clean up the installed CRD defer func() { - require.NoError(GinkgoT(), c.ApiextensionsV1beta1Interface().ApiextensionsV1beta1().CustomResourceDefinitions().Delete(dependentCRD.GetName(), deleteOpts)) + require.NoError(GinkgoT(), c.ApiextensionsInterface().ApiextensionsV1beta1().CustomResourceDefinitions().Delete(dependentCRD.GetName(), deleteOpts)) }() // ensure there is only one installplan @@ -2590,7 +2590,7 @@ type checkInstallPlanFunc func(fip *operatorsv1alpha1.InstallPlan) bool func validateCRDVersions(t GinkgoTInterface, c operatorclient.ClientInterface, name string, expectedVersions map[string]struct{}) { // Retrieve CRD information - crd, err := c.ApiextensionsV1beta1Interface().ApiextensionsV1beta1().CustomResourceDefinitions().Get(name, metav1.GetOptions{}) + crd, err := c.ApiextensionsInterface().ApiextensionsV1beta1().CustomResourceDefinitions().Get(name, metav1.GetOptions{}) require.NoError(t, err) require.Equal(t, len(expectedVersions), len(crd.Spec.Versions), "number of CRD versions don't not match installed") diff --git a/vendor/github.com/onsi/gomega/go.mod b/vendor/github.com/onsi/gomega/go.mod index 6fef041b3b6..65eedf6967a 100644 --- a/vendor/github.com/onsi/gomega/go.mod +++ b/vendor/github.com/onsi/gomega/go.mod @@ -1,7 +1,5 @@ module github.com/onsi/gomega -go 1.14 - require ( github.com/fsnotify/fsnotify v1.4.7 // indirect github.com/golang/protobuf v1.2.0