Skip to content
This repository has been archived by the owner on Feb 6, 2024. It is now read-only.

Add support for the images={} attribute. #8

Merged
merged 2 commits into from
Sep 11, 2017
Merged

Conversation

mattmoor
Copy link
Contributor

@mattmoor mattmoor commented Sep 7, 2017

When the images={} attribute is specified resolution will publish the collection of images, and substitute the exact digest of the published image as part of resolution (avoids race condition).

When the `images={}` attribute is specified resolution will publish the collection of images, and substitute the exact digest of the published image as part of resolution (avoids race condition).
@mattmoor
Copy link
Contributor Author

mattmoor commented Sep 7, 2017

@dlorenc It is notable that this won't work with minikube unless the user's roundtripping through a real registry.

(thinking out loud)
That said, we can probably put together an alternate implementation of Publish() that supports docker loading the image into minikube's docker daemon. If we put this in a little library in rules_docker then we could also use it to implement this, which is a nice two-for-one.

WDYT?

k8s/object.bzl Outdated
# Each images entry results in a single --image_spec argument.
# As part of this walk, we also collect all of the image's input files
# to include as runfiles, so they are accessible to be pushed.
index = 0
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is index used anywhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, where I increment it :) Removed.

@mattmoor mattmoor merged commit 85d1c9a into master Sep 11, 2017
@mattmoor mattmoor mentioned this pull request Sep 18, 2017
@nlopezgi nlopezgi deleted the publish-images branch November 27, 2018 15:48
@rohansingh
Copy link
Contributor

That said, we can probably put together an alternate implementation of Publish() that supports docker loading the image into minikube's docker daemon.

@mattmoor I've put together some rules and a macro for a repo that basically does this. Notes for anyone that wants to take a stab on a real implementation:

  1. The difficult part was deciding how to tag the images locally, since locally-loaded Docker images can't be referenced by digest. I ended up just tagging the images with their digest value.

  2. I didn't want to tackle rewriting the incremental loader from rules_docker in Python. Instead, I just created a separate rule that wraps incremental_load from @io_bazel_rules_docker//container:layer_tools.bzl to load the image, and then wraps k8s_object to do Kube stuff.

  3. Because of the above, I didn't change Publish() to load the image into the Docker daemon. Instead, the resolver just accepts a --no_upload flag. When passed, it does not upload the image. Instead, it resolves the image name using the tag strategy described above.

Obviously this is a bit of a hacky solution, but it seems to work quite reliably. Having the ability to deploy manifests locally like this is really handy.

@mattmoor
Copy link
Contributor Author

FWIW I solved this in ko by simply tagging the image with the digest, see: https://github.com/google/go-containerregistry/blob/master/cmd/ko/README.md

@mattmoor
Copy link
Contributor Author

Note: my bias towards not using :latest was that it doesn't work well with K8s default pull policy.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants