feat(labels): include detailed labels diff in service plan

pkg/client/deploy/deploy.go

@@ -1,11 +1,15 @@
 package deploy
 
 import (
+	"cmp"
 	"context"
 	"errors"
 	"fmt"
+	"maps"
+	"slices"
 	"strconv"
 	"strings"
+	"unicode"
 
 	"charm.land/lipgloss/v2"
 	"charm.land/lipgloss/v2/table"
@@ -50,25 +54,31 @@ type ServicePlan struct {
 // Format renders the service plan as a styled block with a spec diff and nested container operations.
 func (sp *ServicePlan) Format() string {
 	// Determine service-level operation type and extract the old spec from container operations.
-	// Assume replace operations precede remove operations (rolling strategy) so the first replace operation
-	// (if exists) determines the old spec for the diff. Otherwise, fallback to the first remove operation.
+	// Containers may have different specs after a failed deployment. The first replaced container determines the old
+	// spec for the diff because a container updated in place already has the new image. Otherwise, fallback to the
+	// first updated container and then the first removed one.
 	var hasRun, hasRemove bool
-	var oldSpec *api.ServiceSpec
+	var replacedSpec, updatedSpec, removedSpec *api.ServiceSpec
 	for _, op := range sp.Operations {
 		switch o := op.(type) {
 		case *operation.RunContainerOperation:
 			hasRun = true
 		case *operation.ReplaceContainerOperation:
-			if oldSpec == nil {
-				oldSpec = &o.OldContainer.ServiceSpec
+			if replacedSpec == nil {
+				replacedSpec = &o.OldContainer.ServiceSpec
+			}
+		case *operation.UpdateContainerOperation:
+			if updatedSpec == nil {
+				updatedSpec = &o.Container.ServiceSpec
 			}
 		case *operation.RemoveContainerOperation:
 			hasRemove = true
-			if oldSpec == nil {
-				oldSpec = &o.Container.ServiceSpec
+			if removedSpec == nil {
+				removedSpec = &o.Container.ServiceSpec
 			}
 		}
 	}
+	oldSpec := cmp.Or(replacedSpec, updatedSpec, removedSpec)
 
 	// Service line modifier and verb.
 	var modifier, verb string
@@ -91,7 +101,7 @@ func (sp *ServicePlan) Format() string {
 	out.WriteString("\n")
 
 	// Build spec diff table: columns are [modifier, attribute, value or change].
-	// TODO: print diff for all changed attributes, not just image and replicas.
+	// TODO: print diff for all changed attributes, not just image, replicas, and labels.
 	//  Consider reusing the logic in EvalContainerSpecChange to return a structured diff.
 	specTable := table.New().
 		Border(lipgloss.Border{}).
@@ -141,17 +151,23 @@ func (sp *ServicePlan) Format() string {
 		}
 	}
 
-	// Stack "  │ " tree prefixes vertically, then join horizontally with the table.
-	tableStr := specTable.String()
-	treePrefix := tui.Faint.Render("  │ ")
-	treeColRows := make([]string, specTable.GetData().Rows())
-	for i := range treeColRows {
-		treeColRows[i] = treePrefix
-	}
-	treeCol := lipgloss.JoinVertical(lipgloss.Left, treeColRows...)
+	specDiff := specTable.String() + "\n"
 
-	out.WriteString(lipgloss.JoinHorizontal(lipgloss.Top, treeCol, tableStr))
-	out.WriteString("\n")
+	// Render labels as nested blocks outside the table to not widen its attribute column. An existing service without
+	// an old spec only has new containers added to up-to-date ones so its labels didn't change.
+	if sp.IsNewService || oldSpec != nil {
+		var oldLabels, oldContainerLabels map[string]string
+		if oldSpec != nil {
+			oldLabels, oldContainerLabels = oldSpec.Labels, oldSpec.Container.Labels
+		}
+		specDiff += formatLabelsDiff("labels:", oldLabels, sp.Spec.Labels, sp.IsNewService)
+		specDiff += formatLabelsDiff("container labels:", oldContainerLabels, sp.Spec.Container.Labels, sp.IsNewService)
+	}
+
+	treePrefix := tui.Faint.Render("  │ ")
+	for line := range strings.Lines(specDiff) {
+		out.WriteString(treePrefix + line)
+	}
 
 	// Blank separator line before container operations.
 	out.WriteString(tui.Faint.Render("  │"))
@@ -272,6 +288,66 @@ func formatImageDiff(oldImage, newImage string) string {
 		tui.FormatImage(newImage, tui.Green)
 }
 
+// formatLabelsDiff renders a block with the attribute name followed by a nested line for each added, changed, or
+// removed label, sorted by key. It returns an empty string if the labels are the same.
+func formatLabelsDiff(attribute string, oldLabels, newLabels map[string]string, isNewService bool) string {
+	keys := slices.Concat(slices.Collect(maps.Keys(oldLabels)), slices.Collect(maps.Keys(newLabels)))
+	slices.Sort(keys)
+
+	var lines strings.Builder
+	for _, key := range slices.Compact(keys) {
+		oldValue, inOld := oldLabels[key]
+		newValue, inNew := newLabels[key]
+
+		var mod, diff string
+		switch {
+		case !inOld:
+			mod = tui.Green.Render("+")
+			// All attributes of a new service are new so only the green color marks them, without a modifier.
+			if isNewService {
+				mod = " "
+			}
+			diff = tui.Green.Render(key+tui.Faint.Render("=")) + tui.Green.Render(formatLabelValue(newValue))
+		case !inNew:
+			mod = tui.Red.Render("-")
+			diff = tui.Red.Render(key+tui.Faint.Render("=")) + tui.Red.Render(formatLabelValue(oldValue))
+		case oldValue != newValue:
+			mod = tui.Yellow.Render("~")
+			diff = key +
+				tui.Faint.Render("=") +
+				tui.Red.Render(formatLabelValue(oldValue)) + " " +
+				tui.Faint.Render("→") + " " +
+				tui.Green.Render(formatLabelValue(newValue))
+		default:
+			continue
+		}
+
+		lines.WriteString("    " + mod + " " + diff + "\n")
+	}
+	if lines.Len() == 0 {
+		return ""
+	}
+
+	blockMod := tui.Yellow.Render("~")
+	switch {
+	case isNewService:
+		blockMod = " "
+	case len(newLabels) == 0:
+		blockMod = tui.Red.Render("-")
+	}
+
+	return blockMod + " " + tui.Faint.Render(attribute) + "\n" + lines.String()
+}
+
+// formatLabelValue returns the label value on a single line. It quotes and escapes empty values and values with
+// non-printable characters such as newlines so that they're visible in the diff. Other values are returned as is.
+func formatLabelValue(value string) string {
+	if value == "" || strings.ContainsFunc(value, func(r rune) bool { return !unicode.IsPrint(r) }) {
+		return strconv.Quote(value)
+	}
+	return value
+}
+
 // NewDeployment creates a new deployment for the given service specification.
 // If strategy is nil, a default RollingStrategy will be used.
 func NewDeployment(cli Client, spec api.ServiceSpec, strategy Strategy) *Deployment {

pkg/client/deploy/deploy_test.go

@@ -1,9 +1,11 @@
 package deploy
 
 import (
+	"strings"
 	"testing"
 
 	"github.com/charmbracelet/x/ansi"
+	"github.com/docker/docker/api/types/container"
 	"github.com/psviderski/uncloud/pkg/api"
 	"github.com/psviderski/uncloud/pkg/client/operation"
 	"github.com/stretchr/testify/assert"
@@ -59,3 +61,225 @@ func TestServicePlan_FormatSummary(t *testing.T) {
 		})
 	}
 }
+
+func TestServicePlan_Format(t *testing.T) {
+	t.Parallel()
+
+	newContainer := func(spec api.ServiceSpec) api.ServiceContainer {
+		return api.ServiceContainer{
+			Container: api.Container{InspectResponse: container.InspectResponse{
+				ContainerJSONBase: &container.ContainerJSONBase{ID: "0f6df6593e1b0123456789abcdef"},
+			}},
+			ServiceSpec: spec,
+		}
+	}
+	spec := func(image string, labels, containerLabels map[string]string) api.ServiceSpec {
+		return api.ServiceSpec{
+			Name:      "web",
+			Mode:      api.ServiceModeReplicated,
+			Replicas:  1,
+			Labels:    labels,
+			Container: api.ContainerSpec{Image: image, Labels: containerLabels},
+		}
+	}
+
+	tests := []struct {
+		name         string
+		isNewService bool
+		spec         api.ServiceSpec
+		ops          func(spec api.ServiceSpec) []operation.Operation
+		expected     []string
+	}{
+		{
+			name: "service labels added, changed, and removed in place",
+			spec: spec("nginx:1.25", map[string]string{"env": "prod", "team": "platform", "same": "1"}, nil),
+			ops: func(api.ServiceSpec) []operation.Operation {
+				return []operation.Operation{&operation.UpdateContainerOperation{
+					MachineName: "machine-1",
+					Container: newContainer(spec(
+						"nginx:1.25", map[string]string{"old": "legacy", "team": "backend", "same": "1"}, nil,
+					)),
+				}}
+			},
+			expected: []string{
+				"~ update service web",
+				"  │   image: nginx:1.25",
+				"  │ ~ labels:",
+				"  │     + env=prod",
+				"  │     - old=legacy",
+				"  │     ~ team=backend → platform",
+				"  │",
+				"  ╰── ~   update container web/0f6df6593e1b on machine-1 (in place)",
+			},
+		},
+		{
+			name: "container labels changed with image on replace",
+			spec: spec("nginx:1.26", nil, map[string]string{"proxy": "on"}),
+			ops: func(new api.ServiceSpec) []operation.Operation {
+				return []operation.Operation{&operation.ReplaceContainerOperation{
+					Spec:         new,
+					MachineName:  "machine-1",
+					OldContainer: newContainer(spec("nginx:1.25", nil, map[string]string{"proxy": "off"})),
+				}}
+			},
+			expected: []string{
+				"~ update service web",
+				"  │ ~ image: nginx:1.25 → 1.26",
+				"  │ ~ container labels:",
+				"  │     ~ proxy=off → on",
+				"  │",
+				"  ╰── +/- replace container web/0f6df6593e1b on machine-1",
+			},
+		},
+		{
+			name: "no label blocks when labels are the same",
+			spec: spec("nginx:1.26", map[string]string{}, map[string]string{"proxy": "on"}),
+			ops: func(new api.ServiceSpec) []operation.Operation {
+				return []operation.Operation{&operation.ReplaceContainerOperation{
+					Spec:         new,
+					MachineName:  "machine-1",
+					OldContainer: newContainer(spec("nginx:1.25", nil, map[string]string{"proxy": "on"})),
+				}}
+			},
+			expected: []string{
+				"~ update service web",
+				"  │ ~ image: nginx:1.25 → 1.26",
+				"  │",
+				"  ╰── +/- replace container web/0f6df6593e1b on machine-1",
+			},
+		},
+		{
+			name: "replaced container determines old spec over updated one",
+			spec: spec("nginx:1.26", map[string]string{"team": "platform"}, nil),
+			ops: func(new api.ServiceSpec) []operation.Operation {
+				return []operation.Operation{
+					&operation.UpdateContainerOperation{
+						MachineName: "machine-1",
+						Container:   newContainer(spec("nginx:1.26", map[string]string{"team": "backend"}, nil)),
+					},
+					&operation.ReplaceContainerOperation{
+						Spec:         new,
+						MachineName:  "machine-2",
+						OldContainer: newContainer(spec("nginx:1.25", map[string]string{"team": "backend"}, nil)),
+					},
+				}
+			},
+			expected: []string{
+				"~ update service web",
+				"  │ ~ image: nginx:1.25 → 1.26",
+				"  │ ~ labels:",
+				"  │     ~ team=backend → platform",
+				"  │",
+				"  ├── ~   update container web/0f6df6593e1b on machine-1 (in place)",
+				"  ╰── +/- replace container web/0f6df6593e1b on machine-2",
+			},
+		},
+		{
+			name: "all labels removed",
+			spec: spec("nginx:1.25", map[string]string{}, nil),
+			ops: func(api.ServiceSpec) []operation.Operation {
+				return []operation.Operation{&operation.UpdateContainerOperation{
+					MachineName: "machine-1",
+					Container:   newContainer(spec("nginx:1.25", map[string]string{"a": "1", "b": "2"}, nil)),
+				}}
+			},
+			expected: []string{
+				"~ update service web",
+				"  │   image: nginx:1.25",
+				"  │ - labels:",
+				"  │     - a=1",
+				"  │     - b=2",
+				"  │",
+				"  ╰── ~   update container web/0f6df6593e1b on machine-1 (in place)",
+			},
+		},
+		{
+			name: "first labels added to existing service",
+			spec: spec("nginx:1.25", map[string]string{"a": "1"}, nil),
+			ops: func(api.ServiceSpec) []operation.Operation {
+				return []operation.Operation{&operation.UpdateContainerOperation{
+					MachineName: "machine-1",
+					Container:   newContainer(spec("nginx:1.25", nil, nil)),
+				}}
+			},
+			expected: []string{
+				"~ update service web",
+				"  │   image: nginx:1.25",
+				"  │ ~ labels:",
+				"  │     + a=1",
+				"  │",
+				"  ╰── ~   update container web/0f6df6593e1b on machine-1 (in place)",
+			},
+		},
+		{
+			name: "empty and non-printable label values are escaped on a single line",
+			spec: spec("nginx:1.25", map[string]string{
+				"added":    "1\n2",
+				"cleared":  "",
+				"json":     `{"a":1}`,
+				"multi":    "a\nb\nc",
+				"tab":      "a\tb",
+				"trailing": "hello\n",
+				"unicode":  "héllo wörld 🚀",
+			}, nil),
+			ops: func(api.ServiceSpec) []operation.Operation {
+				return []operation.Operation{&operation.UpdateContainerOperation{
+					MachineName: "machine-1",
+					Container: newContainer(spec("nginx:1.25", map[string]string{
+						"cleared":  "x",
+						"multi":    "a\nb",
+						"trailing": "hello",
+					}, nil)),
+				}}
+			},
+			expected: []string{
+				"~ update service web",
+				"  │   image: nginx:1.25",
+				"  │ ~ labels:",
+				`  │     + added="1\n2"`,
+				`  │     ~ cleared=x → ""`,
+				`  │     + json={"a":1}`,
+				`  │     ~ multi="a\nb" → "a\nb\nc"`,
+				`  │     + tab="a\tb"`,
+				`  │     ~ trailing=hello → "hello\n"`,
+				"  │     + unicode=héllo wörld 🚀",
+				"  │",
+				"  ╰── ~   update container web/0f6df6593e1b on machine-1 (in place)",
+			},
+		},
+		{
+			name:         "new service with labels",
+			isNewService: true,
+			spec:         spec("nginx:1.25", map[string]string{"team": "platform"}, map[string]string{"proxy": "on"}),
+			ops: func(new api.ServiceSpec) []operation.Operation {
+				return []operation.Operation{&operation.RunContainerOperation{Spec: new, MachineName: "machine-1"}}
+			},
+			expected: []string{
+				"+ create service web",
+				"  │   image: nginx:1.25",
+				"  │   labels:",
+				"  │       team=platform",
+				"  │   container labels:",
+				"  │       proxy=on",
+				"  │",
+				"  ╰── +   run container web on machine-1",
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			plan := ServicePlan{ServiceName: "web", IsNewService: tt.isNewService, Spec: tt.spec}
+			plan.Operations = tt.ops(tt.spec)
+
+			// The table pads cells to the column width so ignore trailing whitespace.
+			var lines []string
+			for line := range strings.Lines(ansi.Strip(plan.Format())) {
+				lines = append(lines, strings.TrimRight(line, " \n"))
+			}
+			assert.Equal(t, tt.expected, lines)
+		})
+	}
+}