Mark openshift & xKS e2e tests #1216
Open
anandrkskd wants to merge 3 commits into
Open
Conversation
Signed-off-by: Anand Kumar Singh <anandrkskd@gmail.com>
Signed-off-by: Anand Kumar Singh <anandrkskd@gmail.com>
|
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 |
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR derives the Ginkgo CLI version from ChangesOpenShift E2E test and tooling updates
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
Signed-off-by: Anand Kumar Singh <anandrkskd@gmail.com>
![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
+174
to
+175
| // 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
Keep cluster-agnostic cleanup outside the OpenShift gate.
custom-argocd-role is a standard Kubernetes ClusterRole. On xKS, this change leaves it behind between sequential tests, allowing stale state to contaminate later runs. Move its deletion before Line 114; keep only OpenShift-specific resources inside the gate.
As per path instructions, “Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.”
🤖 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 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.
Source: Path instructions
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.
| }) | ||
|
|
||
| It("verifying that ArgoCD CR '.spec.applicationset.sourcenamespaces' and '.spec.sourcenamespaces' correctly control role/rolebindings within the managed namespaces", func() { | ||
| It("verifying that ArgoCD CR '.spec.applicationset.sourcenamespaces' and '.spec.sourcenamespaces' correctly control role/rolebindings within the managed namespaces", Label("openshift"), func() { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Remove openshift labels from portable tests.
These specs use generic Kubernetes and Argo CD capabilities, so --label-filter="!openshift" incorrectly removes them from xKS coverage.
test/openshift/e2e/ginkgo/sequential/1-037_validate_applicationset_in_any_namespace_test.go#L69-L69: Remove the label from the ApplicationSet RBAC test.test/openshift/e2e/ginkgo/sequential/1-114_validate_imagepullpolicy_test.go#L52-L52: Remove the label from the namespace-scoped image policy test.test/openshift/e2e/ginkgo/sequential/1-120_repo_server_system_ca_trust.go#L103-L103: Remove the label from the missing-Secret CA-trust test.test/openshift/e2e/ginkgo/sequential/1-122_validate_namespace_test.go#L74-L74: Remove the label from the generic namespace validation test.
Apply the same correction to the other affected ranges identified in the per-file comments.
📍 Affects 4 files
test/openshift/e2e/ginkgo/sequential/1-037_validate_applicationset_in_any_namespace_test.go#L69-L69(this comment)test/openshift/e2e/ginkgo/sequential/1-114_validate_imagepullpolicy_test.go#L52-L52test/openshift/e2e/ginkgo/sequential/1-120_repo_server_system_ca_trust.go#L103-L103test/openshift/e2e/ginkgo/sequential/1-122_validate_namespace_test.go#L74-L74
🤖 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-037_validate_applicationset_in_any_namespace_test.go`
at 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.
| }) | ||
|
|
||
| It("should handle route enabled configuration correctly", func() { | ||
| It("should handle route enabled configuration correctly", Label("openshift"), func() { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Label the route-toggle scenario consistently.
The newly labeled route-enabled test is followed by an unlabeled test that enables, disables, and recreates an OpenShift Route. With --label-filter="!openshift", that route-dependent scenario can still run on xKS. Add the same label to the route-toggle spec, or remove its OpenShift-only behavior behind an explicit platform guard.
As per PR objectives, !openshift should skip tests that require OpenShift-specific resources.
🤖 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-051_validate_argocd_agent_principal_test.go`
at 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.
| }) | ||
|
|
||
| It("verifies RoleBindings are added to namespace-scoped Namespace when that Namespace is managed by openshift-gitops", func() { | ||
| It("verifies RoleBindings are added to namespace-scoped Namespace when that Namespace is managed by openshift-gitops", Label("openshift"), func() { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not mark standard Kubernetes behavior as OpenShift-only.
These specs do not use Routes, ConsoleLinks, SCCs, or another demonstrated OpenShift-only API. With --label-filter="!openshift", the new labels make xKS skip coverage for generic agent configuration, RBAC, notification source namespaces, and terminating-namespace behavior. Remove the labels from these capability tests, or split any genuinely OpenShift-dependent setup from the portable assertions.
test/openshift/e2e/ginkgo/sequential/1-052_validate_rolebinding_number_test.go#L38-L38: removeLabel("openshift"); the test only validates Namespace labels and RoleBindings.test/openshift/e2e/ginkgo/sequential/1-051_validate_argocd_agent_principal_test.go#L308-L308: keep this image/resource test portable unless its fixture has an undocumented OpenShift-only dependency.test/openshift/e2e/ginkgo/sequential/1-051_validate_argocd_agent_principal_test.go#L349-L349: keep default-image startup coverage portable.test/openshift/e2e/ginkgo/sequential/1-051_validate_argocd_agent_principal_test.go#L411-L411: keep configuration-reflection coverage portable.test/openshift/e2e/ginkgo/sequential/1-051_validate_argocd_agent_principal_test.go#L628-L628: keep ClusterIP service behavior portable.test/openshift/e2e/ginkgo/sequential/1-051_validate_argocd_agent_principal_test.go#L653-L653: keep service-type coverage portable unless an OpenShift-specific LoadBalancer requirement is intentional.test/openshift/e2e/ginkgo/sequential/1-051_validate_argocd_agent_principal_test.go#L678-L678: keep service update coverage portable.test/openshift/e2e/ginkgo/sequential/1-051_validate_argocd_agent_principal_test.go#L720-L720: keep namespace-scoped deployment coverage portable.test/openshift/e2e/ginkgo/sequential/1-051_validate_argocd_agent_principal_test.go#L855-L855: keep NetworkPolicy coverage portable.test/openshift/e2e/ginkgo/sequential/1-051_validate_argocd_agent_principal_test.go#L918-L918: use a capability-specific requirement for ServiceMonitor rather than treating it as inherently OpenShift-only.test/openshift/e2e/ginkgo/sequential/1-052_validate_argocd_agent_agent_test.go#L284-L284: keep agent image/resource coverage portable.test/openshift/e2e/ginkgo/sequential/1-052_validate_argocd_agent_agent_test.go#L371-L371: keep agent configuration-reflection coverage portable.test/openshift/e2e/ginkgo/sequential/1-052_validate_argocd_agent_agent_test.go#L589-L589: use a monitoring capability requirement for ServiceMonitor coverage.test/openshift/e2e/ginkgo/sequential/1-058_validate_notifications_source_namespaces_test.go#L61-L61: keep notification source-namespace behavior portable.test/openshift/e2e/ginkgo/sequential/1-058_validate_notifications_source_namespaces_test.go#L201-L201: keep unmanaged-source-namespace behavior portable.test/openshift/e2e/ginkgo/sequential/1-058_validate_notifications_source_namespaces_test.go#L396-L396: keep disabled-notifications behavior portable.test/openshift/e2e/ginkgo/sequential/1-058_validate_notifications_source_namespaces_test.go#L465-L465: keep source-namespace update behavior portable.test/openshift/e2e/ginkgo/sequential/1-058_validate_notifications_source_namespaces_test.go#L546-L546: keep re-enabling notifications coverage portable.test/openshift/e2e/ginkgo/sequential/1-074_validate_terminating_namespace_block_test.go#L52-L52: remove the label; this scenario uses only standard Namespace, ConfigMap, finalizer, and RBAC behavior.
As per PR objectives, --label-filter="!openshift" is intended to skip OpenShift-dependent tests, not generic Kubernetes behavior.
📍 Affects 5 files
test/openshift/e2e/ginkgo/sequential/1-052_validate_rolebinding_number_test.go#L38-L38(this comment)test/openshift/e2e/ginkgo/sequential/1-051_validate_argocd_agent_principal_test.go#L308-L308test/openshift/e2e/ginkgo/sequential/1-051_validate_argocd_agent_principal_test.go#L349-L349test/openshift/e2e/ginkgo/sequential/1-051_validate_argocd_agent_principal_test.go#L411-L411test/openshift/e2e/ginkgo/sequential/1-051_validate_argocd_agent_principal_test.go#L628-L628test/openshift/e2e/ginkgo/sequential/1-051_validate_argocd_agent_principal_test.go#L653-L653test/openshift/e2e/ginkgo/sequential/1-051_validate_argocd_agent_principal_test.go#L678-L678test/openshift/e2e/ginkgo/sequential/1-051_validate_argocd_agent_principal_test.go#L720-L720test/openshift/e2e/ginkgo/sequential/1-051_validate_argocd_agent_principal_test.go#L855-L855test/openshift/e2e/ginkgo/sequential/1-051_validate_argocd_agent_principal_test.go#L918-L918test/openshift/e2e/ginkgo/sequential/1-052_validate_argocd_agent_agent_test.go#L284-L284test/openshift/e2e/ginkgo/sequential/1-052_validate_argocd_agent_agent_test.go#L371-L371test/openshift/e2e/ginkgo/sequential/1-052_validate_argocd_agent_agent_test.go#L589-L589test/openshift/e2e/ginkgo/sequential/1-058_validate_notifications_source_namespaces_test.go#L61-L61test/openshift/e2e/ginkgo/sequential/1-058_validate_notifications_source_namespaces_test.go#L201-L201test/openshift/e2e/ginkgo/sequential/1-058_validate_notifications_source_namespaces_test.go#L396-L396test/openshift/e2e/ginkgo/sequential/1-058_validate_notifications_source_namespaces_test.go#L465-L465test/openshift/e2e/ginkgo/sequential/1-058_validate_notifications_source_namespaces_test.go#L546-L546test/openshift/e2e/ginkgo/sequential/1-074_validate_terminating_namespace_block_test.go#L52-L52
|
@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
1 participant
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