Skip to content

Commit

Permalink
Better Docker Daemon Support + Cleaner Actions Terminal Output (#1332)
Browse files Browse the repository at this point in the history
## Description

Updates local image behavior, changing back to crane first with docker
as an optional fallback when needed. Large images from docker will still
be slow, but should not cause OOM issues and a new warning was added to
communicate an alternate approach to speed up local dev with large
images. Also cleans up some of the exec package presentation logic.

## Related Issue

Fixes #1214 
Fixes #1267

## Type of change

- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Other (security config, docs update, etc)

## Checklist before merging

- [x] Test, docs, adr added or updated as needed
- [ ] [Contributor Guide
Steps](https://github.com/defenseunicorns/zarf/blob/main/CONTRIBUTING.md#developer-workflow)
followed

---------

Co-authored-by: Wayne Starr <[email protected]>
  • Loading branch information
jeff-mccoy and Racer159 authored Feb 8, 2023
1 parent 1af056a commit b3d8067
Show file tree
Hide file tree
Showing 28 changed files with 450 additions and 335 deletions.
28 changes: 28 additions & 0 deletions adr/0012-local-image-support-via-docker.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
# 12. Local Image Support Via Docker

Date: 2023-02-06

## Status

Accepted

## Context

There has been a long-standing usabilty gap with Zarf when doing local development due to a lack of local image support. A solution was merged in, [#1173](https://github.com/defenseunicorns/zarf/pull/1173), and released in [v0.23.4](https://github.com/defenseunicorns/zarf/releases/tag/v0.23.4) to support this feature. Unfortuantely, we didn't realize there is a [glaring issue](https://github.com/defenseunicorns/zarf/issues/1214) with the implementation that causes Zarf to crash when trying to load large images into the local docker daemon. The docker daemon support in Crane is somewhat naive and can send a machine into an OOM condition due to how the tar stream is loaded into memory from the docker save action. Crane does have an option to avoid this issue, but at the cost of being much slower to load images from docker.

We did extensive investigation into various strategies of loading docker images from the daemon including: crane, skopeo, the docker go client and executing the docker cli directly with varying levels of success. Unfortunately, some of the methods that did work well were up to 3 times slower than the current implementation, though they avoided the OOM issue. Lastly, the docker daemon save operations directly still ended up being slower than crane and docker produced a legacy format that would cause issues with [future package schema changes](https://github.com/defenseunicorns/zarf/issues/1319) we are planning for oci imports.

| | **Docker** | **Crane** |
| -------------------------------------------------- | ---------- | --------- |
| Big Bang Core (cached) | 3m 1s | 1m 58s |
| Big Bang Core (cached + skip-sbom) | 1m 51s | 56s |
| 20 GB Single-Layer Image (local registry) | | 6m 14s |
| 20 GB Single-Layer Image (local registry + cached) | 5m 2s | 2m 10s |

## Decision

We had hoped to leverage docker caching and avoid crane caching moreso, but realized that caching was still occuring via Syft for SBOM. Additionaly, the extremely-large, local-only image is actually the edge case here and we created a recommended workaround in the FAQs as well as an inline alert when a large docker image is detected. This restores behavior to what it was before the docker daemon support was added, but with the added benefit of being able to load images from the docker daemon when they are available locally.

## Consequences

For most cases this will be a seamless transition back to the previous behavior while still supporting local-only images. While this will work for large images too, it will be slow and this is automatically communicated to the user via a warning/recommendation to use a local registry.
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ zarf package create [DIRECTORY] [flags]
--confirm Confirm package creation without prompting
-h, --help help for create
-m, --max-package-size int Specify the maximum size of the package in megabytes, packages larger than this will be split into multiple parts. Use 0 to disable splitting.
--no-local-images Do not use local container images when creating this package
-o, --output-directory string Specify the output directory for the created Zarf package
-s, --sbom View SBOM contents after creating the package
--sbom-out string Specify an output directory for the SBOMs from the created Zarf package
Expand Down
20 changes: 19 additions & 1 deletion docs/4-user-guide/3-zarf-schema.md
Original file line number Diff line number Diff line change
Expand Up @@ -343,12 +343,14 @@ Must be one of:
| **Defined in** | #/definitions/ZarfComponent |

<details>
<summary><strong> <a name="components_items_name"></a>name</strong>
<summary><strong> <a name="components_items_name"></a>name *</strong>

</summary>
&nbsp;
<blockquote>

![Required](https://img.shields.io/badge/Required-red)

**Description:** The name of the component

| | |
Expand Down Expand Up @@ -1062,6 +1064,22 @@ Must be one of:
</blockquote>
</details>

<details>
<summary><strong> <a name="components_items_actions_onCreate_before_items_description"></a>description</strong>

</summary>
&nbsp;
<blockquote>

**Description:** Description of the action to be displayed during package execution instead of the command

| | |
| -------- | -------- |
| **Type** | `string` |

</blockquote>
</details>

</blockquote>
</details>

Expand Down
34 changes: 32 additions & 2 deletions docs/9-faq.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ Zarf is statically compiled and written in [Go](https://golang.org/) and [Rust](

Zarf is under the [Apache License 2.0](https://github.com/defenseunicorns/zarf/blob/main/LICENSE). This is one of the most commonly used licenses for open source software.


## What is the Zarf Agent?

The Zarf Agent is a [Kubernetes Mutating Webhook](https://kubernetes.io/docs/reference/access-authn-authz/admission-controllers/#mutatingadmissionwebhook) that is installed into the cluster during the `zarf init` operation. The Agent is responsible for modifying [Kubernetes PodSpec](https://kubernetes.io/docs/reference/kubernetes-api/workload-resources/pod-v1/#PodSpec) objects [Image](https://kubernetes.io/docs/reference/kubernetes-api/workload-resources/pod-v1/#Container.Image) fields to point to the Zarf Registry. This allows the cluster to pull images from the Zarf Registry instead of the internet without having to modify the original image references. The Agent also modifies [Flux GitRepository](https://fluxcd.io/docs/components/source/gitrepositories/) objects to point to the local Git Server.
Expand All @@ -21,7 +20,7 @@ The Zarf Agent is a [Kubernetes Mutating Webhook](https://kubernetes.io/docs/ref

During early discussions and [subsequent decision](../adr/0005-mutating-webhook.md) to use a Mutating Webhook, we decided to not have the Agent create any secrets in the cluster. This is to avoid the Agent having to have more privileges than it needs as well as avoid collisions with Helm. The Agent today simply responds to requests to patch PodSpec and GitRepository objects.

The Agent does not need to create any secrets in the cluster. Instead, during `zarf init` and `zarf package deploy`, secrets are automatically created as [Helm Postrender Hook](https://helm.sh/docs/topics/advanced/#post-rendering) for any namespaces Zarf sees. If you have resources managed by [Flux](https://fluxcd.io/) that are not in a namespace managed by Zarf, you can either create the secrets manually or include a manifest to create the namespace in your package and let Zarf create the secrets for you.
The Agent does not need to create any secrets in the cluster. Instead, during `zarf init` and `zarf package deploy`, secrets are automatically created as [Helm Postrender Hook](https://helm.sh/docs/topics/advanced/#post-rendering) for any namespaces Zarf sees. If you have resources managed by [Flux](https://fluxcd.io/) that are not in a namespace managed by Zarf, you can either create the secrets manually or include a manifest to create the namespace in your package and let Zarf create the secrets for you.

## How can a Kubernetes resource be excluded from the Zarf Agent?

Expand All @@ -31,6 +30,37 @@ Resources can be excluded at the namespace or resources level by adding the `zar

During the `zarf init` operation, the Zarf Agent will patch any existing namespaces with the `zarf.dev/agent: ignore` label to prevent the Agent from modifying any resources in that namespace. This is done because there is no way to guarantee the images used by pods in existing namespaces are available in the Zarf Registry.

## How can I improve the speed of loading larges images from Docker on `zarf package create`?

Due to some limitations with how Docker provides access to local image layers, `zarf package create` has to rely on `docker save` under the hood which is [very slow overall](https://github.com/defenseunicorns/zarf/issues/1214) and also takes a long time to report progress. We experimented with many ways to improve this, but for now recommend leveraging a local docker registry to speed up the process. This can be done by running a local registry and pushing the images to it before running `zarf package create`. This will allow `zarf package create` to pull the images from the local registry instead of Docker. This can also be combined with [component actions](4-user-guide/5-component-actions.md) to make the process automatic. Given an example image of `my-giant-image:###ZARF_PKG_VAR_IMG###` you could do something like this:

```sh
# Create a local registry
docker run -d -p 5000:5000 --restart=always --name registry registry:2

# Run the package create with a tag variable
zarf package create --set IMG=my-giant-image:v2
```

```yaml
kind: ZarfPackageConfig
metadata:
name: giant-image-example

components:
- name: main
actions:
# runs during "zarf package create"
onCreate:
# runs before the component is created
before:
- cmd: 'docker tag ###ZARF_PKG_VAR_IMG### localhost:5000/###ZARF_PKG_VAR_IMG###'
- cmd: 'docker push localhost:5000/###ZARF_PKG_VAR_IMG###'

images:
- 'localhost:5000/###ZARF_PKG_VAR_IMG###'
```
## What is YOLO Mode and why would I use it?
YOLO Mode is a special package metadata designation that be added to a package prior to `zarf package create` to allow the package to be installed without the need for a `zarf init` operation. In most cases this will not be used, but it can be useful for testing or for environments that manage their own registries and Git servers completely outside of Zarf. This can also be used as a way to transition slowly to using Zarf without having to do a full migration.
Expand Down
14 changes: 14 additions & 0 deletions examples/component-actions/zarf.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,20 @@ components:
# runs after the component is deployed
after:
- cmd: touch test-create-after.txt
- cmd: sleep 1
- cmd: echo "I can print!"
- cmd: sleep 1
- cmd: |
echo "multiline!"
sleep 1
echo "updates!"
sleep 3
echo "in!"
sleep 1
echo "realtime!"
sleep 1
description: multiline & description demo
- cmd: sleep 1

- name: on-deploy
actions:
Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ require (
github.com/goccy/go-yaml v1.9.8
github.com/google/go-containerregistry v0.13.0
github.com/mholt/archiver/v3 v3.5.1
github.com/moby/moby v20.10.23+incompatible
github.com/otiai10/copy v1.9.0
github.com/pkg/errors v0.9.1
github.com/pterm/pterm v0.12.54
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -1266,6 +1266,8 @@ github.com/mitchellh/reflectwalk v1.0.2 h1:G2LzWKi524PWgd3mLHV8Y5k7s6XUvT0Gef6zx
github.com/mitchellh/reflectwalk v1.0.2/go.mod h1:mSTlrgnPZtwu0c4WaC2kGObEpuNDbx0jmZXqmk4esnw=
github.com/moby/locker v1.0.1 h1:fOXqR41zeveg4fFODix+1Ch4mj/gT0NE1XJbp/epuBg=
github.com/moby/locker v1.0.1/go.mod h1:S7SDdo5zpBK84bzzVlKr2V0hz+7x9hWbYC/kq7oQppc=
github.com/moby/moby v20.10.23+incompatible h1:5+Q6jGL7oH89tx+ms0fGsTYEXrQ3P4vuL3i7DayMUuM=
github.com/moby/moby v20.10.23+incompatible/go.mod h1:fDXVQ6+S340veQPv35CzDahGBmHsiclFwfEygB/TWMc=
github.com/moby/spdystream v0.2.0 h1:cjW1zVyyoiM0T7b6UoySUFqzXMoqRckQtXwGPiBhOM8=
github.com/moby/spdystream v0.2.0/go.mod h1:f7i0iNDQJ059oMTcWxx8MA/zKFIuD/lY+0GqbN2Wy8c=
github.com/moby/sys/mountinfo v0.6.0 h1:gUDhXQx58YNrpHlK4nSL+7y2pxFZkUcXqzFDKWdC0Oo=
Expand Down
2 changes: 0 additions & 2 deletions src/cmd/package.go
Original file line number Diff line number Diff line change
Expand Up @@ -248,15 +248,13 @@ func bindCreateFlags() {
v.SetDefault(V_PKG_CREATE_SBOM_OUTPUT, "")
v.SetDefault(V_PKG_CREATE_SKIP_SBOM, false)
v.SetDefault(V_PKG_CREATE_MAX_PACKAGE_SIZE, 0)
v.SetDefault(V_PKG_CREATE_NO_LOCAL_IMAGES, false)

createFlags.StringToStringVar(&pkgConfig.CreateOpts.SetVariables, "set", v.GetStringMapString(V_PKG_CREATE_SET), "Specify package variables to set on the command line (KEY=value)")
createFlags.StringVarP(&pkgConfig.CreateOpts.OutputDirectory, "output-directory", "o", v.GetString(V_PKG_CREATE_OUTPUT_DIR), "Specify the output directory for the created Zarf package")
createFlags.BoolVarP(&pkgConfig.CreateOpts.ViewSBOM, "sbom", "s", v.GetBool(V_PKG_CREATE_SBOM), "View SBOM contents after creating the package")
createFlags.StringVar(&pkgConfig.CreateOpts.SBOMOutputDir, "sbom-out", v.GetString(V_PKG_CREATE_SBOM_OUTPUT), "Specify an output directory for the SBOMs from the created Zarf package")
createFlags.BoolVar(&pkgConfig.CreateOpts.SkipSBOM, "skip-sbom", v.GetBool(V_PKG_CREATE_SKIP_SBOM), "Skip generating SBOM for this package")
createFlags.IntVarP(&pkgConfig.CreateOpts.MaxPackageSizeMB, "max-package-size", "m", v.GetInt(V_PKG_CREATE_MAX_PACKAGE_SIZE), "Specify the maximum size of the package in megabytes, packages larger than this will be split into multiple parts. Use 0 to disable splitting.")
createFlags.BoolVar(&pkgConfig.CreateOpts.NoLocalImages, "no-local-images", v.GetBool(V_PKG_CREATE_NO_LOCAL_IMAGES), "Do not use local container images when creating this package")
}

func bindDeployFlags() {
Expand Down
1 change: 0 additions & 1 deletion src/cmd/viper.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ const (
V_PKG_CREATE_SBOM_OUTPUT = "package.create.sbom_output"
V_PKG_CREATE_SKIP_SBOM = "package.create.skip_sbom"
V_PKG_CREATE_MAX_PACKAGE_SIZE = "package.create.max_package_size"
V_PKG_CREATE_NO_LOCAL_IMAGES = "package.create.no_local_images"

// Package deploy config keys
V_PKG_DEPLOY_SET = "package.deploy.set"
Expand Down
13 changes: 7 additions & 6 deletions src/internal/packager/git/clone.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ func (g *Git) clone(gitDirectory string, gitURL string, onlyFetchRef bool) (*git

return repo, git.ErrRepositoryAlreadyExists
} else if err != nil {
message.Debugf("Failed to clone repo: %s", err.Error())
message.Infof("Falling back to host git for %s", gitURL)
message.Debugf("Failed to clone repo %s: %s", gitURL, err.Error())
g.Spinner.Updatef("Falling back to host git for %s", gitURL)

// If we can't clone with go-git, fallback to the host clone
// Only support "all tags" due to the azure clone url format including a username
Expand All @@ -56,10 +56,11 @@ func (g *Git) clone(gitDirectory string, gitURL string, onlyFetchRef bool) (*git
cmdArgs = append(cmdArgs, "--no-tags")
}

stdOut, stdErr, err := exec.CmdWithContext(context.TODO(), exec.Config{}, "git", cmdArgs...)
g.Spinner.Updatef(stdOut)
message.Debug(stdErr)

execConfig := exec.Config{
Stdout: g.Spinner,
Stderr: g.Spinner,
}
_, _, err := exec.CmdWithContext(context.TODO(), execConfig, "git", cmdArgs...)
if err != nil {
return nil, err
}
Expand Down
8 changes: 5 additions & 3 deletions src/internal/packager/git/fetch.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,8 @@ func (g *Git) fetch(gitDirectory string, fetchOptions *git.FetchOptions) error {
if errors.Is(err, git.ErrTagExists) || errors.Is(err, git.NoErrAlreadyUpToDate) {
message.Debug("Already fetched requested ref")
} else if err != nil {
message.Debugf("Failed to fetch repo: %s", err)
message.Infof("Falling back to host git for %s", gitURL)
message.Debugf("Failed to fetch repo %s: %s", gitURL, err.Error())
g.Spinner.Updatef("Falling back to host git for %s", gitURL)

// If we can't fetch with go-git, fallback to the host fetch
// Only support "all tags" due to the azure fetch url format including a username
Expand All @@ -87,7 +87,9 @@ func (g *Git) fetch(gitDirectory string, fetchOptions *git.FetchOptions) error {
cmdArgs = append(cmdArgs, refspec.String())
}
execCfg := exec.Config{
Dir: gitDirectory,
Dir: gitDirectory,
Stdout: g.Spinner,
Stderr: g.Spinner,
}
_, _, err := exec.CmdWithContext(context.TODO(), execCfg, "git", cmdArgs...)
return err
Expand Down
16 changes: 9 additions & 7 deletions src/internal/packager/git/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,19 @@ import (
)

// DownloadRepoToTemp clones or updates a repo into a temp folder to perform ephemeral actions (i.e. process chart repos).
func (g *Git) DownloadRepoToTemp(gitURL string) string {
path, err := utils.MakeTempDir(config.CommonOptions.TempDirectory)
if err != nil {
message.Fatalf(err, "Unable to create tmpdir: %s", config.CommonOptions.TempDirectory)
func (g *Git) DownloadRepoToTemp(gitURL string) (path string, err error) {
if path, err = utils.MakeTempDir(config.CommonOptions.TempDirectory); err != nil {
return "", fmt.Errorf("unable to create tmpdir: %w", err)
}

// If downloading to temp, grab all tags since the repo isn't being
// packaged anyway, and it saves us from having to fetch the tags
// later if we need them
if err = g.pull(gitURL, path, ""); err != nil {
return "", fmt.Errorf("unable to pull the git repo at %s: %w", gitURL, err)
}

err = g.pull(gitURL, path, "")
return path
return path, nil
}

// Pull clones or updates a git repository into the target folder.
Expand Down Expand Up @@ -124,7 +126,7 @@ func (g *Git) pull(gitURL, targetFolder string, repoName string) error {

_, err = g.removeLocalTagRefs()
if err != nil {
return fmt.Errorf("Unable to remove unneeded local tag refs: %w", err)
return fmt.Errorf("unable to remove unneeded local tag refs: %w", err)
}
_, _ = g.removeLocalBranchRefs()
_, _ = g.removeOnlineRemoteRefs()
Expand Down
12 changes: 7 additions & 5 deletions src/internal/packager/helm/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,23 +54,25 @@ func (h *Helm) DownloadChartFromGit(destination string) string {
// Get the git repo
gitCfg := git.NewWithSpinner(h.Cfg.State.GitServer, spinner)

tempPath := gitCfg.DownloadRepoToTemp(h.Chart.URL)
tempPath, err := gitCfg.DownloadRepoToTemp(h.Chart.URL)
defer os.RemoveAll(tempPath)
if err != nil {
spinner.Fatalf(err, "Unable to download the git repo %s", h.Chart.URL)
}
gitCfg.GitPath = tempPath

// Switch to the correct tag
gitCfg.CheckoutTag(h.Chart.Version)

// Validate the chart
_, err := loader.LoadDir(filepath.Join(tempPath, h.Chart.GitPath))
if err != nil {
chartPath := filepath.Join(tempPath, h.Chart.GitPath)
if _, err = loader.LoadDir(chartPath); err != nil {
spinner.Fatalf(err, "Validation failed for chart %s (%s)", h.Chart.Name, err.Error())
}

// Tell helm where to save the archive and create the package
client.Destination = destination
name, err := client.Run(filepath.Join(tempPath, h.Chart.GitPath), nil)

name, err := client.Run(chartPath, nil)
if err != nil {
spinner.Fatalf(err, "Helm is unable to save the archive and create the package %s", name)
}
Expand Down
2 changes: 0 additions & 2 deletions src/internal/packager/images/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,4 @@ type ImgConfig struct {
NoChecksum bool

Insecure bool

NoLocalImages bool
}
Loading

0 comments on commit b3d8067

Please sign in to comment.