-
Notifications
You must be signed in to change notification settings - Fork 152
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
(WIP) Refactor and modularize OpenShift CI script for better maintainability of our CI execution #1992
base: main
Are you sure you want to change the base?
Conversation
[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.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Hey there and thank you for opening this pull request! 👋🏼 We require pull request titles to follow the Conventional Commits specification and it looks like your proposed title needs to be adjusted. Details:
|
778abaa
to
dd90659
Compare
The image is available at: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @gustavolira, thank you for that, this is definitely needed! I have a couple of thoughts about that though.
- There are already separate files with functions for AKS, GKE (and OSD) located here: https://github.com/janus-idp/backstage-showcase/blob/main/.ibm/pipelines//cluster
Please make sure to take them into account and incorporate them. - We have the
install_oc
andinstall_helm
functions there (inopenshift-ci-tests.sh
orutils.sh
), but they are redundant, since we are already installing both in the https://github.com/janus-idp/backstage-showcase/blob/main/.ibm/images/Dockerfile#L37. I vote for deleting them. - Now we use only a single
openshift-ci-tests.sh
file to run the jobs and then we differentiate it based on the job name. I think we better split it and use theopenshift-ci-tests.sh
for OCP tests and https://github.com/janus-idp/backstage-showcase/blob/main/.ibm/pipelines/kubernetes-tests.sh for Kubernetes tests (GKE and AKS). (I would delete thekubernetes-tests.sh
s content first, as it is not used anymore, and start from scratch.)
To do that, we can edit https://github.com/openshift/release/blob/4d24aecb5f0cb787a9ec0e3e6d1744f234c3e184/ci-operator/step-registry/janus-idp/backstage-showcase/helm-aks-nightly/janus-idp-backstage-showcase-helm-aks-nightly-commands.sh#L19 and https://github.com/openshift/release/blob/4d24aecb5f0cb787a9ec0e3e6d1744f234c3e184/ci-operator/step-registry/janus-idp/backstage-showcase/helm-gke-nightly/janus-idp-backstage-showcase-helm-gke-nightly-commands.sh#L19
That way we can avoid some moreif
s andcase
switches.
I offer my assistance with AKS and GKE.
If you want to test that these changes are not breaking AKS and GKE yourself, you can follow this doc and also set JOB_NAME=periodic-aks
or JOB_NAME=periodic-gke
respectively.
Hi @zdrapela thank you very much for your attention here!
|
480f6cb
to
94ae839
Compare
The image is available at: |
@gustavolira: The following test failed, say
Full PR test history. Your PR dashboard. 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. |
Modularized the OpenShift CI script by separating main functionalities into individual files. This change improves the maintainability and readability of the scripts by isolating specific tasks, such as handling AKS/GKE deployments and finalizers. Signed-off-by: Gustavo Lira <[email protected]>
Renamed, deleted, and refactored scripts to improve maintainability. Loaded utility functions and environment-specific scripts in a cleaner manner. Simplified the main function to use a case statement for deployment initiation. Signed-off-by: Gustavo Lira <[email protected]>
Signed-off-by: Gustavo Lira <[email protected]>
Removed redundant installation commands for Helm and oc from various deployment scripts. The utilities for installing Helm and oc have been eliminated, streamlining the setup process across different environments. Signed-off-by: Gustavo Lira <[email protected]>
Signed-off-by: Gustavo Lira <[email protected]>
b2ae095
to
7398c98
Compare
The image is available at: |
Modularized the OpenShift CI script by separating main functionalities into individual files. This change improves the maintainability and readability of the scripts by isolating specific tasks, such as handling AKS/GKE deployments and finalizers.
Description
Please explain the changes you made here.
Which issue(s) does this PR fix
PR acceptance criteria
Please make sure that the following steps are complete:
How to test changes / Special notes to the reviewer