Skip to content

refactor(ci): modularize pipeline utilities into lib/ structure #3817

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
gustavolira wants to merge 31 commits into
base: main
Choose a base branch
from
Open

refactor(ci): modularize pipeline utilities into lib/ structure #3817

gustavolira wants to merge 31 commits into from
+914 −104

Conversation

@gustavolira
Copy link
Member

@gustavolira gustavolira commented Dec 8, 2025

Description

Extract common functions from utils.sh into focused modules to improve maintainability and reduce code duplication.

Which issue(s) does this PR fix

PR acceptance criteria

Please make sure that the following steps are complete:

  • GitHub Actions are completed and successful
  • Unit Tests are updated and passing
  • E2E Tests are updated and passing
  • Documentation is updated if necessary (requirement for new features)
  • Add a screenshot if the change is UX/UI related

How to test changes / Special notes to the reviewer

@openshift-ci openshift-ci bot requested review from albarbaro and psrna December 8, 2025 14:10
@gustavolira
Copy link
Member Author

gustavolira commented Dec 8, 2025

/review

@rhdh-qodo-merge
Copy link

rhdh-qodo-merge bot commented Dec 8, 2025 •
edited
Loading

PR Reviewer Guide 🔍

(Review updated until commit b3ffc23)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🏅 Score: 86
🔒 Security concerns

Sensitive information handling:
common::oc_login echoes "OCP version: $(oc version)". If oc is configured to print server URLs, ensure no tokens are included. Also, several functions manipulate secrets (Postgres credentials) and patch files using sed; confirm no secrets are logged. Add explicit checks to avoid echoing sensitive env vars (like K8S_CLUSTER_TOKEN) on failures.

🔀 Multiple PR themes

Sub-PR theme: Extract common, wait, and operator modules and wire into utils.sh

Relevant files:

  • .ibm/pipelines/lib/common.sh
  • .ibm/pipelines/lib/k8s-wait.sh
  • .ibm/pipelines/lib/operators.sh
  • .ibm/pipelines/utils.sh
Sub-PR theme: Refactor jobs to use common::oc_login and new lib utilities

Relevant files:

  • .ibm/pipelines/jobs/auth-providers.sh
  • .ibm/pipelines/jobs/ocp-nightly.sh
  • .ibm/pipelines/jobs/ocp-operator.sh
  • .ibm/pipelines/jobs/ocp-pull.sh
  • .ibm/pipelines/jobs/upgrade.sh
Sub-PR theme: Documentation updates for modular pipeline libraries

Relevant files:

  • .ibm/pipelines/README.md
  • .ibm/pipelines/lib/README.md
⚡ Recommended focus areas for review

Matching Pods

k8s_wait::deployment picks the first pod whose name matches the resource string via grep. This can select unintended pods (partial matches, multiple replicas) and miss readiness of Deployments/StatefulSets. Consider scoping by label/owner or using rollout status for Deployment/StatefulSet to avoid flaky waits.

 
for ((i = 1; i <= max_attempts; i++)); do
  local pod_name
  pod_name=$(oc get pods -n "$namespace" | grep "$resource_name" | awk '{print $1}' | head -n 1)

  if [[ -n "$pod_name" ]]; then
    local pod_ready_status
    pod_ready_status=$(oc get pod "$pod_name" -n "$namespace" -o jsonpath='{.status.conditions[?(@.type=="Ready")].status}')

    if [[ "$pod_ready_status" == "True" ]]; then
      log::success "Resource '$resource_name' is ready in namespace '$namespace'"
      return 0
    fi

    local container_statuses
    container_statuses=$(oc get pod "$pod_name" -n "$namespace" -o jsonpath='{.status.containerStatuses}')
    log::debug "Pod '$pod_name' status: Ready=$pod_ready_status, Containers=$container_statuses"
  else
    log::debug "No pods found matching '$resource_name' in namespace '$namespace'"
  fi
📚 Focus areas based on broader codebase context

Missing Dependency Check

