Skip to content

c8d/daemon: Add remote snapshotters to config #44

Open
vvoland wants to merge 39 commits intorumpl:c8dfrom
vvoland:containerd-snapshotters-proxy-plugins
Open

c8d/daemon: Add remote snapshotters to config #44
vvoland wants to merge 39 commits intorumpl:c8dfrom
vvoland:containerd-snapshotters-proxy-plugins

Conversation

@vvoland
Copy link
Copy Markdown
Collaborator

@vvoland vvoland commented Aug 4, 2022

Requires:

This adds the proxy plugin configuration to the generated containerd.toml that is used for the moby's supervised containerd process.
So to use the new snapshotters, you just need to make sure they are running on their default socket address and run the engine with the --containerd-snapshotter.
This also adds --containerd-snapshotter-addr which can be used to override the default snapshotter plugin socket address.

- What I did

- How I did it

- How to verify it

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

vvoland and others added 30 commits August 4, 2022 00:50
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
requies passing context around

Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Initial version that doesn't handle any filters

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
Without this "docker build" fails with:

    Error response from daemon: exporter "image" could not be found

Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
The legacy v1 is not supported by the containerd import

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
To make it possible to load emptyfs which is amd64 only

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
Move opentelemetry dependencies to direct

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
vvoland and others added 9 commits August 5, 2022 17:02
containerd/save: Export only present platform manifests
This change fixes multiple things:
* pass ctx instead of nil because we need it while computing the layer
  size
* don't return an error if the container is not found during the layer
  size calculation, if a container is not found it means that it exited
* create the container with the image it was created from, the image is
  used when computing the layer size

Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
Also moved some layerStore related initialization to the non-c8d case
because otherwise they get treated as a graphdriver plugins.

Co-authored-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The remote snapshotters have to be configured in the `containerd.toml`.
Add the proxy plugin configuration to the contained daemon opts.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
@vvoland vvoland force-pushed the containerd-snapshotters-proxy-plugins branch from c31725d to d930a4a Compare August 9, 2022 14:25
@vvoland vvoland marked this pull request as ready for review August 9, 2022 14:26
@vvoland
Copy link
Copy Markdown
Collaborator Author

vvoland commented Aug 9, 2022

Rebased

@vvoland vvoland requested review from ndeloof, rumpl and thaJeztah August 9, 2022 14:33
Comment on lines +62 to +67
func WithProxyPlugin(name string, plugin config.ProxyPlugin) DaemonOpt {
return func(r *remote) error {
r.ProxyPlugins[name] = plugin
return nil
}
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

FWIW; we discussed some things around this in the maintainers meeting; we want to stop adding new features to the generated configuration (when running a managed containerd / containerd as child process), and instead allow users to create their own containerd.toml for configuring containerd options.

I started some work on a patch to allow that feature (started to dust it off Yesterday evening)

I realise that this PR is easier (for now), and will get us going for the PoC; but possibly this approach will change in upstream (at least).

@thaJeztah
Copy link
Copy Markdown
Collaborator

FWIW; we discussed some things around this in the maintainers meeting; we want to stop adding new features to the generated configuration (when running a managed containerd / containerd as child process), and instead allow users to create their own containerd.toml for configuring containerd options.

I started some work on a patch to allow that feature (started to dust it off Yesterday evening)

I realise that this PR is easier (for now), and will get us going for the PoC; but possibly this approach will change in upstream (at least).

Quick heads up; I pushed a PR with that proposal; moby#43946

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Won't Upstream

Development

Successfully merging this pull request may close these issues.

4 participants