From b7cd9ba1ca56c25a5acc15f55151d674628d4023 Mon Sep 17 00:00:00 2001 From: Daniel Fan Date: Mon, 16 Sep 2024 14:56:48 -0700 Subject: [PATCH 1/2] Add packagemanifest CRD in test ENV Signed-off-by: Daniel Fan --- common/Makefile.common.mk | 3 + .../testutil/packagemanifests_crd.yaml | 164 ++++++++++++++++++ 2 files changed, 167 insertions(+) create mode 100644 controllers/testutil/packagemanifests_crd.yaml diff --git a/common/Makefile.common.mk b/common/Makefile.common.mk index fcc2796f..cea96080 100644 --- a/common/Makefile.common.mk +++ b/common/Makefile.common.mk @@ -82,6 +82,9 @@ fetch-test-crds: tar -zxf v${NAMESPACESCOPE_VERSION}.tar.gz ibm-namespace-scope-operator-${NAMESPACESCOPE_VERSION}/bundle/manifests && mv ibm-namespace-scope-operator-${NAMESPACESCOPE_VERSION}/bundle/manifests/operator.ibm.com_namespacescopes.yaml ${ENVCRDS_DIR}/operator.ibm.com_namespacescopes.yaml ;\ rm -rf ibm-namespace-scope-operator-${NAMESPACESCOPE_VERSION} v${NAMESPACESCOPE_VERSION}.tar.gz ;\ } + @{ \ + cp ./controllers/testutil/packagemanifests_crd.yaml ${ENVCRDS_DIR}/packagemanifests_crd.yaml ;\ + } CONTROLLER_GEN ?= $(shell pwd)/common/bin/controller-gen diff --git a/controllers/testutil/packagemanifests_crd.yaml b/controllers/testutil/packagemanifests_crd.yaml new file mode 100644 index 00000000..4931e9b1 --- /dev/null +++ b/controllers/testutil/packagemanifests_crd.yaml @@ -0,0 +1,164 @@ +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: packagemanifests.packages.operators.coreos.com +spec: + group: packages.operators.coreos.com + versions: + - name: v1 + served: true + storage: true + schema: + openAPIV3Schema: + type: object + properties: + spec: + type: object + status: + type: object + properties: + catalogSource: + type: string + catalogSourceDisplayName: + type: string + catalogSourcePublisher: + type: string + catalogSourceNamespace: + type: string + provider: + type: object + properties: + name: + type: string + url: + type: string + packageName: + type: string + deprecation: + type: object + properties: + message: + type: string + channels: + type: array + items: + type: object + properties: + name: + type: string + currentCSV: + type: string + currentCSVDesc: + type: object + properties: + displayName: + type: string + icon: + type: array + items: + type: object + properties: + base64data: + type: string + mediatype: + type: string + version: + type: object + properties: + version: + type: string + provider: + type: object + properties: + name: + type: string + url: + type: string + annotations: + type: object + additionalProperties: + type: string + keywords: + type: array + items: + type: string + links: + type: array + items: + type: object + properties: + name: + type: string + url: + type: string + maintainers: + type: array + items: + type: object + properties: + name: + type: string + email: + type: string + maturity: + type: string + longDescription: + type: string + installModes: + type: array + items: + type: object + properties: + type: + type: string + supported: + type: boolean + customResourceDefinitions: + type: object + apiServiceDefinitions: + type: object + nativeAPIs: + type: array + items: + type: object + properties: + group: + type: string + version: + type: string + kind: + type: string + minKubeVersion: + type: string + relatedImages: + type: array + items: + type: string + deprecation: + type: object + properties: + message: + type: string + entries: + type: array + items: + type: object + properties: + name: + type: string + version: + type: string + deprecation: + type: object + properties: + message: + type: string + defaultChannel: + type: string + names: + plural: packagemanifests + singular: packagemanifest + kind: PackageManifest + shortNames: + - pm + scope: Namespaced \ No newline at end of file From f1f61bfa1171abe5b83038803a0f83876ad73cf2 Mon Sep 17 00:00:00 2001 From: Daniel Fan Date: Mon, 16 Sep 2024 14:57:18 -0700 Subject: [PATCH 2/2] update test cases Signed-off-by: Daniel Fan --- .../operandrequest_controller.go | 5 ++- .../operandrequest/reconcile_operator.go | 2 +- .../operandrequest/reconcile_operator_test.go | 39 +++++++++++++++++++ controllers/operator/manager.go | 2 +- 4 files changed, 45 insertions(+), 3 deletions(-) diff --git a/controllers/operandrequest/operandrequest_controller.go b/controllers/operandrequest/operandrequest_controller.go index 0f82ae56..29479fc5 100644 --- a/controllers/operandrequest/operandrequest_controller.go +++ b/controllers/operandrequest/operandrequest_controller.go @@ -103,6 +103,9 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Re if err := r.Client.Status().Patch(ctx, requestInstance, client.MergeFrom(existingInstance)); err != nil && !apierrors.IsNotFound(err) { reconcileErr = utilerrors.NewAggregate([]error{reconcileErr, fmt.Errorf("error while patching OperandRequest.Status: %v", err)}) } + if reconcileErr != nil { + klog.Errorf("failed to patch status for OperandRequest %s: %v", req.NamespacedName.String(), reconcileErr) + } }() // Remove finalizer when DeletionTimestamp none zero @@ -180,7 +183,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Re } klog.V(1).Infof("Finished reconciling OperandRequest: %s", req.NamespacedName) - return ctrl.Result{RequeueAfter: constant.DefaultSyncPeriod}, nil + return ctrl.Result{RequeueAfter: constant.DefaultSyncPeriod}, reconcileErr } func (r *Reconciler) checkPermission(ctx context.Context, req ctrl.Request) bool { diff --git a/controllers/operandrequest/reconcile_operator.go b/controllers/operandrequest/reconcile_operator.go index 459edf8c..d53c3368 100644 --- a/controllers/operandrequest/reconcile_operator.go +++ b/controllers/operandrequest/reconcile_operator.go @@ -763,7 +763,7 @@ func checkSubAnnotationsForUninstall(reqName, reqNs, opName, installMode string, uninstallOperator = false } - if _, ok := sub.Labels[constant.OpreqLabel]; !ok { + if value, ok := sub.Labels[constant.OpreqLabel]; !ok || value != "true" { uninstallOperator = false } diff --git a/controllers/operandrequest/reconcile_operator_test.go b/controllers/operandrequest/reconcile_operator_test.go index 35ef023e..3232d703 100644 --- a/controllers/operandrequest/reconcile_operator_test.go +++ b/controllers/operandrequest/reconcile_operator_test.go @@ -174,6 +174,9 @@ func TestCheckSubAnnotationsForUninstall(t *testing.T) { reqNsNew + "." + reqNameB + "." + opNameV4 + "/request": opChannelV4, reqNsNew + "." + reqNameB + "." + opNameV4 + "/operatorNamespace": reqNsNew, }, + Labels: map[string]string{ + constant.OpreqLabel: "true", + }, }, } @@ -198,6 +201,9 @@ func TestCheckSubAnnotationsForUninstall(t *testing.T) { reqNsNew + "." + reqNameB + "." + opNameV4 + "/request": opChannelV4, reqNsNew + "." + reqNameB + "." + opNameV4 + "/operatorNamespace": reqNsNew, }, + Labels: map[string]string{ + constant.OpreqLabel: "true", + }, }, } @@ -224,6 +230,9 @@ func TestCheckSubAnnotationsForUninstall(t *testing.T) { reqNsOld + "." + reqNameD + "." + opNameD + "/request": opChannelD, reqNsOld + "." + reqNameD + "." + opNameD + "/operatorNamespace": reqNsOld, }, + Labels: map[string]string{ + constant.OpreqLabel: "true", + }, }, } @@ -250,6 +259,9 @@ func TestCheckSubAnnotationsForUninstall(t *testing.T) { reqNsOld + "." + reqNameB + "." + opNameV4 + "/request": opChannelV4, reqNsOld + "." + reqNameB + "." + opNameV4 + "/operatorNamespace": reqNsOld, }, + Labels: map[string]string{ + constant.OpreqLabel: "true", + }, }, } @@ -275,6 +287,9 @@ func TestCheckSubAnnotationsForUninstall(t *testing.T) { reqNsNew + "." + reqNameB + "." + opNameV4 + "/request": opChannelV4, reqNsNew + "." + reqNameB + "." + opNameV4 + "/operatorNamespace": reqNsNew, }, + Labels: map[string]string{ + constant.OpreqLabel: "true", + }, }, } @@ -287,4 +302,28 @@ func TestCheckSubAnnotationsForUninstall(t *testing.T) { assert.NotContains(t, sub.Annotations, reqNsNew+"."+reqNameA+"."+opNameV4+"/operatorNamespace") assert.Contains(t, sub.Annotations, reqNsNew+"."+reqNameB+"."+opNameV4+"/request") assert.Contains(t, sub.Annotations, reqNsNew+"."+reqNameB+"."+opNameV4+"/operatorNamespace") + + // Test case 8: uninstallOperator is false, uninstallOperand is true for operator with general installMode + // The operator should be NOT uninstalled because operator is not been managed by ODLM. + // The operand should be uninstalled because no other OperandRequest is requesting the same operand. + sub = &olmv1alpha1.Subscription{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: reqNsOld, + Annotations: map[string]string{ + reqNsOld + "." + reqNameA + "." + opNameV4 + "/request": opChannelV4, + reqNsOld + "." + reqNameA + "." + opNameV4 + "/operatorNamespace": reqNsOld, + }, + Labels: map[string]string{ + constant.OpreqLabel: "false", + }, + }, + } + + uninstallOperator, uninstallOperand = checkSubAnnotationsForUninstall(reqNameA, reqNsOld, opNameV4, operatorv1alpha1.InstallModeNamespace, sub) + + assert.False(t, uninstallOperator) + assert.True(t, uninstallOperand) + + assert.NotContains(t, sub.Annotations, reqNsOld+"."+reqNameA+"."+opNameV4+"/request") + assert.NotContains(t, sub.Annotations, reqNsOld+"."+reqNameA+"."+opNameV4+"/operatorNamespace") } diff --git a/controllers/operator/manager.go b/controllers/operator/manager.go index 7c0cab6b..6cb646fd 100644 --- a/controllers/operator/manager.go +++ b/controllers/operator/manager.go @@ -154,7 +154,7 @@ func (m *ODLMOperator) GetCatalogSourceAndChannelFromPackage(ctx context.Context switch number { case 0: klog.V(2).Infof("Not found PackageManifest %s in the namespace %s has channel %s", packageName, namespace, channel) - return "", "", "", nil + return opregCatalog, opregCatalogNs, channel, nil default: // Check if the CatalogSource and CatalogSource namespace are specified in OperandRegistry if opregCatalog != "" && opregCatalogNs != "" {