-
Notifications
You must be signed in to change notification settings - Fork 69
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
[no-relnotes] Use reusable workflows for CI #246
Conversation
.github/workflows/image.yaml
Outdated
@@ -56,8 +45,10 @@ jobs: | |||
password: ${{ secrets.GITHUB_TOKEN }} | |||
- name: Build image | |||
env: | |||
IMAGE_NAME: ghcr.io/${LOWERCASE_REPO_OWNER}/k8s-dra-driver | |||
VERSION: ${COMMIT_SHORT_SHA} | |||
IMAGE_NAME: ghcr.io/nvidia/k8s-device-plugin |
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.
This doesn't seem right
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.
My bad, I was testing the workflows and forgot to edit this bit, it is now fixed
.github/workflows/golang.yaml
Outdated
@@ -1,4 +1,4 @@ | |||
# Copyright 2024 NVIDIA CORPORATION | |||
# Copyright 2024-2025 NVIDIA CORPORATION |
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.
Just keep the year the file was originally created
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.
done
Rebased, ready for review |
.github/workflows/image.yaml
Outdated
- name: Set up QEMU | ||
uses: docker/setup-qemu-action@v3 | ||
with: | ||
image: tonistiigi/binfmt:master |
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.
I believe this needs to match: https://github.com/NVIDIA/k8s-samples/blob/a0d87c7bc5f572d13da40596a6394adb3ad1df17/.github/workflows/image.yaml#L57
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.
thank you, done
.github/workflows/image.yaml
Outdated
@@ -56,8 +45,10 @@ jobs: | |||
password: ${{ secrets.GITHUB_TOKEN }} | |||
- name: Build image | |||
env: | |||
IMAGE_NAME: ghcr.io/${LOWERCASE_REPO_OWNER}/k8s-dra-driver-gpu | |||
VERSION: ${COMMIT_SHORT_SHA} | |||
IMAGE_NAME: ghcr.io/nvidia/k8s-dra-driver |
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.
Is this:
IMAGE_NAME: ghcr.io/nvidia/k8s-dra-driver | |
IMAGE_NAME: ghcr.io/nvidia/k8s-dra-driver-gpu |
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.
thank you, done
/ok to test |
secrets: inherit | ||
with: | ||
version: ${{ needs.basic.outputs.version }} | ||
build_multi_arch_images: ${{ github.ref_name == 'main' || startsWith(github.ref_name, 'release-') }} |
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.
Question (not a blocker): given that we've optimized the multi-arch builds in this repo to use GOARCH
, should we not just drop this or set it to true
all the time?
.github/workflows/image.yaml
Outdated
build_multi_arch_images: | ||
required: true | ||
type: string |
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.
build_multi_arch_images: | |
required: true | |
type: string | |
build_multi_arch_images: | |
required: false | |
type: string | |
default: true |
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.
MInor comments. Not blockers though.
This commit introduces the following changes to the CI structure. A more agressive split of the CI steps to reusable workflows. We now have the following high-level workflows: - A set of basic checks that are run on PR and can be invoked from a workflow - A full ci pipeline that is run on push to main and release-* branches (as well as PR copy bot branches) - A standalone definition for CodeQL Signed-off-by: Carlos Eduardo Arango Gutierrez <[email protected]>
Changes adopted, thanks for the suggestions |
This commit introduces the following changes to the CI structure. A more agressive split of the CI steps to reusable workflows. We now have the following high-level workflows: