-
Notifications
You must be signed in to change notification settings - Fork 345
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
Update our registry to dockerhub and use our latest kind-node #927
Conversation
ifndef KIND_K8S_TAG | ||
$(error KIND_K8S_TAG is undefined) | ||
endif | ||
echo --kube-root=$(K8S_PATH) tagging as --image $(REGISTRY)/kind-node:$(KIND_K8S_TAG) |
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.
Was this a debugging line? I think the output from make is verbose and will show the kind build node-image...
command details. If you wanted to include it, it might be better as part of the kind_images
target as that is where the building/tagging takes place.
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.
Well it was partially a debug line and partially a helper since I thought it might make sense in our steps to do something like make check-kind-env
and verify the path and versions are what we want. The output from the build starting isn't actually that great at the start; once things are tagged all is clear, but its not that optimal to either waste that time or even start/stop that mid build IMO.
Kind kept failing in CI but I couldn't repro locally. I just updated the version of kind we download to see if that fixes it (I am on v0.6.0, manually built from github) and CI was on v0.4.0. I updated to v0.5.1, which is the latest actual release. |
Hm so the issue right now is that I updated the tagging in the scripts for the main sonobuoy image so that gets loaded into the cluster, but then the codebase itself is looking for the I'll have to simultaneously update the code that specifies the |
The only consequence of this is that if you run from master, you'll have to specify |
New Dockerhub org for sonobuoy so we will start moving our image publishing there. As of now we do not have our main images published there (another PR will do that) but I've manually built/pushed kind-node:v1.16.0 and so we can use it in our CI. - updates makefile to make/build based on the sonobuoy repo - updates the default repo to grab the sonobuoy image from - updates our test to use the custom kind image - updates our makefile to build the custom kind image Fixes #913 Signed-off-by: John Schnake <[email protected]>
Asking for re-review since I had to update some sonobuoy defaults and tests since last approved. |
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.
Still LGTM! I'm also fine with needing to specify the image for now 👍
What this PR does / why we need it:
New Dockerhub org for sonobuoy so we will start moving our
image publishing there. As of now we do not have our main images
published there (another PR will do that) but I've manually
built/pushed kind-node:v1.16.0 and so we can use it in our CI.
Which issue(s) this PR fixes
Fixes #913
Special notes for your reviewer:
Release note: