Repository navigation
OSAC-1733: Build multiple components from one checkout via a components array - #283
Merged
eliorerz merged 1 commit intoJul 31, 2026
Conversation
|
@eliorerz: This pull request references OSAC-1733 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: eliorerz The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
WalkthroughBMaaS, CaaS, and VMaaS full-install workflows now accept JSON arrays of components, build and load each configured image, apply aggregated installer overrides and CRDs, and clean up all recorded component images. ChangesMulti-component E2E installation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Workflow
participant ComponentRepositories
participant ClusterNode
participant OSACInstaller
Workflow->>ComponentRepositories: Clone and build configured components
ComponentRepositories-->>Workflow: Images, metadata, and optional CRDs
Workflow->>ClusterNode: Load component images
Workflow->>OSACInstaller: Pass aggregated build results
OSACInstaller->>ClusterNode: Apply image overrides and CRDs
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 11✅ Passed checks (11 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Merged
3 of 5 tasks
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/e2e-vmaas-full-install.yml (1)
746-753: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winClean up all locally loaded component images in teardown.
.github/scripts/teardown.shonly prunes${COMPONENT_IMAGE:-}, but this workflow can also loadCOMPONENT2_IMAGEfromlocalhost/component2-pr:${GITHUB_RUN_ID}directly onto the runner. Add the samepodman rmibranch for${COMPONENT2_IMAGE:-}to prevent accumulated images on reused self-hosted runners.🤖 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/e2e-vmaas-full-install.yml around lines 746 - 753, Update teardown.sh to also remove the locally loaded COMPONENT2_IMAGE, using the same conditional podman rmi branch already applied to COMPONENT_IMAGE; preserve the existing cleanup behavior and reference the component2-pr image value supplied by this workflow.
🤖 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.
Outside diff comments:
In @.github/workflows/e2e-vmaas-full-install.yml:
- Around line 746-753: Update teardown.sh to also remove the locally loaded
COMPONENT2_IMAGE, using the same conditional podman rmi branch already applied
to COMPONENT_IMAGE; preserve the existing cleanup behavior and reference the
component2-pr image value supplied by this workflow.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: osac-project/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b67813d1-c4ea-4392-91d2-4a125c6c7ba1
📒 Files selected for processing (2)
.github/workflows/e2e-bmaas-full-install.yml.github/workflows/e2e-vmaas-full-install.yml
eliorerz
force-pushed
the
support-monorepo-multi-component-e2e
branch
from
July 31, 2026 01:07
b6ed717 to
df6db4b
Compare
eliorerz
changed the title
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/scripts/replace-installer-submodule.sh (1)
69-69: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftMonorepo component subdirectory is dropped everywhere it matters. The new monorepo contract puts a component under
<dir>/(fulfillment-service/Containerfile), but both the submodule copy and the CRD capture still assume the component is at the checkout root.
.github/scripts/replace-installer-submodule.sh#L69-L69: copy${COMPONENT_SRC}/<containerfile dir>/.into the staging dir when the containerfile is nested, instead of the whole checkout..github/workflows/e2e-bmaas-full-install.yml#L328-L333: resolveconfig/crd/basesunder${src}/$(dirname "${file}")before decidingcrds_dir..github/workflows/e2e-vmaas-full-install.yml#L346-L351: apply the samedirname-based resolution for the CRD capture.🤖 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/scripts/replace-installer-submodule.sh at line 69, Update .github/scripts/replace-installer-submodule.sh lines 69-69 to copy the nested containerfile directory from COMPONENT_SRC into STAGING instead of always copying the checkout root. Update .github/workflows/e2e-bmaas-full-install.yml lines 328-333 and .github/workflows/e2e-vmaas-full-install.yml lines 346-351 to resolve config/crd/bases beneath the directory returned by dirname of the containerfile path before selecting crds_dir.
🤖 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/scripts/replace-installer-submodule.sh:
- Around line 33-37: Update the repository-name resolution around
COMPONENT_CONTAINERFILE to use the containerfile’s basename rather than dirname,
while preserving COMPONENT_REPO_NAME as the fallback for standalone or unmapped
paths. In the SUBMODULE_MAP lookup and warning path, make an unmapped component
fatal by exiting nonzero so stale submodule pins cannot proceed silently.
In @.github/scripts/teardown.sh:
- Around line 61-64: Update the component-image cleanup flow around
COMPONENT_BUILD_RESULTS to parse and validate the JSON before entering the loop,
failing teardown if jq is unavailable or parsing fails. Filter out missing,
null, and empty .image values so the loop passes only non-empty image strings to
podman rmi.
In @.github/workflows/e2e-bmaas-full-install.yml:
- Around line 335-336: Add shell: bash to the image-loading steps in
.github/workflows/e2e-bmaas-full-install.yml lines 335-336,
.github/workflows/e2e-vmaas-full-install.yml lines 353-354, and
.github/workflows/e2e-caas-full-install.yml lines 329-330, and begin each script
with set -euo pipefail so podman save failures propagate through the pipe.
- Around line 141-164: Replace the duplicated validation loops with a shared
composite action at .github/actions/validate-components. The action must
tolerate empty-string input consistently with the build step, validate each
component’s repo, ref, imageKey, containerfile, and buildType, and preserve the
existing JSON-array and org/repo checks. Update
.github/workflows/e2e-bmaas-full-install.yml lines 141-164,
.github/workflows/e2e-caas-full-install.yml lines 141-164, and
.github/workflows/e2e-vmaas-full-install.yml lines 141-164 to invoke the shared
action.
- Around line 141-164: Update the Validate components input step to treat an
empty COMPONENTS_JSON value consistently with the build-step guard by skipping
component validation/build processing when it is empty. For non-empty input,
validate each component’s buildType against the supported values and require
containerfile when that build type uses it, rejecting containerfile values
containing path traversal such as “../”.
- Around line 309-315: Replace the unpinned, system-level ansible-builder
installation in the build_type=="ansible-builder" branches with an exact,
identical ansible-builder version installed in an isolated venv or pipx
environment, and invoke that environment for ansible-builder create. Apply the
same change in .github/workflows/e2e-bmaas-full-install.yml lines 309-315,
.github/workflows/e2e-vmaas-full-install.yml lines 327-333, and
.github/workflows/e2e-caas-full-install.yml lines 313-319.
In @.github/workflows/e2e-caas-full-install.yml:
- Around line 332-335: Update the CaaS result construction around the jq
expression in the workflow so crdsDir is populated using the same CRD capture
flow as e2e-vmaas-full-install.yml, rather than hardcoded to an empty string.
Preserve the existing CRD-apply branch behavior so operator PR runs use the
captured CRDs.
---
Outside diff comments:
In @.github/scripts/replace-installer-submodule.sh:
- Line 69: Update .github/scripts/replace-installer-submodule.sh lines 69-69 to
copy the nested containerfile directory from COMPONENT_SRC into STAGING instead
of always copying the checkout root. Update
.github/workflows/e2e-bmaas-full-install.yml lines 328-333 and
.github/workflows/e2e-vmaas-full-install.yml lines 346-351 to resolve
config/crd/bases beneath the directory returned by dirname of the containerfile
path before selecting crds_dir.
🪄 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: osac-project/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 2d260e0a-8a8e-428b-9bbf-5a1211b6b4db
📒 Files selected for processing (5)
.github/scripts/replace-installer-submodule.sh.github/scripts/teardown.sh.github/workflows/e2e-bmaas-full-install.yml.github/workflows/e2e-caas-full-install.yml.github/workflows/e2e-vmaas-full-install.yml
Comment on lines
+335
to
+336
| echo "Loading image onto node ${NODE_NAME}..." | ||
| podman save "${tag}" | oc debug "node/${NODE_NAME}" -t=false -- chroot /host podman load |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Image-load pipe hides podman save failures in all three workflows. These steps use the default bash -e {0} shell (no pipefail), so only oc debug's exit status is checked and a missing image surfaces much later as a rollout timeout.
.github/workflows/e2e-bmaas-full-install.yml#L335-L336: addshell: bashplusset -euo pipefailat the top of this step's script..github/workflows/e2e-vmaas-full-install.yml#L353-L354: add the sameshell: bash/set -euo pipefail..github/workflows/e2e-caas-full-install.yml#L329-L330: add the sameshell: bash/set -euo pipefail.
📍 Affects 3 files
.github/workflows/e2e-bmaas-full-install.yml#L335-L336(this comment).github/workflows/e2e-vmaas-full-install.yml#L353-L354.github/workflows/e2e-caas-full-install.yml#L329-L330
🤖 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/e2e-bmaas-full-install.yml around lines 335 - 336, Add
shell: bash to the image-loading steps in
.github/workflows/e2e-bmaas-full-install.yml lines 335-336,
.github/workflows/e2e-vmaas-full-install.yml lines 353-354, and
.github/workflows/e2e-caas-full-install.yml lines 329-330, and begin each script
with set -euo pipefail so podman save failures propagate through the pipe.
Merged
1 of 3 tasks
…ts array Lets a single e2e run build any number of components from source, instead of just one. Replaces the previous single-component inputs with one `components` input: a JSON array, one entry per component (repo, ref, containerfile, imageKey, buildType). Applied identically to e2e-bmaas-full-install.yml, e2e-vmaas-full-install.yml, and e2e-caas-full-install.yml: - A validation step checks the array shape and required fields per entry. - A loop builds/loads each component's image and records the result (image tag, key, repo, ref, containerfile, CRDs dir) after every component, so a failure partway through still leaves earlier successfully-built images tracked for teardown to clean up. - Loops (instead of single-component blocks) handle the installer-submodule replace and the image-overrides/CRD-apply logic at install time. Backward compatible: callers still using the old single-component inputs (component-repo, component-ref, component-image-key, component-containerfile, component-build-type) keep working unchanged -- those inputs are still declared, and a synthesized one-entry components array is used when the new input is left empty. fulfillment-service, osac-operator, and osac-aap all still call these workflows with the old inputs today, so this avoids breaking their CI on merge. Bugs fixed along the way: - replace-installer-submodule.sh picked which osac-installer submodule to replace by the component's repo name, which breaks for a monorepo caller since every component shares the same repo name (e.g. osac-project/osac). It now uses the component's containerfile path to identify both which submodule to replace and which subdirectory of the checkout to copy -- copying the whole checkout into one component's submodule was also wrong for monorepo callers. An unmapped or missing submodule is now a hard failure instead of a silent warning. - The "build the OSAC CLI from source" check was keyed off a component-repo substring match on "fulfillment-service", which also breaks for a monorepo caller. Now keyed off imageKey == "service.images.service", which identifies that component regardless of repo name. - teardown.sh now loops over every built component's image instead of a single COMPONENT_IMAGE var, validates COMPONENT_BUILD_RESULTS is a JSON array before looping (with a warning instead of a silent no-op on malformed input), and skips a literal "null" image value. - Pinned ansible-builder's version instead of an unpinned pip install, and added an explicit shell: bash to the image-build step.
eliorerz
force-pushed
the
support-monorepo-multi-component-e2e
branch
from
July 31, 2026 01:46
ad7a0f7 to
ed4f252
Compare
eliorerz
changed the title
1 task
This was referenced Aug 3, 2026
This branch was previously deployed
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
2 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.
Summary
Lets a single e2e run build any number of components from source, instead of just one. Replaces the previous single-component inputs (
component-repo,component-ref,component-image-key,component-containerfile,component-build-type) with onecomponentsinput: a JSON array string, one entry per component (repo,ref,containerfile,imageKey,buildType), e.g.:[ {"repo": "osac-project/osac", "ref": "my-branch", "containerfile": "fulfillment-service/Containerfile", "imageKey": "service.images.service"}, {"repo": "osac-project/osac", "ref": "my-branch", "containerfile": "osac-operator/Containerfile", "imageKey": "operator.image.repository"} ]Applied identically to
e2e-bmaas-full-install.yml,e2e-vmaas-full-install.yml, ande2e-caas-full-install.yml:Backward compatible. Callers still using the old single-component inputs keep working unchanged — those inputs are still declared, and a synthesized one-entry
componentsarray is used when the new input is left empty.fulfillment-service,osac-operator, andosac-aapall still call these workflows with the old inputs today, so this avoids breaking their CI on merge.Bugs fixed along the way
replace-installer-submodule.shpicked whichosac-installersubmodule to replace by the component's repo name, which breaks for a monorepo caller since every component shares the same repo name (e.g.osac-project/osac). It now uses the component's containerfile path to identify both which submodule to replace and which subdirectory of the checkout to copy — copying the whole checkout into one component's submodule was also wrong for monorepo callers. An unmapped or missing submodule is now a hard failure instead of a silent warning.component-reposubstring match on"fulfillment-service", which also breaks for a monorepo caller. Now keyed offimageKey == "service.images.service", which identifies that component regardless of repo name.teardown.shnow loops over every built component's image instead of a singleCOMPONENT_IMAGEvar, validatesCOMPONENT_BUILD_RESULTSis a JSON array before looping (with a warning instead of a silent no-op on malformed input), and skips a literal"null"image value.ansible-builder's version instead of an unpinned pip install, and added an explicitshell: bashto the image-build step.Verification
pre-commit run --all-filespasses (shellcheck skipped — needs Docker, unavailable in this dev environment; ran locally installed shellcheck directly on every touched script and on each workflow's extractedrun:blocks instead, no issues beyond pre-existing/unrelated style notes).osac-ci/cluster-tool runner infra) — needs a realworkflow_dispatchor the pairedosac-project/osaccaller PR to verify the multi-component build + merge + install path on real infra.