-
Notifications
You must be signed in to change notification settings - Fork 190
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
Allow use of locally built images for CI/testing #759
Allow use of locally built images for CI/testing #759
Conversation
3594ee7
to
cb76de9
Compare
Build FAILURE, see build http://10.8.144.11:8080/job/dev-tools/1079/ |
Build FAILURE, see build http://10.8.144.11:8080/job/dev-tools/1080/ |
cb76de9
to
06c332d
Compare
Build FAILURE, see build http://10.8.144.11:8080/job/dev-tools/1118/ |
Build SUCCESS, see build http://10.8.144.11:8080/job/dev-tools/1120/ |
04_setup_ironic.sh
Outdated
export $IMAGE_VAR=192.168.111.1:5000/localimages/${!IMAGE_VAR} | ||
|
||
clone_installer | ||
cd /home/notstack/go/src/github.com/openshift/installer |
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 should use $OPENSHIFT_INSTALL_PATH
I think this needs an update since #778 landed, perhaps we can move the image build into the 03 script and rename it? Also it'd be nice if we could figure out some way to not hijack git tracked files in-place, don't have a good suggestion of how we do that yet though. |
This could be dropped in favor of looking ahead to OpenShift CI, where it would be building custom ironic images if necessary and including them in the release image for that CI job. Now that openshift/installer#2234 has landed, we really shouldn't be using metal3-io ironic images anywhere in OpenShift. |
I've been wondering about this recently, should we just drop the metal3 ironic images and work directly on the openshift ones? |
06c332d
to
dfb3b35
Compare
Build FAILURE, see build http://10.8.144.11:8080/job/dev-tools/1141/ |
dfb3b35
to
f7acf18
Compare
Build SUCCESS, see build http://10.8.144.11:8080/job/dev-tools/1144/ |
@russellb So it seems we're blocked on making changes to metal3-io repos without something like this, and we're blocked on output images from openshift on code data, so it seems if we don't have a happy place to even test changes :\ I feel like Derek is raising a good point, seems like we at least need to drop something somewhere to unblock things. |
The metal3-io images are not used anywhere in OpenShift directly. They are used directly by metal3-dev-env, though. We still need them for the usage of metal3-io outside of OpenShift. |
ok, so I guess the answer is the I/we need to switch to using metal3-dev-env when doing development on the ironic-images |
f7acf18
to
43bab70
Compare
Now uses a local release image, and should make it possible to use custom versions of any of the images mentioned in the release image (not just the ironic images) |
Build SUCCESS, see build http://10.8.144.11:8080/job/dev-tools/1183/ |
43bab70
to
1814b8f
Compare
Build SUCCESS, see build http://10.8.144.11:8080/job/dev-tools/1185/ |
1814b8f
to
fde5aed
Compare
Build SUCCESS, see build http://10.8.144.11:8080/job/dev-tools/1186/ |
This is working great for me, the only thing missing I think is to open tcp/5000 in the firewall as I had to do that manually.
That was with |
09a75b4
to
bf5f6be
Compare
Build SUCCESS, see build http://10.8.144.11:8080/job/dev-tools/1189/ |
Build SUCCESS, see build http://10.8.144.11:8080/job/dev-tools/1190/ |
@@ -39,6 +39,11 @@ source $CONFIG | |||
export OPENSHIFT_RELEASE_IMAGE="${OPENSHIFT_RELEASE_IMAGE:-registry.svc.ci.openshift.org/ocp/release:4.2}" | |||
export OPENSHIFT_INSTALL_PATH="$GOPATH/src/github.com/openshift/installer" | |||
|
|||
if env | grep -q "_LOCAL_IMAGE=" ; then | |||
# We need a custome installer (allows http image pulls for local images) |
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 more info on this - is it a configuration that's turned off in the release-payload installer build?
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.
Its needed because in 04_setup_ironic.sh we are adding a file into the filesystem
echo -e "[registries.insecure]\nregistries = ['192.168.111.1:5000']" > $OPENSHIFT_INSTALL_PATH/data/data/bootstrap/baremetal/files/etc/containers/registries.conf
we need to build a custom installer including this file
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.
Ah I see, thanks - I saw the 99_local-registry.yaml.optional
and assumed we could do all the local-registry config via an asset, but I guess we need registries.conf before the MCO runs.
Modifying the installer checkout seems fine, but one alternative would be to generate the ignition-configs via the installer, then inject the registries.conf via ignition - we did that early in dev-scripts IIRC for some other things.
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.
Modifying the installer checkout seems fine, but one alternative would be to generate the ignition-configs via the installer, then inject the registries.conf via ignition - we did that early in dev-scripts IIRC for some other things.
I'll look into doing this as an alternative
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'll look into doing this as an alternative
Not necessarily saying that we should, just wanted to mention it as a possible alternative 👍
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 is working well for me, I think it's ready to merge if you're happy @derekhiggins ?
I guess some update to the README would be nice, to document the new/easier test workflow? |
I'll update the README then I'm happy to merge if you are |
bf5f6be
to
22f8200
Compare
Add the ability to use a local release image containing entries referencing custom container images. The local references can be container images or git repositories which will be built to container images.
22f8200
to
cda0d11
Compare
Build SUCCESS, see build http://10.8.144.11:8080/job/dev-tools/1191/ |
Add the ability for the bootstrap node to pull localy
built images for development and ci jobs.