cherry-pick(release-v1.3.x): Test upgrade SBO with OLM

.github/workflows/pr-checks-build-images.yaml

@@ -46,19 +46,23 @@ jobs:
           export TAG=pr-${PR_NUMBER}-${PR_SHA:0:8}
           export OPERATOR_IMAGE_REF=${OPERATOR_REPO_REF}:${TAG}
           export OPERATOR_BUNDLE_IMAGE_REF=${OPERATOR_IMAGE_REF}-bundle
-          export OPERATOR_INDEX_IMAGE_REF=${OPERATOR_IMAGE_REF}-index
+          export OPERATOR_INDEX_IMAGE_REF=$(make operator-index-image-ref)
+          export OPERATOR_UPGRADE_INDEX_IMAGE_REF=${OPERATOR_IMAGE_REF}-index
+          export OPERATOR_BUNDLE_VERSION=$(make operator-version)-${TAG}
 
           which podman
 
           make opm
 
-          OPM=.github/actions/setup-podman/opm BUILDAH_FORMAT=docker make SKIP_REGISTRY_LOGIN=true release-operator -o registry-login
+          podman pull ${OPERATOR_INDEX_IMAGE_REF}
+
+          OPM=.github/actions/setup-podman/opm BUILDAH_FORMAT=docker make SKIP_REGISTRY_LOGIN=true push-index-image-upgrade -o registry-login
 
           mkdir -p ${ARTIFACTS}
 
           echo "export OPERATOR_IMAGE_REF=${OPERATOR_IMAGE_REF}" >> ${ARTIFACTS}/operator.refs
           echo "export OPERATOR_BUNDLE_IMAGE_REF=${OPERATOR_BUNDLE_IMAGE_REF}" >> ${ARTIFACTS}/operator.refs
-          echo "export OPERATOR_INDEX_IMAGE_REF=${OPERATOR_INDEX_IMAGE_REF}" >> ${ARTIFACTS}/operator.refs
+          echo "export OPERATOR_INDEX_IMAGE_REF=${OPERATOR_UPGRADE_INDEX_IMAGE_REF}" >> ${ARTIFACTS}/operator.refs
 
           podman stop reg
           tar -czvf ${ARTIFACTS}/registry.tar.gz -C ${GITHUB_WORKSPACE} registry

.github/workflows/pr-checks.yaml

@@ -70,7 +70,7 @@ jobs:
     timeout-minutes: 90
 
     env:
-      EXTRA_BEHAVE_ARGS: "--tags=~@knative --tags=~@openshift --tags=~@examples --tags=~@supported-operator --tags=~@optional-annotations --tags=~@workload-resource-mapping --tags=~@disable-github-actions"
+      EXTRA_BEHAVE_ARGS: "--tags=~@knative --tags=~@openshift --tags=~@examples --tags=~@supported-operator --tags=~@optional-annotations --tags=~@workload-resource-mapping --tags=~@upgrade-with-olm --tags=~@disable-github-actions"
       TEST_RUN: Acceptance_tests_Kubernetes_with_OLM
 
     steps:

.gitignore

@@ -112,3 +112,5 @@ bundle
 bundle.Dockerfile
 config/crd/bases/_.yaml
 .cache
+
+service-binding-operator-index*

hack/upgrade-sbo-env.sh

@@ -0,0 +1,17 @@
+#!/bin/bash -x
+
+OUTPUT_DIR=${OUTPUT_DIR:-out}
+mkdir -p $OUTPUT_DIR
+
+export TEST_ACCEPTANCE_START_SBO=scenarios
+export TEST_OPERATOR_INDEX_IMAGE=${OPERATOR_INDEX_IMAGE_REF:-quay.io/redhat-developer/servicebinding-operator:index}
+export TEST_OPERATOR_CHANNEL=candidate
+operator_index_yaml=$OUTPUT_DIR/operator-index.yaml
+
+opm render ${TEST_OPERATOR_INDEX_IMAGE} -o yaml > $operator_index_yaml
+yq_exp='select(.schema=="olm.channel") | select(.name=="'${TEST_OPERATOR_CHANNEL}'").entries[] | select(.replaces == null).name'
+export TEST_OPERATOR_CSV=$(yq eval "$yq_exp" "$operator_index_yaml")
+yq_exp='select(.schema=="olm.channel") | select(.name=="'${TEST_OPERATOR_CHANNEL}'").package'
+export TEST_OPERATOR_PACKAGE=$(yq eval "$yq_exp" "$operator_index_yaml")
+
+env | grep TEST_OPERATOR

