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

DockerImage build step #555

Open
4 of 12 tasks
anti-social opened this issue Dec 9, 2021 · 14 comments
Open
4 of 12 tasks

DockerImage build step #555

anti-social opened this issue Dec 9, 2021 · 14 comments

Comments

@anti-social
Copy link
Contributor

anti-social commented Dec 9, 2021

Have a PoC of the !DockerImage build step:

containers:
  python:
    setup:
    - !DockerImage
      image: library/python
      version: 3.10-slim

What do you think about it?

TODO:

  • Build step options: registry, image, tag, path
  • Short form: !DockerImage python, !DockerImage python:3.10-slim
  • Auth with credentials; credentials from vagga settings
  • Registries that don't support authentication
  • Detect OS release
  • Cache layers; wait if another process is downloading the layer
  • Downloading layers progress
  • Maximum layers cache size
  • Push image to a docker registry
  • Use docker registry as an image cache
  • Image entrypoint
  • Capsule command to run docker images?

Superseeds #502

@tailhook
Copy link
Owner

tailhook commented Dec 9, 2021

Sounds good if you can make fetching the image from the repository. Having local docker daemon for this task is a bit ugly.

I wanted to do that at some point but don't remember why I didn't.

@anti-social
Copy link
Contributor Author

Of course I don't use docker for that. There is docker registry client. The only thing it should be patched to support streamed downloading of layers: anti-social/dkregistry-rs@02f1075 (please review when have time).

@tailhook
Copy link
Owner

Of course I don't use docker for that. There is docker registry client. The only thing it should be patched to support streamed downloading of layers: anti-social/dkregistry-rs@02f1075 (please review when have time).

Looks fine. There is also https://crates.io/crates/oci-registry-client which has streaming API (as far as I can see in front page example). Although, it is newer and has little downloads.

I would also prefer streaming API like in surf rather than in reqwest, but both libraries are using reqwest and it's better to get things done than perfect.

@anti-social anti-social mentioned this issue Dec 12, 2021
@anti-social
Copy link
Contributor Author

anti-social commented Dec 13, 2021

Looked at https://crates.io/crates/oci-registry-client - it is not able to fetch oauth url from the registry. You should pass the url when creating a client.

@anti-social
Copy link
Contributor Author

Should we detect OS after unpacking a docker image so we could use distro specific steps?

- !DockerImage ubuntu:focal
- !Install [bc]

@tailhook
Copy link
Owner

tailhook commented Jan 12, 2022

Should we detect OS after unpacking a docker image so we could use distro specific steps?

- !DockerImage ubuntu:focal
- !Install [bc]

Sounds useful, but we have to have some control over that. Just brainstorming here:

- !DockerImage ubuntu:focal
- !Assume ubuntu
- !Install [bc]

Or:

- !DockerImage ubuntu:focal
- !Assume
   - !Ubuntu focal
- !Install [bc]

Or:

setup:
- !DockerImage ubuntu:focal
- !Install [bc]
assumptions:
  distro: ubuntu

Maybe we can generalize to other assumptions here. I.e. instead of:

- !PipConfig
  install-python: false

Have something like this:

assumptions:
  python-installed: true

So after using docker image, you can mark all things (yes, yet another syntax proposal):

- !DockerImage python-3.6
- !Assume
  distro: ubuntu  # this also reads /etc/os-release for version automatically
  python-exe: /usr/bin/python3.6

As part of this proposal, by default we should read /etc/os-release, but allow opt-out of this somehow like:

- !DockerImage
  version: ubuntu
  auto-detect: false

Or:

auto-detect-distro: false
setup:
- !DockerImage ubuntu

Or:

- !DockerImage ubuntu
- !Assume no-distribution

Or:

- !DisableAutoDetect
- !DockerImage ubuntu

What do you think?

@anti-social
Copy link
Contributor Author

anti-social commented Jan 14, 2022

I also thought about a specific build step to detect a container distro. And yes, it will be better to make it explicit.

I think we don't have to explicitly specify a distro like:

- !Assume ubuntu

as we could read distro directly from the /etc/os-release file. So possibly it is worth to make !Assume's argument optional. But then assume name is misleading.

Should we only try to detect distro after the first build step in the following 2 examples?

setup:
- !DockerImage ubuntu:focal
- !Install [bc]
assumptions:
  distro: ubuntu
auto-detect-distro: false
setup:
- !DockerImage ubuntu

