Skip to content

Conversation

@jianzhangbjz
Copy link
Member

@jianzhangbjz jianzhangbjz commented Dec 1, 2025

Description

=== RUN   TestClusterExtensionVersionUpdate
    single_namespace_support_test.go:361: When a cluster extension is installed from a catalog
    single_namespace_support_test.go:362: When resolving upgrade edges
    helpers.go:229: 
        	Error Trace:	/home/runner/work/operator-controller/operator-controller/test/helpers/helpers.go:229
        	Error:      	Received unexpected error:
        	            	object is being deleted: clustercatalogs.olm.operatorframework.io "test-catalog" already exists
        	Test:       	TestClusterExtensionVersionUpdate
--- FAIL: TestClusterExtensionVersionUpdate (0.01s)

Reviewer Checklist

  • API Go Documentation
  • Tests: Unit Tests (and E2E Tests, if appropriate)
  • Comprehensive Commit Messages
  • Links to related GitHub Issue(s)

Assisted-by: Claude Code

@jianzhangbjz jianzhangbjz requested a review from a team as a code owner December 1, 2025 02:31
@openshift-ci openshift-ci bot requested review from oceanc80 and perdasilva December 1, 2025 02:31
@netlify
Copy link

netlify bot commented Dec 1, 2025

Deploy Preview for olmv1 ready!

Name Link
🔨 Latest commit 33414a7
🔍 Latest deploy log https://app.netlify.com/projects/olmv1/deploys/692e426dfb88490008f0a9ab
😎 Deploy Preview https://deploy-preview-2367--olmv1.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@jianzhangbjz jianzhangbjz force-pushed the e2e branch 3 times, most recently from ad9ea17 to 18931ab Compare December 1, 2025 04:24
@codecov
Copy link

codecov bot commented Dec 1, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.51%. Comparing base (b23e124) to head (33414a7).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2367      +/-   ##
==========================================
+ Coverage   70.58%   74.51%   +3.92%     
==========================================
  Files          93       93              
  Lines        7333     7333              
==========================================
+ Hits         5176     5464     +288     
+ Misses       1721     1435     -286     
+ Partials      436      434       -2     
Flag Coverage Δ
e2e 44.51% <ø> (+0.04%) ⬆️
experimental-e2e 48.78% <ø> (?)
unit 58.61% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dtfranz
Copy link
Contributor

dtfranz commented Dec 1, 2025

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Dec 1, 2025
@jianzhangbjz
Copy link
Member Author

/approve

@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Dec 1, 2025
@jianzhangbjz
Copy link
Member Author

It failed at the below, which I don't think this PR introduced it.

=== RUN   TestClusterExtensionRecoversFromExistingDeploymentWhenFailureFixed
    cluster_extension_install_test.go:679: When a cluster extension is installed from a catalog
    cluster_extension_install_test.go:680: When the extension bundle format is registry+v1
    helpers.go:263: Ensuring ClusterCatalog has Status.Condition of Progressing with a status == True and reason == Succeeded
    helpers.go:273: Checking that catalog has the expected metadata label
    helpers.go:278: Ensuring ClusterCatalog has Status.Condition of Type = Serving with status == True
    cluster_extension_install_test.go:703: By creating a new Deployment that can not be adopted
    cluster_extension_install_test.go:746: It resolves the specified package with correct bundle path
    cluster_extension_install_test.go:747: By creating the ClusterExtension resource
    cluster_extension_install_test.go:750: By eventually reporting Progressing == True with Reason Retrying
    cluster_extension_install_test.go:751: 
        	Error Trace:	/home/runner/work/operator-controller/operator-controller/test/e2e/cluster_extension_install_test.go:756
        	            				/opt/hostedtoolcache/go/1.24.6/x64/src/runtime/asm_amd64.s:1700
        	Error:      	Not equal: 
        	            	expected: "Retrying"
        	            	actual  : "RolloutInProgress"
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1 +1 @@
        	            	-Retrying
        	            	+RolloutInProgress
    cluster_extension_install_test.go:751: 
        	Error Trace:	/home/runner/work/operator-controller/operator-controller/test/e2e/cluster_extension_install_test.go:751
        	Error:      	Condition never satisfied
        	Test:       	TestClusterExtensionRecoversFromExistingDeploymentWhenFailureFixed
    helpers.go:321: By deleting ClusterCatalog "test-catalog-x8krz9gf"
    helpers.go:330: By deleting ClusterExtension "clusterextension-p8zl88ng"
    helpers.go:294: By waiting for CustomResourceDefinitions of "clusterextension-p8zl88ng" to be deleted
    helpers.go:302: By waiting for ClusterRoleBindings of "clusterextension-p8zl88ng" to be deleted
    helpers.go:310: By waiting for ClusterRoles of "clusterextension-p8zl88ng" to be deleted
    helpers.go:340: By deleting ServiceAccount "clusterextension-p8zl88ng"
    helpers.go:349: By deleting Namespace "clusterextension-p8zl88ng"
