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

add: volume preloading via ?preload=true directive in Acornfile (manager#1598) #2351

Merged
merged 12 commits into from
Nov 21, 2023

Conversation

iwilltry42
Copy link
Contributor

@iwilltry42 iwilltry42 commented Nov 21, 2023

Ref https://github.com/acorn-io/manager/issues/1598

Defining a volume, e.g. dirs: "/usr/share/nginx/html": "volume://myvol?preload=true" will pre-populate the volume with the data that the used container image has at the defined place.
Usually the directory would be empty (overlayed with the empty volume), so we're mimicking the Docker copy-up behavior here using initContainers.
Note: The data lands in the data/ subdirectory of the volume.

Note 2: There's no validation if ?preload is put on e.g. secret:// - didn't see that anywhere else, so we just do nothing in that case

What is this?

First off: Darren wants it. It's a pre-requisite for an upcoming dev flow.
The example above dirs: "/usr/share/nginx/html": "volume://myvol?preload=true" will do the following:

  • Add an initContainer to the resulting Deployment which copies the static busybox binary from the Acorn runtime base image to a shared ephemeral volume

  • Add another initContainer that's running the target app's image and uses the shared busybox binary to copy the contents of /usr/share/nginx/html within the image to the volume myvol's subPath data/

    • this is also placing a file .preload-done into myvol's root, that we use to avoid doing the process twice (i.e. preloading happens only once in the app lifecycle)
  • The data/ subPath is then mounted into the actual application container on /usr/share/nginx/html, so that the image contents are there

  • This is mimicking the default Docker behavior, which is not present in Kubernetes. Without this, the mountPath will just be empty if the volume is fresh.

Checklist

  • The title of this PR would make a good line in Acorn's Release Note's Changelog
  • The title of this PR ends with a link to the main issue being address in parentheses, like: This is a title (#1216). Here's an example
  • All relevant issues are referenced in the PR description. NOTE: don't use GitHub keywords that auto-close issues
  • Commits follow contributing guidance -> will be squashed
  • Automated tests added to cover the changes. If tests couldn't be added, an explanation is provided in the Verification and Testing section
  • Changes to user-facing functionality, API, CLI, and upgrade impacts are clearly called out in PR description
  • PR has at least two approvals before merging (or a reasonable exception, like it's just a docs change)

pkg/controller/appdefinition/deploy.go Outdated Show resolved Hide resolved
// Data will be copied to the data/ subdirectory and we'll drop a .preload-done file in the root to indicate that
// the copy has been completed (and should not be repeated).
initContainers = append(initContainers, corev1.Container{
Name: "acorn-preload-dir-" + sanitizeVolumeName(src),
Copy link
Contributor

Choose a reason for hiding this comment

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

It is possible, though unlikely, for a user to define an init container with this name. Should we add some randomness to it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll just add the app UID to it :)
It's already unlikely enough, given that we're using the path hash, so this should do it.

pkg/apis/internal.acorn.io/v1/unmarshal.go Show resolved Hide resolved
pkg/apis/internal.acorn.io/v1/unmarshal.go Outdated Show resolved Hide resolved
pkg/appdefinition/acornfile-schema.acorn Outdated Show resolved Hide resolved
pkg/controller/appdefinition/deploy_test.go Outdated Show resolved Hide resolved
@iwilltry42
Copy link
Contributor Author

@g-linville I addressed your comments, please have another look whenever you have some time :)

@iwilltry42 iwilltry42 merged commit 87d1e0f into acorn-io:main Nov 21, 2023
4 checks passed
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.

4 participants