make/acceptance.mk

@@ -44,7 +44,7 @@ ifeq ($(TEST_ACCEPTANCE_START_SBO), local)
 test-acceptance-setup: stop-local build test-cleanup create-test-namespace deploy-test-3rd-party-crds
 	$(Q)echo "Starting local SBO instance"
 	$(eval TEST_ACCEPTANCE_SBO_STARTED := $(shell ZAP_FLAGS="$(ZAP_FLAGS)" OUTPUT="$(TEST_ACCEPTANCE_OUTPUT_DIR)" RUN_IN_BACKGROUND=true ./hack/deploy-sbo-local.sh))
-else ifeq ($(TEST_ACCEPTANCE_START_SBO), remote)
+else ifeq ($(TEST_ACCEPTANCE_START_SBO), $(filter $(TEST_ACCEPTANCE_START_SBO), remote scenarios))
 test-acceptance-setup: test-cleanup create-test-namespace
 else ifeq ($(TEST_ACCEPTANCE_START_SBO), operator-hub)
 test-acceptance-setup:

make/build.mk

@@ -26,7 +26,7 @@ manifests: controller-gen
 bundle: manifests kustomize yq kubectl-slice operator-sdk push-image
 #	$(OPERATOR_SDK) generate kustomize manifests -q
 	cd config/manager && $(KUSTOMIZE) edit set image controller=$(OPERATOR_REPO_REF)@$(OPERATOR_IMAGE_SHA_REF)
-	$(KUSTOMIZE) build config/manifests | $(OPERATOR_SDK) generate bundle -q --overwrite --version $(VERSION) $(BUNDLE_METADATA_OPTS)
+	$(KUSTOMIZE) build config/manifests | $(OPERATOR_SDK) generate bundle -q --overwrite --version $(OPERATOR_BUNDLE_VERSION) $(BUNDLE_METADATA_OPTS)
 	$(YQ) e -i '.metadata.annotations.containerImage="$(OPERATOR_REPO_REF)@$(OPERATOR_IMAGE_SHA_REF)"' bundle/manifests/service-binding-operator.clusterserviceversion.yaml
 	# this is needed because $(OPERATOR_SDK) 1.16 filters out aggregated cluster role and the accompanied binding
 	$(KUSTOMIZE) build config/manifests | $(YQ) e 'select((.kind == "ClusterRole" and .metadata.name == "service-binding-controller-role") or (.kind == "ClusterRoleBinding" and .metadata.name == "service-binding-controller-rolebinding"))' - | $(KUBECTL_SLICE) -o bundle/manifests -t '{{.metadata.name}}_{{.apiVersion | replace "/" "_"}}_{{.kind | lower}}.yaml'
@@ -67,10 +67,26 @@ index-image: opm push-bundle-image
 	@echo "package: $(CSV_PACKAGE_NAME)" >> $(OPERATOR_INDEX_YAML)
 	@echo "name: $(DEFAULT_OPERATOR_CHANNEL)" >> $(OPERATOR_INDEX_YAML)
 	@echo "entries:" >> $(OPERATOR_INDEX_YAML)
-	@echo "- name: $(CSV_PACKAGE_NAME).v$(VERSION)" >> $(OPERATOR_INDEX_YAML)
+	@echo "- name: $(CSV_PACKAGE_NAME).v$(OPERATOR_BUNDLE_VERSION)" >> $(OPERATOR_INDEX_YAML)
 	$(OPM) validate $(OPERATOR_INDEX_NAME)
 	$(CONTAINER_RUNTIME) build -f $(OPERATOR_INDEX_NAME).Dockerfile -t $(OPERATOR_INDEX_IMAGE_REF) .
 
