Skip to content
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 new build command to build from a base image files and params #1195

Closed
wants to merge 5 commits into from

Conversation

ehmm
Copy link
Contributor

@ehmm ehmm commented Dec 2, 2021

This is still a WIP/POC - but do you think this has room in this repo?

We want to use crane a bit like ko to build images quickly from a base image and lists of files - but instead of building go binaries we would like to take an assorted list of files and package as layers - also support envvars and custom entrypoints.

The code is quick simple (as you can see) - and borrows from mutate and append - and it makes building custom docker images locally a very painless process.

@codecov-commenter
Copy link

codecov-commenter commented Dec 2, 2021

Codecov Report

Merging #1195 (96b9dc0) into main (63e6f4e) will increase coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1195   +/-   ##
=======================================
  Coverage   74.75%   74.76%           
=======================================
  Files         111      111           
  Lines        8007     8008    +1     
=======================================
+ Hits         5986     5987    +1     
  Misses       1441     1441           
  Partials      580      580           
Impacted Files Coverage Δ
internal/windows/windows.go 67.69% <0.00%> (+0.50%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 63e6f4e...96b9dc0. Read the comment docs.

@imjasonh
Copy link
Collaborator

imjasonh commented Dec 2, 2021

Thanks for suggesting this! 🎉

AIUI this is effectively codifying this recipe into a top-level crane command, adding env var support, etc.

I'm very interested in promoting and generalizing this workflow for other use cases than ko's -- I know there are folks working on making this work for .Net applications on Windows, and I've toyed with this sort of thing for JavaScript in deno-image and (mostly unsuccessfully so far) with Rust. I think the idea has lots of merit. 😄

I'm a little hesitant to add this with first-class support into crane though, for roughly two reasons:

  1. It's already doable, or mostly doable, today with crane, using crane mutate and crane append -- we'd need to add --env support to crane mutate, and whatever other config fields we'd need, but I think we should do that anyway. It might be worthwhile to package append-and-mutate into a better UX, but that could even be a separate tool/script, like in deno-image.
  2. crane build sounds like it would be doing the actual work to build the binary, when in fact it's just packaging up a previously built artifact. The specific way that artifact needs to be laid out is very dependent on the use case -- it's easy with a Go binary or deno-compiled JS executable; it's harder for Python, PHP, Java, etc. I wouldn't want to build all that situation-specific logic into crane, but I wouldn't mind some other higher-level tool taking that on (ko, deno-image, your own build script).

Like I said, I am a very strong proponent of the general direction, I just think crane is a building block toward it, and maybe shouldn't become the full solution. I'm happy to discuss improvements to crane or these Go libraries that would make it easier to consume in higher-level contexts though, and I'd love to hear more about your use case and how we can help make it awesome.

@ehmm
Copy link
Contributor Author

ehmm commented Dec 6, 2021

Hi @imjasonh - regarding your comments - I can agree that this isn't very native to crane - especially creating the tar layers - although it is useful especially as their is Windows specific knowledge in crane (I would also like to add Windows support).

Regarding using append/mutate and this comment - I don't want to push images in between - and prefer to have everything as one step - but can see a separate util functions:
Create layer - this isn't necessarily part of crane and can be done as p2 and maybe not as part of crane

I would still think a command - that is equivalent to append+mutate that takes a base layer - a set of layers and envvars/cmdline/entrypoint and creates an image in one go - without passing through appends/mutate and without reading the previous envvars/cmdline/entrypoint might be useful.

If build is a confusing name what about compose (hopefully won't be confused with docker compose) - this would still allow quick image building - but would make more sense in the crane util

@imjasonh
Copy link
Collaborator

imjasonh commented Dec 6, 2021

Maybe we could just add an --append=some.tar flag to crane mutate to avoid the push? I think that and an env setting option on crane mutate could do everything you want without a new top-level command.

@jonjohnsonjr
Copy link
Collaborator

Related: #1134

I'm interested in this in general, but I haven't been able to come up with a great way to describe layers declaratively. If you buy into bazel, rules_docker supports this, but that's not a great answer if you don't want bazel.

Maybe we could just add an --append=some.tar flag to crane mutate to avoid the push?

This is reasonable to me and pushes the layer construction responsibility out of crane, which is nice, but something we might want to eventually figure out ourselves.

cc @loosebazooka

@loosebazooka
Copy link

I forgot about a lot of the work I did here: https://github.com/GoogleContainerTools/jib/tree/master/jib-cli#fully-annotated-build-file-jibyaml to do this using jib as the underlying library. Perhaps there's something in there (ignoring the java specific stuff) that makes sense here.

@imjasonh
Copy link
Collaborator

imjasonh commented Dec 9, 2021

I forgot about a lot of the work I did here: https://github.com/GoogleContainerTools/jib/tree/master/jib-cli#fully-annotated-build-file-jibyaml to do this using jib as the underlying library. Perhaps there's something in there (ignoring the java specific stuff) that makes sense here.

That looks really cool, thanks for sharing! I've also toyed with this idea a bit in https://github.com/imjasonh/diy -- I think there's a niche need for this, if we want to push it forward together.

@thesayyn
Copy link
Collaborator

I have come across this PR through #1134. Right now we are trying to use crane for our new bazel rules. (https://github.com/thesayyn/rules_container).

Our main goal is to build images with crane without ever depending on a container runtime and crane seems like a perfect tool for this job. However, it falls short in some cases such as interacting with the local image directly.

main pain points mostly arise from the need for a remote registry to mutate images. I believe that we could arrange the mutation commands (append, mutate, etc) to only interact with an intermediate OCI container tar that resides in the FS.

so generally one should have to pull and push when really needed

  • pull the image and save it as tar
  • mutate the image on the fs (append, mutate)
  • push it back to the registry with new tags etc.

my approach is here to make mutation commands pipeable instead of using a config file to build containers.

this would give us a big advantage since we could split the image building steps into chunks and get consistent incremental builds when running under bazel.

i have already worked on it a bit to get it running under bazel; thesayyn@e0a9948

@jonjohnsonjr
Copy link
Collaborator

@thesayyn I'd like crane to be able to operate on local OCI image layouts (not a tarball, but just a directory), some wild ramblings here: #891 (comment)

It's unclear to me if the best approach would be adapting existing commands to also support layout or having a separate command for layout ops altogether... #896 (comment)

@thesayyn
Copy link
Collaborator

thesayyn commented Dec 14, 2021

@thesayyn I'd like crane to be able to operate on local OCI image layouts (not a tarball, but just a directory), some wild ramblings here: #891 (comment)

It's unclear to me if the best approach would be adapting existing commands to also support layout or having a separate command for layout ops altogether... #896 (comment)

Isn’t the layout feature for building multi-platform images?

I believe that we are on the right path as long as we don’t adopt an instruction language like Dockerfile. I would rather have every command split be able to consume whatever is in the filesystem whether a directory containing uncompressed or compressed layers.

crane layout sounds good to preserve backward compatibility.

nevertheless, I am ready to push this forward and have the resources.

I guess the collective goal is here to add new functionality which would solve a lot of problems at once.

@thesayyn
Copy link
Collaborator

thesayyn commented Dec 15, 2021

To clarify, what we need here is to be able to pull platform-specific layers and the manifest for that platform and mutate it in the fs and push this mutated image back to the registry. everything we have done until this point is platform-specific and we haven’t even done anything on the image index.

after this point, we could worry about fat images. perhaps a top-level command that takes final tarballs for each platform generates the image-index manifest and builds a fat image.

I believe this way, the maintenance burden would be minimal because many of these features are already there but they needed to be slightly changed.

What do you think? @jonjohnsonjr @imjasonh

@imjasonh
Copy link
Collaborator

What do you think? @jonjohnsonjr @imjasonh

It sounds like everything you want to do is already possible using the go packages today, even if crane itself doesn't yet support everything.

It might be a better idea to write your own binaries using the go packages, to get an idea exactly what you'd want crane to do for you, with an eye toward contributing it up into crane when it's stabilized.

This prevents you from being blocked on getting everything you need into crane, and reduces pressure on maintainers to unblock you. It may also reduce long-running maintenance burden if we accidentally add something to crane you didn't actually need, but we can't remove easily once it's used by someone else.

I'm more than happy to help with that, in terms of help and advice, and/or contributing code if that's needed.

@thesayyn
Copy link
Collaborator

Well, I agree with everything you said, I wouldn’t accept something that wouldn’t be used by many people and would create a maintenance burden to me. However, one thing that I hesitate is that we don’t want to repeat what rules_docker people have done already. https://github.com/bazelbuild/rules_docker/blob/v0.19.0/container/go/cmd/join_layers/join_layers.go.

I can see that the patch is really small (it's just adding support of loading tarballs to mutate command) and it is not really worth it for us to maintain our version of crane mutate command just for this.

Would it be still okay if I send it as a PR and move the conversation over there? I believe everything would be much clearer once you see the patch. No pressure though.

@imjasonh
Copy link
Collaborator

Would it be still okay if I send it as a PR and move the conversation over there? I believe everything would be much clearer once you see the patch. No pressure though.

Absolutely. If the behavior seems too Bazel-specific, or otherwise easy to build and maintain yourself outside of crane, we can discuss it. I think we'll just need to find a good balance that keeps us all happy long-term. 😄

AIUI the issue that led to rules_docker having a need for its own join_layers.go was that they use a different tarball format than go-containerregistry or crane, so it didn't make sense to support that in crane, but it's also annoying to build custom support for that inside rules_docker. Let's try to avoid that situation in new code, and I think we'll all be much better off 😅 .

@imjasonh
Copy link
Collaborator

Closing this issue since I think we're in agreement that we won't merge this code as-is in favor of #1199 (thanks again btw 🎉).

Feel free to keep the conversation going in here though, or open a new issue or discussion if you'd prefer.

@imjasonh imjasonh closed this Dec 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants