-
Notifications
You must be signed in to change notification settings - Fork 18
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
Add GH Actions + Refactors #48
Add GH Actions + Refactors #48
Conversation
it is unused Signed-off-by: adrianc <adrianc@nvidia.com>
some general improvements in makefile. - use go install to install binaries - use "set" as cover mode - dont use alternative gopath - remove un-needed variables Signed-off-by: adrianc <adrianc@nvidia.com>
- Add Dockerfiles for arm64/ppc64le - Remove un-needed dependencies Signed-off-by: adrianc <adrianc@nvidia.com>
Signed-off-by: adrianc <adrianc@nvidia.com>
- build/test workflow to run build and test targets - codeql workflow - static scan workflow (linters/shellcheck/hadolint) - image push workflows for master/branch - support multi-arch build Signed-off-by: adrianc <adrianc@nvidia.com>
Signed-off-by: adrianc <adrianc@nvidia.com>
jobs: | ||
build-and-push-amd64-rdma-cni: | ||
name: image push amd64 | ||
runs-on: ubuntu-20.04 |
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.
any reason why not to use ubuntu-latest
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.
essentially copied either from sriov-cni or sriov-device-plugin.
maybe for stability ? so u know what u get when building image.
that said. i can change to latest if u think it better :)
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.
it good to pin it, what go me confused is that the build and test are on ubunut-latest.
Maybe we should move all to ubuntu-22.04
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.
ack. will pin all and move to 22.04.
make clean && \ | ||
make build | ||
|
||
FROM alpine | ||
FROM alpine:3 |
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.
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.
essentially copied either from sriov-cni or sriov-device-plugin.
in build image it will build with latest golang compiler.
we didnt hit any issues with it until now as it is.
i can change if u like
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 think it better to use the same image from compile and run.
so with should use explicitly golang:alpine3 and not golang:alpine
Once alpine:4 will be release I think this will break.
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.
there is no alpine3 tag.
currently it will build against latest version of go (which is generally good) and will run on latest version of alpine3 (technically what runs is just the shell script, CNI is invoked from host ).
which i think it fine. in case it breaks for some glibc when alpine4 is out we can handle it then.
LMK if current setting is fine or take your pick from : https://hub.docker.com/_/golang
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.
let keep it as is.
Signed-off-by: adrianc <adrianc@nvidia.com>
Refer to individual commits for more info