+.PHONY: index-image-upgrade
+index-image-upgrade: OPERATOR_BUNDLE_VERSION ?= $(VERSION)-$(GIT_COMMIT_ID)
+index-image-upgrade: opm push-bundle-image
+	mkdir -p $(OPERATOR_INDEX_DIR)
+	-$(OPM) generate dockerfile $(OPERATOR_INDEX_NAME)
+	$(OPM) render $(OPERATOR_INDEX_IMAGE_REF) --output=yaml > $(OPERATOR_INDEX_YAML)
+	$(OPM) render $(OPERATOR_BUNDLE_IMAGE_REF) --output=yaml >> $(OPERATOR_INDEX_YAML)
+	$(YQ) eval -i '(select(.schema=="olm.channel").entries) += {"name": "$(CSV_PACKAGE_NAME).v$(OPERATOR_BUNDLE_VERSION)", "replaces": "'$$(yq eval 'select(.schema=="olm.channel") | select(.name=="$(DEFAULT_OPERATOR_CHANNEL)").entries[] | select(.replaces == null).name' $(OPERATOR_INDEX_YAML))'"}' $(OPERATOR_INDEX_YAML)
+	$(OPM) validate $(OPERATOR_INDEX_NAME)
+	$(CONTAINER_RUNTIME) build -f $(OPERATOR_INDEX_NAME).Dockerfile -t $(OPERATOR_UPGRADE_INDEX_IMAGE_REF) .
+
+.PHONY: push-index-image-upgrade
+# push upgrade index image
+push-index-image-upgrade: index-image-upgrade registry-login
+	$(Q)$(CONTAINER_RUNTIME) push $(OPERATOR_UPGRADE_INDEX_IMAGE_REF)
+
 .PHONY: push-index-image
 # push index image
 push-index-image: index-image registry-login

make/version.mk

@@ -10,8 +10,15 @@ OPERATOR_REGISTRY ?= quay.io
 OPERATOR_REPO_REF ?= $(OPERATOR_REGISTRY)/redhat-developer/servicebinding-operator
 OPERATOR_IMAGE_REF ?= $(OPERATOR_REPO_REF):$(GIT_COMMIT_ID)
 OPERATOR_IMAGE_SHA_REF ?= $(shell $(CONTAINER_RUNTIME) inspect --format='{{index .RepoDigests 0}}' $(OPERATOR_IMAGE_REF) | cut -f 2 -d '@')
+OPERATOR_BUNDLE_VERSION ?= $(VERSION)
 OPERATOR_BUNDLE_IMAGE_REF ?= $(OPERATOR_REPO_REF):bundle-$(VERSION)-$(GIT_COMMIT_ID)
 OPERATOR_INDEX_IMAGE_REF ?= $(OPERATOR_REPO_REF):index
+OPERATOR_UPGRADE_INDEX_IMAGE_REF ?= $(OPERATOR_REPO_REF):upgrade-index
+
+.PHONY: operator-version
+# Prints operator version
+operator-version:
+	@echo $(VERSION)
 
 .PHONY: operator-repo-ref
 # Prints operator repo ref

test/acceptance/features/environment.py

@@ -29,10 +29,7 @@
 
 def before_all(_context):
     if ctx.cli == "oc":
-        output, code = cmd.run("oc version --client | grep Client")
-        assert code == 0, f"Checking oc version failed: {output}"
-
-        oc_ver = output.split()[2]
+        oc_ver = ctx.cli_version
         assert semver.compare(oc_ver, "4.5.0") > 0, f"oc version is required 4.5+, but is {oc_ver}."
 
         namespace = os.getenv("TEST_NAMESPACE")
@@ -41,7 +38,7 @@ def before_all(_context):
 
     start_sbo = os.getenv("TEST_ACCEPTANCE_START_SBO")
     assert start_sbo is not None, "TEST_ACCEPTANCE_START_SBO is not set. It should be one of local, remote or operator-hub"
