From 3f95583dd4f95b299d029ec63959102bb9527d1f Mon Sep 17 00:00:00 2001 From: Pasha Sviderski Date: Thu, 19 Mar 2026 15:29:21 +1000 Subject: [PATCH] fix: plan formatting for service scaling (update verb), fix image and replicas diff --- cmd/uncloud/caddy/deploy.go | 13 ++++--------- internal/cli/tui/style.go | 14 ++++++++++---- pkg/client/deploy/deploy.go | 36 +++++++++++++++++++---------------- pkg/client/deploy/strategy.go | 1 + 4 files changed, 35 insertions(+), 29 deletions(-) diff --git a/cmd/uncloud/caddy/deploy.go b/cmd/uncloud/caddy/deploy.go index 7d66338e..c0e1d77c 100644 --- a/cmd/uncloud/caddy/deploy.go +++ b/cmd/uncloud/caddy/deploy.go @@ -10,7 +10,6 @@ import ( "strings" "charm.land/lipgloss/v2" - "github.com/distribution/reference" "github.com/docker/cli/cli/streams" "github.com/docker/compose/v2/pkg/progress" "github.com/psviderski/uncloud/internal/cli" @@ -89,20 +88,17 @@ func runDeploy(ctx context.Context, uncli *cli.CLI, opts deployOptions) error { if len(currentImages) > 1 { formattedImages := make([]string, len(currentImages)) for i, img := range currentImages { - ref, _ := reference.ParseDockerRef(img) - formattedImages[i] = tui.FormatImage(ref, lipgloss.NewStyle()) + formattedImages[i] = tui.FormatImage(img, lipgloss.NewStyle()) } fmt.Println(tui.Faint.Render("current images (multiple versions detected): ") + strings.Join(formattedImages, tui.Faint.Render(", "))) } else { - ref, _ := reference.ParseDockerRef(currentImages[0]) - fmt.Println(tui.Faint.Render("current image: ") + tui.FormatImage(ref, lipgloss.NewStyle())) + fmt.Println(tui.Faint.Render("current image: ") + tui.FormatImage(currentImages[0], lipgloss.NewStyle())) } } if opts.image != "" { - ref, _ := reference.ParseDockerRef(opts.image) - fmt.Println(tui.Faint.Render("target image: ") + tui.FormatImage(ref, tui.Green)) + fmt.Println(tui.Faint.Render("target image: ") + tui.FormatImage(opts.image, tui.Green)) } fmt.Println() @@ -117,8 +113,7 @@ func runDeploy(ctx context.Context, uncli *cli.CLI, opts deployOptions) error { } if opts.image == "" { - ref, _ := reference.ParseDockerRef(d.Spec.Container.Image) - fmt.Println(tui.Faint.Render("target image: ") + tui.FormatImage(ref, + fmt.Println(tui.Faint.Render("target image: ") + tui.FormatImage(d.Spec.Container.Image, tui.Green) + tui.Faint.Render(" (latest stable)")) } diff --git a/internal/cli/tui/style.go b/internal/cli/tui/style.go index 75bc1316..f5bed0db 100644 --- a/internal/cli/tui/style.go +++ b/internal/cli/tui/style.go @@ -20,11 +20,17 @@ var ( ) // FormatImage renders an image reference with the given style, using a faint colon separator for tagged images. -func FormatImage(image reference.Named, style lipgloss.Style) string { - if tagged, ok := image.(reference.NamedTagged); ok { - return style.Render(reference.FamiliarName(image)) + +// Returns the styled original string if parsing fails. +func FormatImage(image string, style lipgloss.Style) string { + ref, err := reference.ParseDockerRef(image) + if err != nil { + return style.Render(image) + } + + if tagged, ok := ref.(reference.NamedTagged); ok { + return style.Render(reference.FamiliarName(ref)) + Faint.Render(":") + style.Render(tagged.Tag()) } - return style.Render(reference.FamiliarString(image)) + return style.Render(reference.FamiliarString(ref)) } diff --git a/pkg/client/deploy/deploy.go b/pkg/client/deploy/deploy.go index e81c7e58..b8779b07 100644 --- a/pkg/client/deploy/deploy.go +++ b/pkg/client/deploy/deploy.go @@ -40,6 +40,8 @@ type Deployment struct { type ServicePlan struct { ServiceID string ServiceName string + // IsNewService indicates this plan creates a new service (first deployment) rather than updating an existing one. + IsNewService bool // Spec is the desired service spec being deployed. Spec api.ServiceSpec operation.SequenceOperation @@ -50,14 +52,13 @@ 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. - var hasRun, hasReplace, hasRemove bool + var hasRun, hasRemove bool var oldSpec *api.ServiceSpec for _, op := range sp.Operations { switch o := op.(type) { case *operation.RunContainerOperation: hasRun = true case *operation.ReplaceContainerOperation: - hasReplace = true if oldSpec == nil { oldSpec = &o.OldContainer.ServiceSpec } @@ -72,7 +73,7 @@ func (sp *ServicePlan) Format() string { // Service line modifier and verb. var modifier, verb string switch { - case hasRun && !hasReplace && !hasRemove: + case sp.IsNewService: modifier = tui.BoldGreen.Render("+") verb = "create" // TODO: when service removal via a deployment is supported, handle "remove" verb here as well. @@ -110,7 +111,11 @@ func (sp *ServicePlan) Format() string { // Image row. if oldSpec == nil { - specTable.Row("", "image:", formatImageDiff("", sp.Spec.Container.Image)) + if sp.IsNewService { + specTable.Row("", "image:", tui.FormatImage(sp.Spec.Container.Image, tui.Green)) + } else { + specTable.Row("", "image:", tui.FormatImage(sp.Spec.Container.Image, lipgloss.NewStyle())) + } } else { mod := "" if oldSpec.Container.Image != sp.Spec.Container.Image { @@ -122,8 +127,10 @@ func (sp *ServicePlan) Format() string { // Replicas row for replicated services. if sp.Spec.Mode == api.ServiceModeReplicated { replicasStr := fmt.Sprintf("%d", sp.Spec.Replicas) - if oldSpec == nil { - specTable.Row("", "replicas:", tui.Green.Render(replicasStr)) + if sp.IsNewService { + if sp.Spec.Replicas > 1 { + specTable.Row("", "replicas:", tui.Green.Render(replicasStr)) + } } else if sp.Spec.Replicas > 1 || hasRun || hasRemove { mod := "" if hasRun || hasRemove { @@ -221,27 +228,24 @@ func (sp *ServicePlan) FormatSummary() string { // formatImageDiff formats the image for display. If oldImage is empty, it formats newImage as a new (green) value. // Otherwise, it renders the diff between oldImage and newImage. func formatImageDiff(oldImage, newImage string) string { - newRef, _ := reference.ParseDockerRef(newImage) // ignore error since the image was already validated - - // Create case: no old image. if oldImage == "" { - return tui.FormatImage(newRef, tui.Green) + return tui.FormatImage(newImage, tui.Green) } - // Update case: no change. if oldImage == newImage { - return tui.FormatImage(newRef, lipgloss.NewStyle()) + return tui.FormatImage(newImage, lipgloss.NewStyle()) } oldRef, _ := reference.ParseDockerRef(oldImage) + newRef, _ := reference.ParseDockerRef(newImage) // If either uses a digest, show full old → new. _, oldDigested := oldRef.(reference.Digested) _, newDigested := newRef.(reference.Digested) if oldDigested || newDigested { - return tui.FormatImage(oldRef, tui.Red) + " " + + return tui.FormatImage(oldImage, tui.Red) + " " + tui.Faint.Render("→") + " " + - tui.FormatImage(newRef, tui.Green) + tui.FormatImage(newImage, tui.Green) } // If repos match and both are tagged, show only tag diff. @@ -256,9 +260,9 @@ func formatImageDiff(oldImage, newImage string) string { } // Different repos: full old → new. - return tui.FormatImage(oldRef, tui.Red) + " " + + return tui.FormatImage(oldImage, tui.Red) + " " + tui.Faint.Render("→") + " " + - tui.FormatImage(newRef, tui.Green) + tui.FormatImage(newImage, tui.Green) } // NewDeployment creates a new deployment for the given service specification. diff --git a/pkg/client/deploy/strategy.go b/pkg/client/deploy/strategy.go index 57b5f91a..5bf8fb67 100644 --- a/pkg/client/deploy/strategy.go +++ b/pkg/client/deploy/strategy.go @@ -446,6 +446,7 @@ func newEmptyServicePlan(svc *api.Service, spec api.ServiceSpec) (ServicePlan, e plan.ServiceID = svc.ID plan.ServiceName = svc.Name } else { + plan.IsNewService = true var err error plan.ServiceID, err = secret.NewID() if err != nil {