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

Embed the envbuilder binary in images #216

Closed
Tracked by #132
mafredri opened this issue Jun 1, 2024 · 6 comments
Closed
Tracked by #132

Embed the envbuilder binary in images #216

mafredri opened this issue Jun 1, 2024 · 6 comments
Assignees

Comments

@mafredri
Copy link
Member

mafredri commented Jun 1, 2024

Depends on #197

As part of #128, we want to embed the envbuilder binary in container images that are pushed to a registry (see #213 for the push implementation).

This could be as simple as appending COPY /.envbuilder/bin/envbuilder /.envbuilder/bin/envbuilder to the Dockerfile. The binary could be located elsewhere too, so a lookup may be necessary.

This will embed an eventually stale envbuilder binary in the image, but we consider this OK as our recommendation is to lock the envbuilder version, updating it will result in a new build.

Acceptance Criteria:

  1. Build an image using envbuilder with ENVBUILDER_CACHE_REPO=localhost:5000/envbuilder-cache or similar (after feat: push final image to cache repo #197 is merged)
  2. Pull the resulting image
  3. Validate that the same envbuilder binary is present in the same location in the resulting image
@BrunoQuaresma
Copy link
Contributor

BrunoQuaresma commented Jun 7, 2024

I think it is already done. When I try to run envbuilder inside a container from an integration test, it works and it looks like the binary is already in the image.

execContainer(t, ctr, "/.envbuilder/bin/envbuilder --help")
USAGE:
  envbuilder

OPTIONS:
      --base-image-cache-dir string, $ENVBUILDER_BASE_IMAGE_CACHE_DIR
          The path to a directory where the base image can be found. This should
          be a read-only directory solely mounted for the purpose of caching the
          base image.

      --build-context-path string, $ENVBUILDER_BUILD_CONTEXT_PATH
          Can be specified when a DockerfilePath is specified outside the base
          WorkspaceFolder. This path MUST be relative to the WorkspaceFolder
          path into which the repo is cloned.

      --cache-repo string, $ENVBUILDER_CACHE_REPO
          The name of the container registry to push the cache image to. If this
          is empty, the cache will not be pushed.

      --cache-ttl-days int, $ENVBUILDER_CACHE_TTL_DAYS
          The number of days to use cached layers before expiring them. Defaults
          to 7 days.

      --coder-agent-subsystem string-array, $CODER_AGENT_SUBSYSTEM
          Coder agent subsystems to report when forwarding logs...+6238 more

@johnstcn
Copy link
Member

^ It needs to be in the built and pushed image. See the acceptance criteria I added in the issue description.

@BrunoQuaresma
Copy link
Contributor

Looks like it is already there. Here is the testing that I'm using to check that.

func TestEmbedBinaryImage(t *testing.T) {
	t.Parallel()

	srv := createGitServer(t, gitServerOptions{
		files: map[string]string{
			".devcontainer/Dockerfile": fmt.Sprintf("FROM %s\nRUN date --utc > /root/date.txt", testImageAlpine),
			".devcontainer/devcontainer.json": `{
			"name": "Test",
			"build": {
				"dockerfile": "Dockerfile"
			},
		}`,
		},
	})

	testReg := setupInMemoryRegistry(t, setupInMemoryRegistryOpts{})
	testRepo := testReg + "/test"
	ref, err := name.ParseReference(testRepo + ":latest")
	require.NoError(t, err)

	_, err = runEnvbuilder(t, options{env: []string{
		envbuilderEnv("GIT_URL", srv.URL),
		envbuilderEnv("CACHE_REPO", testRepo),
		envbuilderEnv("PUSH_IMAGE", "1"),
	}})
	require.NoError(t, err)

	_, err = remote.Image(ref)
	require.NoError(t, err, "expected image to be present after build + push")

	ctr, err := runEnvbuilder(t, options{env: []string{
		envbuilderEnv("FALLBACK_IMAGE", ref.String()),
	}})
	require.NoError(t, err)

	out := execContainer(t, ctr, "[[ -f \"/.envbuilder/bin/envbuilder\" ]] && echo \"exists\"")
	require.Equal(t, "exists", strings.TrimSpace(out))
}

@BrunoQuaresma
Copy link
Contributor

BrunoQuaresma commented Jun 13, 2024

Related to the following statement:

The binary could be located elsewhere too, so a lookup may be necessary.

Since we control where the binary is, do we need to lookup? Asking because we would have to install whereis in the image to do something like whereis envbuilder and use the path to copy the binary. Maybe another way would be to expose an option to tell envbuilder where the binary is ENVBUILDER_BIN_PATH.

@johnstcn
Copy link
Member

Related to the following statement:

The binary could be located elsewhere too, so a lookup may be necessary.

Since we control where the binary is, do we need to lookup? Asking because we would have to install whereis in the image to do something like whereis envbuilder and use the path to copy the binary. Maybe another way would be to expose an option to tell envbuilder where the binary is ENVBUILDER_BIN_PATH.

I don't think it's unreasonable to require the binary to be at a specific path.

@johnstcn johnstcn self-assigned this Jun 14, 2024
@BrunoQuaresma BrunoQuaresma removed their assignment Jun 21, 2024
@johnstcn
Copy link
Member

fixed by #234

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

No branches or pull requests

4 participants