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

RHOAIENG-7525: Build opendatahub-io/notebooks in GitHub Action with caching #543

Merged

Conversation

jiridanek
Copy link
Member

@jiridanek jiridanek commented May 27, 2024

https://issues.redhat.com/browse/RHOAIENG-7525

Description

  • create a python script that analyzes dependencies between make targets
  • create github actions build for all notebook images
  • use caching to speed up the build
  • PRs only read from cache, branch builds also push into cache
  • build only images that changed compared to the PR target branch and images that depend on these
    • want to postpone this to subsequent PR
  • images are named as to distinguish PRs from branch builds
  • put PR number into the image name, and also probably git ref hash, so that the build works correctly in presence of multiple concurrent PRs/branches to be tested.
  • it is clear to GitHub users that the GHA image builds are not release builds, we release through openshift-ci builds
    • the naming with branch name and hash should make it clear this is not user-facing build; and if somebody uses this anyways, then it is not a big problem, it is as if they did the build from that branch at that commit themselves
  • check that the autogenerated gha workflow is still up to date; run generating script again and if there is change, fail

How Has This Been Tested?

See GHA run

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

Copy link
Contributor

openshift-ci bot commented May 27, 2024

Hi @jiridanek. Thanks for your PR.

I'm waiting for a opendatahub-io member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

Makefile Show resolved Hide resolved
gen_gha_matrix_jobs.py Outdated Show resolved Hide resolved
gha_lvm_overlay.bash Outdated Show resolved Hide resolved
gen_gha_matrix_jobs.py Outdated Show resolved Hide resolved
@jiridanek
Copy link
Member Author

@jiridanek jiridanek force-pushed the jd_try_dep_inside_nodes branch 3 times, most recently from 6e21ada to 5c3ccae Compare May 31, 2024 08:03
@jiridanek jiridanek force-pushed the jd_try_dep_inside_nodes branch 2 times, most recently from 2bbd779 to 6243293 Compare May 31, 2024 08:34
@jiridanek
Copy link
Member Author

/ok-to-test

@jiridanek jiridanek force-pushed the jd_try_dep_inside_nodes branch 8 times, most recently from fe9e503 to e0a43e7 Compare May 31, 2024 17:40
ci/storage.conf Outdated Show resolved Hide resolved
@jiridanek
Copy link
Member Author

/label tide/merge-method-squash

Copy link
Contributor

openshift-ci bot commented May 31, 2024

@jiridanek: The label(s) tide/merge-method-squash cannot be applied, because the repository doesn't have them.

In response to this:

/label tide/merge-method-squash

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.

@jiridanek
Copy link
Member Author

/override ci/prow/images
/override ci/prow/notebooks-e2e-tests

Copy link
Contributor

openshift-ci bot commented Jun 6, 2024

@jiridanek: jiridanek unauthorized: /override is restricted to Repo administrators, approvers in top level OWNERS file, and the following github teams:.

In response to this:

/override ci/prow/images
/override ci/prow/notebooks-e2e-tests

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.

@jiridanek jiridanek changed the title Build opendatahub-io/notebooks in GitHub Action with caching RHOAIENG-7525: Build opendatahub-io/notebooks in GitHub Action with caching Jun 7, 2024
@jiridanek jiridanek force-pushed the jd_try_dep_inside_nodes branch 2 times, most recently from fa89076 to 0a78449 Compare June 7, 2024 20:52
Copy link
Member

@atheo89 atheo89 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work, @jiridanek! I finally had the time to take a look at this, and I don't have any comments to add. Very neat work!

/lgtm

Copy link
Contributor

openshift-ci bot commented Jun 12, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: atheo89

The full list of commands accepted by this bot can be found here.

The pull request process is described 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

@jiridanek
Copy link
Member Author

/test notebook-jupyter-intel-pyt-ubi9-python-3-9-pr-image-mirror
/test notebook-jupyter-intel-tf-ubi9-python-3-9-pr-image-mirror
/test images

@jiridanek
Copy link
Member Author

/override ci/prow/notebooks-e2e-tests

as it is failing on issue unrelated to GitHub Actions

Copy link
Contributor

openshift-ci bot commented Jun 12, 2024

@jiridanek: jiridanek unauthorized: /override is restricted to Repo administrators, approvers in top level OWNERS file, and the following github teams:.

In response to this:

/override ci/prow/notebooks-e2e-tests

as it is failing on issue unrelated to GitHub Actions

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.

@atheo89
Copy link
Member

atheo89 commented Jun 12, 2024

/override ci/prow/notebooks-e2e-tests

Copy link
Contributor

openshift-ci bot commented Jun 12, 2024

@atheo89: Overrode contexts on behalf of atheo89: ci/prow/notebooks-e2e-tests

In response to this:

/override ci/prow/notebooks-e2e-tests

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.

@jiridanek
Copy link
Member Author

/retest

Copy link
Contributor

openshift-ci bot commented Jun 12, 2024

New changes are detected. LGTM label has been removed.

@jiridanek jiridanek added the lgtm label Jun 12, 2024
@atheo89
Copy link
Member

atheo89 commented Jun 12, 2024

/override ci/prow/notebooks-e2e-tests

Copy link
Contributor

openshift-ci bot commented Jun 12, 2024

@atheo89: Overrode contexts on behalf of atheo89: ci/prow/notebooks-e2e-tests

In response to this:

/override ci/prow/notebooks-e2e-tests

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.

@openshift-merge-bot openshift-merge-bot bot merged commit 4d4841f into opendatahub-io:main Jun 12, 2024
33 checks passed
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.

3 participants