USHIFT-6748: [QE] Migrate OTP tests into RF: operators#6502
USHIFT-6748: [QE] Migrate OTP tests into RF: operators#6502agullon wants to merge 1 commit intoopenshift:mainfrom
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@agullon: This pull request references USHIFT-6748 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds OLM test assets and Robot Framework tests: new CatalogSource, Subscription, and OperatorGroup manifests for single- and all-namespace installs; updated Robot keywords and new tests validating install flows, OperatorGroup conflict detection/recovery, CSV/subscription polling, and NetworkPolicy assertions. Changes
Sequence Diagram(s)sequenceDiagram
participant Tester as Test Runner
participant API as Kubernetes API
participant OLM as OLM Controller
participant Operator as Operator Deployment
participant NP as NetworkPolicy Validator
Tester->>API: apply OperatorGroup, CatalogSource, Subscription
API->>OLM: create CatalogSource & Subscription resources
OLM-->>API: emit CatalogSource readiness events
OLM->>API: create InstallPlan / CSV
API->>Operator: create Deployment/Pods
Operator-->>OLM: update CSV status
Tester->>API: poll Subscription.status.currentCSV and verify CSV existence
Tester->>NP: validate NetworkPolicy selectors, types, and spec
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 10✅ Passed checks (10 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
openshift-ci
bot
added
the
do-not-merge/work-in-progress
|
@agullon: This pull request references USHIFT-6748 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: agullon The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/suites/optional/olm.robot`:
- Line 87: The VAR declaration currently creates a variable named "csv=" due to
the extra equals sign; change the variable declaration from using "VAR ${csv}=
${EMPTY}" to the correct Robot Framework syntax "VAR ${csv} ${EMPTY}" so that
subsequent usages of ${csv} (e.g., where ${csv} is referenced later in the test)
resolve correctly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: ddf25b2d-7445-4f3c-a2ec-45367c69010c
📒 Files selected for processing (7)
test/assets/olm/nginx-ok-catalog-source-all.yamltest/assets/olm/nginx-ok-catalog-source-single.yamltest/assets/olm/nginx-ok1-subscription.yamltest/assets/olm/nginx-ok2-subscription.yamltest/assets/olm/og-all.yamltest/assets/olm/og-single.yamltest/suites/optional/olm.robot
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
|
@agullon: This pull request references USHIFT-6748 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
@agullon: This pull request references USHIFT-6748 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
agullon
changed the title
|
@agullon: This pull request references USHIFT-6748 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
test/suites/optional/olm.robot (1)
352-359: Bulk CSV cleanup may affect parallel tests.
csv --all -n ${OPERATORS_NAMESPACE}deletes all CSVs in the namespace, which could interfere with other tests running concurrently. Consider scoping by label if parallel execution is expected.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/suites/optional/olm.robot` around lines 352 - 359, The bulk CSV deletion using "Oc Delete csv --all -n ${OPERATORS_NAMESPACE} --ignore-not-found" can remove CSVs from parallel tests; change the ELSE branch in the cleanup to avoid --all and instead target CSVs by a test-specific label or selector (e.g., use "Oc Delete csv -n ${OPERATORS_NAMESPACE} --selector <test-label>=<value> --ignore-not-found") or skip bulk deletion when ${csv} is empty; update the "Oc Delete" invocation and ensure the test applies a unique label to CSVs so the selector scopes deletions to this test only.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/suites/optional/olm.robot`:
- Line 57: The test calls the Robot keyword Wait For Deployments Deletion
without the required ${namespace} argument, so the teardown will fail; update
the call to pass the namespace as the first argument followed by the deployments
list (i.e., call Wait For Deployments Deletion ${namespace}
@{deployments}) so the keyword receives both the namespace and the deployments
to wait on.
---
Nitpick comments:
In `@test/suites/optional/olm.robot`:
- Around line 352-359: The bulk CSV deletion using "Oc Delete csv --all -n
${OPERATORS_NAMESPACE} --ignore-not-found" can remove CSVs from parallel tests;
change the ELSE branch in the cleanup to avoid --all and instead target CSVs by
a test-specific label or selector (e.g., use "Oc Delete csv -n
${OPERATORS_NAMESPACE} --selector <test-label>=<value> --ignore-not-found") or
skip bulk deletion when ${csv} is empty; update the "Oc Delete" invocation and
ensure the test applies a unique label to CSVs so the selector scopes deletions
to this test only.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 79a52d10-ef59-4b48-9b96-9eb60fe2c231
📒 Files selected for processing (7)
test/assets/olm/nginx-ok-catalog-source-all.yamltest/assets/olm/nginx-ok-catalog-source-single.yamltest/assets/olm/nginx-ok1-subscription.yamltest/assets/olm/nginx-ok2-subscription.yamltest/assets/olm/og-all.yamltest/assets/olm/og-single.yamltest/suites/optional/olm.robot
✅ Files skipped from review due to trivial changes (6)
- test/assets/olm/og-single.yaml
- test/assets/olm/og-all.yaml
- test/assets/olm/nginx-ok2-subscription.yaml
- test/assets/olm/nginx-ok-catalog-source-all.yaml
- test/assets/olm/nginx-ok1-subscription.yaml
- test/assets/olm/nginx-ok-catalog-source-single.yaml
|
@agullon: This pull request references USHIFT-6748 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
test/suites/optional/olm.robot (1)
262-267:⚠️ Potential issue | 🔴 Critical
Wait For Deployments Deletionsignature change breaks an existing caller.After adding
${namespace}to this keyword, the teardown call at Line 57 still passes only@{deployments}, which will fail at runtime due to argument mismatch.Proposed fix
- ... Wait For Deployments Deletion @{deployments} + ... Wait For Deployments Deletion ${OPERATORS_NAMESPACE} @{deployments}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/suites/optional/olm.robot` around lines 262 - 267, The keyword Wait For Deployments Deletion was changed to require a ${namespace}, breaking existing callers that only pass @{deployments}; make the namespace optional by changing the signature to accept a default (e.g. [Arguments] ${namespace}=${DEFAULT_NAMESPACE} @{deployments}) and update the body to use ${namespace} as before (Oc Wait -n ${namespace} ...). This preserves existing callers while allowing explicit namespaces when provided.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@test/suites/optional/olm.robot`:
- Around line 262-267: The keyword Wait For Deployments Deletion was changed to
require a ${namespace}, breaking existing callers that only pass @{deployments};
make the namespace optional by changing the signature to accept a default (e.g.
[Arguments] ${namespace}=${DEFAULT_NAMESPACE} @{deployments}) and update
the body to use ${namespace} as before (Oc Wait -n ${namespace} ...). This
preserves existing callers while allowing explicit namespaces when provided.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 1b4d4e84-80ee-465d-9913-3ef3ee5e3f86
📒 Files selected for processing (7)
test/assets/olm/nginx-ok-catalog-source-all.yamltest/assets/olm/nginx-ok-catalog-source-single.yamltest/assets/olm/nginx-ok1-subscription.yamltest/assets/olm/nginx-ok2-subscription.yamltest/assets/olm/og-all.yamltest/assets/olm/og-single.yamltest/suites/optional/olm.robot
✅ Files skipped from review due to trivial changes (6)
- test/assets/olm/og-all.yaml
- test/assets/olm/nginx-ok2-subscription.yaml
- test/assets/olm/nginx-ok-catalog-source-all.yaml
- test/assets/olm/og-single.yaml
- test/assets/olm/nginx-ok1-subscription.yaml
- test/assets/olm/nginx-ok-catalog-source-single.yaml
|
@agullon: This pull request references USHIFT-6748 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/suites/optional/olm.robot`:
- Around line 102-103: Increase the retry window for the CSV propagation check:
update the call to the Robot Framework keyword Wait Until Keyword Succeeds that
invokes CSV Should Exist In Namespace for ${csv} in the default namespace so the
first argument (total timeout) is 10m instead of 2m, keeping the retry interval
(10s) the same; locate the invocation of Wait Until Keyword Succeeds and the
nested CSV Should Exist In Namespace (${csv} default) and change only the
timeout value.
- Around line 355-357: Replace the dangerous global CSV deletion (the Oc Delete
call that uses "csv --all -n ${OPERATORS_NAMESPACE} --ignore-not-found") with a
scoped cleanup keyword such as Cleanup CSVs For Subscription that takes
${namespace} and ${subscription_name}, uses Oc Get JsonPath to list
clusterserviceversions (.items[*].metadata.name), Splits that result into
@{csv_list}, iterates the list and only calls Oc Delete for names that start
with "${subscription_name}." (use an IF with startswith) so only CSVs belonging
to that subscription are removed; update the test to call this keyword instead
of deleting all CSVs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: ad7db79d-1983-4393-852c-f048d313d189
📒 Files selected for processing (7)
test/assets/olm/nginx-ok-catalog-source-all.yamltest/assets/olm/nginx-ok-catalog-source-single.yamltest/assets/olm/nginx-ok1-subscription.yamltest/assets/olm/nginx-ok2-subscription.yamltest/assets/olm/og-all.yamltest/assets/olm/og-single.yamltest/suites/optional/olm.robot
✅ Files skipped from review due to trivial changes (6)
- test/assets/olm/og-all.yaml
- test/assets/olm/og-single.yaml
- test/assets/olm/nginx-ok2-subscription.yaml
- test/assets/olm/nginx-ok1-subscription.yaml
- test/assets/olm/nginx-ok-catalog-source-single.yaml
- test/assets/olm/nginx-ok-catalog-source-all.yaml
…twork policies Migrate three openshift-tests-private Ginkgo tests (69867, 69868, 83581) to MicroShift Robot Framework test suite: - Install Operator In Single Namespace Mode: creates a dedicated namespace with a SingleNamespace OperatorGroup and verifies nginx-ok1-1399 CSV installation and expected operator resources. - Install Operator In All Namespaces Mode With OperatorGroup Conflict: verifies MultipleOperatorGroupsFound error when two AllNamespaces OGs coexist, then confirms successful install after removing the extra OG. Also verifies CSV propagation to the default namespace. - OLM Network Policies Are Correctly Configured: validates the four NetworkPolicies deployed by MicroShift OLM (catalog-operator, default-deny-all-traffic, olm-operator, default-allow-all). Adds asset YAMLs for OperatorGroups, CatalogSources, and Subscriptions used by the new tests. Also fixes a pre-existing bug in Get CSV Name From Subscription where it ignored its arguments and always used hardcoded suite-level variables, and a pre-existing bug where Wait For Deployments Deletion was called without the required namespace argument. pre-commit.check-secrets: ENABLED
|
@agullon: This pull request references USHIFT-6748 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
@agullon: This pull request references USHIFT-6748 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
@agullon: This pull request references USHIFT-6748 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
@agullon: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
2 participants
Summary
Migrates three OLM Ginkgo tests from openshift-tests-private to MicroShift Robot Framework:
Also fixes pre-existing bugs in
Get CSV Name From Subscription(ignored arguments) andWait For Deployments Deletion(missing namespace argument).Companion PR removing the original Ginkgo tests: openshift/openshift-tests-private#29784
Test plan
test/suites/optional/olm.roboton a MicroShift instance with OLM installedJira: https://issues.redhat.com/browse/USHIFT-6748