Merge pull request juju#23105 from SimonRichardson/rollout-pods

juju#23105

When reconciling the units during scaling, we previously always scaled from top down - so if you had 0,1,2 and scaled down by one at a time, it would do 0,1 and then 0. This is normally fine, but it doesn't allow you to rollout the oldest unit (or pod in this instance). For instance, if you had new constraints (think affinities) then you'd have to scale to zero, thus removing all pods. This is very unfortunate. Instead, if we scale from bottom up, we can then roll in new units (pods) with new constraints, allowing for a more fluid system.

This was spotted when testing juju#23076 and wanting to scale back down. 

## QA steps

```sh
$ juju bootstrap microk8s test
$ juju add-model foo
$ juju deploy prometheus-k8s prom -n 3
$ juju remove-unit prom --num-units=2
```

Removes 0, 1.


## Links


**Jira card:** [JUJU-10258](https://warthogs.atlassian.net/browse/JUJU-10258)


[JUJU-10258]: https://warthogs.atlassian.net/browse/JUJU-10258?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ

Author: jujubot

apiserver/restrict_caasmodel.go

@@ -72,6 +72,7 @@ var commonModelFacadeNames = set.NewStrings(
 	"StorageProvisioner",
 	"StringsWatcher",
 	"Subnets",
+	"Tracer",
 	"Undertaker",
 	"Uniter",
 	"Upgrader",

apiserver/restrict_caasmodel_test.go

@@ -30,6 +30,7 @@ func (s *RestrictCAASModelSuite) SetUpSuite(c *tc.C) {
 
 func (s *RestrictCAASModelSuite) TestAllowed(c *tc.C) {
 	s.assertMethod(c, "CAASApplication", 1, "UnitIntroduction")
+	s.assertMethod(c, "Tracer", 1, "GetControllerTracingConfig")
 }
 
 func (s *RestrictCAASModelSuite) TestSubnetsAllowed(c *tc.C) {

internal/worker/caasapplicationprovisioner/ops.go

@@ -9,6 +9,7 @@ import (
 	"fmt"
 	"io"
 	"reflect"
+	"sort"
 	"strings"
 	"time"
 
@@ -634,12 +635,12 @@ func reconcileDeadUnitScale(
 	}
 
 	desiredScale := ps.ScaleTarget
-	unitsToRemove := 0
+	threshold := unitRemovalThreshold(unitNamesAndLives, desiredScale)
 
+	unitsToRemove := 0
 	var deadUnits []coreunit.Name
 	for unitName, unitLife := range unitNamesAndLives {
-		if unitName.Number() < desiredScale {
-			// This is a unit we want to keep.
+		if unitName.Number() >= threshold {
 			continue
 		}
 		unitsToRemove++
@@ -676,6 +677,9 @@ func reconcileDeadUnitScale(
 		return tryAgain
 	}
 
+	sort.Slice(deadUnits, func(i, j int) bool {
+		return deadUnits[i].Number() < deadUnits[j].Number()
+	})
 	for _, deadUnit := range deadUnits {
 		logger.Infof(ctx, "removing dead unit %s", deadUnit)
 		if err := facade.RemoveUnit(ctx, string(deadUnit)); err != nil && !errors.Is(err, errors.NotFound) {
@@ -729,15 +733,7 @@ func ensureScale(
 		return err
 	}
 
-	unitScale := 0
-	for unitName := range units {
-		nextUnitNumber := unitName.Number() + 1
-		if nextUnitNumber > unitScale {
-			unitScale = nextUnitNumber
-		}
-	}
-
-	if ps.ScaleTarget >= unitScale {
+	if ps.ScaleTarget >= len(units) {
 		storageUniqueID := appUUID.String()[:6]
 		err := ensureScaleWithFsAttachments(
 			ctx,
@@ -773,10 +769,10 @@ func ensureScale(
 		return nil
 	}
 
+	threshold := unitRemovalThreshold(units, ps.ScaleTarget)
 	var unitsToDestroy []string
 	for unitName, unitLife := range units {
-		if unitName.Number() < ps.ScaleTarget {
-			// This is a unit we want to keep.
+		if unitName.Number() >= threshold {
 			continue
 		}
 		if unitLife == life.Alive {
@@ -802,6 +798,28 @@ func getStorageUniqueID(appUUID coreapplication.UUID) string {
 	return appUUID.String()[:6]
 }
 
+// unitRemovalThreshold returns the lowest ordinal that should be kept when
+// scaling down to targetScale units. Unit ordinals lower than the returned
+// threshold are removed, keeping the highest targetScale ordinals. The
+// threshold is derived from the sorted ordinal set rather than assuming
+// ordinals are contiguous (0..N-1), so it remains correct when ordinals are
+// sparse (e.g. 0, 2, 4).
+func unitRemovalThreshold(units map[coreunit.Name]life.Value, targetScale int) int {
+	ordinals := make([]int, 0, len(units))
+	for unitName := range units {
+		ordinals = append(ordinals, unitName.Number())
+	}
+	sort.Ints(ordinals)
+
+	if targetScale >= len(ordinals) {
+		return 0
+	}
+	if targetScale <= 0 {
+		return ordinals[len(ordinals)-1] + 1
+	}
+	return ordinals[len(ordinals)-targetScale]
+}
+
 func setOperatorStatus(
 	ctx context.Context,
 	appName string, s status.Status, reason string, data map[string]any,