From df08a17e9846ee7a2caaa688b20e1d6085decd9c Mon Sep 17 00:00:00 2001 From: fahed dorgaa Date: Fri, 3 Feb 2023 10:38:13 +0100 Subject: [PATCH] handle container deps removal with postsop hooks Signed-off-by: fahed dorgaa --- cmd/nerdctl/container_restart_linux_test.go | 6 +++--- cmd/nerdctl/container_run.go | 1 - cmd/nerdctl/container_run_network_linux_test.go | 1 + pkg/cmd/container/remove.go | 4 +++- pkg/dnsutil/hostsstore/hostsstore.go | 2 ++ pkg/ocihook/ocihook.go | 11 ++++++++++- 6 files changed, 19 insertions(+), 6 deletions(-) diff --git a/cmd/nerdctl/container_restart_linux_test.go b/cmd/nerdctl/container_restart_linux_test.go index b04e0346fc7..669d94d6640 100644 --- a/cmd/nerdctl/container_restart_linux_test.go +++ b/cmd/nerdctl/container_restart_linux_test.go @@ -51,11 +51,11 @@ func TestRestartPIDContainer(t *testing.T) { base := testutil.NewBase(t) baseContainerName := testutil.Identifier(t) - base.Cmd("run", "-d", "--name", baseContainerName, testutil.AlpineImage, "sleep", "infinity").Run() + base.Cmd("run", "-d", "--name", baseContainerName, testutil.AlpineImage, "sleep", "infinity").AssertOK() defer base.Cmd("rm", "-f", baseContainerName).Run() - sharedContainerName := testutil.Identifier(t) - base.Cmd("run", "-d", "--name", sharedContainerName, fmt.Sprintf("--pid=container:%s", baseContainerName), testutil.AlpineImage, "sleep", "infinity").Run() + sharedContainerName := fmt.Sprintf("%s-shared", baseContainerName) + base.Cmd("run", "-d", "--name", sharedContainerName, fmt.Sprintf("--pid=container:%s", baseContainerName), testutil.AlpineImage, "sleep", "infinity").AssertOK() defer base.Cmd("rm", "-f", sharedContainerName).Run() base.Cmd("restart", baseContainerName).AssertOK() diff --git a/cmd/nerdctl/container_run.go b/cmd/nerdctl/container_run.go index 6475f0e453f..c26bf008876 100644 --- a/cmd/nerdctl/container_run.go +++ b/cmd/nerdctl/container_run.go @@ -358,7 +358,6 @@ func runAction(cmd *cobra.Command, args []string) error { return err } logURI := lab[labels.LogURI] - task, err := taskutil.NewTask(ctx, client, c, false, flagI, flagT, flagD, con, logURI) if err != nil { return err diff --git a/cmd/nerdctl/container_run_network_linux_test.go b/cmd/nerdctl/container_run_network_linux_test.go index 0bf6bfcdb6d..db7c4e6e96e 100644 --- a/cmd/nerdctl/container_run_network_linux_test.go +++ b/cmd/nerdctl/container_run_network_linux_test.go @@ -404,6 +404,7 @@ func TestRunWithInvalidPortThenCleanUp(t *testing.T) { t.Parallel() base := testutil.NewBase(t) containerName := testutil.Identifier(t) + defer base.Cmd("rm", "-f", containerName).Run() base.Cmd("run", "--rm", "--name", containerName, "-p", "22200-22299:22200-22299", testutil.CommonImage).AssertFail() base.Cmd("run", "--rm", "--name", containerName, "-p", "22200-22299:22200-22299", testutil.CommonImage).AssertCombinedOutContains(errdefs.ErrInvalidArgument.Error()) base.Cmd("run", "--rm", "--name", containerName, testutil.CommonImage).AssertOK() diff --git a/pkg/cmd/container/remove.go b/pkg/cmd/container/remove.go index ee4481ce6d4..3150328dbb5 100644 --- a/pkg/cmd/container/remove.go +++ b/pkg/cmd/container/remove.go @@ -122,10 +122,10 @@ func RemoveContainer(ctx context.Context, c containerd.Container, globalOptions if retErr != nil { return } - if err := os.RemoveAll(stateDir); err != nil { logrus.WithError(retErr).Warnf("failed to remove container state dir %s", stateDir) } + // enforce release name here in case the poststop hook name release fails if name != "" { if err := namst.Release(name, id); err != nil { logrus.WithError(retErr).Warnf("failed to release container name %s", name) @@ -135,6 +135,8 @@ func RemoveContainer(ctx context.Context, c containerd.Container, globalOptions logrus.WithError(retErr).Warnf("failed to remove hosts file for container %q", id) } }() + + // volume removal is not handled by the poststop hook lifecycle because it depends on removeAnonVolumes option if anonVolumesJSON, ok := l[labels.AnonymousVolumes]; ok && removeAnonVolumes { var anonVolumes []string if err := json.Unmarshal([]byte(anonVolumesJSON), &anonVolumes); err != nil { diff --git a/pkg/dnsutil/hostsstore/hostsstore.go b/pkg/dnsutil/hostsstore/hostsstore.go index d190be858bc..f90e7ad8752 100644 --- a/pkg/dnsutil/hostsstore/hostsstore.go +++ b/pkg/dnsutil/hostsstore/hostsstore.go @@ -138,6 +138,8 @@ func (x *store) Acquire(meta Meta) error { return lockutil.WithDirLock(x.hostsD, fn) } +// Release is triggered by Poststop hooks. +// It is called after the containerd task is deleted but before the delete operation returns. func (x *store) Release(ns, id string) error { fn := func() error { metaPath := filepath.Join(x.hostsD, ns, id, metaJSON) diff --git a/pkg/ocihook/ocihook.go b/pkg/ocihook/ocihook.go index 8c9554a15af..ef61e1fd992 100644 --- a/pkg/ocihook/ocihook.go +++ b/pkg/ocihook/ocihook.go @@ -32,6 +32,7 @@ import ( "github.com/containerd/nerdctl/pkg/bypass4netnsutil" "github.com/containerd/nerdctl/pkg/dnsutil/hostsstore" "github.com/containerd/nerdctl/pkg/labels" + "github.com/containerd/nerdctl/pkg/namestore" "github.com/containerd/nerdctl/pkg/netutil" "github.com/containerd/nerdctl/pkg/netutil/nettype" "github.com/containerd/nerdctl/pkg/rootlessutil" @@ -445,6 +446,7 @@ func onCreateRuntime(opts *handlerOpts) error { func onPostStop(opts *handlerOpts) error { ctx := context.Background() + ns := opts.state.Annotations[labels.Namespace] if opts.cni != nil { var err error b4nnEnabled, err := bypass4netnsutil.IsBypass4netnsEnabled(opts.state.Annotations) @@ -497,10 +499,17 @@ func onPostStop(opts *handlerOpts) error { if err != nil { return err } - ns := opts.state.Annotations[labels.Namespace] if err := hs.Release(ns, opts.state.ID); err != nil { return err } } + namst, err := namestore.New(opts.dataStore, ns) + if err != nil { + return err + } + name := opts.state.Annotations[labels.Name] + if err := namst.Release(name, opts.state.ID); err != nil { + return fmt.Errorf("failed to release container name %s: %w", name, err) + } return nil }