-
Notifications
You must be signed in to change notification settings - Fork 68
🐛 Fix testCatalogName conflict #2367
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
✅ Deploy Preview for olmv1 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
ad9ea17 to
18931ab
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 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
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
/lgtm |
|
/approve |
|
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) |
|
Anyway, fix this in this PR. |
|
/approve |
|
/retest |
rashmigottipati
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/approve
| // 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) | ||
| } |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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, |
There was a problem hiding this comment.
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: Catalogand 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: 1There was a problem hiding this comment.
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.
|
/lgtm |
34394ce
into
operator-framework:main
pedjak
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
|
[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 |
Description
Reviewer Checklist
Assisted-by: Claude Code