From 350f257f8a66a6ee39d1dc4838295e48ec54286b Mon Sep 17 00:00:00 2001
From: Nicolas De Loof <nicolas.deloof@gmail.com>
Date: Wed, 6 Nov 2024 13:18:00 +0100
Subject: [PATCH] remove obsolete containers first on scale down

Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
---
 pkg/compose/convergence.go        |  5 ++++-
 pkg/e2e/fixtures/scale/Dockerfile |  3 +++
 pkg/e2e/fixtures/scale/build.yaml |  3 +++
 pkg/e2e/scale_test.go             | 31 +++++++++++++++++++++++++++++++
 4 files changed, 41 insertions(+), 1 deletion(-)
 create mode 100644 pkg/e2e/fixtures/scale/Dockerfile
 create mode 100644 pkg/e2e/fixtures/scale/build.yaml

diff --git a/pkg/compose/convergence.go b/pkg/compose/convergence.go
index 2c2c85f82e0..d57c13b8b52 100644
--- a/pkg/compose/convergence.go
+++ b/pkg/compose/convergence.go
@@ -20,6 +20,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"slices"
 	"sort"
 	"strconv"
 	"strings"
@@ -136,16 +137,18 @@ func (c *convergence) ensureService(ctx context.Context, project *types.Project,
 		ni, erri := strconv.Atoi(containers[i].Labels[api.ContainerNumberLabel])
 		nj, errj := strconv.Atoi(containers[j].Labels[api.ContainerNumberLabel])
 		if erri == nil && errj == nil {
-			return ni < nj
+			return ni > nj
 		}
 
 		// If we don't get a container number (?) just sort by creation date
 		return containers[i].Created < containers[j].Created
 	})
 
+	slices.Reverse(containers)
 	for i, container := range containers {
 		if i >= expected {
 			// Scale Down
+			// As we sorted containers, obsolete ones and/or highest number will be removed
 			container := container
 			traceOpts := append(tracing.ServiceOptions(service), tracing.ContainerOptions(container)...)
 			eg.Go(tracing.SpanWrapFuncForErrGroup(ctx, "service/scale/down", traceOpts, func(ctx context.Context) error {
diff --git a/pkg/e2e/fixtures/scale/Dockerfile b/pkg/e2e/fixtures/scale/Dockerfile
new file mode 100644
index 00000000000..23a491297e9
--- /dev/null
+++ b/pkg/e2e/fixtures/scale/Dockerfile
@@ -0,0 +1,3 @@
+FROM nginx:alpine
+ARG FOO
+LABEL FOO=$FOO
\ No newline at end of file
diff --git a/pkg/e2e/fixtures/scale/build.yaml b/pkg/e2e/fixtures/scale/build.yaml
new file mode 100644
index 00000000000..cd109c7a849
--- /dev/null
+++ b/pkg/e2e/fixtures/scale/build.yaml
@@ -0,0 +1,3 @@
+services:
+  test:
+    build: .
diff --git a/pkg/e2e/scale_test.go b/pkg/e2e/scale_test.go
index 21595dd4773..7a859187bae 100644
--- a/pkg/e2e/scale_test.go
+++ b/pkg/e2e/scale_test.go
@@ -184,3 +184,34 @@ func checkServiceContainer(t *testing.T, stdout, containerName, containerState s
 	}
 	testify.Fail(t, errMessage, stdout)
 }
+
+func TestScaleDownNoRecreate(t *testing.T) {
+	const projectName = "scale-down-recreated-test"
+	c := NewCLI(t, WithEnv(
+		"COMPOSE_PROJECT_NAME="+projectName))
+
+	reset := func() {
+		c.RunDockerComposeCmd(t, "down", "--rmi", "all")
+	}
+	t.Cleanup(reset)
+	c.RunDockerComposeCmd(t, "-f", "fixtures/scale/build.yaml", "build", "--build-arg", "FOO=test")
+	c.RunDockerComposeCmd(t, "-f", "fixtures/scale/build.yaml", "up", "-d", "--scale", "test=2")
+
+	c.RunDockerComposeCmd(t, "-f", "fixtures/scale/build.yaml", "build", "--build-arg", "FOO=updated")
+	c.RunDockerComposeCmd(t, "-f", "fixtures/scale/build.yaml", "up", "-d", "--scale", "test=4", "--no-recreate")
+
+	res := c.RunDockerComposeCmd(t, "ps", "--format", "{{.Name}}", "test")
+	res.Assert(t, icmd.Success)
+	assert.Check(t, strings.Contains(res.Stdout(), "scale-down-recreated-test-test-1"))
+	assert.Check(t, strings.Contains(res.Stdout(), "scale-down-recreated-test-test-2"))
+	assert.Check(t, strings.Contains(res.Stdout(), "scale-down-recreated-test-test-3"))
+	assert.Check(t, strings.Contains(res.Stdout(), "scale-down-recreated-test-test-4"))
+
+	t.Log("scale down removes obsolete replica #1 and #2")
+	c.NewDockerComposeCmd(t, "--project-directory", "fixtures/scale", "up", "-d", "--scale", "test=2")
+
+	res = c.RunDockerComposeCmd(t, "ps", "--format", "{{.Name}}", "test")
+	res.Assert(t, icmd.Success)
+	assert.Check(t, strings.Contains(res.Stdout(), "scale-down-recreated-test-test-3"))
+	assert.Check(t, strings.Contains(res.Stdout(), "scale-down-recreated-test-test-4"))
+}