Skip to content
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

feat(containers): Remove reliance on chained builds #908

Conversation

andyatmiami
Copy link
Contributor

@andyatmiami andyatmiami commented Feb 19, 2025

Description

warning: this is only a partial change with 2024a changes included to solicit initial feedback on approach

Commit contains following changes:

  • Refactors all Dockerfiles to to be "self-contained" - not rely on any "build chains" for internal images
    • please note in the final form of this PR - many intermediate stages will be combined... this current form makes it easier to understand how the Dockerfiles present in the "build chain" are being combined - long term - we'd expect the following stages: base, base-<accelerator> (if applicable), <final>
  • Dockerfile file now has a file extension/suffix that indicates CPU or accelerator
  • LABEL directives now should be consistent/accurate
    • this probably needs a little more attention
  • base/ directory removed as its no longer relevant/required
  • Makefile updated to remove pre-reqs on container image targets
  • Makefile updated to handle support better across releases
    • read: MOWR variables
  • rocm/ related directories removed as ROCm was not actually supported in 2024a
  • wheel / setuptools explicitly added to runtime- images Pipfile
  • pytorch Makefile targets now contain cuda- prefix
  • ENV directive in Dockerfile now properly uses = (vs. whitespace)
  • change in buildinputs to pull file paths from the terminal layer
    • this change particularly needs refined - but its "crude yet effective" in its current form

Related-to: https://issues.redhat.com/browse/RHOAIENG-19048

How Has This Been Tested?

Merge criteria:

  • The commits are squashed in a cohesive manner and have meaningful messages.
  • Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
  • The developer has manually tested the changes and verified that the changes work

**warning: this is only a partial change with 2024a changes included to solicit initial feedback on approach**

Commit contains following changes:

- Refactors all `Dockerfile`s to to be "self-contained" - not rely on any "build chains" for internal images
    - please note in the final form of this PR - many intermediate stages will be combined... this current form makes it easier to understand how the `Dockerfile`s present in the "build chain" are being combined
        - long term - we'd expect the following stages: `base`, `base-<accelerator>` (if applicable), `<final>`
- `Dockerfile` file now has a file extension/suffix that indicates CPU or accelerator
- `LABEL` directives now should be consistent/accurate
    - this probably needs a little more attention
- `base/` directory removed as its no longer relevant/required
- Makefile updated to handle support better across releases
    - _read: MOWR variables_
- `wheel` / `setuptools` explicitly added to `runtime-` images `Pipfile`
- `pytorch` `Makefile` targets now contain `cuda-` prefix
- `ENV` directive in `Dockerfile` now properly uses `=` (vs. whitespace)
- change in `buildinputs` to pull file paths from the terminal layer
    - this change particularly needs refined - but its "crude yet effective" in its current form

Related-to: https://issues.redhat.com/browse/RHOAIENG-19048
Copy link
Contributor

openshift-ci bot commented Feb 19, 2025

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

Copy link
Contributor

openshift-ci bot commented Feb 19, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from andyatmiami. For more information see the Code Review Process.

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added size/xxl and removed size/xxl labels Feb 19, 2025
this is a partial/incomplete set of changes...

will not ever merge these changes... going to use as a staging ground as we spin up a new branch off main based on recent change in direction.
@openshift-ci openshift-ci bot added size/xxl and removed size/xxl labels Feb 24, 2025
@jiridanek
Copy link
Member

@andyatmiami
Copy link
Contributor Author

closing this PR as I will be (soon!) opened another to replace it...

conceptually... changes in the new PR are the same - just adapted to note require the 2024a + 2024b subdirectories now.

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

Successfully merging this pull request may close these issues.

2 participants