-    assert start_sbo in {"local", "remote", "operator-hub"}, "TEST_ACCEPTANCE_START_SBO should be one of local, remote or operator-hub"
+    assert start_sbo in {"local", "remote", "operator-hub", "scenarios"}, "TEST_ACCEPTANCE_START_SBO should be one of local, remote or operator-hub"
 
     if start_sbo == "local":
         assert not str(os.getenv("TEST_ACCEPTANCE_SBO_STARTED")).startswith("FAILED"), "TEST_ACCEPTANCE_SBO_STARTED shoud not be FAILED."
@@ -53,6 +50,8 @@ def before_all(_context):
         output = str(output).strip()
         assert output != "", "Unable to find SBO's deployment in any namespace."
         _context.sbo_namespace = output
+    elif start_sbo == "scenarios":
+        print("INFO: The scenarios are responsible for installing and running SBO...")
     else:
         assert False, f"TEST_ACCEPTANCE_START_SBO={start_sbo} is currently unsupported."
     ctx.no_scenarios_failed = True

test/acceptance/features/steps/environment.py

@@ -1,14 +1,29 @@
 import os
+from steps.command import Command
 
 
 class Environment(object):
     cli = "oc"
+    cli_version = None
 
-    def __init__(self, cli):
+    def __init__(self, cli, cli_version=None):
         self.cli = cli
+        self.cli_version = cli_version
 
 
 # This is a global context (complementing behave's context)
 # to be accesible from any place, even where behave's context is not available.
 global ctx
-ctx = Environment(os.getenv("TEST_ACCEPTANCE_CLI", "oc"))
+cmd = Command()
+cli = os.getenv("TEST_ACCEPTANCE_CLI", "oc")
+cli_version = None
+if cli == "oc":
+    output, code = cmd.run("oc version --client | grep Client")
+    assert code == 0, f"Checking {cli} version failed: {output}"
+    cli_version = output.split()[2]
+elif cli == "kubectl":
+    output, code = cmd.run("kubectl version -o json | jq -rc '.clientVersion.gitVersion'")
+    assert code == 0, f"Checking {cli} version failed: {output}"
+    cli_version = output.split(sep="v")[1]
+
+ctx = Environment(cli, cli_version)

test/acceptance/features/steps/olm.py

@@ -51,13 +51,22 @@ def install_catalog_source(self):
                 return False
         return self.openshift.wait_for_package_manifest(self.package_name, self.operator_catalog_source_name, self.operator_catalog_channel)
 
+    def csv_version_resolved(self, csv_version=None):
+        if csv_version is None:
+            if self.operator_subscription_csv_version is None:
+                return self.openshift.get_current_csv(self.package_name, self.operator_catalog_source_name, self.operator_catalog_channel)
+            else:
+                return self.operator_subscription_csv_version
+        else:
+            return csv_version
+
     def install_operator_subscription(self, csv_version=None, install_mode=InstallMode.Automatic):
-        csv_version_resolved = self.operator_subscription_csv_version if csv_version is None else csv_version
+        csv_version_resolved = self.csv_version_resolved(csv_version)
         install_sub_output = self.openshift.create_operator_subscription(
             self.package_name, self.operator_catalog_source_name, self.operator_catalog_channel, self.operator_catalog_namespace,
             csv_version_resolved, install_mode)
         if re.search(r'.*subscription.operators.coreos.com/%s\s(unchanged|created)' % self.package_name, install_sub_output) is None:
             print("Failed to create {} operator subscription".format(self.package_name))
             return False
-        self.openshift.approve_operator_subscription(self.package_name, csv_version_resolved)
+        self.openshift.approve_operator_subscription(self.package_name)
         return True

test/acceptance/features/steps/openshift.py

@@ -3,6 +3,7 @@
 import time
 import base64
 import json
+import semver
 from environment import ctx
 from command import Command
 from behave import step