The function common::oc_login calls log::error, but this module does not source the logging library. Ensure lib/log.sh is sourced or avoid using log:: calls here to prevent runtime errors when used standalone. (Ref 8)

 
common::oc_login() {
  if ! command -v oc &> /dev/null; then
    log::error "oc command not found. Please install OpenShift CLI."
    return 1
  fi

  oc login --token="${K8S_CLUSTER_TOKEN}" --server="${K8S_CLUSTER_URL}" --insecure-skip-tls-verify=true
  echo "OCP version: $(oc version)"
  return 0

Reference reasoning: Other bash utilities in the referenced scripts rely on local logging helpers before calling log functions, demonstrating the pattern of ensuring dependencies are available within the script context.

📄 References
  1. redhat-developer/rhdh-operator/hack/validate-image-digests.sh [1-55]
  2. redhat-developer/rhdh-operator/hack/validate-image-digests.sh [56-69]
  3. redhat-developer/rhdh-operator/hack/validate-image-digests.sh [104-145]
  4. redhat-developer/rhdh-operator/hack/validate-image-digests.sh [71-103]
  5. redhat-developer/rhdh-operator/hack/validate-image-digests.sh [146-157]
  6. redhat-developer/rhdh-operator/hack/validate-image-digests.sh [159-190]
  7. redhat-developer/rhdh-operator/hack/validate-image-digests.sh [192-199]
  8. redhat-developer/rhdh-operator/config/profile/rhdh/plugin-infra/plugin-infra.sh [1-49]

@gustavolira
Copy link
Member Author

gustavolira commented Dec 8, 2025

/review
-i
--pr_reviewer.require_score_review=true
--pr_reviewer.require_can_be_split_review=true
--pr_reviewer.require_soc2_ticket=false
--pr_reviewer.num_code_suggestions="2"

@rhdh-qodo-merge
Copy link

rhdh-qodo-merge bot commented Dec 8, 2025

Persistent review updated to latest commit b3ffc23

@gustavolira
Copy link
Member Author

gustavolira commented Dec 8, 2025

/improve
--pr_code_suggestions.num_code_suggestions_per_chunk="2"
--pr_code_suggestions.commitable_code_suggestions=true

rhdh-qodo-merge[bot]
rhdh-qodo-merge bot reviewed Dec 8, 2025
View reviewed changes
@github-actions
Copy link
Contributor

github-actions bot commented Dec 8, 2025

The image is available at:

@github-actions
Copy link
Contributor

github-actions bot commented Dec 8, 2025

The image is available at:

@zdrapela
Copy link
Member

zdrapela commented Dec 9, 2025

Just testing my privileges /approve

@zdrapela
Copy link
Member

zdrapela commented Dec 9, 2025

/approve

@openshift-ci openshift-ci bot added the approved label Dec 9, 2025
@zdrapela
Copy link
Member

zdrapela commented Dec 9, 2025

/unapprove

@zdrapela
Copy link
Member

zdrapela commented Dec 9, 2025

/remove-approve

@openshift-ci openshift-ci bot removed the approved label Dec 9, 2025
@zdrapela
Copy link
Member

zdrapela commented Dec 9, 2025

/lgtm

@gustavolira
fix(ci): enhance wait mechanism for database resource creation in ope…
02dd774 
…rator deployment

Updated the `deploy_rhdh_operator` function to wait for either a PostgresCluster or StatefulSet resource to be created by the operator. Improved logging to provide clarity on which resource is being checked and added error handling for cases where neither resource is created within the specified wait time. This change enhances the reliability of the deployment process and aids in troubleshooting.
@gustavolira
fix(ci): streamline wait logic in operator deployment for database re…
42d3765 
…sources

Refined the `deploy_rhdh_operator` function to eliminate unnecessary whitespace and improve the clarity of the wait mechanism for database resource creation. This update enhances the readability of the code while maintaining the existing functionality and logging for resource checks.
@github-actions
Copy link
Contributor

github-actions bot commented Jan 7, 2026

The image is available at:

/test e2e-ocp-helm

@gustavolira
Copy link
Member Author

gustavolira commented Jan 7, 2026

/test e2e-ocp-operator-nightly

@gustavolira gustavolira closed this Jan 7, 2026
@gustavolira gustavolira reopened this Jan 7, 2026
@gustavolira
Copy link
Member Author

gustavolira commented Jan 7, 2026

/test e2e-ocp-operator-nightly

@github-actions
Copy link
Contributor

github-actions bot commented Jan 7, 2026

🚫 Image Push Skipped.

The container image push was skipped because the build was skipped (either due to [skip-build] tag or no relevant changes with existing image)

/test e2e-ocp-helm

@gustavolira
Copy link
Member Author

gustavolira commented Jan 8, 2026

/test e2e-ocp-operator-nightly

@gustavolira
fix(ci): enhance orchestrator plugins enabling process in utils.sh
945de68 
Refined the `enable_orchestrator_plugins_op` function to improve the process of enabling orchestrator plugins. This update includes extracting and merging custom and default dynamic plugins, applying the merged configmap, and restarting the Backstage deployment. Enhanced logging and error handling were added to ensure clarity and reliability during the plugin enabling process.
@github-actions
Copy link
Contributor

github-actions bot commented Jan 8, 2026

🚫 Image Push Skipped.

The container image push was skipped because the build was skipped (either due to [skip-build] tag or no relevant changes with existing image)

/test e2e-ocp-helm

@gustavolira
Copy link
Member Author

gustavolira commented Jan 8, 2026

/test e2e-ocp-operator-nightly

@gustavolira
fix(ci): improve error handling and logging in operator deployment fo…
a93421e 
…r Backstage resource

Enhanced the `deploy_rhdh_operator` function to log an error if the Backstage deployment is not created within the specified wait time. Added additional logging to check the status of the Backstage CR and the operator logs for better troubleshooting. This change improves the reliability of the deployment process and aids in identifying issues during the Backstage resource creation.
@github-actions
Copy link
Contributor

github-actions bot commented Jan 8, 2026

🚫 Image Push Skipped.

The container image push was skipped because the build was skipped (either due to [skip-build] tag or no relevant changes with existing image)

/test e2e-ocp-helm

@gustavolira
Copy link
Member Author

gustavolira commented Jan 8, 2026

/test e2e-ocp-operator-nightly

1 similar comment
@gustavolira
Copy link
Member Author

gustavolira commented Jan 8, 2026

/test e2e-ocp-operator-nightly

@gustavolira
fix(ci): refine plugin merging logic in enable_orchestrator_plugins_o…
36b154a 
…p function

Updated the plugin merging process in the `enable_orchestrator_plugins_op` function to utilize yq for improved clarity and efficiency. The new implementation merges default and custom plugins while ensuring deduplication by package name, enhancing the robustness of plugin management and maintaining custom plugin precedence.
@github-actions
Copy link
Contributor

github-actions bot commented Jan 8, 2026

The image is available at:

/test e2e-ocp-helm

@gustavolira
Copy link
Member Author

gustavolira commented Jan 8, 2026

/test e2e-ocp-operator-nightly

@gustavolira
fix(ci): update logging for orchestrator plugins enabling process in …
403a04e 
…utils.sh

Removed the wait logic for Backstage deployment readiness after enabling orchestrator plugins. Updated logging to clarify that deployment verification will occur in subsequent calls, enhancing the clarity of the process.
@sonarqubecloud
Copy link

sonarqubecloud bot commented Jan 8, 2026

Quality Gate Passed Quality Gate passed

Issues
4 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@github-actions
Copy link
Contributor

github-actions bot commented Jan 9, 2026

The image is available at:

/test e2e-ocp-helm

@openshift-ci
Copy link

openshift-ci bot commented Jan 9, 2026

@gustavolira: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-aks-helm-nightly 7fef4a8 link false /test e2e-aks-helm-nightly
ci/prow/e2e-ocp-operator-nightly 36b154a link false /test e2e-ocp-operator-nightly
ci/prow/e2e-ocp-helm 403a04e link true /test e2e-ocp-helm

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

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

Reviewers

@albarbaro albarbaro Awaiting requested review from albarbaro

@psrna psrna Awaiting requested review from psrna

2 more reviewers

@zdrapela zdrapela zdrapela left review comments

@rhdh-qodo-merge rhdh-qodo-merge[bot] rhdh-qodo-merge[bot] left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@zdrapela zdrapela

Labels

approved Possible security concern Review effort 3/5 Stale

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@gustavolira @zdrapela @openshift-merge-robot