[shiftstack] Bootstrap oc when prepare stage is skipped - #4195
tusharjadhav3302 wants to merge 1 commit into
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. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
Hi @tusharjadhav3302. Thanks for your PR. I'm waiting for a openstack-k8s-operators member to verify that this patch is reasonable to test. If it is, they should reply with Tip We noticed you've done this a few times! Consider joining the org to skip this step and gain Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
01becc7 to
0238c9c
Compare
|
/ok-to-test |
|
Build failed (check pipeline). Post ✔️ openstack-k8s-operators-content-provider SUCCESS in 2h 37m 21s |
|
Build failed (check pipeline). Post ✔️ openstack-k8s-operators-content-provider SUCCESS in 2h 11m 01s |
|
/retest-required |
|
Build succeeded (check pipeline). ✔️ openstack-k8s-operators-content-provider SUCCESS in 2h 54m 30s |
|
Hi, I think we on the framework team might be missing a bit of context from the summary you provided. I'm not certain what phased pipeline b is, but after doing quite a bit of digging it looks like this pr is trying to address a situation that arose because of the way some job was split into several jobs. To my understanding, it looks like there used to be one shiftstackclient pod created and used across a single monolithic test suite, but those tests have now been broken up into multiple phases. Now a new shiftstackclient pod is created several times, so some amount of configuration (oc client install, /etc/hosts entries, etc) needs to be put in all the right places in the new pod before testing can resume. Correct me if my understanding is incorrect on any of the above This leaves me wondering, is this something that could be handled from the shiftstack qa side? For example, could there be a Additionally, if this does need to live in the cifmw, I will ask that you maintain a clean git history. We don't squash commits when we merge, so instead of committing something broken then immediately fixing it with a second commit, just amend the initial commit. Thanks! |
|
Hi @michburk — thanks for digging in. Your reading is mostly correct; a bit more context: What changed Why Why this lives in cifmw (not a QA
A QA Happy to move the bootstrap into shiftstack-qa later if that team prefers ownership there; this PR unblocks Pipeline B without changing the monolithic path. On history: we will amend / leave a clean single commit for merge (no broken-then-fix pair). |
Phased Pipeline B run-tests recreates the shiftstackclient pod but omits the QA prepare stage. Without prepare, verification fails on missing oc, /etc/hosts FIP entries, clouds.shiftstack, and artifacts/resources.yml. When stages_override is set and does not include prepare, restore that cold-start environment from the installation PVC and install-config (same sources prepare uses) before running tests. Use a literal block scalar for the in-pod shell so newlines are preserved. Signed-off-by: Tushar Jadhav <tjadhav@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
f60fe4a to
ff2eca5
Compare
|
Build failed (check pipeline). Post ✔️ openstack-k8s-operators-content-provider SUCCESS in 3h 20m 37s |
|
Thanks for cleaning up the git history, I'm still a little unclear on why this logic needs to live in the cifmw. To these two points:
and
The instruction to skip the And to this point:
I noticed you were active in the shiftstack qa repo and you are listed as an approver/reviewer there, which is largely why I'm asking you if this logic is better handled in that repo, sorry if I'm misunderstanding your role there. I'm mostly concerned with not duplicating code across repos. If you change anything about the configurations in the prepare step of the shiftstack qa repo, the logic here in the cifmw would diverge. To me, it would make more sense to track all of that in one repo, and the shiftstack qa repo already owns similar logic in the And again I'm by no means an expert on the shiftstack qa repo, I've just glanced over it to understand the gist of how things work. Is there some concrete technical reason why something like a
but isn't a similar task already handled by the |
|
Hi @michburk — thanks again for the careful review and for pushing on ownership/duplication. You were right. The We moved that work into shiftstack-qa as a dedicated stage:
Callers that omit We validated Pipeline B on warm serval70 without this PR (no Depends-On on #4195; CIF stayed on main). Closing this PR in favor of shiftstack-qa#43. Appreciate you taking the time to dig through the phased-job context. |
|
Closing in favor of shiftstack-qa#43 (prepare_client_pod). See reply to @michburk above. |
3 participants
Summary
Phased Pipeline B (
run-tests) setscifmw_shiftstack_stages_overrideto verification/test stages only and omitsprepare. The shiftstack role always recreates theshiftstackclientpod, and QA only installs/usr/local/bin/ocduringprepare. Result: verification fails immediately withoc: command not found.When
stages_overrideis non-empty and does not includeprepare, bootstrap a stable OpenShift client into the pod (same cold-start approach as shiftstack-qaget_openshift_release_binaries) before running tests.roles/shiftstack/tasks/bootstrap_oc_client.ymlstages_override: []) are unchangedcifmw_shiftstack_bootstrap_oc_client: falseTest plan
shiftstackclientpod → confirmocmissing → run bootstrap →ocon PATH (Zuul failure mode gone)Made with Cursor