Mark openshift & xKS e2e tests #1216
Open
anandrkskd wants to merge 10 commits into
Open
Conversation
|
Skipping CI for Draft Pull Request. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
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:
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR aligns the Ginkgo CLI with ChangesOpenShift E2E test and tooling updates
Estimated code review effort: 3 (Moderate) | ~25 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test/openshift/e2e/ginkgo/parallel/1-031_validate_toolchain_test.go (1)
134-165: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAdd
Label("openshift")to this spec This test usesRouteandoc, so it should be excluded from non-OpenShift runs; otherwise--label-filter="!openshift"can still pick it up and fail on xKS.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/openshift/e2e/ginkgo/parallel/1-031_validate_toolchain_test.go` around lines 134 - 165, Add the Ginkgo Label("openshift") to the spec containing the Route and oc-based toolchain version checks, ensuring it is excluded when the !openshift label filter is used. Keep the existing test logic unchanged.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/openshift/e2e/ginkgo/fixture/fixture.go`:
- Around line 174-175: Move the custom-argocd-role deletion in the fixture
cleanup flow before the OpenShift-specific gate, near the setup preceding line
114. Keep deletion of this standard ClusterRole unconditional so xKS runs also
remove stale state, while leaving only OpenShift-specific resource cleanup
inside the gate.
- Around line 996-1009: Update IsOperatorRunningOnOLM to determine whether the
tested operator is OLM-managed by inspecting its relevant Subscription/CSV or
ownership chain, rather than counting cluster-wide CRDs whose group contains
operators.coreos.com. Preserve the false result for manually deployed operators
and return true only when the operator’s own resources demonstrate OLM
ownership.
In `@test/openshift/e2e/ginkgo/parallel/1-031_validate_toolchain_test.go`:
- Around line 93-104: After creating the ArgoCD resource in the validation
setup, wait until it reports Available and its server endpoint is ready before
invoking argocd version --server. Update the readiness flow around getPodName
and serverRoute to check the Argo CD status condition, not merely pod Running
state or route existence, while preserving the existing version-check behavior
once availability is confirmed.
In `@test/openshift/e2e/ginkgo/parallel/1-050_validate_sso_test.go`:
- Line 70: Apply the existing "openshift" label to the sibling SSO test at the
second It block, preferably by labeling the surrounding Context so both specs
that set OpenShiftOAuth: true are excluded by --label-filter="!openshift".
In
`@test/openshift/e2e/ginkgo/parallel/1-055_drop_resource_customizations_test.go`:
- Line 49: Remove the "openshift" label from the test case in the It block for
resource customization conversion, or replace it with the appropriate
conversion-webhook capability label if one exists. Keep the test classified as a
general namespace-scoped Argo CD v1alpha1-to-v1beta1 conversion test.
In
`@test/openshift/e2e/ginkgo/parallel/1-069_validate_redis_secure_comm_autotls_ha_test.go`:
- Line 61: Align labels with platform requirements across the three tests: in
test/openshift/e2e/ginkgo/parallel/1-069_validate_redis_secure_comm_autotls_ha_test.go:61,
add Label("openshift") to the HA AutoTLS test; in
test/openshift/e2e/ginkgo/parallel/1-082_validate_node_placement_test.go:52,
remove Label("openshift"); and in
test/openshift/e2e/ginkgo/parallel/1-084_validate_status_host_ingress_test.go:50,
remove Label("openshift") while preserving the existing RunningOnOpenShift()
guard.
In
`@test/openshift/e2e/ginkgo/parallel/1-103_argocd_alpha_to_beta_conversion_test.go`:
- Line 49: Update the spec declaration in the alpha-to-beta conversion test to
include the conversionWebhook label alongside openshift, ensuring it is selected
by conversionWebhook-filtered runs like the neighboring conversion specs.
In
`@test/openshift/e2e/ginkgo/sequential/1-037_validate_applicationset_in_any_namespace_test.go`:
- Line 69: Remove the openshift label from the affected portable tests: the
ApplicationSet RBAC test in
test/openshift/e2e/ginkgo/sequential/1-037_validate_applicationset_in_any_namespace_test.go:69,
the image policy test in
test/openshift/e2e/ginkgo/sequential/1-114_validate_imagepullpolicy_test.go:52,
the missing-Secret CA-trust test in
test/openshift/e2e/ginkgo/sequential/1-120_repo_server_system_ca_trust.go:103,
and the namespace validation test in
test/openshift/e2e/ginkgo/sequential/1-122_validate_namespace_test.go:74. Apply
the same label removal to any additional affected ranges covered by the per-file
comments.
In
`@test/openshift/e2e/ginkgo/sequential/1-051_validate_argocd_agent_principal_test.go`:
- Line 542: Apply the openshift label to the adjacent route-toggle Ginkgo spec
that enables, disables, and recreates an OpenShift Route, matching the label on
“should handle route enabled configuration correctly.” Ensure route-dependent
behavior is skipped when the label filter excludes OpenShift.
---
Outside diff comments:
In `@test/openshift/e2e/ginkgo/parallel/1-031_validate_toolchain_test.go`:
- Around line 134-165: Add the Ginkgo Label("openshift") to the spec containing
the Route and oc-based toolchain version checks, ensuring it is excluded when
the !openshift label filter is used. Keep the existing test logic unchanged.
🪄 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), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 2bff0d3c-c021-470a-89cb-c31c0e40a0aa
📒 Files selected for processing (74)
Makefiletest/e2e/argocd_route_test.gotest/openshift/e2e/ginkgo/fixture/fixture.gotest/openshift/e2e/ginkgo/parallel/1-001_alpha_to_beta_dex_conversion_test.gotest/openshift/e2e/ginkgo/parallel/1-003_alpha_to_beta_sso_conflict_conversion_test.gotest/openshift/e2e/ginkgo/parallel/1-003_validate_console_link_test.gotest/openshift/e2e/ginkgo/parallel/1-004_beta_to_alpha_conversion_test.gotest/openshift/e2e/ginkgo/parallel/1-005_validate_metrics_test.gotest/openshift/e2e/ginkgo/parallel/1-005_validate_route_tls_test.gotest/openshift/e2e/ginkgo/parallel/1-023_validate_repo_server_tls_test.gotest/openshift/e2e/ginkgo/parallel/1-030_validate_reencrypt_test.gotest/openshift/e2e/ginkgo/parallel/1-031_validate_toolchain_test.gotest/openshift/e2e/ginkgo/parallel/1-041_validate_argocd_sync_alert_test.gotest/openshift/e2e/ginkgo/parallel/1-042_validate_status_host_test.gotest/openshift/e2e/ginkgo/parallel/1-050_validate_sso_test.gotest/openshift/e2e/ginkgo/parallel/1-053_validate_cluster_admin_rbac_test.gotest/openshift/e2e/ginkgo/parallel/1-054_validate_deploymentconfig_test.gotest/openshift/e2e/ginkgo/parallel/1-055_drop_resource_customizations_test.gotest/openshift/e2e/ginkgo/parallel/1-063_validate_dex_liveness_probe_test.gotest/openshift/e2e/ginkgo/parallel/1-065_validate_redis_ha_anti_affinity_test.gotest/openshift/e2e/ginkgo/parallel/1-067_validate_redis_secure_comm_no_autotls_ha_test.gotest/openshift/e2e/ginkgo/parallel/1-069_validate_redis_secure_comm_autotls_ha_test.gotest/openshift/e2e/ginkgo/parallel/1-081_validate_applicationset_deployment_test.gotest/openshift/e2e/ginkgo/parallel/1-082_validate_node_placement_test.gotest/openshift/e2e/ginkgo/parallel/1-084_validate_status_host_ingress_test.gotest/openshift/e2e/ginkgo/parallel/1-096-validate_home_env_argocd_controller_test.gotest/openshift/e2e/ginkgo/parallel/1-103_argocd_alpha_to_beta_conversion_test.gotest/openshift/e2e/ginkgo/parallel/1-109_validate_reencrypt_termination_policy_test.gotest/openshift/e2e/ginkgo/parallel/1-125_validate_server_serving_cert_annotation_restore_test.gotest/openshift/e2e/ginkgo/parallel/1-126_validate_servicemonitor_metrics_config_test.gotest/openshift/e2e/ginkgo/sequential/1-002-validate_backend_service_test.gotest/openshift/e2e/ginkgo/sequential/1-004_validate_argocd_installation_test.gotest/openshift/e2e/ginkgo/sequential/1-006_validate_machine_config_test.gotest/openshift/e2e/ginkgo/sequential/1-008_validate-4.9CI-Failures_test.gotest/openshift/e2e/ginkgo/sequential/1-010_validate-ootb-manage-other-namespace_test.gotest/openshift/e2e/ginkgo/sequential/1-018_validate_disable_default_instance_test.gotest/openshift/e2e/ginkgo/sequential/1-020_validate_redis_ha_nonha_test.gotest/openshift/e2e/ginkgo/sequential/1-026-validate_backend_service_permissions_test.gotest/openshift/e2e/ginkgo/sequential/1-027_validate_operand_from_git_test.gotest/openshift/e2e/ginkgo/sequential/1-028-validate_run_on_infra_test.gotest/openshift/e2e/ginkgo/sequential/1-035_validate_argocd_secret_repopulate_test.gotest/openshift/e2e/ginkgo/sequential/1-036_validate_role_rolebinding_for_source_namespace_test.gotest/openshift/e2e/ginkgo/sequential/1-037_validate_applicationset_in_any_namespace_test.gotest/openshift/e2e/ginkgo/sequential/1-040_validate_quoted_RBAC_group_names_test.gotest/openshift/e2e/ginkgo/sequential/1-051_validate_argocd_agent_principal_test.gotest/openshift/e2e/ginkgo/sequential/1-052_validate_argocd_agent_agent_test.gotest/openshift/e2e/ginkgo/sequential/1-052_validate_rolebinding_number_test.gotest/openshift/e2e/ginkgo/sequential/1-053_validate_argocd_agent_principal_connected_test.gotest/openshift/e2e/ginkgo/sequential/1-058_validate_notifications_source_namespaces_test.gotest/openshift/e2e/ginkgo/sequential/1-059_validate_argocd_agent_terminal_streaming_test.gotest/openshift/e2e/ginkgo/sequential/1-064_validate_tcp_reset_error_test.gotest/openshift/e2e/ginkgo/sequential/1-071_validate_SCC_HA_test.gotest/openshift/e2e/ginkgo/sequential/1-071_validate_node_selectors_test.gotest/openshift/e2e/ginkgo/sequential/1-074_validate_terminating_namespace_block_test.gotest/openshift/e2e/ginkgo/sequential/1-083_validate_apps_in_any_namespace_test.gotest/openshift/e2e/ginkgo/sequential/1-084_validate_prune_templates.gotest/openshift/e2e/ginkgo/sequential/1-086_validate_default_argocd_role_test.gotest/openshift/e2e/ginkgo/sequential/1-092_validate_workload_status_monitoring_alert.gotest/openshift/e2e/ginkgo/sequential/1-100_validate_rollouts_resources_creation_test.gotest/openshift/e2e/ginkgo/sequential/1-101_validate_rollout_policyrules_test.gotest/openshift/e2e/ginkgo/sequential/1-102_validate_handle_terminating_namespaces_test.gotest/openshift/e2e/ginkgo/sequential/1-103-validate-rollouts-imagepullpolicy.gotest/openshift/e2e/ginkgo/sequential/1-105_validate_default_argocd_route_test.gotest/openshift/e2e/ginkgo/sequential/1-106_validate_argocd_metrics_controller_test.gotest/openshift/e2e/ginkgo/sequential/1-107_validate_redis_scc_test.gotest/openshift/e2e/ginkgo/sequential/1-110_validate_podsecurity_alerts_test.gotest/openshift/e2e/ginkgo/sequential/1-111_validate_default_argocd_route_test.gotest/openshift/e2e/ginkgo/sequential/1-112_validate_rollout_plugin_support_test.gotest/openshift/e2e/ginkgo/sequential/1-114_validate_imagepullpolicy_test.gotest/openshift/e2e/ginkgo/sequential/1-120_repo_server_system_ca_trust.gotest/openshift/e2e/ginkgo/sequential/1-120_validate_running_must_gather.gotest/openshift/e2e/ginkgo/sequential/1-121-valiate_resource_constraints_gitopsservice_test.gotest/openshift/e2e/ginkgo/sequential/1-122_validate_namespace_test.gotest/openshift/e2e/ginkgo/sequential/1-123_validate_list_order_comparison_test.go
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
argoproj-labs/argocd-operator(manual)
Comment on lines
+996
to
+1009
| // IsOperatorRunningOnOLM returns true if the operator is running on OLM, false otherwise. | ||
| func IsOperatorRunningOnOLM() bool { | ||
| k8sClient, _ := utils.GetE2ETestKubeClient() | ||
|
|
||
| crdList := crdv1.CustomResourceDefinitionList{} | ||
| Expect(k8sClient.List(context.Background(), &crdList)).To(Succeed()) | ||
|
|
||
| olmAPIsFound := 0 | ||
| for _, crd := range crdList.Items { | ||
| if strings.Contains(crd.Spec.Group, "operators.coreos.com") { | ||
| olmAPIsFound++ | ||
| } | ||
| } | ||
| return olmAPIsFound > 0 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C4 --type go '\bIsOperatorRunningOnOLM\s*\(' .
rg -n -C4 --type go 'ClusterServiceVersion|Subscription|OwnerReferences|gitops-operator' test/openshift/e2e/ginkgoRepository: redhat-developer/gitops-operator
Length of output: 50389
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## IsOperatorRunningOnOLM definition\n'
sed -n '980,1035p' test/openshift/e2e/ginkgo/fixture/fixture.go
printf '\n## OLM-related helpers/usages in fixture tree\n'
rg -n -C3 'IsOperatorRunningOnOLM|ClusterServiceVersion|Subscription|OwnerReferences|ManagedFields|operator Subscription|operator Deployment' test/openshift/e2e/ginkgo/fixture test/openshift/e2e/ginkgo/sequential test/openshift/e2e/ginkgo/parallelRepository: redhat-developer/gitops-operator
Length of output: 50388
test/openshift/e2e/ginkgo/fixture/fixture.go:996-1009 Check the operator’s install source, not cluster-wide OLM CRDs. This returns true on any OLM-enabled cluster, so a manually deployed operator can be misclassified as OLM-managed. Use the operator’s Subscription/CSV or ownership chain instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/openshift/e2e/ginkgo/fixture/fixture.go` around lines 996 - 1009, Update
IsOperatorRunningOnOLM to determine whether the tested operator is OLM-managed
by inspecting its relevant Subscription/CSV or ownership chain, rather than
counting cluster-wide CRDs whose group contains operators.coreos.com. Preserve
the false result for manually deployed operators and return true only when the
operator’s own resources demonstrate OLM ownership.
Source: Path instructions
Comment on lines
+93
to
+104
| argocdInstance := &argov1beta1api.ArgoCD{ | ||
| ObjectMeta: metav1.ObjectMeta{ | ||
| Name: "openshift-gitops", | ||
| Namespace: ns.Name, | ||
| }, | ||
| Spec: argov1beta1api.ArgoCDSpec{ | ||
| SSO: &argov1beta1api.ArgoCDSSOSpec{ | ||
| Provider: "dex", | ||
| }, | ||
| }, | ||
| } | ||
| Expect(k8sClient.Create(context.Background(), argocdInstance)).To(Succeed()) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C4 --type go 'Server\.Route|BeAvailable|k8sClient\.Create|ExistByName|getPodName' \
test/openshift/e2e/ginkgoRepository: redhat-developer/gitops-operator
Length of output: 50389
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="test/openshift/e2e/ginkgo/parallel/1-031_validate_toolchain_test.go"
echo "== file outline =="
ast-grep outline "$file" --view expanded || true
echo
echo "== relevant lines 1-180 =="
sed -n '1,180p' "$file" | cat -n
echo
echo "== route / availability / query usage =="
rg -n 'Server\.Route|BeAvailable|openshift-gitops-server|Route|Ready|Running|k8sClient\.Create|getPodName|ExistByName|Eventually\(' "$file"Repository: redhat-developer/gitops-operator
Length of output: 8929
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Server.Route usage in this repo =="
rg -n 'Server\.Route\.Enabled|Route:\s*&argov1beta1api\.ArgoCDRouteSpec|RouteEnabled|route enabled|BeAvailable\(\)' .
echo
echo "== ArgoCDSpec definition =="
spec_file=$(fd -a '.*v1beta1.*\.go$' api 2>/dev/null | head -n 20)
printf '%s\n' "$spec_file"Repository: redhat-developer/gitops-operator
Length of output: 38050
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files containing ArgoCDSpec and route defaults =="
fd -a '.*\.go$' api controllers test | sed -n '1,120p'
echo
echo "== repo-wide route enabled/default searches =="
rg -n 'Server\.Route\.Enabled|Route\s*:\s*&|BeAvailable\(\)|Default.*Route|route.*enabled' api controllers test config .Repository: redhat-developer/gitops-operator
Length of output: 50390
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== route enabled / availability / defaulting references =="
rg -n 'Server\.Route\.Enabled|RouteEnabled|BeAvailable\(|Available\)|Ready|Default.*Route|Route:\s*&' api controllers test config
echo
echo "== ArgoCDSpec and Route-related types =="
fd -a 'v1beta1*.go' api | head -n 20Repository: redhat-developer/gitops-operator
Length of output: 50388
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== route-related defaults and examples =="
rg -n 'Route\.Enabled|Server:|Route:' api config controllers test | sed -n '1,220p'
echo
echo "== similar e2e patterns around availability =="
rg -n 'BeAvailable\(\)|ExistByName\(\)' test/openshift/e2e/ginkgo | sed -n '1,220p'Repository: redhat-developer/gitops-operator
Length of output: 50388
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== ArgoCD creations without explicit Server.Route.Enabled =="
rg -n -U 'ArgoCD\{[\s\S]{0,220}?SSO:\s*&argov1beta1api\.ArgoCDSSOSpec' test/openshift/e2e/ginkgo | sed -n '1,200p'
echo
echo "== files where route is checked after create =="
rg -n 'Route\{ObjectMeta: metav1\.ObjectMeta\{Name: "openshift-gitops-server"|BeAvailable\(\)' test/openshift/e2e/ginkgo | sed -n '1,200p'Repository: redhat-developer/gitops-operator
Length of output: 36386
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== api tree =="
fd -t f . api | sed -n '1,200p'
echo
echo "== route references in api =="
rg -n 'Route|Dex|SSO|Available' apiRepository: redhat-developer/gitops-operator
Length of output: 324
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== server version / route host usages =="
rg -n 'argocd version --short --server|serverRoute\.Spec\.Host|openshift-gitops-server' test/openshift/e2e/ginkgo
echo
echo "== ArgoCD create patterns in toolchain tests =="
sed -n '86,170p' test/openshift/e2e/ginkgo/parallel/1-031_validate_toolchain_test.go | cat -nRepository: redhat-developer/gitops-operator
Length of output: 9187
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== controllers/argocd/argocd.go around getArgoServerSpec =="
sed -n '140,290p' controllers/argocd/argocd.go | cat -n
echo
echo "== test/openshift/e2e/ginkgo/fixture/argocd/fixture.go BeAvailable =="
sed -n '1,140p' test/openshift/e2e/ginkgo/fixture/argocd/fixture.go | cat -n
echo
echo "== 1-031 around route host and waits =="
sed -n '86,170p' test/openshift/e2e/ginkgo/parallel/1-031_validate_toolchain_test.go | cat -nRepository: redhat-developer/gitops-operator
Length of output: 15636
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== controllers/argocd/argocd.go getArgoServerSpec references =="
rg -n -C3 'getArgoServerSpec|Route: argoapp\.ArgoCDRouteSpec|BeAvailable\(' controllers/argocd/argocd.go test/openshift/e2e/ginkgo/fixture/argocd/fixture.go
echo
echo "== small excerpt of the relevant lines =="
sed -n '160,190p' controllers/argocd/argocd.go | cat -n
sed -n '60,110p' test/openshift/e2e/ginkgo/fixture/argocd/fixture.go | cat -nRepository: redhat-developer/gitops-operator
Length of output: 5536
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== controller code that mutates defaults / route settings =="
rg -n -C4 'Server\.Route\.Enabled|getArgoServerSpec|Route\.Enabled =|Route:.*Enabled|openshift-gitops' controllers test/openshift/e2e/ginkgo | sed -n '1,240p'
echo
echo "== 1-031 rest of file for any availability wait =="
sed -n '170,260p' test/openshift/e2e/ginkgo/parallel/1-031_validate_toolchain_test.go | cat -n
echo
echo "== 1-031 fixture helpers used in test =="
rg -n -C3 'WaitForAllPodsInTheNamespaceToBeReady|BeAvailable|HavePhase|GetOpenShiftGitOpsNSArgoCD' test/openshift/e2e/ginkgo/fixture test/openshift/e2e/ginkgo/parallel/1-031_validate_toolchain_test.goRepository: redhat-developer/gitops-operator
Length of output: 35974
Wait for Argo CD to become Available before using argocd version --server. getPodName only checks Running, and serverRoute only checks object existence; this can race API readiness and make the version check flaky.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/openshift/e2e/ginkgo/parallel/1-031_validate_toolchain_test.go` around
lines 93 - 104, After creating the ArgoCD resource in the validation setup, wait
until it reports Available and its server endpoint is ready before invoking
argocd version --server. Update the readiness flow around getPodName and
serverRoute to check the Argo CD status condition, not merely pod Running state
or route existence, while preserving the existing version-check behavior once
availability is confirmed.
Source: Path instructions
| } | ||
| }) | ||
| It("ensures the conditions in status when external Authentication is enabled on clusters; above 4.20 by default in openshit is enabled", func() { | ||
| It("ensures the conditions in status when external Authentication is enabled on clusters; above 4.20 by default in openshift is enabled", Label("openshift"), func() { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Label the sibling OpenShift SSO test as well.
The second It block at Line 124 also sets OpenShiftOAuth: true but is not labeled. With --label-filter="!openshift", it will still run on xKS and can fail; apply the label to the surrounding Context or to both specs.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/openshift/e2e/ginkgo/parallel/1-050_validate_sso_test.go` at line 70,
Apply the existing "openshift" label to the sibling SSO test at the second It
block, preferably by labeling the surrounding Context so both specs that set
OpenShiftOAuth: true are excluded by --label-filter="!openshift".
| }) | ||
|
|
||
| It("verifies that resource customization is dropped in conversion from ArgoCD v1alpha1 to v1beta1", func() { | ||
| It("verifies that resource customization is dropped in conversion from ArgoCD v1alpha1 to v1beta1", Label("openshift"), func() { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not classify this conversion test as OpenShift-only.
This test uses namespace-scoped Argo CD resources and conversion APIs but no OpenShift-specific capability. With --label-filter="!openshift", it will be skipped on xKS unnecessarily; remove this label or use a capability-specific label for the conversion webhook.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@test/openshift/e2e/ginkgo/parallel/1-055_drop_resource_customizations_test.go`
at line 49, Remove the "openshift" label from the test case in the It block for
resource customization conversion, or replace it with the appropriate
conversion-webhook capability label if one exists. Keep the test classified as a
general namespace-scoped Argo CD v1alpha1-to-v1beta1 conversion test.
| }) | ||
|
|
||
| It("verifying when HA is enabled that Argo CD starts successfully in HA mode, and that AutoTLS can be enabled", func() { | ||
| It("verifying when HA is enabled that Argo CD starts successfully in HA mode, and that AutoTLS can be enabled", Label("HA"), func() { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Align platform labels with actual test requirements.
The current labels cause incorrect selection when running xKS with --label-filter="!openshift":
test/openshift/e2e/ginkgo/parallel/1-069_validate_redis_secure_comm_autotls_ha_test.go#L61-L61: addLabel("openshift"), because the test configuresRedis.AutoTLS = "openshift".test/openshift/e2e/ginkgo/parallel/1-082_validate_node_placement_test.go#L52-L52: removeLabel("openshift"); the supplied test uses only generic Kubernetes resources and should remain available on xKS.test/openshift/e2e/ginkgo/parallel/1-084_validate_status_host_ingress_test.go#L50-L50: removeLabel("openshift")unless this test is intentionally excluded from xKS; its OpenShift-specific assertion is already guarded byRunningOnOpenShift().
📍 Affects 3 files
test/openshift/e2e/ginkgo/parallel/1-069_validate_redis_secure_comm_autotls_ha_test.go#L61-L61(this comment)test/openshift/e2e/ginkgo/parallel/1-082_validate_node_placement_test.go#L52-L52test/openshift/e2e/ginkgo/parallel/1-084_validate_status_host_ingress_test.go#L50-L50
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@test/openshift/e2e/ginkgo/parallel/1-069_validate_redis_secure_comm_autotls_ha_test.go`
at line 61, Align labels with platform requirements across the three tests: in
test/openshift/e2e/ginkgo/parallel/1-069_validate_redis_secure_comm_autotls_ha_test.go:61,
add Label("openshift") to the HA AutoTLS test; in
test/openshift/e2e/ginkgo/parallel/1-082_validate_node_placement_test.go:52,
remove Label("openshift"); and in
test/openshift/e2e/ginkgo/parallel/1-084_validate_status_host_ingress_test.go:50,
remove Label("openshift") while preserving the existing RunningOnOpenShift()
guard.
| }) | ||
|
|
||
| It("ensures that creation of a v1alpha1 ArgoCD CR with SSO fields will be translated into a v1beta1 ArgoCD CR via webhook", func() { | ||
| It("ensures that creation of a v1alpha1 ArgoCD CR with SSO fields will be translated into a v1beta1 ArgoCD CR via webhook", Label("openshift"), func() { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Add the conversionWebhook label.
This spec explicitly validates alpha-to-beta conversion via the webhook, but is tagged only openshift. It will therefore be omitted from conversionWebhook-filtered runs, unlike the neighboring conversion specs.
Proposed fix
- It("ensures that creation of a v1alpha1 ArgoCD CR with SSO fields will be translated into a v1beta1 ArgoCD CR via webhook", Label("openshift"), func() {
+ It("ensures that creation of a v1alpha1 ArgoCD CR with SSO fields will be translated into a v1beta1 ArgoCD CR via webhook", Label("openshift", "conversionWebhook"), func() {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| It("ensures that creation of a v1alpha1 ArgoCD CR with SSO fields will be translated into a v1beta1 ArgoCD CR via webhook", Label("openshift"), func() { | |
| It("ensures that creation of a v1alpha1 ArgoCD CR with SSO fields will be translated into a v1beta1 ArgoCD CR via webhook", Label("openshift", "conversionWebhook"), func() { |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@test/openshift/e2e/ginkgo/parallel/1-103_argocd_alpha_to_beta_conversion_test.go`
at line 49, Update the spec declaration in the alpha-to-beta conversion test to
include the conversionWebhook label alongside openshift, ensuring it is selected
by conversionWebhook-filtered runs like the neighboring conversion specs.
Signed-off-by: Anand Kumar Singh <anandrkskd@gmail.com>
Signed-off-by: Anand Kumar Singh <anandrkskd@gmail.com>
Signed-off-by: Anand Kumar Singh <anandrkskd@gmail.com>
Signed-off-by: Anand Kumar Singh <anandrkskd@gmail.com>
Signed-off-by: Anand Kumar Singh <anandrkskd@gmail.com>
assisted-by: claude-code Signed-off-by: Anand Kumar Singh <anandrkskd@gmail.com>
anandrkskd
force-pushed
the
mark-openshift-xks-tests
branch
from
July 20, 2026 09:41
8e74cae to
dbcc49c
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/kind-ci-automation.yaml (1)
18-20: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winDisable credential persistence in checkout.
To prevent the GitHub token from being persisted in the local Git configuration and potentially exposed in subsequent steps or artifacts, configure
actions/checkoutto not persist credentials.🛡️ Proposed fix
- name: Checkout code uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 + with: + persist-credentials: false🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/kind-ci-automation.yaml around lines 18 - 20, Update the actions/checkout step to disable credential persistence by setting its persist-credentials option to false, while keeping the pinned checkout action version unchanged.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/kind-ci-automation.yaml:
- Around line 9-12: Add a top-level permissions declaration near the workflow’s
existing global env settings, configuring the workflow token with read-only
repository contents access and no broader default permissions. Keep the existing
IMG and LOG_DIR environment values unchanged.
In
`@test/openshift/e2e/ginkgo/sequential/1-018_validate_disable_default_instance_test.go`:
- Line 43: Label every OpenShift/OLM-dependent spec with notOnXKS: update the
second It declaration in
test/openshift/e2e/ginkgo/sequential/1-018_validate_disable_default_instance_test.go
at line 103, the subscription-level spec in
test/openshift/e2e/ginkgo/sequential/1-103-validate-rollouts-imagepullpolicy.go
at line 118, and the operator-level environment-variable spec in
test/openshift/e2e/ginkgo/sequential/1-114_validate_imagepullpolicy_test.go at
line 241; the anchor site at
test/openshift/e2e/ginkgo/sequential/1-018_validate_disable_default_instance_test.go:43
is context only.
In
`@test/openshift/e2e/ginkgo/sequential/1-102_validate_handle_terminating_namespaces_test.go`:
- Line 84: Remove the notOnXKS label from the affected portable Kubernetes
specs:
test/openshift/e2e/ginkgo/sequential/1-102_validate_handle_terminating_namespaces_test.go:84,
test/openshift/e2e/ginkgo/sequential/1-114_validate_imagepullpolicy_test.go:52,
and both namespace tests at
test/openshift/e2e/ginkgo/sequential/1-122_validate_namespace_test.go:74 and
:166. Preserve each spec body and all other labels unchanged.
---
Outside diff comments:
In @.github/workflows/kind-ci-automation.yaml:
- Around line 18-20: Update the actions/checkout step to disable credential
persistence by setting its persist-credentials option to false, while keeping
the pinned checkout action version unchanged.
🪄 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), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: e28a9ee3-4fd5-4d75-8512-5ad060812b6c
📒 Files selected for processing (76)
.github/workflows/kind-ci-automation.yamlMakefiletest/e2e/argocd_route_test.gotest/openshift/e2e/ginkgo/fixture/fixture.gotest/openshift/e2e/ginkgo/parallel/1-001_alpha_to_beta_dex_conversion_test.gotest/openshift/e2e/ginkgo/parallel/1-003_alpha_to_beta_sso_conflict_conversion_test.gotest/openshift/e2e/ginkgo/parallel/1-003_validate_console_link_test.gotest/openshift/e2e/ginkgo/parallel/1-004_beta_to_alpha_conversion_test.gotest/openshift/e2e/ginkgo/parallel/1-005_validate_metrics_test.gotest/openshift/e2e/ginkgo/parallel/1-005_validate_route_tls_test.gotest/openshift/e2e/ginkgo/parallel/1-023_validate_repo_server_tls_test.gotest/openshift/e2e/ginkgo/parallel/1-030_validate_reencrypt_test.gotest/openshift/e2e/ginkgo/parallel/1-031_validate_toolchain_test.gotest/openshift/e2e/ginkgo/parallel/1-041_validate_argocd_sync_alert_test.gotest/openshift/e2e/ginkgo/parallel/1-042_validate_status_host_test.gotest/openshift/e2e/ginkgo/parallel/1-050_validate_sso_test.gotest/openshift/e2e/ginkgo/parallel/1-053_validate_cluster_admin_rbac_test.gotest/openshift/e2e/ginkgo/parallel/1-054_validate_deploymentconfig_test.gotest/openshift/e2e/ginkgo/parallel/1-055_drop_resource_customizations_test.gotest/openshift/e2e/ginkgo/parallel/1-063_validate_dex_liveness_probe_test.gotest/openshift/e2e/ginkgo/parallel/1-065_validate_redis_ha_anti_affinity_test.gotest/openshift/e2e/ginkgo/parallel/1-067_validate_redis_secure_comm_no_autotls_ha_test.gotest/openshift/e2e/ginkgo/parallel/1-069_validate_redis_secure_comm_autotls_ha_test.gotest/openshift/e2e/ginkgo/parallel/1-081_validate_applicationset_deployment_test.gotest/openshift/e2e/ginkgo/parallel/1-082_validate_node_placement_test.gotest/openshift/e2e/ginkgo/parallel/1-084_validate_status_host_ingress_test.gotest/openshift/e2e/ginkgo/parallel/1-096-validate_home_env_argocd_controller_test.gotest/openshift/e2e/ginkgo/parallel/1-103_argocd_alpha_to_beta_conversion_test.gotest/openshift/e2e/ginkgo/parallel/1-109_validate_reencrypt_termination_policy_test.gotest/openshift/e2e/ginkgo/parallel/1-125_validate_server_serving_cert_annotation_restore_test.gotest/openshift/e2e/ginkgo/parallel/1-126_validate_servicemonitor_metrics_config_test.gotest/openshift/e2e/ginkgo/sequential/1-002-validate_backend_service_test.gotest/openshift/e2e/ginkgo/sequential/1-004_validate_argocd_installation_test.gotest/openshift/e2e/ginkgo/sequential/1-006_validate_machine_config_test.gotest/openshift/e2e/ginkgo/sequential/1-008_validate-4.9CI-Failures_test.gotest/openshift/e2e/ginkgo/sequential/1-010_validate-ootb-manage-other-namespace_test.gotest/openshift/e2e/ginkgo/sequential/1-018_validate_disable_default_instance_test.gotest/openshift/e2e/ginkgo/sequential/1-020_validate_redis_ha_nonha_test.gotest/openshift/e2e/ginkgo/sequential/1-026-validate_backend_service_permissions_test.gotest/openshift/e2e/ginkgo/sequential/1-027_validate_operand_from_git_test.gotest/openshift/e2e/ginkgo/sequential/1-028-validate_run_on_infra_test.gotest/openshift/e2e/ginkgo/sequential/1-035_validate_argocd_secret_repopulate_test.gotest/openshift/e2e/ginkgo/sequential/1-036_validate_role_rolebinding_for_source_namespace_test.gotest/openshift/e2e/ginkgo/sequential/1-037_validate_applicationset_in_any_namespace_test.gotest/openshift/e2e/ginkgo/sequential/1-040_validate_quoted_RBAC_group_names_test.gotest/openshift/e2e/ginkgo/sequential/1-051_validate_argocd_agent_principal_test.gotest/openshift/e2e/ginkgo/sequential/1-052_validate_argocd_agent_agent_test.gotest/openshift/e2e/ginkgo/sequential/1-052_validate_rolebinding_number_test.gotest/openshift/e2e/ginkgo/sequential/1-053_validate_argocd_agent_principal_connected_test.gotest/openshift/e2e/ginkgo/sequential/1-058_validate_notifications_source_namespaces_test.gotest/openshift/e2e/ginkgo/sequential/1-059_validate_argocd_agent_terminal_streaming_test.gotest/openshift/e2e/ginkgo/sequential/1-064_validate_tcp_reset_error_test.gotest/openshift/e2e/ginkgo/sequential/1-071_validate_SCC_HA_test.gotest/openshift/e2e/ginkgo/sequential/1-071_validate_node_selectors_test.gotest/openshift/e2e/ginkgo/sequential/1-074_validate_terminating_namespace_block_test.gotest/openshift/e2e/ginkgo/sequential/1-078_validate_default_argocd_consoleLink_test.gotest/openshift/e2e/ginkgo/sequential/1-083_validate_apps_in_any_namespace_test.gotest/openshift/e2e/ginkgo/sequential/1-084_validate_prune_templates.gotest/openshift/e2e/ginkgo/sequential/1-086_validate_default_argocd_role_test.gotest/openshift/e2e/ginkgo/sequential/1-092_validate_workload_status_monitoring_alert.gotest/openshift/e2e/ginkgo/sequential/1-100_validate_rollouts_resources_creation_test.gotest/openshift/e2e/ginkgo/sequential/1-101_validate_rollout_policyrules_test.gotest/openshift/e2e/ginkgo/sequential/1-102_validate_handle_terminating_namespaces_test.gotest/openshift/e2e/ginkgo/sequential/1-103-validate-rollouts-imagepullpolicy.gotest/openshift/e2e/ginkgo/sequential/1-105_validate_default_argocd_route_test.gotest/openshift/e2e/ginkgo/sequential/1-106_validate_argocd_metrics_controller_test.gotest/openshift/e2e/ginkgo/sequential/1-107_validate_redis_scc_test.gotest/openshift/e2e/ginkgo/sequential/1-110_validate_podsecurity_alerts_test.gotest/openshift/e2e/ginkgo/sequential/1-111_validate_default_argocd_route_test.gotest/openshift/e2e/ginkgo/sequential/1-112_validate_rollout_plugin_support_test.gotest/openshift/e2e/ginkgo/sequential/1-114_validate_imagepullpolicy_test.gotest/openshift/e2e/ginkgo/sequential/1-120_repo_server_system_ca_trust.gotest/openshift/e2e/ginkgo/sequential/1-120_validate_running_must_gather.gotest/openshift/e2e/ginkgo/sequential/1-121-valiate_resource_constraints_gitopsservice_test.gotest/openshift/e2e/ginkgo/sequential/1-122_validate_namespace_test.gotest/openshift/e2e/ginkgo/sequential/1-123_validate_list_order_comparison_test.go
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
argoproj-labs/argocd-operator(manual)
🚧 Files skipped from review as they are similar to previous changes (42)
- test/openshift/e2e/ginkgo/sequential/1-026-validate_backend_service_permissions_test.go
- test/openshift/e2e/ginkgo/sequential/1-040_validate_quoted_RBAC_group_names_test.go
- test/openshift/e2e/ginkgo/parallel/1-005_validate_metrics_test.go
- test/openshift/e2e/ginkgo/parallel/1-109_validate_reencrypt_termination_policy_test.go
- test/openshift/e2e/ginkgo/parallel/1-081_validate_applicationset_deployment_test.go
- test/openshift/e2e/ginkgo/sequential/1-106_validate_argocd_metrics_controller_test.go
- test/openshift/e2e/ginkgo/sequential/1-111_validate_default_argocd_route_test.go
- test/openshift/e2e/ginkgo/sequential/1-008_validate-4.9CI-Failures_test.go
- test/openshift/e2e/ginkgo/sequential/1-105_validate_default_argocd_route_test.go
- test/openshift/e2e/ginkgo/sequential/1-110_validate_podsecurity_alerts_test.go
- test/openshift/e2e/ginkgo/parallel/1-004_beta_to_alpha_conversion_test.go
- test/openshift/e2e/ginkgo/parallel/1-030_validate_reencrypt_test.go
- test/openshift/e2e/ginkgo/sequential/1-027_validate_operand_from_git_test.go
- test/openshift/e2e/ginkgo/sequential/1-120_validate_running_must_gather.go
- test/openshift/e2e/ginkgo/parallel/1-103_argocd_alpha_to_beta_conversion_test.go
- test/e2e/argocd_route_test.go
- test/openshift/e2e/ginkgo/sequential/1-020_validate_redis_ha_nonha_test.go
- test/openshift/e2e/ginkgo/sequential/1-123_validate_list_order_comparison_test.go
- test/openshift/e2e/ginkgo/sequential/1-004_validate_argocd_installation_test.go
- test/openshift/e2e/ginkgo/parallel/1-084_validate_status_host_ingress_test.go
- test/openshift/e2e/ginkgo/sequential/1-071_validate_node_selectors_test.go
- test/openshift/e2e/ginkgo/sequential/1-092_validate_workload_status_monitoring_alert.go
- test/openshift/e2e/ginkgo/parallel/1-082_validate_node_placement_test.go
- test/openshift/e2e/ginkgo/parallel/1-023_validate_repo_server_tls_test.go
- test/openshift/e2e/ginkgo/sequential/1-101_validate_rollout_policyrules_test.go
- test/openshift/e2e/ginkgo/sequential/1-035_validate_argocd_secret_repopulate_test.go
- test/openshift/e2e/ginkgo/sequential/1-083_validate_apps_in_any_namespace_test.go
- test/openshift/e2e/ginkgo/sequential/1-074_validate_terminating_namespace_block_test.go
- test/openshift/e2e/ginkgo/parallel/1-042_validate_status_host_test.go
- test/openshift/e2e/ginkgo/parallel/1-005_validate_route_tls_test.go
- test/openshift/e2e/ginkgo/parallel/1-003_alpha_to_beta_sso_conflict_conversion_test.go
- test/openshift/e2e/ginkgo/sequential/1-064_validate_tcp_reset_error_test.go
- test/openshift/e2e/ginkgo/sequential/1-028-validate_run_on_infra_test.go
- test/openshift/e2e/ginkgo/parallel/1-125_validate_server_serving_cert_annotation_restore_test.go
- test/openshift/e2e/ginkgo/parallel/1-001_alpha_to_beta_dex_conversion_test.go
- test/openshift/e2e/ginkgo/sequential/1-006_validate_machine_config_test.go
- test/openshift/e2e/ginkgo/sequential/1-052_validate_argocd_agent_agent_test.go
- test/openshift/e2e/ginkgo/sequential/1-037_validate_applicationset_in_any_namespace_test.go
- test/openshift/e2e/ginkgo/sequential/1-120_repo_server_system_ca_trust.go
- test/openshift/e2e/ginkgo/parallel/1-069_validate_redis_secure_comm_autotls_ha_test.go
- test/openshift/e2e/ginkgo/fixture/fixture.go
- test/openshift/e2e/ginkgo/parallel/1-031_validate_toolchain_test.go
| }) | ||
|
|
||
| It("verifies that the default ArgoCD instance from openshift-gitops namespace is recreated when deleted manually", func() { | ||
| It("verifies that the default ArgoCD instance from openshift-gitops namespace is recreated when deleted manually", Label("notOnXKS"), func() { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Label every OpenShift/OLM-dependent spec.
The following specs still modify operator subscriptions or deployments and will run under --label-filter="!notOnXKS", causing xKS failures or unsupported mutations:
test/openshift/e2e/ginkgo/sequential/1-018_validate_disable_default_instance_test.go#L43-L43: addLabel("notOnXKS")to the secondItdeclaration at Line 103.test/openshift/e2e/ginkgo/sequential/1-103-validate-rollouts-imagepullpolicy.go#L75-L75: add the label to the subscription-level spec at Line 118.test/openshift/e2e/ginkgo/sequential/1-114_validate_imagepullpolicy_test.go#L197-L197: add the label to the operator-level environment-variable spec at Line 241.
📍 Affects 3 files
test/openshift/e2e/ginkgo/sequential/1-018_validate_disable_default_instance_test.go#L43-L43(this comment)test/openshift/e2e/ginkgo/sequential/1-103-validate-rollouts-imagepullpolicy.go#L75-L75test/openshift/e2e/ginkgo/sequential/1-114_validate_imagepullpolicy_test.go#L197-L197
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@test/openshift/e2e/ginkgo/sequential/1-018_validate_disable_default_instance_test.go`
at line 43, Label every OpenShift/OLM-dependent spec with notOnXKS: update the
second It declaration in
test/openshift/e2e/ginkgo/sequential/1-018_validate_disable_default_instance_test.go
at line 103, the subscription-level spec in
test/openshift/e2e/ginkgo/sequential/1-103-validate-rollouts-imagepullpolicy.go
at line 118, and the operator-level environment-variable spec in
test/openshift/e2e/ginkgo/sequential/1-114_validate_imagepullpolicy_test.go at
line 241; the anchor site at
test/openshift/e2e/ginkgo/sequential/1-018_validate_disable_default_instance_test.go:43
is context only.
Signed-off-by: Anand Kumar Singh <anandrkskd@gmail.com>
| .PHONY: e2e-xks-tests-sequential-ginkgo | ||
| e2e-xks-tests-sequential-ginkgo: ginkgo ## Runs Ginkgo e2e sequential tests | ||
| @echo "Running GitOps Operator sequential Ginkgo E2E tests..." | ||
| $(GINKGO_CLI) -v --trace --label-filter="!notOnXKS" --timeout 240m -r ./test/openshift/e2e/ginkgo/sequential |
Collaborator
There was a problem hiding this comment.
Can we rename the label notOnXKS to something positive in grammatical sense? openshift, maybe.
I envision to have multiple labels representing runtime environments or even their individual capabilities and the negation can get confusing, same as declaring what are the non-xks attributes.
Contributor
Author
There was a problem hiding this comment.
I agree; I was considering using the same labels, such as openshift for tests unique to openshift and xKS or any other platform-dependent labels for their particular environment.
I talked this over with @varshab1210, and she suggested that we create a label indicating where not to run it, like notOnXKS, notOnOpenshift or something along those lines.
Member
There was a problem hiding this comment.
I envision to have multiple labels representing runtime environments or even their individual capabilities and the negation can get confusing, same as declaring what are the non-xks attributes.
I made this suggestion to reduce label usage. If we consider adding openshift and xKS labels we would need to add them on every test going forward because we expect features to work on both platforms and hence label it accordingly (unless I misunderstood the intention), whereas if we use something like skip_on_xKS, it needs to be added only when a particular feature is not going to be supported on xKS or skip_on_openshift for that matter for other way around.
@anandrkskd when I initially reviewed the PR, I thought you were using openshift as a way to skip the run on xKS. That seemed a little confusing to me.
I am okay either way, as long as we have a README.md describing the scenarios for labeling
Contributor
Author
There was a problem hiding this comment.
Yes, currently we are using it to skip test that are currently only compatible with openshift, we should only add labels for tests that are platform specific, either openshift or xks. Labels are not required on every test aa most test should be generic and should work on both the platforms.
I will update the Readme.md to make it more transparent as an action item.
Collaborator
There was a problem hiding this comment.
We seem to have a different understanding about the semantics of such labels.
What does openshift mean, for example? Is the test specific for openshift so it does not need to run on any other platforms, or it means the test is relevant for openshift and potentially for other platforms as well.
I find the second meaning less practical because - as Varsha pointed out - most of the tests would have most of the labels and the lists will only ever expand. Also, forgetting to add label(s) will cause tests are not executed creating a false sense of security.
I suggest the meaning of specfic: "this tests depends on a feature specific to given environment". That way, most of the tests depending on k8s essentials can remain unlabeled. And we will only bother the authors of platform-specific tests. The downside here is that the test executions will have to exclude N-1 platforms when launching, like --label-filter=!openshift && !..., but like not a big problem given this is done on handful of places (one?) and for handful of non-k8s environments. Here, forgetting to add label to test (forgetting to indicate that tests require special features of a platform) will cause test failure on other platforms, pointing out the mistake.
Contributor
Author
There was a problem hiding this comment.
I suggest the meaning of specfic: "this tests depends on a feature specific to given environment". That way, most of the tests depending on k8s essentials can remain unlabeled. And we will only bother the authors of platform-specific tests. The downside here is that the test executions will have to exclude N-1 platforms when launching, like --label-filter=!openshift && !..., but like not a big problem given this is done on handful of places (one?) and for handful of non-k8s environments. Here, forgetting to add label to test (forgetting to indicate that tests require special features of a platform) will cause test failure on other platforms, pointing out the mistake.
I agree with what you have mentioned above, labeling depends on a feature specific to platform. When we label a test with openshift it should not run on other platforms, or else it will fails.
Member
There was a problem hiding this comment.
My only concern is people adding the label openshift thinking "Yes, the feature is supported on Openshift" instead of thinking if the feature is only supported on Openshift. We don't want to skip a test run on xKS accidentally. Or someone going through the test thinking tests labelled as openshift are the ones supported on Openshift. A README.md would help clarify this though so its fine by me.
Collaborator
There was a problem hiding this comment.
Agreed this needs to be documented thoroughly. @anandrkskd, could you?
thinking "Yes, the feature is supported on Openshift"
If we cant to be super clear about this, we can introduce a dummy wrapper function for Label like Requires("openshift"). But I am not sure it is worth it.
Contributor
Author
There was a problem hiding this comment.
I will update the readme, if it is still not clear, we can work towards a wrapper function as last resort.
WDYT?
Signed-off-by: Anand Kumar Singh <anandrkskd@gmail.com>
Signed-off-by: Anand Kumar Singh <anandrkskd@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/openshift/e2e/ginkgo/fixture/fixture.go`:
- Around line 112-114: Update the cleanup around the ClusterRole deletion in the
fixture setup to handle the error returned by k8sClient.Delete: ignore it only
when apierrors.IsNotFound(err) is true, and propagate or fail the cleanup for
all other errors so custom-argocd-role cannot remain behind.
In
`@test/openshift/e2e/ginkgo/sequential/1-074_validate_terminating_namespace_block_test.go`:
- Line 52: Remove the misspelled OpenShift label from the affected It(...)
blocks so these portable Kubernetes regression tests run in xKS. Apply this to
test/openshift/e2e/ginkgo/sequential/1-074_validate_terminating_namespace_block_test.go:52-52,
test/openshift/e2e/ginkgo/sequential/1-102_validate_handle_terminating_namespaces_test.go:84-84,
and
test/openshift/e2e/ginkgo/sequential/1-114_validate_imagepullpolicy_test.go:52-52,
141-141, and 197-197; make no other changes.
In `@test/openshift/e2e/README.md`:
- Line 107: The OpenShift label is misspelled as openshfit across documentation,
Ginkgo tests, and the Makefile’s XKS_LABEL_FILTER. Replace every occurrence with
openshift, preserving the existing label combinations, including the conversion
test’s conversionWebhook label; update test/openshift/e2e/README.md lines
107-107, 114-114, 122-122, 127-127, 132-132, and 141-141, all listed Ginkgo
sites in the specified files, and the corresponding Makefile definition.
🪄 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), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 6cc9e939-ffa8-4557-8b1c-764bf21b18e5
📒 Files selected for processing (74)
.github/workflows/kind-ci-automation.yamlMakefiletest/e2e/argocd_route_test.gotest/kind-config.yamltest/openshift/e2e/README.mdtest/openshift/e2e/ginkgo/fixture/fixture.gotest/openshift/e2e/ginkgo/parallel/1-001_alpha_to_beta_dex_conversion_test.gotest/openshift/e2e/ginkgo/parallel/1-003_alpha_to_beta_sso_conflict_conversion_test.gotest/openshift/e2e/ginkgo/parallel/1-003_validate_console_link_test.gotest/openshift/e2e/ginkgo/parallel/1-004_beta_to_alpha_conversion_test.gotest/openshift/e2e/ginkgo/parallel/1-005_validate_metrics_test.gotest/openshift/e2e/ginkgo/parallel/1-005_validate_route_tls_test.gotest/openshift/e2e/ginkgo/parallel/1-023_validate_repo_server_tls_test.gotest/openshift/e2e/ginkgo/parallel/1-030_validate_reencrypt_test.gotest/openshift/e2e/ginkgo/parallel/1-041_validate_argocd_sync_alert_test.gotest/openshift/e2e/ginkgo/parallel/1-042_validate_status_host_test.gotest/openshift/e2e/ginkgo/parallel/1-050_validate_sso_test.gotest/openshift/e2e/ginkgo/parallel/1-053_validate_cluster_admin_rbac_test.gotest/openshift/e2e/ginkgo/parallel/1-054_validate_deploymentconfig_test.gotest/openshift/e2e/ginkgo/parallel/1-055_drop_resource_customizations_test.gotest/openshift/e2e/ginkgo/parallel/1-063_validate_dex_liveness_probe_test.gotest/openshift/e2e/ginkgo/parallel/1-081_validate_applicationset_deployment_test.gotest/openshift/e2e/ginkgo/parallel/1-082_validate_node_placement_test.gotest/openshift/e2e/ginkgo/parallel/1-084_validate_status_host_ingress_test.gotest/openshift/e2e/ginkgo/parallel/1-096-validate_home_env_argocd_controller_test.gotest/openshift/e2e/ginkgo/parallel/1-103_argocd_alpha_to_beta_conversion_test.gotest/openshift/e2e/ginkgo/parallel/1-109_validate_reencrypt_termination_policy_test.gotest/openshift/e2e/ginkgo/parallel/1-125_validate_server_serving_cert_annotation_restore_test.gotest/openshift/e2e/ginkgo/parallel/1-126_validate_servicemonitor_metrics_config_test.gotest/openshift/e2e/ginkgo/sequential/1-002-validate_backend_service_test.gotest/openshift/e2e/ginkgo/sequential/1-004_validate_argocd_installation_test.gotest/openshift/e2e/ginkgo/sequential/1-006_validate_machine_config_test.gotest/openshift/e2e/ginkgo/sequential/1-008_validate-4.9CI-Failures_test.gotest/openshift/e2e/ginkgo/sequential/1-010_validate-ootb-manage-other-namespace_test.gotest/openshift/e2e/ginkgo/sequential/1-018_validate_disable_default_instance_test.gotest/openshift/e2e/ginkgo/sequential/1-020_validate_redis_ha_nonha_test.gotest/openshift/e2e/ginkgo/sequential/1-026-validate_backend_service_permissions_test.gotest/openshift/e2e/ginkgo/sequential/1-027_validate_operand_from_git_test.gotest/openshift/e2e/ginkgo/sequential/1-028-validate_run_on_infra_test.gotest/openshift/e2e/ginkgo/sequential/1-035_validate_argocd_secret_repopulate_test.gotest/openshift/e2e/ginkgo/sequential/1-036_validate_role_rolebinding_for_source_namespace_test.gotest/openshift/e2e/ginkgo/sequential/1-037_validate_applicationset_in_any_namespace_test.gotest/openshift/e2e/ginkgo/sequential/1-040_validate_quoted_RBAC_group_names_test.gotest/openshift/e2e/ginkgo/sequential/1-051_validate_argocd_agent_principal_test.gotest/openshift/e2e/ginkgo/sequential/1-052_validate_argocd_agent_agent_test.gotest/openshift/e2e/ginkgo/sequential/1-052_validate_rolebinding_number_test.gotest/openshift/e2e/ginkgo/sequential/1-053_validate_argocd_agent_principal_connected_test.gotest/openshift/e2e/ginkgo/sequential/1-058_validate_notifications_source_namespaces_test.gotest/openshift/e2e/ginkgo/sequential/1-059_validate_argocd_agent_terminal_streaming_test.gotest/openshift/e2e/ginkgo/sequential/1-064_validate_tcp_reset_error_test.gotest/openshift/e2e/ginkgo/sequential/1-071_validate_SCC_HA_test.gotest/openshift/e2e/ginkgo/sequential/1-071_validate_node_selectors_test.gotest/openshift/e2e/ginkgo/sequential/1-074_validate_terminating_namespace_block_test.gotest/openshift/e2e/ginkgo/sequential/1-078_validate_default_argocd_consoleLink_test.gotest/openshift/e2e/ginkgo/sequential/1-083_validate_apps_in_any_namespace_test.gotest/openshift/e2e/ginkgo/sequential/1-084_validate_prune_templates.gotest/openshift/e2e/ginkgo/sequential/1-086_validate_default_argocd_role_test.gotest/openshift/e2e/ginkgo/sequential/1-092_validate_workload_status_monitoring_alert.gotest/openshift/e2e/ginkgo/sequential/1-100_validate_rollouts_resources_creation_test.gotest/openshift/e2e/ginkgo/sequential/1-101_validate_rollout_policyrules_test.gotest/openshift/e2e/ginkgo/sequential/1-102_validate_handle_terminating_namespaces_test.gotest/openshift/e2e/ginkgo/sequential/1-103-validate-rollouts-imagepullpolicy.gotest/openshift/e2e/ginkgo/sequential/1-105_validate_default_argocd_route_test.gotest/openshift/e2e/ginkgo/sequential/1-106_validate_argocd_metrics_controller_test.gotest/openshift/e2e/ginkgo/sequential/1-107_validate_redis_scc_test.gotest/openshift/e2e/ginkgo/sequential/1-110_validate_podsecurity_alerts_test.gotest/openshift/e2e/ginkgo/sequential/1-111_validate_default_argocd_route_test.gotest/openshift/e2e/ginkgo/sequential/1-112_validate_rollout_plugin_support_test.gotest/openshift/e2e/ginkgo/sequential/1-114_validate_imagepullpolicy_test.gotest/openshift/e2e/ginkgo/sequential/1-120_repo_server_system_ca_trust.gotest/openshift/e2e/ginkgo/sequential/1-120_validate_running_must_gather.gotest/openshift/e2e/ginkgo/sequential/1-121-valiate_resource_constraints_gitopsservice_test.gotest/openshift/e2e/ginkgo/sequential/1-122_validate_namespace_test.gotest/openshift/e2e/ginkgo/sequential/1-123_validate_list_order_comparison_test.go
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
argoproj-labs/argocd-operator(manual)
🚧 Files skipped from review as they are similar to previous changes (50)
- test/openshift/e2e/ginkgo/sequential/1-035_validate_argocd_secret_repopulate_test.go
- test/openshift/e2e/ginkgo/sequential/1-004_validate_argocd_installation_test.go
- test/openshift/e2e/ginkgo/parallel/1-003_validate_console_link_test.go
- test/openshift/e2e/ginkgo/sequential/1-002-validate_backend_service_test.go
- test/openshift/e2e/ginkgo/sequential/1-078_validate_default_argocd_consoleLink_test.go
- test/openshift/e2e/ginkgo/sequential/1-053_validate_argocd_agent_principal_connected_test.go
- test/openshift/e2e/ginkgo/sequential/1-086_validate_default_argocd_role_test.go
- test/openshift/e2e/ginkgo/sequential/1-111_validate_default_argocd_route_test.go
- test/openshift/e2e/ginkgo/parallel/1-084_validate_status_host_ingress_test.go
- test/openshift/e2e/ginkgo/parallel/1-109_validate_reencrypt_termination_policy_test.go
- test/openshift/e2e/ginkgo/parallel/1-081_validate_applicationset_deployment_test.go
- test/openshift/e2e/ginkgo/sequential/1-084_validate_prune_templates.go
- test/openshift/e2e/ginkgo/parallel/1-041_validate_argocd_sync_alert_test.go
- test/openshift/e2e/ginkgo/sequential/1-105_validate_default_argocd_route_test.go
- test/openshift/e2e/ginkgo/parallel/1-005_validate_metrics_test.go
- test/openshift/e2e/ginkgo/sequential/1-106_validate_argocd_metrics_controller_test.go
- test/openshift/e2e/ginkgo/sequential/1-040_validate_quoted_RBAC_group_names_test.go
- test/openshift/e2e/ginkgo/sequential/1-107_validate_redis_scc_test.go
- test/openshift/e2e/ginkgo/sequential/1-006_validate_machine_config_test.go
- test/openshift/e2e/ginkgo/parallel/1-030_validate_reencrypt_test.go
- test/openshift/e2e/ginkgo/parallel/1-082_validate_node_placement_test.go
- test/openshift/e2e/ginkgo/parallel/1-096-validate_home_env_argocd_controller_test.go
- test/e2e/argocd_route_test.go
- test/openshift/e2e/ginkgo/sequential/1-052_validate_rolebinding_number_test.go
- test/openshift/e2e/ginkgo/sequential/1-123_validate_list_order_comparison_test.go
- test/openshift/e2e/ginkgo/parallel/1-103_argocd_alpha_to_beta_conversion_test.go
- test/openshift/e2e/ginkgo/parallel/1-053_validate_cluster_admin_rbac_test.go
- test/openshift/e2e/ginkgo/sequential/1-008_validate-4.9CI-Failures_test.go
- test/openshift/e2e/ginkgo/parallel/1-042_validate_status_host_test.go
- test/openshift/e2e/ginkgo/sequential/1-071_validate_node_selectors_test.go
- test/openshift/e2e/ginkgo/parallel/1-005_validate_route_tls_test.go
- test/openshift/e2e/ginkgo/sequential/1-037_validate_applicationset_in_any_namespace_test.go
- test/openshift/e2e/ginkgo/sequential/1-101_validate_rollout_policyrules_test.go
- test/openshift/e2e/ginkgo/parallel/1-126_validate_servicemonitor_metrics_config_test.go
- test/openshift/e2e/ginkgo/parallel/1-054_validate_deploymentconfig_test.go
- test/openshift/e2e/ginkgo/sequential/1-122_validate_namespace_test.go
- test/openshift/e2e/ginkgo/sequential/1-058_validate_notifications_source_namespaces_test.go
- test/openshift/e2e/ginkgo/sequential/1-020_validate_redis_ha_nonha_test.go
- test/openshift/e2e/ginkgo/parallel/1-004_beta_to_alpha_conversion_test.go
- test/openshift/e2e/ginkgo/sequential/1-059_validate_argocd_agent_terminal_streaming_test.go
- test/openshift/e2e/ginkgo/sequential/1-121-valiate_resource_constraints_gitopsservice_test.go
- test/openshift/e2e/ginkgo/sequential/1-120_validate_running_must_gather.go
- test/openshift/e2e/ginkgo/parallel/1-063_validate_dex_liveness_probe_test.go
- test/openshift/e2e/ginkgo/sequential/1-052_validate_argocd_agent_agent_test.go
- test/openshift/e2e/ginkgo/parallel/1-055_drop_resource_customizations_test.go
- test/openshift/e2e/ginkgo/sequential/1-027_validate_operand_from_git_test.go
- test/openshift/e2e/ginkgo/sequential/1-100_validate_rollouts_resources_creation_test.go
- test/openshift/e2e/ginkgo/sequential/1-051_validate_argocd_agent_principal_test.go
- Makefile
- .github/workflows/kind-ci-automation.yaml
Comment on lines
+112
to
114
| // Clean up old cluster-scoped role from 1-034 | ||
| _ = k8sClient.Delete(ctx, &rbacv1.ClusterRole{ObjectMeta: metav1.ObjectMeta{Name: "custom-argocd-role"}}) | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
git ls-files test/openshift/e2e/ginkgo/fixture/fixture.go
echo '---'
cat -n test/openshift/e2e/ginkgo/fixture/fixture.go | sed -n '90,140p'
echo '---'
rg -n "custom-argocd-role|RunningOnOpenShift|IsOperatorRunningOnOLM|Delete\\(ctx, &rbacv1.ClusterRole" test/openshift/e2e/ginkgo -SRepository: redhat-developer/gitops-operator
Length of output: 6326
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo 'FILE:'
cat -n test/openshift/e2e/ginkgo/fixture/fixture.go | sed -n '100,130p'
echo 'MATCHES:'
rg -n "custom-argocd-role|RunningOnOpenShift|IsOperatorRunningOnOLM|apierr.IsNotFound|Delete\\(" test/openshift/e2e/ginkgo -SRepository: redhat-developer/gitops-operator
Length of output: 18454
Don't ignore ClusterRole delete errors. Ignore only NotFound; unexpected API failures can leave custom-argocd-role behind and contaminate later xKS runs.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/openshift/e2e/ginkgo/fixture/fixture.go` around lines 112 - 114, Update
the cleanup around the ClusterRole deletion in the fixture setup to handle the
error returned by k8sClient.Delete: ignore it only when
apierrors.IsNotFound(err) is true, and propagate or fail the cleanup for all
other errors so custom-argocd-role cannot remain behind.
Source: Path instructions
| }) | ||
|
|
||
| It("ensures that if one managed namespace is stuck in deleting state -- due to a finalizer -- it does not block other managed namespaces from being reconciled", func() { | ||
| It("ensures that if one managed namespace is stuck in deleting state -- due to a finalizer -- it does not block other managed namespaces from being reconciled", Label("openshfit"), func() { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not exclude portable Kubernetes regression tests from xKS.
These test specs validate core Argo CD behavior using standard namespace-scoped Kubernetes resources (Namespaces, ConfigMaps, Deployments, StatefulSets). Their test bodies do not require OpenShift-specific APIs (Routes, ConsoleLinks, SCCs, or OLM). Tagging them with the OpenShift label incorrectly excludes them from the xKS test suite, resulting in a loss of valid test coverage.
test/openshift/e2e/ginkgo/sequential/1-074_validate_terminating_namespace_block_test.go#L52-L52: removeLabel("openshfit")from theIt(...)block.test/openshift/e2e/ginkgo/sequential/1-102_validate_handle_terminating_namespaces_test.go#L84-L84: removeLabel("openshfit")from theIt(...)block.test/openshift/e2e/ginkgo/sequential/1-114_validate_imagepullpolicy_test.go#L52-L52: removeLabel("openshfit")from theIt(...)block.test/openshift/e2e/ginkgo/sequential/1-114_validate_imagepullpolicy_test.go#L141-L141: removeLabel("openshfit")from theIt(...)block.test/openshift/e2e/ginkgo/sequential/1-114_validate_imagepullpolicy_test.go#L197-L197: removeLabel("openshfit")from theIt(...)block.
📍 Affects 3 files
test/openshift/e2e/ginkgo/sequential/1-074_validate_terminating_namespace_block_test.go#L52-L52(this comment)test/openshift/e2e/ginkgo/sequential/1-102_validate_handle_terminating_namespaces_test.go#L84-L84test/openshift/e2e/ginkgo/sequential/1-114_validate_imagepullpolicy_test.go#L52-L52test/openshift/e2e/ginkgo/sequential/1-114_validate_imagepullpolicy_test.go#L141-L141test/openshift/e2e/ginkgo/sequential/1-114_validate_imagepullpolicy_test.go#L197-L197
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@test/openshift/e2e/ginkgo/sequential/1-074_validate_terminating_namespace_block_test.go`
at line 52, Remove the misspelled OpenShift label from the affected It(...)
blocks so these portable Kubernetes regression tests run in xKS. Apply this to
test/openshift/e2e/ginkgo/sequential/1-074_validate_terminating_namespace_block_test.go:52-52,
test/openshift/e2e/ginkgo/sequential/1-102_validate_handle_terminating_namespaces_test.go:84-84,
and
test/openshift/e2e/ginkgo/sequential/1-114_validate_imagepullpolicy_test.go:52-52,
141-141, and 197-197; make no other changes.
|
|
||
| | Label | Meaning | When to use | | ||
| |-------|---------|-------------| | ||
| | `openshfit` | Test requires OpenShift-specific features (Routes, ConsoleLinks, OLM Subscriptions, SCCs, etc.) and cannot run on vanilla Kubernetes (KinD, EKS, GKE, AKS). | Add when the test depends on OpenShift APIs or resources not available on non-OCP clusters. | |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the typo in the OpenShift label name across all documentation and tests.
The label is consistently misspelled as openshfit instead of openshift in the documentation and test files. This mismatch causes confusion and contradicts the PR description, which specifies !openshift. Please correct the spelling across the codebase (including in the XKS_LABEL_FILTER definition in the Makefile).
test/openshift/e2e/README.md#L107-L107: changeopenshfittoopenshift.test/openshift/e2e/README.md#L114-L114: changeopenshfittoopenshift.test/openshift/e2e/README.md#L122-L122: changeopenshfittoopenshift.test/openshift/e2e/README.md#L127-L127: changeopenshfittoopenshift.test/openshift/e2e/README.md#L132-L132: changeopenshfittoopenshift.test/openshift/e2e/README.md#L141-L141: changeopenshfittoopenshift.test/openshift/e2e/ginkgo/parallel/1-001_alpha_to_beta_dex_conversion_test.go#L50-L50: changeLabel("openshfit")toLabel("openshift", "conversionWebhook").test/openshift/e2e/ginkgo/sequential/1-083_validate_apps_in_any_namespace_test.go#L57-L57: changeLabel("openshfit")toLabel("openshift").test/openshift/e2e/ginkgo/sequential/1-092_validate_workload_status_monitoring_alert.go#L48-L48: changeLabel("openshfit")toLabel("openshift").test/openshift/e2e/ginkgo/sequential/1-103-validate-rollouts-imagepullpolicy.go#L51-L51: changeLabel("openshfit")toLabel("openshift").test/openshift/e2e/ginkgo/sequential/1-103-validate-rollouts-imagepullpolicy.go#L75-L75: changeLabel("openshfit")toLabel("openshift").test/openshift/e2e/ginkgo/sequential/1-103-validate-rollouts-imagepullpolicy.go#L118-L118: changeLabel("openshfit")toLabel("openshift").test/openshift/e2e/ginkgo/sequential/1-110_validate_podsecurity_alerts_test.go#L31-L31: changeLabel("openshfit")toLabel("openshift").test/openshift/e2e/ginkgo/sequential/1-112_validate_rollout_plugin_support_test.go#L38-L38: changeLabel("openshfit")toLabel("openshift").test/openshift/e2e/ginkgo/sequential/1-120_repo_server_system_ca_trust.go#L84-L84: changeLabel("openshfit")toLabel("openshift").test/openshift/e2e/ginkgo/sequential/1-120_repo_server_system_ca_trust.go#L103-L103: changeLabel("openshfit")toLabel("openshift").test/openshift/e2e/ginkgo/sequential/1-120_repo_server_system_ca_trust.go#L165-L165: changeLabel("openshfit")toLabel("openshift").test/openshift/e2e/ginkgo/sequential/1-120_repo_server_system_ca_trust.go#L213-L213: changeLabel("openshfit")toLabel("openshift").test/openshift/e2e/ginkgo/sequential/1-120_repo_server_system_ca_trust.go#L245-L245: changeLabel("openshfit")toLabel("openshift").test/openshift/e2e/ginkgo/sequential/1-120_repo_server_system_ca_trust.go#L259-L259: changeLabel("openshfit")toLabel("openshift").
🧰 Tools
🪛 LanguageTool
[grammar] ~107-~107: Ensure spelling is correct
Context: ...e | |-------|---------|-------------| | openshfit | Test requires OpenShift-specific feat...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
📍 Affects 8 files
test/openshift/e2e/README.md#L107-L107(this comment)test/openshift/e2e/README.md#L114-L114test/openshift/e2e/README.md#L122-L122test/openshift/e2e/README.md#L127-L127test/openshift/e2e/README.md#L132-L132test/openshift/e2e/README.md#L141-L141test/openshift/e2e/ginkgo/parallel/1-001_alpha_to_beta_dex_conversion_test.go#L50-L50test/openshift/e2e/ginkgo/sequential/1-083_validate_apps_in_any_namespace_test.go#L57-L57test/openshift/e2e/ginkgo/sequential/1-092_validate_workload_status_monitoring_alert.go#L48-L48test/openshift/e2e/ginkgo/sequential/1-103-validate-rollouts-imagepullpolicy.go#L51-L51test/openshift/e2e/ginkgo/sequential/1-103-validate-rollouts-imagepullpolicy.go#L75-L75test/openshift/e2e/ginkgo/sequential/1-103-validate-rollouts-imagepullpolicy.go#L118-L118test/openshift/e2e/ginkgo/sequential/1-110_validate_podsecurity_alerts_test.go#L31-L31test/openshift/e2e/ginkgo/sequential/1-112_validate_rollout_plugin_support_test.go#L38-L38test/openshift/e2e/ginkgo/sequential/1-120_repo_server_system_ca_trust.go#L84-L84test/openshift/e2e/ginkgo/sequential/1-120_repo_server_system_ca_trust.go#L103-L103test/openshift/e2e/ginkgo/sequential/1-120_repo_server_system_ca_trust.go#L165-L165test/openshift/e2e/ginkgo/sequential/1-120_repo_server_system_ca_trust.go#L213-L213test/openshift/e2e/ginkgo/sequential/1-120_repo_server_system_ca_trust.go#L245-L245test/openshift/e2e/ginkgo/sequential/1-120_repo_server_system_ca_trust.go#L259-L259
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/openshift/e2e/README.md` at line 107, The OpenShift label is misspelled
as openshfit across documentation, Ginkgo tests, and the Makefile’s
XKS_LABEL_FILTER. Replace every occurrence with openshift, preserving the
existing label combinations, including the conversion test’s conversionWebhook
label; update test/openshift/e2e/README.md lines 107-107, 114-114, 122-122,
127-127, 132-132, and 141-141, all listed Ginkgo sites in the specified files,
and the corresponding Makefile definition.
Source: Linked repositories
|
@anandrkskd: The following tests failed, say
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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
/kind enhancement
What does this PR do / why we need it:
Operator currently only tested on OpenShift. This PR enables running e2e suite on non-OpenShift Kubernetes (xKS) by separating OpenShift-dependent tests via labels. Run --label-filter="!openshift" on xKS to skip tests requiring Routes/ConsoleLinks/SCCs/OLM. Broadens CI coverage to vanilla k8s environments.
Currently we are only labellling tests that are passing on openshift and failing on xks, this currenlty helps with the filtering.
Have you updated the necessary documentation?
Which issue(s) this PR fixes:
Fixes #?
Test acceptance criteria:
How to test changes / Special notes to the reviewer:
NON_OLM=true ./bin/ginkgo -v --trace -p --timeout 240m -r ./test/openshift/e2e/ginkgo/sequential
NON_OLM=true ./bin/ginkgo -v --trace -p --timeout 240m -r ./test/openshift/e2e/ginkgo/parallel