Skip to content

Commit cdd9a79

Browse files
djdongjindjdongjin
authored
Merge pull request containerd#3457 from haytok/fix-to-clean-up-orphaned-files
fix: Cleaning up orphaned directories and files when containers creat…
2 parents cd4a62a + 2c2745e commit cdd9a79

File tree

4 files changed

+181
-32
lines changed

4 files changed

+181
-32
lines changed

‎cmd/nerdctl/container/container_create_linux_test.go

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,22 @@
1717
package container
1818

1919
import (
20+
"errors"
2021
"fmt"
22+
"os"
23+
"path/filepath"
2124
"strings"
2225
"testing"
2326

27+
"github.com/opencontainers/go-digest"
2428
"gotest.tools/v3/assert"
2529

30+
"github.com/containerd/containerd/v2/defaults"
31+
2632
"github.com/containerd/nerdctl/v2/pkg/testutil"
33+
"github.com/containerd/nerdctl/v2/pkg/testutil/nerdtest"
2734
"github.com/containerd/nerdctl/v2/pkg/testutil/nettestutil"
35+
"github.com/containerd/nerdctl/v2/pkg/testutil/test"
2836
)
2937

3038
func TestCreate(t *testing.T) {
@@ -174,3 +182,121 @@ func TestCreateWithTty(t *testing.T) {
174182
withTtyContainer := base.InspectContainer(withTtyContainerName)
175183
assert.Equal(base.T, 0, withTtyContainer.State.ExitCode)
176184
}
185+
186+
// TestIssue2993 tests https://github.com/containerd/nerdctl/issues/2993
187+
func TestIssue2993(t *testing.T) {
188+
testutil.DockerIncompatible(t)
189+
190+
nerdtest.Setup()
191+
192+
const (
193+
containersPathKey = "containersPath"
194+
etchostsPathKey = "etchostsPath"
195+
)
196+
197+
getAddrHash := func(addr string) string {
198+
const addrHashLen = 8
199+
200+
d := digest.SHA256.FromString(addr)
201+
h := d.Encoded()[0:addrHashLen]
202+
203+
return h
204+
}
205+
206+
testCase := &test.Group{
207+
{
208+
Description: "Issue #2993 - nerdctl no longer leaks containers and etchosts directories and files when container creation fails.",
209+
Require: nerdtest.Private,
210+
Setup: func(data test.Data, helpers test.Helpers) {
211+
helpers.Ensure("run", "--name", data.Identifier(), "-d", testutil.AlpineImage, "sleep", "infinity")
212+
213+
dataRoot := string(data.ReadConfig(nerdtest.DataRoot))
214+
h := getAddrHash(defaults.DefaultAddress)
215+
dataStore := filepath.Join(dataRoot, h)
216+
namespace := data.Identifier()
217+
218+
containersPath := filepath.Join(dataStore, "containers", namespace)
219+
containersDirs, err := os.ReadDir(containersPath)
220+
assert.NilError(t, err)
221+
assert.Equal(t, len(containersDirs), 1)
222+
223+
etchostsPath := filepath.Join(dataStore, "etchosts", namespace)
224+
etchostsDirs, err := os.ReadDir(etchostsPath)
225+
assert.NilError(t, err)
226+
assert.Equal(t, len(etchostsDirs), 1)
227+
228+
data.Set(containersPathKey, containersPath)
229+
data.Set(etchostsPathKey, etchostsPath)
230+
},
231+
Cleanup: func(data test.Data, helpers test.Helpers) {
232+
helpers.Anyhow("rm", "-f", data.Identifier())
233+
},
234+
Command: func(data test.Data, helpers test.Helpers) test.Command {
235+
return helpers.Command("run", "--name", data.Identifier(), "-d", testutil.AlpineImage, "sleep", "infinity")
236+
},
237+
Expected: func(data test.Data, helpers test.Helpers) *test.Expected {
238+
return &test.Expected{
239+
ExitCode: 1,
240+
Errors: []error{errors.New("is already used by ID")},
241+
Output: func(stdout string, info string, t *testing.T) {
242+
containersDirs, err := os.ReadDir(data.Get(containersPathKey))
243+
assert.NilError(t, err)
244+
assert.Equal(t, len(containersDirs), 1)
245+
246+
etchostsDirs, err := os.ReadDir(data.Get(etchostsPathKey))
247+
assert.NilError(t, err)
248+
assert.Equal(t, len(etchostsDirs), 1)
249+
},
250+
}
251+
},
252+
},
253+
{
254+
Description: "Issue #2993 - nerdctl no longer leaks containers and etchosts directories and files when containers are removed.",
255+
Require: nerdtest.Private,
256+
Setup: func(data test.Data, helpers test.Helpers) {
257+
helpers.Ensure("run", "--name", data.Identifier(), "-d", testutil.AlpineImage, "sleep", "infinity")
258+
259+
dataRoot := string(data.ReadConfig(nerdtest.DataRoot))
260+
h := getAddrHash(defaults.DefaultAddress)
261+
dataStore := filepath.Join(dataRoot, h)
262+
namespace := data.Identifier()
263+
264+
containersPath := filepath.Join(dataStore, "containers", namespace)
265+
containersDirs, err := os.ReadDir(containersPath)
266+
assert.NilError(t, err)
267+
assert.Equal(t, len(containersDirs), 1)
268+
269+
etchostsPath := filepath.Join(dataStore, "etchosts", namespace)
270+
etchostsDirs, err := os.ReadDir(etchostsPath)
271+
assert.NilError(t, err)
272+
assert.Equal(t, len(etchostsDirs), 1)
273+
274+
data.Set(containersPathKey, containersPath)
275+
data.Set(etchostsPathKey, etchostsPath)
276+
},
277+
Cleanup: func(data test.Data, helpers test.Helpers) {
278+
helpers.Anyhow("rm", "-f", data.Identifier())
279+
},
280+
Command: func(data test.Data, helpers test.Helpers) test.Command {
281+
return helpers.Command("rm", "-f", data.Identifier())
282+
},
283+
Expected: func(data test.Data, helpers test.Helpers) *test.Expected {
284+
return &test.Expected{
285+
ExitCode: 0,
286+
Errors: []error{},
287+
Output: func(stdout string, info string, t *testing.T) {
288+
containersDirs, err := os.ReadDir(data.Get(containersPathKey))
289+
assert.NilError(t, err)
290+
assert.Equal(t, len(containersDirs), 0)
291+
292+
etchostsDirs, err := os.ReadDir(data.Get(etchostsPathKey))
293+
assert.NilError(t, err)
294+
assert.Equal(t, len(etchostsDirs), 0)
295+
},
296+
}
297+
},
298+
},
299+
}
300+
301+
testCase.Run(t)
302+
}

‎pkg/cmd/container/create.go

Lines changed: 47 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ import (
4646
"github.com/containerd/nerdctl/v2/pkg/cmd/image"
4747
"github.com/containerd/nerdctl/v2/pkg/cmd/volume"
4848
"github.com/containerd/nerdctl/v2/pkg/containerutil"
49+
"github.com/containerd/nerdctl/v2/pkg/dnsutil/hostsstore"
4950
"github.com/containerd/nerdctl/v2/pkg/flagutil"
5051
"github.com/containerd/nerdctl/v2/pkg/idgen"
5152
"github.com/containerd/nerdctl/v2/pkg/imgutil"
@@ -118,7 +119,7 @@ func Create(ctx context.Context, client *containerd.Client, args []string, netMa
118119

119120
platformOpts, err := setPlatformOptions(ctx, client, id, netManager.NetworkOptions().UTSNamespace, &internalLabels, options)
120121
if err != nil {
121-
return nil, nil, err
122+
return nil, generateRemoveStateDirFunc(ctx, id, internalLabels), err
122123
}
123124
opts = append(opts, platformOpts...)
124125

@@ -130,7 +131,7 @@ func Create(ctx context.Context, client *containerd.Client, args []string, netMa
130131
}
131132
ocispecPlatforms, err := platformutil.NewOCISpecPlatformSlice(false, platformSS)
132133
if err != nil {
133-
return nil, nil, err
134+
return nil, generateRemoveStateDirFunc(ctx, id, internalLabels), err
134135
}
135136
rawRef := args[0]
136137

@@ -141,13 +142,13 @@ func Create(ctx context.Context, client *containerd.Client, args []string, netMa
141142

142143
ensuredImage, err = image.EnsureImage(ctx, client, rawRef, options.ImagePullOpt)
143144
if err != nil {
144-
return nil, nil, err
145+
return nil, generateRemoveStateDirFunc(ctx, id, internalLabels), err
145146
}
146147
}
147148

148149
rootfsOpts, rootfsCOpts, err := generateRootfsOpts(args, id, ensuredImage, options)
149150
if err != nil {
150-
return nil, nil, err
151+
return nil, generateRemoveStateDirFunc(ctx, id, internalLabels), err
151152
}
152153
opts = append(opts, rootfsOpts...)
153154
cOpts = append(cOpts, rootfsCOpts...)
@@ -158,12 +159,12 @@ func Create(ctx context.Context, client *containerd.Client, args []string, netMa
158159

159160
envs, err := flagutil.MergeEnvFileAndOSEnv(options.EnvFile, options.Env)
160161
if err != nil {
161-
return nil, nil, err
162+
return nil, generateRemoveStateDirFunc(ctx, id, internalLabels), err
162163
}
163164

164165
if options.Interactive {
165166
if options.Detach {
166-
return nil, nil, errors.New("currently flag -i and -d cannot be specified together (FIXME)")
167+
return nil, generateRemoveStateDirFunc(ctx, id, internalLabels), errors.New("currently flag -i and -d cannot be specified together (FIXME)")
167168
}
168169
}
169170

@@ -174,7 +175,7 @@ func Create(ctx context.Context, client *containerd.Client, args []string, netMa
174175
var mountOpts []oci.SpecOpts
175176
mountOpts, internalLabels.anonVolumes, internalLabels.mountPoints, err = generateMountOpts(ctx, client, ensuredImage, volStore, options)
176177
if err != nil {
177-
return nil, nil, err
178+
return nil, generateRemoveStateDirFunc(ctx, id, internalLabels), err
178179
}
179180
opts = append(opts, mountOpts...)
180181

@@ -186,30 +187,30 @@ func Create(ctx context.Context, client *containerd.Client, args []string, netMa
186187
// 3, nerdctl start/restart demo
187188
logConfig, err := generateLogConfig(dataStore, id, options.LogDriver, options.LogOpt, options.GOptions.Namespace)
188189
if err != nil {
189-
return nil, nil, err
190+
return nil, generateRemoveStateDirFunc(ctx, id, internalLabels), err
190191
}
191192
internalLabels.logURI = logConfig.LogURI
192193

193194
restartOpts, err := generateRestartOpts(ctx, client, options.Restart, logConfig.LogURI, options.InRun)
194195
if err != nil {
195-
return nil, nil, err
196+
return nil, generateRemoveStateDirFunc(ctx, id, internalLabels), err
196197
}
197198
cOpts = append(cOpts, restartOpts...)
198199

199200
if err = netManager.VerifyNetworkOptions(ctx); err != nil {
200-
return nil, nil, fmt.Errorf("failed to verify networking settings: %s", err)
201+
return nil, generateRemoveStateDirFunc(ctx, id, internalLabels), fmt.Errorf("failed to verify networking settings: %s", err)
201202
}
202203

203204
netOpts, netNewContainerOpts, err := netManager.ContainerNetworkingOpts(ctx, id)
204205
if err != nil {
205-
return nil, nil, fmt.Errorf("failed to generate networking spec options: %s", err)
206+
return nil, generateRemoveOrphanedDirsFunc(ctx, id, dataStore, internalLabels), fmt.Errorf("failed to generate networking spec options: %s", err)
206207
}
207208
opts = append(opts, netOpts...)
208209
cOpts = append(cOpts, netNewContainerOpts...)
209210

210211
netLabelOpts, err := netManager.InternalNetworkingOptionLabels(ctx)
211212
if err != nil {
212-
return nil, nil, fmt.Errorf("failed to generate internal networking labels: %s", err)
213+
return nil, generateRemoveOrphanedDirsFunc(ctx, id, dataStore, internalLabels), fmt.Errorf("failed to generate internal networking labels: %s", err)
213214
}
214215

215216
envs = append(envs, "HOSTNAME="+netLabelOpts.Hostname)
@@ -230,37 +231,37 @@ func Create(ctx context.Context, client *containerd.Client, args []string, netMa
230231
if runtime.GOOS != "windows" {
231232
hookOpt, err := withNerdctlOCIHook(options.NerdctlCmd, options.NerdctlArgs)
232233
if err != nil {
233-
return nil, nil, err
234+
return nil, generateRemoveOrphanedDirsFunc(ctx, id, dataStore, internalLabels), err
234235
}
235236
opts = append(opts, hookOpt)
236237
}
237238

238239
uOpts, err := generateUserOpts(options.User)
239240
if err != nil {
240-
return nil, nil, err
241+
return nil, generateRemoveOrphanedDirsFunc(ctx, id, dataStore, internalLabels), err
241242
}
242243
opts = append(opts, uOpts...)
243244
gOpts, err := generateGroupsOpts(options.GroupAdd)
244245
if err != nil {
245-
return nil, nil, err
246+
return nil, generateRemoveOrphanedDirsFunc(ctx, id, dataStore, internalLabels), err
246247
}
247248
opts = append(opts, gOpts...)
248249

249250
umaskOpts, err := generateUmaskOpts(options.Umask)
250251
if err != nil {
251-
return nil, nil, err
252+
return nil, generateRemoveOrphanedDirsFunc(ctx, id, dataStore, internalLabels), err
252253
}
253254
opts = append(opts, umaskOpts...)
254255

255256
rtCOpts, err := generateRuntimeCOpts(options.GOptions.CgroupManager, options.Runtime)
256257
if err != nil {
257-
return nil, nil, err
258+
return nil, generateRemoveOrphanedDirsFunc(ctx, id, dataStore, internalLabels), err
258259
}
259260
cOpts = append(cOpts, rtCOpts...)
260261

261262
lCOpts, err := withContainerLabels(options.Label, options.LabelFile, ensuredImage)
262263
if err != nil {
263-
return nil, nil, err
264+
return nil, generateRemoveOrphanedDirsFunc(ctx, id, dataStore, internalLabels), err
264265
}
265266
cOpts = append(cOpts, lCOpts...)
266267

@@ -276,25 +277,25 @@ func Create(ctx context.Context, client *containerd.Client, args []string, netMa
276277
if options.Name != "" {
277278
containerNameStore, err = namestore.New(dataStore, options.GOptions.Namespace)
278279
if err != nil {
279-
return nil, nil, err
280+
return nil, generateRemoveOrphanedDirsFunc(ctx, id, dataStore, internalLabels), err
280281
}
281282
if err := containerNameStore.Acquire(options.Name, id); err != nil {
282-
return nil, nil, err
283+
return nil, generateRemoveOrphanedDirsFunc(ctx, id, dataStore, internalLabels), err
283284
}
284285
}
285286
internalLabels.name = options.Name
286287
internalLabels.pidFile = options.PidFile
287288
internalLabels.extraHosts = strutil.DedupeStrSlice(netManager.NetworkOptions().AddHost)
288289
for i, host := range internalLabels.extraHosts {
289290
if _, err := dockercliopts.ValidateExtraHost(host); err != nil {
290-
return nil, nil, err
291+
return nil, generateRemoveOrphanedDirsFunc(ctx, id, dataStore, internalLabels), err
291292
}
292293
parts := strings.SplitN(host, ":", 2)
293294
// If the IP Address is a string called "host-gateway", replace this value with the IP address stored
294295
// in the daemon level HostGateway IP config variable.
295296
if len(parts) == 2 && parts[1] == dockeropts.HostGatewayName {
296297
if options.GOptions.HostGatewayIP == "" {
297-
return nil, nil, fmt.Errorf("unable to derive the IP value for host-gateway")
298+
return nil, generateRemoveOrphanedDirsFunc(ctx, id, dataStore, internalLabels), fmt.Errorf("unable to derive the IP value for host-gateway")
298299
}
299300
parts[1] = options.GOptions.HostGatewayIP
300301
internalLabels.extraHosts[i] = fmt.Sprintf(`%s:%s`, parts[0], parts[1])
@@ -304,7 +305,7 @@ func Create(ctx context.Context, client *containerd.Client, args []string, netMa
304305
// TODO: abolish internal labels and only use annotations
305306
ilOpt, err := withInternalLabels(internalLabels)
306307
if err != nil {
307-
return nil, nil, err
308+
return nil, generateRemoveOrphanedDirsFunc(ctx, id, dataStore, internalLabels), err
308309
}
309310
cOpts = append(cOpts, ilOpt)
310311

@@ -817,6 +818,29 @@ func generateLogConfig(dataStore string, id string, logDriver string, logOpt []s
817818
return logConfig, nil
818819
}
819820

821+
func generateRemoveStateDirFunc(ctx context.Context, id string, internalLabels internalLabels) func() {
822+
return func() {
823+
if rmErr := os.RemoveAll(internalLabels.stateDir); rmErr != nil {
824+
log.G(ctx).WithError(rmErr).Warnf("failed to remove container %q state dir %q", id, internalLabels.stateDir)
825+
}
826+
}
827+
}
828+
829+
func generateRemoveOrphanedDirsFunc(ctx context.Context, id, dataStore string, internalLabels internalLabels) func() {
830+
return func() {
831+
if rmErr := os.RemoveAll(internalLabels.stateDir); rmErr != nil {
832+
log.G(ctx).WithError(rmErr).Warnf("failed to remove container %q state dir %q", id, internalLabels.stateDir)
833+
}
834+
835+
hs, err := hostsstore.New(dataStore, internalLabels.namespace)
836+
if err != nil {
837+
log.G(ctx).WithError(err).Warnf("failed to instantiate hostsstore for %q", internalLabels.namespace)
838+
} else if err = hs.Delete(id); err != nil {
839+
log.G(ctx).WithError(err).Warnf("failed to remove an etchosts directory for container %q", id)
840+
}
841+
}
842+
}
843+
820844
func generateGcFunc(ctx context.Context, container containerd.Container, ns, id, name, dataStore string, containerErr error, containerNameStore namestore.NameStore, netManager containerutil.NetworkOptionsManager, internalLabels internalLabels) func() {
821845
return func() {
822846
if containerErr == nil {

‎pkg/cmd/container/remove.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,7 @@ func RemoveContainer(ctx context.Context, c containerd.Container, globalOptions
233233
hs, err := hostsstore.New(dataStore, containerNamespace)
234234
if err != nil {
235235
log.G(ctx).WithError(err).Warnf("failed to instantiate hostsstore for %q", containerNamespace)
236-
} else if err = hs.DeallocHostsFile(id); err != nil {
236+
} else if err = hs.Delete(id); err != nil {
237237
// De-allocate hosts file - soft failure
238238
log.G(ctx).WithError(err).Warnf("failed to remove hosts file for container %q", id)
239239
}

‎pkg/dnsutil/hostsstore/hostsstore.go

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ type Store interface {
8989
Release(id string) error
9090
Update(id, newName string) error
9191
HostsPath(id string) (location string, err error)
92-
DeallocHostsFile(id string) (err error)
92+
Delete(id string) (err error)
9393
AllocHostsFile(id string, content []byte) (location string, err error)
9494
}
9595

@@ -185,14 +185,13 @@ func (x *hostsStore) AllocHostsFile(id string, content []byte) (location string,
185185
return x.safeStore.Location(id, hostsFile)
186186
}
187187

188-
func (x *hostsStore) DeallocHostsFile(id string) (err error) {
189-
defer func() {
190-
if err != nil {
191-
err = errors.Join(ErrHostsStore, err)
192-
}
193-
}()
188+
func (x *hostsStore) Delete(id string) (err error) {
189+
err = x.safeStore.WithLock(func() error { return x.safeStore.Delete(id) })
190+
if err != nil {
191+
err = errors.Join(ErrHostsStore, err)
192+
}
194193

195-
return x.safeStore.WithLock(func() error { return x.safeStore.Delete(id, hostsFile) })
194+
return err
196195
}
197196

198197
func (x *hostsStore) HostsPath(id string) (location string, err error) {

0 commit comments

Comments
 (0)