🐛 Verify pre-existing extension migration in upgrade E2E

test/e2e/steps/hooks.go

@@ -7,7 +7,6 @@ import (
 	"fmt"
 	"os/exec"
 	"path/filepath"
-	"regexp"
 	"strconv"
 	"strings"
 
@@ -16,6 +15,7 @@ import (
 	"github.com/spf13/pflag"
 	"golang.org/x/sync/errgroup"
 	appsv1 "k8s.io/api/apps/v1"
+	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/component-base/featuregate"
@@ -59,6 +59,9 @@ type scenarioContext struct {
 	extensionObjects     []client.Object
 	revisionSecrets      []resource
 	resourceUIDs         map[string]types.UID
+	upgradeObjects       []*unstructured.Unstructured
+	upgradePods          []corev1.Pod
+	upgradeStorageUIDs   map[string]types.UID
 	proxy                *recordingProxy
 	catalogAddr          string
 	catalogCleanup       func()
@@ -159,10 +162,17 @@ func BeforeSuite() {
 		featureGates[catalogdHAFeature] = true
 	}
 
+	if err := refreshOLMConfiguration(); err != nil {
+		logger.Info("OLM deployments not found; skipping feature gate detection (upgrade scenarios will install OLM in Background)")
+	}
+}
+
+// refreshOLMConfiguration must also run after installing or upgrading OLM:
+// extension resource discovery depends on the deployed runtime's feature gates.
+func refreshOLMConfiguration() error {
 	olm, catalogdDep, err := detectOLMDeployments()
 	if err != nil {
-		logger.Info("OLM deployments not found; skipping feature gate detection (upgrade scenarios will install OLM in Background)")
-		return
+		return err
 	}
 	olmNamespace = olm.Namespace
 	componentNamespaces["operator-controller"] = olmNamespace
@@ -185,22 +195,26 @@ func BeforeSuite() {
 		}
 	}
 
-	featureGatePattern := regexp.MustCompile(`--feature-gates=([[:alnum:]]+)=(true|false)`)
+	featureGates[features.BoxcutterRuntime] = false
 	for _, c := range olm.Spec.Template.Spec.Containers {
 		if c.Name == "manager" {
 			for _, arg := range c.Args {
-				if matches := featureGatePattern.FindStringSubmatch(arg); matches != nil {
-					v, err := strconv.ParseBool(matches[2])
-					if err != nil {
-						panic(fmt.Errorf("failed to parse feature gate %q=%q: %v", matches[1], matches[2], err))
+				if gates, found := strings.CutPrefix(arg, "--feature-gates="); found {
+					for _, gate := range strings.Split(gates, ",") {
+						name, value, _ := strings.Cut(gate, "=")
+						v, err := strconv.ParseBool(value)
+						if err != nil {
+							return fmt.Errorf("failed to parse feature gate %q: %w", gate, err)
+						}
+						featureGates[featuregate.Feature(name)] = v
 					}
-					featureGates[featuregate.Feature(matches[1])] = v
 				}
 			}
 		}
 	}
 
 	logger.Info(fmt.Sprintf("Enabled feature gates: %v", featureGates))
+	return nil
 }
 
 func CheckFeatureTags(ctx context.Context, sc *godog.Scenario) (context.Context, error) {

test/e2e/steps/steps.go

@@ -236,6 +236,11 @@ func RegisterSteps(sc *godog.ScenarioContext) {
 	// Upgrade-specific steps
 	sc.Step(`^(?i)the latest stable OLM release is installed$`, LatestStableOLMReleaseIsInstalled)
 	sc.Step(`^(?i)OLM is upgraded$`, OLMIsUpgraded)
+	sc.Step(`^(?i)Helm-backed extension resource identities are remembered$`, RememberHelmExtensionIdentities)
+	sc.Step(`^(?i)pre-existing ClusterExtension storage has transitioned after OLM upgrade$`, PreExistingExtensionStorageTransitioned)
+	sc.Step(`^(?i)updated ClusterExtension storage is healthy$`, UpdatedExtensionStorageIsHealthy)
+	sc.Step(`^(?i)extension resources keep their pre-upgrade identities$`, ExtensionKeepsPreUpgradeIdentities)
+	sc.Step(`^(?i)retained extension resources keep their pre-upgrade identities$`, RetainedExtensionKeepsPreUpgradeIdentities)
 	sc.Step(`^(?i)(catalogd|operator-controller) is ready to reconcile resources$`, ComponentIsReadyToReconcile)
 	sc.Step(`^(?i)all (ClusterCatalog|ClusterExtension) resources are reconciled$`, allResourcesAreReconciled)
 	sc.Step(`^(?i)ClusterExtension is reconciled$`, ClusterExtensionIsReconciled)
@@ -2555,11 +2560,18 @@ func helmReleaseFromSecret(secret *corev1.Secret) (*release.Release, error) {
 		return nil, err
 	}
 
-	var rel release.Release
-	if err = json.Unmarshal(releaseJsonBytes, &rel); err != nil {
+	// The chunked storage driver wraps release.Release because Helm excludes
+	// Labels from its JSON representation. Restore those labels explicitly so
+	// storage assertions can verify the persisted bundle metadata.
+	var wrapped struct {
+		release.Release
+		Labels map[string]string `json:"labels"`
+	}
+	if err = json.Unmarshal(releaseJsonBytes, &wrapped); err != nil {
 		return nil, err
 	}
-	return &rel, nil
+	wrapped.Release.Labels = wrapped.Labels
+	return &wrapped.Release, nil
 }
 
 // collectHelmReleaseObjects returns a slice of client.Object containing the manifests in rel

test/e2e/steps/upgrade_steps.go

@@ -8,42 +8,48 @@ import (
 	"os"
 	"os/exec"
 	"strings"
-	"sync"
 
+	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/wait"
 
 	catalogdv1 "github.com/operator-framework/operator-controller/api/v1"
+	"github.com/operator-framework/operator-controller/internal/operator-controller/features"
 )
 
+// waitForUpgradeCheck retains the last failed assertion, including controller
+// conditions, so migration failures report their cause rather than just a timeout.
+func waitForUpgradeCheck(ctx context.Context, check func() error) error {
+	var lastErr error
+	err := wait.PollUntilContextTimeout(ctx, tick, timeout, true, func(context.Context) (bool, error) {
+		lastErr = check()
+		return lastErr == nil, nil
+	})
+	if err != nil {
+		return fmt.Errorf("upgrade assertion timed out: %w: %v", err, lastErr)
+	}
+	return nil
+}
+
 // leaseNames maps component labels to their leader election lease names.
 var leaseNames = map[string]string{
 	"catalogd":            "catalogd-operator-lock",
 	"operator-controller": "9c4404e7.operatorframework.io",
 }
 
-var (
-	installOperator = sync.OnceValue(func() error {
-		err := runInstallScript("RELEASE_INSTALL")
-		if err != nil {
-			return err
-		}
-		olm, _, err := detectOLMDeployments()
-		if err != nil {
-			return err
-		}
-		olmNamespace = olm.Namespace
-		return nil
-	})
-	upgradeOperator = sync.OnceValue(func() error {
-		return runInstallScript("RELEASE_UPGRADE")
-	})
-)
-
 // LatestStableOLMReleaseIsInstalled downloads and executes the latest stable OLM release install script.
-// Uses sync.Once to ensure the install only happens once across multiple scenarios.
 func LatestStableOLMReleaseIsInstalled(_ context.Context) error {
-	return installOperator()
+	if err := runInstallScript("RELEASE_INSTALL"); err != nil {
+		return err
+	}
+	if err := refreshOLMConfiguration(); err != nil {
+		return err
+	}
+	if featureGates[features.BoxcutterRuntime] {
+		return fmt.Errorf("upgrade coverage requires an initial standard OLM installation")
+	}
+	return nil
 }
 
 func runInstallScript(envVar string) error {
@@ -72,9 +78,11 @@ func runInstallScript(envVar string) error {
 }
 
 // OLMIsUpgraded applies the locally built OLM manifest to upgrade OLM.
-// Uses sync.Once to ensure the upgrade only happens once across multiple scenarios.
-func OLMIsUpgraded(ctx context.Context) error {
-	return upgradeOperator()
+func OLMIsUpgraded(_ context.Context) error {
+	if err := runInstallScript("RELEASE_UPGRADE"); err != nil {
+		return err
+	}
+	return refreshOLMConfiguration()
 }
 
 // ComponentIsReadyToReconcile waits for the named component's deployment to be fully rolled out,
@@ -105,17 +113,55 @@ func ComponentIsReadyToReconcile(ctx context.Context, component string) error {
 	}
 
 	// Leader election can take up to LeaseDuration (137s) + RetryPeriod (26s) ≈ 163s in the worst case
-	waitFor(ctx, func() bool {
+	if err := waitForUpgradeCheck(ctx, func() error {
 		output, err := k8sClient(ctx, "get", "lease", leaseName, "-n", ns,
 			"-o", "jsonpath={.spec.holderIdentity}")
 		if err != nil || output == "" {
-			return false
+			return fmt.Errorf("waiting for %s leader lease: %s (%v)", component, output, err)
 		}
 		// Extract pod name from lease holder identity (format: <pod-name>_<suffix>)
 		podName := strings.Split(output, "_")[0]
+		// A lease left by the previous controller is insufficient. Require its
+		// holder to belong to the fully rolled-out deployment's current ReplicaSet.
+		podJSON, err := k8sClient(ctx, "get", "pod", podName, "-n", ns, "-o", "json")
+		if err != nil {
+			return fmt.Errorf("reading %s leader pod: %w", component, err)
+		}
+		var pod corev1.Pod
+		if err := json.Unmarshal([]byte(podJSON), &pod); err != nil {
+			return err
+		}
+		if !podReady(&pod) || metav1.GetControllerOf(&pod) == nil {
+			return fmt.Errorf("%s leader pod %s is not ready or has no controller", component, podName)
+		}
+		rsJSON, err := k8sClient(ctx, "get", "replicaset", metav1.GetControllerOf(&pod).Name, "-n", ns, "-o", "json")
+		if err != nil {
+			return err
+		}
+		depJSON, err := k8sClient(ctx, "get", "deployment", depName, "-n", ns, "-o", "json")
+		if err != nil {
+			return err
+		}
+		var rs appsv1.ReplicaSet
+		var dep appsv1.Deployment
+		if err := json.Unmarshal([]byte(rsJSON), &rs); err != nil {
+			return err
+		}
+		if err := json.Unmarshal([]byte(depJSON), &dep); err != nil {
+			return err
+		}
+		owner := metav1.GetControllerOf(&rs)
+		revision := dep.Annotations["deployment.kubernetes.io/revision"]
+		if owner == nil || owner.Kind != "Deployment" || owner.UID != dep.UID ||
+			metav1.GetControllerOf(&pod).Kind != "ReplicaSet" || metav1.GetControllerOf(&pod).UID != rs.UID ||
+			!generationObserved(dep.Generation, dep.Status.ObservedGeneration) || revision == "" || rs.Annotations["deployment.kubernetes.io/revision"] != revision {
+			return fmt.Errorf("%s lease holder %s is not from deployment revision %s", component, podName, revision)
+		}
 		sc.leaderPods[component] = podName
-		return true
-	})
+		return nil
+	}); err != nil {
+		return err
+	}
 
 	logger.V(1).Info("Component is ready to reconcile", "component", component, "leaderPod", sc.leaderPods[component])
 	return nil