Skip to content

Commit 3373d0c

Browse files
nervghalexey-igrychev
authored andcommitted
fix(host-cleanup): don't stop removing werf containers on paused or running ones
Signed-off-by: Alexandr Zaytsev <alexandr.zaytsev@flant.com>
1 parent d5c548c commit 3373d0c

File tree

7 files changed

+131
-4
lines changed

7 files changed

+131
-4
lines changed

Diff for: pkg/container_backend/docker_server_backend.go

+10-1
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package container_backend
22

33
import (
44
"context"
5+
"errors"
56
"fmt"
67
"os"
78
"path/filepath"
@@ -272,7 +273,15 @@ func (backend *DockerServerBackend) Rmi(ctx context.Context, ref string, opts Rm
272273
}
273274

274275
func (backend *DockerServerBackend) Rm(ctx context.Context, ref string, opts RmOpts) error {
275-
return docker.ContainerRemove(ctx, ref, types.ContainerRemoveOptions{Force: opts.Force})
276+
err := docker.ContainerRemove(ctx, ref, types.ContainerRemoveOptions{Force: opts.Force})
277+
switch {
278+
case docker.IsErrContainerPaused(err):
279+
return errors.Join(ErrCannotRemovePausedContainer, err)
280+
case docker.IsErrContainerRunning(err):
281+
return errors.Join(ErrCannotRemoveRunningContainer, err)
282+
default:
283+
return err // err or nil
284+
}
276285
}
277286

278287
func (backend *DockerServerBackend) PushImage(ctx context.Context, img LegacyImageInterface) error {

Diff for: pkg/container_backend/errors.go

+5-1
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,8 @@ package container_backend
22

33
import "errors"
44

5-
var ErrUnsupportedFeature = errors.New("unsupported feature")
5+
var (
6+
ErrUnsupportedFeature = errors.New("unsupported feature")
7+
ErrCannotRemovePausedContainer = errors.New("cannot remove paused container")
8+
ErrCannotRemoveRunningContainer = errors.New("cannot remove running container")
9+
)

Diff for: pkg/docker/errors.go

+33
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
package docker
2+
3+
import (
4+
"strings"
5+
6+
"github.com/pkg/errors"
7+
)
8+
9+
// IsErrContainerPaused returns true if err is "container is paused" error
10+
// https://github.com/moby/moby/blob/25.0/daemon/delete.go#L94
11+
func IsErrContainerPaused(err error) bool {
12+
if err == nil {
13+
return false
14+
}
15+
cause := errors.Cause(err)
16+
if !strings.HasPrefix(cause.Error(), "cannot remove container") {
17+
return false
18+
}
19+
return strings.HasSuffix(cause.Error(), "container is paused and must be unpaused first")
20+
}
21+
22+
// IsErrContainerRunning returns true if err is "container is running" error
23+
// https://github.com/moby/moby/blob/25.0/daemon/delete.go#L96
24+
func IsErrContainerRunning(err error) bool {
25+
if err == nil {
26+
return false
27+
}
28+
cause := errors.Cause(err)
29+
if !strings.HasPrefix(cause.Error(), "cannot remove container") {
30+
return false
31+
}
32+
return strings.HasSuffix(cause.Error(), "container is running: stop the container before removing or force remove")
33+
}

Diff for: pkg/docker/errors_test.go

+34
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
package docker
2+
3+
import (
4+
"errors"
5+
6+
. "github.com/onsi/ginkgo/v2"
7+
. "github.com/onsi/gomega"
8+
errorsPkg "github.com/pkg/errors"
9+
)
10+
11+
var _ = Describe("docker errors", func() {
12+
Describe("IsErrContainerPaused", func() {
13+
It("should return true if err is \"container is paused\" error", func() {
14+
causeErr := errors.New("cannot remove container XXXX: container is paused and must be unpaused first")
15+
err0 := errorsPkg.WithMessage(causeErr, "some text")
16+
Expect(IsErrContainerPaused(err0)).To(BeTrue())
17+
})
18+
It("should return false otherwise", func() {
19+
err0 := errors.New("some err")
20+
Expect(IsErrContainerPaused(err0)).To(BeFalse())
21+
})
22+
})
23+
Describe("IsErrContainerRunning", func() {
24+
It("should return true if err is \"container is paused\" error", func() {
25+
causeErr := errors.New("cannot remove container XXXX: container is running: stop the container before removing or force remove")
26+
err0 := errorsPkg.WithMessage(causeErr, "some text")
27+
Expect(IsErrContainerRunning(err0)).To(BeTrue())
28+
})
29+
It("should return false otherwise", func() {
30+
err0 := errors.New("some err")
31+
Expect(IsErrContainerRunning(err0)).To(BeFalse())
32+
})
33+
})
34+
})

Diff for: pkg/docker/suite_test.go

+13
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
package docker
2+
3+
import (
4+
"testing"
5+
6+
. "github.com/onsi/ginkgo/v2"
7+
. "github.com/onsi/gomega"
8+
)
9+
10+
func TestDocker(t *testing.T) {
11+
RegisterFailHandler(Fail)
12+
RunSpecs(t, "Docker Suite")
13+
}

Diff for: pkg/host_cleaning/local_backend_cleaner.go

+9-2
Original file line numberDiff line numberDiff line change
@@ -416,7 +416,7 @@ func (cleaner *LocalBackendCleaner) RunGC(ctx context.Context, options RunGCOpti
416416
}
417417

418418
if vu.Percentage() <= options.AllowedStorageVolumeUsagePercentage {
419-
logboek.Context(ctx).LogBlock("Check storage", cleaner.BackendName()).Do(func() {
419+
logboek.Context(ctx).LogBlock("Check storage").Do(func() {
420420
logboek.Context(ctx).LogF("Total freed space: %s\n", utils.RedF("%s", humanize.Bytes(vuBefore.UsedBytes-vu.UsedBytes)))
421421
logboek.Context(ctx).LogF("Volume usage: %s / %s\n", humanize.Bytes(vu.UsedBytes), humanize.Bytes(vu.TotalBytes))
422422
logboek.Context(ctx).LogF("Allowed percentage level exceeded: %s > %s — %s\n", utils.RedF("%0.2f%%", vu.Percentage()), utils.YellowF("%0.2f%%", options.AllowedStorageVolumeUsagePercentage), utils.RedF("HIGH VOLUME USAGE"))
@@ -574,7 +574,14 @@ func (cleaner *LocalBackendCleaner) doSafeCleanupWerfContainers(ctx context.Cont
574574
err := cleaner.backend.Rm(ctx, container.ID, container_backend.RmOpts{
575575
Force: options.Force,
576576
})
577-
if err != nil {
577+
switch {
578+
case errors.Is(err, container_backend.ErrCannotRemovePausedContainer):
579+
logboek.Context(ctx).Warn().LogF("Ignore paused container %s\n", logContainerName(container))
580+
return nil
581+
case errors.Is(err, container_backend.ErrCannotRemoveRunningContainer):
582+
logboek.Context(ctx).Warn().LogF("Ignore running container %s\n", logContainerName(container))
583+
return nil
584+
case err != nil:
578585
return fmt.Errorf("failed to remove container %s: %w", logContainerName(container), err)
579586
}
580587
return nil

Diff for: pkg/host_cleaning/local_backend_cleaner_test.go

+27
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,33 @@ var _ = Describe("LocalBackendCleaner", func() {
187187
})
188188
})
189189

190+
Describe("doSafeCleanupWerfContainers", func() {
191+
It("should not return err if backend.Rm() returns 'container is paused' error", func() {
192+
container := image.Container{
193+
ID: "id-stage",
194+
Names: []string{fmt.Sprintf("/%s", image.StageContainerNamePrefix)},
195+
}
196+
197+
backend.EXPECT().Rm(ctx, container.ID, container_backend.RmOpts{Force: false}).Return(container_backend.ErrCannotRemovePausedContainer)
198+
stubs.StubFunc(&cleaner.volumeutilsGetVolumeUsageByPath, volumeutils.VolumeUsage{}, nil)
199+
200+
_, err := cleaner.doSafeCleanupWerfContainers(ctx, RunGCOptions{}, volumeutils.VolumeUsage{}, image.ContainerList{container})
201+
Expect(err).To(Succeed())
202+
})
203+
It("should not return err if backend.Rm() returns 'container is running' error", func() {
204+
container := image.Container{
205+
ID: "id-import-server",
206+
Names: []string{fmt.Sprintf("/%s", image.ImportServerContainerNamePrefix)},
207+
}
208+
209+
backend.EXPECT().Rm(ctx, container.ID, container_backend.RmOpts{Force: false}).Return(container_backend.ErrCannotRemoveRunningContainer)
210+
stubs.StubFunc(&cleaner.volumeutilsGetVolumeUsageByPath, volumeutils.VolumeUsage{}, nil)
211+
212+
_, err := cleaner.doSafeCleanupWerfContainers(ctx, RunGCOptions{}, volumeutils.VolumeUsage{}, image.ContainerList{container})
213+
Expect(err).To(Succeed())
214+
})
215+
})
216+
190217
Describe("werfImages", func() {
191218
It("should return images as merged and sorted result of several backend calls", func() {
192219
expectedImages := []image.Summary{

0 commit comments

Comments
 (0)