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 support for local docker image and image archives on package create #1173

Merged
merged 9 commits into from
Jan 12, 2023

Conversation

YrrepNoj
Copy link
Contributor

@YrrepNoj YrrepNoj commented Jan 9, 2023

Description

Fixes #951

This PR introduces functionality to be able use local images when building a Zarf package.

This can be done in two ways:

  1. you can modify the zarf.yaml images key to specify a path to an image tarball (that has been generated using the docker save {IMAGE} -o my-image.tar command.
    i.e.
images:
  - "/home/user/jperry/mypackage/my-image.tar"
  1. You can pull the image to your local daemon and Zarf will attempt to use those images prior to pulling from a remote registry. (keeping your zarf.yaml specifications the same as before!)

@YrrepNoj
Copy link
Contributor Author

YrrepNoj commented Jan 9, 2023

Currently brainstorms a good way to test this that doesn't require its own test workflow.. The alternative is having all of the workflows pull an image (probably the dos-games image) to be used locally.

@runyontr
Copy link
Contributor

runyontr commented Jan 9, 2023

This looks great! The workflow I've been thinking about for this, is that I'm building an image locally as part of development of an application or a Zarf package (e.g. during the Zarf Controller) and didn't want to have to push the image to the registry, just so that zarf can pull it down to put into my Zarf package. In this workflow, I wouldn't expect the upstream registry to have an image with the same tag as the one defined in the zarf.yaml

What happens when there's a local copy of the image (e.g. from a previous build) but the upstream image has been updated. This can be an anti-pattern to tags being immutable, but occurs regularly when IronBank rebuilds images for Big Bang.

Options:

  1. Ignore the local version
  2. Ignore the upstream image all together
  3. Ask the end user
  4. Pull the newest version (maybe this is by timestamp on the image metadata?)

@jeff-mccoy
Copy link
Contributor

One immediate way we can test this is the motivation for moving this issue up: the agent image in PRs. Right now we push the agent image to GHCR for PRs, but that prevents 3rd party PRs from working properly. If we had agent images not push to GHCR but instead build and use them locally, that would test part of this code and solve our other concern. It would also be a good example of a package dev workflow where you are rebuilding an image multiple times.

@RothAndrew
Copy link
Contributor

This looks great! The workflow I've been thinking about for this, is that I'm building an image locally as part of development of an application or a Zarf package (e.g. during the Zarf Controller) and didn't want to have to push the image to the registry, just so that zarf can pull it down to put into my Zarf package. In this workflow, I wouldn't expect the upstream registry to have an image with the same tag as the one defined in the zarf.yaml

What happens when there's a local copy of the image (e.g. from a previous build) but the upstream image has been updated. This can be an anti-pattern to tags being immutable, but occurs regularly when IronBank rebuilds images for Big Bang.

Options:

  1. Ignore the local version
  2. Ignore the upstream image all together
  3. Ask the end user
  4. Pull the newest version (maybe this is by timestamp on the image metadata?)

For me the ideal capability would be:

  • Preserve existing functionality unless an optional configuration flag is set. Something like allowLocalImages: True
  • If allowLocalImages == True
    • First check local
    • Then check zarf cache (which in reality also checks remote to ensure that the cached image is the latest one)
    • Finally pull from remote

This set up lets me do things like:

  • Do local development without having to change the package definition by building local images with the same tag that the package already expects
  • Cache Registry1 images locally during development or testing, without having to change the package definition

All while preserving existing functionality for the people who are perfectly happy with how things already work.

@RothAndrew
Copy link
Contributor

What happens when there's a local copy of the image (e.g. from a previous build) but the upstream image has been updated.

^-- reason why I believe this should be an opt-in capability

@runyontr
Copy link
Contributor

runyontr commented Jan 9, 2023

Are you thinking this is defined in the component, or an argument to the zarf package command for the entire package?

@RothAndrew
Copy link
Contributor

For my use case I would prefer if it were an argument (also configurable in the zarf config toml). If it were a new noun in the zarf.yaml I would need to add it for dev/test and pull it back out for delivery (since I want the final delivered package to always be built from the source of truth registry.

@jeff-mccoy
Copy link
Contributor

I do hate how Iron bank abuses tags and don't think that should hold us back--we also shouldn't encourage the use of latest or other tag reuse, which is a bad practice in general. This could be solved with digest, but that has outstanding issues with how we use Crane today. I think we need to discuss this some more; my preference would be to prefer cache first, as without it, we're 3making moot a significant point of this work, which is to help efficiently deal with giant image collections--like Big Bang.

@RothAndrew
Copy link
Contributor

By "prefer cache first" are you talking about the .zarf-cache from crane?

@jeff-mccoy
Copy link
Contributor

My ideal state would be:

  1. Check local docker
  2. Check .zarf-cache
  3. Pull online

2/3 are sort of already combined, it would be nice if 1 could somehow check if there was an update first.

@RothAndrew
Copy link
Contributor

I don't see any of this as encouraging the use of latest or other tag reuse, rather as an acknowledgement that that use exists and empathizing with it.

My ideal state would be:

Check local docker
Check .zarf-cache
Pull online
2/3 are sort of already combined, it would be nice if 1 could somehow check if there was an update first.

I'd need the ability to tell zarf to not check if there was an update when checking local docker, or to allow the use of a not-latest image. Without one, this feature is useless to me, as the .zarf-cache already has cached images, but they aren't the latest since Registry1 regularly rebuilds their images.

I think we're actually almost talking about the same thing. Your way would preserve existing functionality, which is to always grab the latest image in the registry if it is different from the local one. All I'd be asking for in addition is the ability to tell zarf to not do that.

@RothAndrew
Copy link
Contributor

I think to summarize:

  • There's only contention around specifying images that exist both locally and in a registry. Specifying a file path to a tarball or a tag that only exists locally like my-cool-image:local only has one "path" and that's met already.
  • The way it is right now that @YrrepNoj has created changes existing functionality, by changing default behavior from "always make sure the image being used is the one that is present in the registry" to "if a local image matches the repo:tag use it"
  • It sounds like @jeff-mccoy and I are both recommending that we preserve existing functionality, by having zarf always grab what matches in the registry, just in slightly different ways
  • I'm additionally asking that we add the ability to tell Zarf to NOT match local images against the registry.

My reasoning for the additional ask is that Registry1 rebuilds their images frequently, and it significantly increases the development feedback loops on large packages that use Registry1. This is, in our opinion, not the right way to do things, but it's something we have to live with as heavy users of Registry1. Without this additional ability to tell zarf to use the local images even if they are "outdated" developers of large packages that use Registry1 haven't had any of their pain alleviated.

Additionally, if I'm able to tell Zarf to use local images even if they don't match the registry, as a developer of a software application I can create new images locally that use the same tag that is already configured in the zarf package, without needing to modify the package definition. This saves time and reduces accidental errors.

@jeff-mccoy
Copy link
Contributor

Thought about this some more. We all agree tag reuse is a bad practice we don't need to encourage, I think. We also understand that Iron Bank is super lame about doing just that by rebuilding images nightly. The reality is with IB, the day after you cut a Zarf package when using these images, they will be out of date the very next day. Whether that delta is one day or 100, it will always be out of date within 24 hours. That is one of the reasons the whole scenario is a bit asinine.

In this PR, if an image exists with a given tag via docker pull, then that will be served from docker. This guarantees that it will still be an image published with the same tag, just as it would be if a Zarf package were created and shipped to an airgap system. The next day there would be a new image and so and so forth. No one anywhere will churn their entire Big Bang stack and replace all the images nightly. It's just a moot point.

So since this is silly behavior, I'm fine with this being seen as a breaking change to discourage the poor practice. However, I agree we should add a flag to skip-local or something like that so that if you don't want to use local, you don't have to. But the workaround to maintain existing functionality is quite simple: don't keep an old copy of those images in Docker or run a docker pull on images you want to force an update of if you want to keep them there too.

@jeff-mccoy jeff-mccoy enabled auto-merge (squash) January 12, 2023 04:52
@jeff-mccoy jeff-mccoy changed the title Build packages using local container images where available Add support for local docker image and image archives on package create Jan 12, 2023
@jeff-mccoy jeff-mccoy merged commit 00736dd into main Jan 12, 2023
@jeff-mccoy jeff-mccoy deleted the 951-local-docker-images branch January 12, 2023 05:17
Noxsios pushed a commit that referenced this pull request Mar 8, 2023
This PR introduces functionality to be able use local images when
building a Zarf package.

This can be done in two ways:

1. you can modify the `zarf.yaml` images key to specify a path to an
image tarball (that has been generated using the `docker save {IMAGE} -o
my-image.tar` command.
i.e.
```
images:
  - "/home/user/jperry/mypackage/my-image.tar"
```

2. You can pull the image to your local daemon and Zarf will attempt to
use those images prior to pulling from a remote registry. (keeping your
`zarf.yaml` specifications the same as before!)

Co-authored-by: Megamind <[email protected]>
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.

Ability to use local docker images when building a Zarf Package
4 participants