@@ -411,26 +412,50 @@ def get_install_plan_for_subscription(self, subscription_name, subscription_name
 
         return install_plan.strip()
 
-    def approve_operator_subscription_in_namespace(self, name, namespace, csv_version=None):
+    def get_current_csv_for_subscription(self, subscription_name, subscription_namespace):
+        cmd = f"{ctx.cli} get subscription {subscription_name} -n {subscription_namespace} -o json | jq -rc '.status.currentCSV'"
+        (output, exit_code) = self.cmd.run(cmd)
+        assert exit_code == 0, f"Unable to get currentCSV for the '{subscription_name}' subscription:\n {output}"
+        return output.strip()
+
+    def get_installed_csv_for_subscription(self, subscription_name, subscription_namespace):
+        cmd = f"{ctx.cli} get subscription {subscription_name} -n {subscription_namespace} -o json | jq -rc '.status.installedCSV'"
+        (output, exit_code) = self.cmd.run(cmd)
+        assert exit_code == 0, f"Unable to get installedCSV for the '{subscription_name}' subscription:\n {output}"
+        return output.strip()
+
+    def approve_operator_subscription_in_namespace(self, name, namespace):
         # get the install plan
         install_plan = self.get_install_plan_for_subscription(name, namespace)
 
-        # patch CSV for install plan
-        if csv_version is not None:
-            print(f"Patching {install_plan} install plan for {csv_version} CSV")
-            patch = f'{{"spec": {{"clusterServiceVersionNames": ["{csv_version}"] }}}}'
-            cmd = f"{ctx.cli} patch installplan {install_plan} -n {namespace} -p '{patch}' --type=merge"
-            (output, exit_code) = self.cmd.run(cmd)
-            assert exit_code == 0, f"Unable to patch CSV version for '{install_plan}' install plan:\n {output}"
+        # get current CSV for subscription
+        csv_version = self.get_current_csv_for_subscription(name, namespace)
 
         # approve install plan
-        print(f"Approving {install_plan} install plan")
+        print(f"Approving {install_plan} install plan for {csv_version}")
         cmd = f'{ctx.cli} -n {namespace} patch installplan {install_plan} --type merge --patch \'{{"spec": {{"approved": true}}}}\''
         (output, exit_code) = self.cmd.run(cmd)
         assert exit_code == 0, f"Unable to patch the {install_plan} install plan to approve it:\n{output}"
 
-    def approve_operator_subscription(self, name, csv_version=None):
-        self.approve_operator_subscription_in_namespace(name, self.operators_namespace, csv_version)
+        # wait for install plan to complete
+        print(f"Waiting for {install_plan} install plan for {csv_version} to complete")
+        cmd = f'{ctx.cli} wait --for=condition=Installed=True InstallPlan/{install_plan} -n {namespace} --timeout=120s'
+        (output, exit_code) = self.cmd.run(cmd)
+        assert exit_code == 0, f"Unable to patch the {install_plan} install plan to approve it:\n{output}"
+
+        # wait for CSV to install successfully
+        print(f"Waiting for {csv_version} CSV to install successfully")
+        if ctx.cli == "oc" and semver.compare(ctx.cli_version, "4.10.0") < 0:
+            cmd = f"{ctx.cli} get csv/{csv_version} -n {namespace} -o jsonpath='{{.status.phase}}'"
+            (output, exit_code) = polling2.poll(target=lambda: tuple(self.cmd.run(cmd)), check_success=lambda o: o[1] == 0 and o[0].startswith(
+                "Succeeded"), step=5, timeout=120, ignore_exceptions=(ValueError,))
+        else:
+            cmd = f"{ctx.cli} wait --for=jsonpath='.status.phase'=Succeeded csv/{csv_version} -n {namespace} --timeout=120s"
+            (output, exit_code) = self.cmd.run(cmd)
+        assert exit_code == 0, f"Unable to install {csv_version} CSV successfully:\n{output}"
+
+    def approve_operator_subscription(self, name):
+        self.approve_operator_subscription_in_namespace(name, self.operators_namespace)
 
     def get_resource_list_in_namespace(self, resource_plural, name_pattern, namespace):
         print(f"Searching for {resource_plural} that matches {name_pattern} in {namespace} namespace")

test/acceptance/features/steps/servicebindingoperator.py

@@ -1,10 +1,14 @@
 import os
+from command import Command
+from steps.environment import ctx
+import polling2
 
 from openshift import Openshift
 
 
 class Servicebindingoperator():
     openshift = Openshift()
+    cmd = Command()
     name = ""
     namespace = ""
 
@@ -14,26 +18,51 @@ def __init__(self,  name="service-binding-operator", namespace="openshift-operat
         self.namespace = namespace
         self.name = name
 
-    def check_crd(self):
+    def check_crd(self, wait=False):
         crd_type = "crd"
         crd_name = "servicebindings.servicebinding.io"
-        assert self.openshift.is_resource_in(crd_type, crd_name), f"CRD '{crd_name}' does not exist"
-        return True
+        if wait:
+            return polling2.poll(target=lambda: self.openshift.is_resource_in(crd_type, crd_name),
+                                 step=5, timeout=400, ignore_exceptions=(ValueError,))
+        else:
+            return self.openshift.is_resource_in(crd_type, crd_name)
 
-    def check_deployment(self):
-        sbo_namespace = self.openshift.lookup_namespace_for_resource("deployments", "service-binding-operator")
+    def check_deployment(self, wait=False, csv_version=None):
+        if wait:
+            sbo_namespace = polling2.poll(
+                target=lambda: self.openshift.lookup_namespace_for_resource("deployments", "service-binding-operator"),
+                check_success=lambda o: o is not None, step=5, timeout=400, ignore_exceptions=(ValueError,))
+        else:
+            sbo_namespace = self.openshift.lookup_namespace_for_resource("deployments", "service-binding-operator")
+        if sbo_namespace is None:
+            return False
+
+        if csv_version is not None:
+            cmd = f"{ctx.cli} wait --for=jsonpath='{{.metadata.labels.olm\\.owner}}'={csv_version} \
+                deployment/service-binding-operator -n {sbo_namespace} --timeout=300s"
+            output, code = self.cmd.run(cmd)
+            assert code == 0, f"Non-zero return code while trying to check SBO deployment is owned by {csv_version} CSV: {output}"
+
+            cmd = f"{ctx.cli} wait --for=jsonpath='{{.metadata.ownerReferences[0].name}}'={csv_version} \
+                secrets/service-binding-operator-service-cert -n {sbo_namespace}"
+            output, code = polling2.poll(target=lambda: tuple(self.cmd.run(cmd)),
+                                         check_success=lambda o: o[1] == 0, step=5, timeout=300, ignore_exceptions=(ValueError,))
+            assert code == 0, f"Non-zero return code while trying to check SBO cert secret is owned by {csv_version} CSV: {output}"
+
+        output, code = self.cmd.run(f"{ctx.cli} rollout status -w deployment/service-binding-operator -n {sbo_namespace}")
+        assert code == 0, f"Non-zero return code while trying to SBO is healthy: {output}"
         return sbo_namespace is not None
 
-    def is_running(self):
+    def is_running(self, wait=False, csv_version=None):
         start_sbo = os.getenv("TEST_ACCEPTANCE_START_SBO")
         assert start_sbo is not None, "TEST_ACCEPTANCE_START_SBO is not set. It should be one of local, remote or operator-hub"
-        assert start_sbo in {"local", "remote", "operator-hub"}, "TEST_ACCEPTANCE_START_SBO should be one of local, remote or operator-hub"
+        assert start_sbo in {"local", "remote", "operator-hub", "scenarios"}, "TEST_ACCEPTANCE_START_SBO should be one of local, remote or operator-hub"
 
         if start_sbo == "local":
-            assert not os.getenv("TEST_ACCEPTANCE_SBO_STARTED").startswith("FAILED"), "TEST_ACCEPTANCE_SBO_STARTED shoud not be FAILED."
-            return self.check_crd()
-        elif start_sbo == "remote":
-            return self.check_crd() and self.check_deployment()
+            assert not os.getenv("TEST_ACCEPTANCE_SBO_STARTED").startswith("FAILED"), "TEST_ACCEPTANCE_SBO_STARTED should not be FAILED."
+            return self.check_crd(wait)
+        elif start_sbo == "remote" or start_sbo == "scenarios":
+            return self.check_crd(wait) and self.check_deployment(wait=wait, csv_version=csv_version)
         elif start_sbo == "operator-hub":
             return False