--- FAIL: TestClusterExtensionRecoversFromExistingDeploymentWhenFailureFixed (106.27s)

@jianzhangbjz
Copy link
Member Author

Anyway, fix this in this PR.

@jianzhangbjz jianzhangbjz changed the title 🐛 fix testCatalogName conflict 🐛 Fix testCatalogName conflict and Explicitly set CollisionProtection to Prevent in GenerateRevision Dec 1, 2025
@perdasilva
Copy link
Contributor

/approve

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 1, 2025
@jianzhangbjz
Copy link
Member Author

/retest

Copy link
Member

@rashmigottipati rashmigottipati left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/approve

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Dec 1, 2025
Comment on lines 78 to 81
// If the CRD doesn't exist (e.g., in non-experimental environments), skip this step silently
if !apierrors.IsNotFound(err) && !strings.Contains(err.Error(), "no matches for kind") {
fmt.Printf("Failed to list cluster extension revisions: %v\n", err)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this change need, it only reduces maybe a number of cases where this is printed out. BTW, we should log it, not print out directly on the console.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you're right. I will remove it.

objs = append(objs, ocv1.ClusterExtensionRevisionObject{
Object: unstr,
Object: unstr,
CollisionProtection: ocv1.CollisionProtectionPrevent,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this change is really not necessary, because in the CRD the default value is already and it going to applied by API server if missing from the payload. Server-side apply works like this:

  • patch with the values that controllers is interested to managed are sent to API server
  • API server applies the patch,
  • applies defaults for the missing fields,
  • perform validations
  • persist the resource in etcd.

I have deployed OLM from the latest main, and created simple test-extension:

apiVersion: olm.operatorframework.io/v1
kind: ClusterExtension
metadata:
  name: test-clusterextension
spec:
  namespace: foo
  serviceAccount:
    name: test-operator-installer-sa
  source:
    catalog:
      packageName: test
      selector:
        matchLabels:
          olm.operatorframework.io/metadata.name: test-catalog
      upgradeConstraintPolicy: SelfCertified
      version: 1.0.0
    sourceType: Catalog

and the created revision contains the default collision protection:

apiVersion: olm.operatorframework.io/v1
kind: ClusterExtensionRevision
metadata:
  annotations:
    olm.operatorframework.io/bundle-name: test-operator.1.0.0
    olm.operatorframework.io/bundle-reference: docker-registry.operator-controller-e2e.svc.cluster.local:5000/bundles/registry-v1/test-operator:v1.0.0
    olm.operatorframework.io/bundle-version: 1.0.0
    olm.operatorframework.io/package-name: test
  creationTimestamp: "2025-12-01T15:10:21Z"
  finalizers:
  - olm.operatorframework.io/teardown
  generation: 1
  labels:
    olm.operatorframework.io/owner-name: test-clusterextension
  name: test-clusterextension-1
spec:
  lifecycleState: Active
  phases:
  - name: policies
    objects:
    - collisionProtection: Prevent
      object:
        apiVersion: networking.k8s.io/v1
        kind: NetworkPolicy
        metadata:
          labels:
            olm.operatorframework.io/owner-kind: ClusterExtension
            olm.operatorframework.io/owner-name: test-clusterextension
          name: test-operator-network-policy
          namespace: foo
        spec:
          podSelector: {}
          policyTypes:
          - Ingress
  - name: rbac
    objects:
    - collisionProtection: Prevent
      object:
        apiVersion: v1
        kind: ServiceAccount
        metadata:
          labels:
            olm.operatorframework.io/owner-kind: ClusterExtension
            olm.operatorframework.io/owner-name: test-clusterextension
          name: simple-bundle-manager
          namespace: foo
    - collisionProtection: Prevent
      object:
        apiVersion: rbac.authorization.k8s.io/v1
        kind: ClusterRole
        metadata:
          labels:
            olm.operatorframework.io/owner-kind: ClusterExtension
            olm.operatorframework.io/owner-name: test-clusterextension
          name: testoperator.v1.0.-1eqned1rve17v8ggw7pxcudwuwxtutac7n2t3grh27tm
        rules:
        - apiGroups:
          - ""
          resources:
          - configmaps
          - serviceaccounts
          verbs:
          - get
          - list
          - watch
          - create
          - update
          - patch
          - delete
        - apiGroups:
          - networking.k8s.io
          resources:
          - networkpolicies
          verbs:
          - get
          - list
          - create
          - update
          - delete
        - apiGroups:
          - coordination.k8s.io
          resources:
          - leases
          verbs:
          - get
          - list
          - watch
          - create
          - update
          - patch
          - delete
        - apiGroups:
          - ""
          resources:
          - events
          verbs:
          - create
          - patch
        - apiGroups:
          - ""
          resources:
          - namespaces
          verbs:
          - get
          - list
          - watch
    - collisionProtection: Prevent
      object:
        apiVersion: rbac.authorization.k8s.io/v1
        kind: ClusterRole
        metadata:
          labels:
            olm.operatorframework.io/owner-kind: ClusterExtension
            olm.operatorframework.io/owner-name: test-clusterextension
          name: testoperator.v1.0.0-t88i5epjh8oxp4klplhjyrsekwcp92b27w03ayr1ku5
        rules:
        - apiGroups:
          - authentication.k8s.io
          resources:
          - tokenreviews
          verbs:
          - create
        - apiGroups:
          - authorization.k8s.io
          resources:
          - subjectaccessreviews
          verbs:
          - create
    - collisionProtection: Prevent
      object:
        apiVersion: rbac.authorization.k8s.io/v1
        kind: ClusterRoleBinding
        metadata:
          labels:
            olm.operatorframework.io/owner-kind: ClusterExtension
            olm.operatorframework.io/owner-name: test-clusterextension
          name: testoperator.v1.0.-1eqned1rve17v8ggw7pxcudwuwxtutac7n2t3grh27tm
        roleRef:
          apiGroup: rbac.authorization.k8s.io
          kind: ClusterRole
          name: testoperator.v1.0.-1eqned1rve17v8ggw7pxcudwuwxtutac7n2t3grh27tm
        subjects:
        - kind: ServiceAccount
          name: simple-bundle-manager
          namespace: foo
    - collisionProtection: Prevent
      object:
        apiVersion: rbac.authorization.k8s.io/v1
        kind: ClusterRoleBinding
        metadata:
          labels:
            olm.operatorframework.io/owner-kind: ClusterExtension
            olm.operatorframework.io/owner-name: test-clusterextension
          name: testoperator.v1.0.0-t88i5epjh8oxp4klplhjyrsekwcp92b27w03ayr1ku5
        roleRef:
          apiGroup: rbac.authorization.k8s.io
          kind: ClusterRole
          name: testoperator.v1.0.0-t88i5epjh8oxp4klplhjyrsekwcp92b27w03ayr1ku5
        subjects:
        - kind: ServiceAccount
          name: simple-bundle-manager
          namespace: foo
  - name: crds
    objects:
    - collisionProtection: Prevent
      object:
        apiVersion: apiextensions.k8s.io/v1
        kind: CustomResourceDefinition
        metadata:
          annotations:
            controller-gen.kubebuilder.io/version: v0.16.1
          labels:
            olm.operatorframework.io/owner-kind: ClusterExtension
            olm.operatorframework.io/owner-name: test-clusterextension
          name: olme2etests.olm.operatorframework.io
        spec:
          group: olm.operatorframework.io
          names:
            kind: OLME2ETest
            listKind: OLME2ETestList
            plural: olme2etests
            singular: olme2etest
          scope: Cluster
          versions:
          - name: v1
            schema:
              openAPIV3Schema:
                properties:
                  spec:
                    properties:
                      testField:
                        type: string
                    type: object
                type: object
            served: true
            storage: true
  - name: deploy
    objects:
    - collisionProtection: Prevent
      object:
        apiVersion: v1
        data:
          httpd.sh: |
            #!/bin/sh
            echo true > /var/www/started
            echo true > /var/www/ready
            echo true > /var/www/live
            exec httpd -f -h /var/www -p 80
        kind: ConfigMap
        metadata:
          labels:
            olm.operatorframework.io/owner-kind: ClusterExtension
            olm.operatorframework.io/owner-name: test-clusterextension
          name: httpd-script
          namespace: foo
    - collisionProtection: Prevent
      object:
        apiVersion: v1
        data:
          name: test-configmap
          version: v1.0.0
        kind: ConfigMap
        metadata:
          annotations:
            shouldNotTemplate: |
              The namespace is {{ $labels.namespace }}. The templated $labels.namespace is NOT expected to be processed by OLM's rendering engine for registry+v1 bundles.
          labels:
            olm.operatorframework.io/owner-kind: ClusterExtension
            olm.operatorframework.io/owner-name: test-clusterextension
          name: test-configmap
          namespace: foo
    - collisionProtection: Prevent
      object:
        apiVersion: apps/v1
        kind: Deployment
        metadata:
          labels:
            app.kubernetes.io/component: controller
            app.kubernetes.io/name: test-operator
            app.kubernetes.io/version: 1.0.0
            olm.operatorframework.io/owner-kind: ClusterExtension
            olm.operatorframework.io/owner-name: test-clusterextension
          name: test-operator
          namespace: foo
        spec:
          replicas: 1
          revisionHistoryLimit: 1
          selector:
            matchLabels:
              app: olme2etest
          strategy: {}
          template:
            metadata:
              annotations:
                alm-examples: |-
                  [
                    {
                      "apiVersion": "olme2etests.olm.operatorframework.io/v1",
                      "kind": "OLME2ETests",
                      "metadata": {
                        "labels": {
                          "app.kubernetes.io/managed-by": "kustomize",
                          "app.kubernetes.io/name": "test"
                        },
                        "name": "test-sample"
                      },
                      "spec": null
                    }
                  ]
                capabilities: Basic Install
                createdAt: "2024-10-24T19:21:40Z"
                olm.targetNamespaces: ""
                operators.operatorframework.io/builder: operator-sdk-v1.34.1
                operators.operatorframework.io/project_layout: go.kubebuilder.io/v4
              labels:
                app: olme2etest
            spec:
              containers:
              - command:
                - /scripts/httpd.sh
                image: busybox:1.36
                livenessProbe:
                  failureThreshold: 1
                  httpGet:
                    path: /live
                    port: 80
                  periodSeconds: 2
                name: busybox-httpd-container
                ports:
                - containerPort: 80
                readinessProbe:
                  httpGet:
                    path: /ready
                    port: 80
                  initialDelaySeconds: 1
                  periodSeconds: 1
                resources: {}
                startupProbe:
                  failureThreshold: 30
                  httpGet:
                    path: /started
                    port: 80
                  periodSeconds: 10
                volumeMounts:
                - mountPath: /scripts
                  name: scripts
                  readOnly: true
              serviceAccountName: simple-bundle-manager
              terminationGracePeriodSeconds: 0
              volumes:
              - configMap:
                  defaultMode: 493
                  name: httpd-script
                name: scripts
  revision: 1

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you're correct. I will revert it.

@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Dec 1, 2025
@jianzhangbjz jianzhangbjz changed the title 🐛 Fix testCatalogName conflict and Explicitly set CollisionProtection to Prevent in GenerateRevision 🐛 Fix testCatalogName conflict Dec 2, 2025
@perdasilva
Copy link
Contributor

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Dec 2, 2025
@jianzhangbjz jianzhangbjz requested a review from pedjak December 2, 2025 08:43
@openshift-merge-bot openshift-merge-bot bot merged commit 34394ce into operator-framework:main Dec 2, 2025
28 checks passed
Copy link
Contributor

@pedjak pedjak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@openshift-ci
Copy link

openshift-ci bot commented Dec 2, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jianzhangbjz, pedjak, perdasilva, rashmigottipati

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants