-
Notifications
You must be signed in to change notification settings - Fork 65
ci: add container builds and publish workflow #605
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
ci: add container builds and publish workflow #605
Conversation
6b7edf5
to
e1b1b05
Compare
/hold We want the following PR merged first (which will require a rebase on this PR) |
/ok-to-test |
e1b1b05
to
d25ab9b
Compare
/unhold |
d25ab9b
to
bfd6ee6
Compare
This commit adds a GHA workflow to support publishing a container image for the `workspaces/controller` component as well as some updates to the `Makefile` to establish reasonable (but overridable) defaults. Key behavior: - Publishes controller images on any workspaces/ directory change - Builds images with commit SHA tags by default - "release" images with use the tag as defined by the `VERSION` file - a `-dirty` suffix is added to the tag if the codebase is not `porcelain` - no `latest` tag is ever produced on images - Uses `ghcr.io/kubeflow/notebooks` registry with configurable naming example image name from "random" commit: - `ghcr.io/kubeflow/notebooks/workspaces-controller:sha-3fa851ab3173942dbaa1a609468e7f9eadf5f4e4` example image name from release: - `ghcr.io/kubeflow/notebooks/workspaces-controller:v2.0.0` Signed-off-by: Andy Stoneberg <astonebe@redhat.com>
bfd6ee6
to
828230d
Compare
Signed-off-by: Mathew Wicks <5735406+thesuperzapper@users.noreply.github.com>
@andyatmiami I have pushed a significant change in 288d0cf, which it would be great if you can test in a fork. It makes lots of improvements, but one of the biggest is also testing the image builds on any PRs to the various components. It also replaces the need for #611 and #610 by combining them into this PR |
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.
Overall I really like these changes.
- The shared
Build Image
workflow does an excellent job encapsulating this logic to be reused acrosscontroller
,backend
,frontend
- I am happy to see us pull the trigger (pun intended!) and abandon the
CONTAINER_TOOL
Makefile
var
and simplify/align the container-runtime related commands to be more consistent with how Kubeflow traditionally has implemented them
Review comments I have made are moreso clarifying questions/open statements for discussion - I do NOT feel so strongly on any of them such that they should hold up this PR.
I will now look into testing/validating these changes. Thanks!
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.
Whoops, forgot to include this comment on prior review 😞
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.
Hehe, ok, yet another comment/discussion point I want to raise around another possible optimization...
Signed-off-by: Andy Stoneberg <astonebe@redhat.com>
Latest VerificationPR Push
Push
Push |
with.semver_raw has an incorrect reference to version_raw output that results in an empty string always being passed - which prevented semver tags from ever being produced (even when they should be) Signed-off-by: Andy Stoneberg <astonebe@redhat.com>
needs.check_version stores all its 'interesting data' under the 'outputs' attribute. This was not properly reflected in the 'with' block for images job. Signed-off-by: Andy Stoneberg <astonebe@redhat.com>
@andyatmiami thanks for all the back and forward on this, will be great to have container images being published for Notebooks 2.0! /lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: thesuperzapper 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 |
1e5c049
into
kubeflow:notebooks-v2
closes: #606
closes: #607
closes: #608
This commit adds a GHA workflow to support publishing a container image for the
workspaces/
components as well as some updates to theMakefiles
to establish reasonable (but overridable) defaults.Key behavior:
VERSION
file-dirty
suffix is added to the tag if the codebase is notporcelain
latest
tag is ever produced on imagesghcr.io/kubeflow/notebooks
registry with configurable namingexample image name from "random" commit:
ghcr.io/kubeflow/notebooks/workspaces-controller:sha-3fa851ab3173942dbaa1a609468e7f9eadf5f4e4
ghcr.io/kubeflow/notebooks/workspaces-backend:sha-3fa851ab3173942dbaa1a609468e7f9eadf5f4e4
ghcr.io/kubeflow/notebooks/workspaces-frontend:sha-3fa851ab3173942dbaa1a609468e7f9eadf5f4e4
example image name from release:
ghcr.io/kubeflow/notebooks/workspaces-controller:2.0.0
ghcr.io/kubeflow/notebooks/workspaces-backend:2.0.0
ghcr.io/kubeflow/notebooks/workspaces-frontend:2.0.0