@tailhook
Copy link
Owner

At the moment, I'm thinking that we may do:

- !Assume
  distro: ubuntu <-- only overrides this option

You can reset assumptions via:

- !Assume
  reset: true <-- or maybe a `clean: true`
  distro: ubuntu

Or like this:

- !AssumeNothing  # or AssumeReset or ForgetAssumptions or AssumeClean?
- !Assume
  distro: ubuntu  # << this adds ubuntu to clear set of assuptions

The core here:

  1. Build step is more flexible, as we can't assume before DockerImage, or someone may deinstall things especially for inherited containers. This is irrelevant to common code, but may be important for niche use cases.
  2. !Assume is additive, so you can set specific things after specific steps, kinda:
- !DockerImage ubuntu
- !Assume  # auto-determine distro
- !Sh 'apt-get install python3.7'
- !Assume python3: /usr/bin/python3.7

Maybe stucture it like this?

  1. !AutoDetect (everything, but don't fail) and !AutoDetect [distibution, python] (fails if can't find python)
  2. !Assume with struct of values that are specified by user, not autodetected
  3. !AssumeForget to cleanup

And also DockerImage automatically provides !AutoDetect with no args (auto-detect does not fail, and you can clean with !AssumeForget if it's wrong).

Still don't like !AssumeForget name.

@anti-social
Copy link
Contributor Author

anti-social commented Feb 8, 2022

I think DockerImage should not do any autodetections, as autodetecting mangles a container. If a user has only DockerImage build step he/she should get a container identical to the docker image.

@anti-social
Copy link
Contributor Author

Another question: would it be useful to store docker image ENTRYPOINT within a container to have a possibility not to describe run command option:

containers:
  elasticsearch:
    setup:
    - !DockerImage elasticsearch:7.17.0

commands:
  elasticsearch: !Command
    container: elasticsearch

In other case we need to find out manually how to run such a container.

@tailhook
Copy link
Owner

tailhook commented Feb 8, 2022

I think DockerImage should not do any autodetections, as autodetecting mangles a container. If a user has only DockerImage build step he/she should get a container identical to the docker image.

In which sense does it mangle? It does not do any sort of thing, until you do any command like !Install.

This approach makes sense, though, if we expect that common case would be to just use docker image verbatim without any additional commands.

Another question: would it be useful to store docker image ENTRYPOINT within a container to have a possibility not to describe run command option:

It makes sense to save it in some way, but I don't think that introducing commands that don't have run is a good idea:

  • entrypoint is used to wrap command so run: [something] should pass something to the entry point rather than running directly, but this contradicts with everything else in vagga, so doesn't sound like a good idea at the moment.
  • One alternative is to save entrypoint as a script in say /docker-entrypoint.sh so you can run it. But it alters image a bit and may conflict with existing commands
  • Another alternative is to add run: !DockerEntrypoint and run: !DockerEntrypoint [something]
  • On the other hand, we already have default-shell in container config so technically we could add entrypoint on config too. But it depending on build steps is kinda very different from all other config we have now.
  • also not clear how run: "xxx" vs run: [xxx, yyy] would be handled by entrypoint.

Too many questions at this point :( Also a lot of the same comes to volumes I think, no?

@anti-social
Copy link
Contributor Author

anti-social commented Feb 9, 2022

In which sense does it mangle? It does not do any sort of thing, until you do any command like !Install.

init_ubuntu_core is called before any !Install commands. It configures some system tools like apt, generates locales and can even install packages. Although we possibly could make such an initialization lazy.

@anti-social
Copy link
Contributor Author

Another alternative is to add run: !DockerEntrypoint and run: !DockerEntrypoint [something]

+1

Either it can be another kind of a command:

commands:
  elasticsearch: !DockerEntrypoint
    container: elasticsearch

@tailhook
Copy link
Owner

In which sense does it mangle? It does not do any sort of thing, until you do any command like !Install.

init_ubuntu_core is called before any !Install commands. It configures some system tools like apt, generates locales and can even install packages. Although we possibly could make such an initialization lazy.

Sure. We definitely should not do any of these.

Another alternative is to add run: !DockerEntrypoint and run: !DockerEntrypoint [something]

+1

Either it can be another kind of a command:

commands:
  elasticsearch: !DockerEntrypoint
    container: elasticsearch

This is a bit weird, but would probably be the most straightforwards currently.

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

2 participants