From 95fe3df6632fa3201b05e7558e49ba0a727c1897 Mon Sep 17 00:00:00 2001 From: Siva Date: Mon, 25 May 2026 20:50:07 +0530 Subject: [PATCH] feat: add Image and ResolvedImage fields to SwarmOpts --- server/internal/database/spec.go | 9 +- server/internal/database/spec_test.go | 32 ++++ .../orchestrator/swarm/orchestrator.go | 35 +++- .../swarm/resolve_instance_images_test.go | 154 ++++++++++++++++++ 4 files changed, 227 insertions(+), 3 deletions(-) create mode 100644 server/internal/orchestrator/swarm/resolve_instance_images_test.go diff --git a/server/internal/database/spec.go b/server/internal/database/spec.go index eb921b87..6d490ae1 100644 --- a/server/internal/database/spec.go +++ b/server/internal/database/spec.go @@ -30,7 +30,12 @@ type ExtraNetworkSpec struct { type SwarmOpts struct { ExtraVolumes []ExtraVolumesSpec `json:"extra_volumes,omitempty"` ExtraNetworks []ExtraNetworkSpec `json:"extra_networks,omitempty"` - ExtraLabels map[string]string `json:"extra_labels,omitempty"` // optional, used for custom labels on the swarm service + ExtraLabels map[string]string `json:"extra_labels,omitempty"` + // Image is a user-specified override. Never written by the CP. + Image string `json:"image,omitempty"` + // ResolvedImage is the CP-managed image tag. Written at instance creation, + // upgrade application, and lazy backfill. Never set simultaneously with Image. + ResolvedImage string `json:"resolved_image,omitempty"` } type OrchestratorOpts struct { Swarm *SwarmOpts `json:"docker,omitempty"` @@ -297,6 +302,8 @@ func (d *SwarmOpts) Clone() *SwarmOpts { ExtraVolumes: clonedVolumes, ExtraNetworks: clonedNetworks, ExtraLabels: maps.Clone(d.ExtraLabels), + Image: d.Image, + ResolvedImage: d.ResolvedImage, } } diff --git a/server/internal/database/spec_test.go b/server/internal/database/spec_test.go index 4b7a2692..7019924d 100644 --- a/server/internal/database/spec_test.go +++ b/server/internal/database/spec_test.go @@ -510,6 +510,38 @@ func TestSpec(t *testing.T) { }) } +func TestSwarmOptsClone(t *testing.T) { + t.Run("copies Image and ResolvedImage", func(t *testing.T) { + orig := &database.SwarmOpts{ + Image: "custom-registry/pgedge:dev", + ResolvedImage: "registry/pgedge:17.9-spock5.0.6-standard-1", + ExtraLabels: map[string]string{"k": "v"}, + } + cloned := orig.Clone() + + assert.Equal(t, orig.Image, cloned.Image) + assert.Equal(t, orig.ResolvedImage, cloned.ResolvedImage) + }) + + t.Run("clone is independent of original", func(t *testing.T) { + orig := &database.SwarmOpts{ + Image: "original-image", + ResolvedImage: "original-resolved", + } + cloned := orig.Clone() + cloned.Image = "mutated-image" + cloned.ResolvedImage = "mutated-resolved" + + assert.Equal(t, "original-image", orig.Image) + assert.Equal(t, "original-resolved", orig.ResolvedImage) + }) + + t.Run("nil clone returns nil", func(t *testing.T) { + var s *database.SwarmOpts + assert.Nil(t, s.Clone()) + }) +} + func TestSpec_NodeInstances_DBOwner(t *testing.T) { minimalSpec := func(users []*database.User) *database.Spec { return &database.Spec{ diff --git a/server/internal/orchestrator/swarm/orchestrator.go b/server/internal/orchestrator/swarm/orchestrator.go index 45386a74..6ea8e9f0 100644 --- a/server/internal/orchestrator/swarm/orchestrator.go +++ b/server/internal/orchestrator/swarm/orchestrator.go @@ -171,10 +171,41 @@ func ServiceInstanceName(databaseID, serviceID, hostID string) string { return fmt.Sprintf("%s-%s-%s", databaseID, serviceID, base36[:8]) } +// resolveInstanceImages returns an Images for the instance using the precedence: +// 1. SwarmOpts.Image (user override) — manifest lookup skipped entirely +// 2. SwarmOpts.ResolvedImage (CP-managed, already stored) +// 3. Manifest lookup — result written to SwarmOpts.ResolvedImage (lazy backfill) +func (o *Orchestrator) resolveInstanceImages(spec *database.InstanceSpec) (*Images, error) { + var swarmOpts *database.SwarmOpts + if spec.OrchestratorOpts != nil { + swarmOpts = spec.OrchestratorOpts.Swarm + } + + switch { + case swarmOpts != nil && swarmOpts.Image != "": + return &Images{PgEdgeImage: swarmOpts.Image}, nil + case swarmOpts != nil && swarmOpts.ResolvedImage != "": + return &Images{PgEdgeImage: swarmOpts.ResolvedImage}, nil + default: + manifested, err := o.versions.GetImages(spec.PgEdgeVersion) + if err != nil { + return nil, fmt.Errorf("failed to get images: %w", err) + } + if spec.OrchestratorOpts == nil { + spec.OrchestratorOpts = &database.OrchestratorOpts{} + } + if spec.OrchestratorOpts.Swarm == nil { + spec.OrchestratorOpts.Swarm = &database.SwarmOpts{} + } + spec.OrchestratorOpts.Swarm.ResolvedImage = manifested.PgEdgeImage + return &Images{PgEdgeImage: manifested.PgEdgeImage}, nil + } +} + func (o *Orchestrator) instanceResources(spec *database.InstanceSpec, scripts database.Scripts) (*database.InstanceResource, []resource.Resource, []resource.Resource, error) { - images, err := o.versions.GetImages(spec.PgEdgeVersion) + images, err := o.resolveInstanceImages(spec) if err != nil { - return nil, nil, nil, fmt.Errorf("failed to get images: %w", err) + return nil, nil, nil, err } instanceHostname := fmt.Sprintf("postgres-%s", spec.InstanceID) diff --git a/server/internal/orchestrator/swarm/resolve_instance_images_test.go b/server/internal/orchestrator/swarm/resolve_instance_images_test.go new file mode 100644 index 00000000..1eabe184 --- /dev/null +++ b/server/internal/orchestrator/swarm/resolve_instance_images_test.go @@ -0,0 +1,154 @@ +package swarm + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/pgEdge/control-plane/server/internal/config" + "github.com/pgEdge/control-plane/server/internal/database" + "github.com/pgEdge/control-plane/server/internal/ds" +) + +func TestResolveInstanceImages(t *testing.T) { + o := &Orchestrator{ + versions: NewVersions(config.Config{ + DockerSwarm: config.DockerSwarm{ + ImageRepositoryHost: "registry.example.com/pgedge", + }, + }), + } + + knownVersion := ds.MustParsePgEdgeVersion("17.9", "5") + unknownVersion := ds.MustParsePgEdgeVersion("99.99", "5") + + t.Run("Image override used directly, manifest not consulted", func(t *testing.T) { + spec := &database.InstanceSpec{ + PgEdgeVersion: knownVersion, + OrchestratorOpts: &database.OrchestratorOpts{ + Swarm: &database.SwarmOpts{ + Image: "my-registry/pgedge:dev-build", + }, + }, + } + + images, err := o.resolveInstanceImages(spec) + require.NoError(t, err) + assert.Equal(t, "my-registry/pgedge:dev-build", images.PgEdgeImage) + // ResolvedImage must not be written when Image is set + assert.Empty(t, spec.OrchestratorOpts.Swarm.ResolvedImage) + }) + + t.Run("Image override works for unknown version (bypasses manifest)", func(t *testing.T) { + spec := &database.InstanceSpec{ + PgEdgeVersion: unknownVersion, + OrchestratorOpts: &database.OrchestratorOpts{ + Swarm: &database.SwarmOpts{ + Image: "my-registry/pgedge:dev-build", + }, + }, + } + + images, err := o.resolveInstanceImages(spec) + require.NoError(t, err) + assert.Equal(t, "my-registry/pgedge:dev-build", images.PgEdgeImage) + }) + + t.Run("Image takes precedence over ResolvedImage", func(t *testing.T) { + spec := &database.InstanceSpec{ + PgEdgeVersion: knownVersion, + OrchestratorOpts: &database.OrchestratorOpts{ + Swarm: &database.SwarmOpts{ + Image: "custom-override:latest", + ResolvedImage: "previously-resolved:tag", + }, + }, + } + + images, err := o.resolveInstanceImages(spec) + require.NoError(t, err) + assert.Equal(t, "custom-override:latest", images.PgEdgeImage) + // ResolvedImage must not be touched when Image wins + assert.Equal(t, "previously-resolved:tag", spec.OrchestratorOpts.Swarm.ResolvedImage) + }) + + t.Run("ResolvedImage used when Image is empty", func(t *testing.T) { + spec := &database.InstanceSpec{ + PgEdgeVersion: knownVersion, + OrchestratorOpts: &database.OrchestratorOpts{ + Swarm: &database.SwarmOpts{ + ResolvedImage: "registry.example.com/pgedge:previously-pinned", + }, + }, + } + + images, err := o.resolveInstanceImages(spec) + require.NoError(t, err) + assert.Equal(t, "registry.example.com/pgedge:previously-pinned", images.PgEdgeImage) + // ResolvedImage must not be modified + assert.Equal(t, "registry.example.com/pgedge:previously-pinned", spec.OrchestratorOpts.Swarm.ResolvedImage) + }) + + t.Run("lazy backfill: resolves from manifest and writes ResolvedImage", func(t *testing.T) { + spec := &database.InstanceSpec{ + PgEdgeVersion: knownVersion, + } + + images, err := o.resolveInstanceImages(spec) + require.NoError(t, err) + assert.NotEmpty(t, images.PgEdgeImage) + require.NotNil(t, spec.OrchestratorOpts) + require.NotNil(t, spec.OrchestratorOpts.Swarm) + assert.Equal(t, images.PgEdgeImage, spec.OrchestratorOpts.Swarm.ResolvedImage) + }) + + t.Run("lazy backfill: initialises nil OrchestratorOpts", func(t *testing.T) { + spec := &database.InstanceSpec{ + PgEdgeVersion: knownVersion, + OrchestratorOpts: nil, + } + + _, err := o.resolveInstanceImages(spec) + require.NoError(t, err) + require.NotNil(t, spec.OrchestratorOpts) + require.NotNil(t, spec.OrchestratorOpts.Swarm) + assert.NotEmpty(t, spec.OrchestratorOpts.Swarm.ResolvedImage) + }) + + t.Run("lazy backfill: initialises nil Swarm inside existing OrchestratorOpts", func(t *testing.T) { + spec := &database.InstanceSpec{ + PgEdgeVersion: knownVersion, + OrchestratorOpts: &database.OrchestratorOpts{Swarm: nil}, + } + + _, err := o.resolveInstanceImages(spec) + require.NoError(t, err) + require.NotNil(t, spec.OrchestratorOpts.Swarm) + assert.NotEmpty(t, spec.OrchestratorOpts.Swarm.ResolvedImage) + }) + + t.Run("lazy backfill: unknown version returns error", func(t *testing.T) { + spec := &database.InstanceSpec{ + PgEdgeVersion: unknownVersion, + } + + _, err := o.resolveInstanceImages(spec) + assert.Error(t, err) + }) + + t.Run("manifest cache not mutated by lazy backfill", func(t *testing.T) { + manifestImage, err := o.versions.GetImages(knownVersion) + require.NoError(t, err) + original := manifestImage.PgEdgeImage + + spec := &database.InstanceSpec{PgEdgeVersion: knownVersion} + _, err = o.resolveInstanceImages(spec) + require.NoError(t, err) + + // Calling GetImages again must return the same value — cache untouched + after, err := o.versions.GetImages(knownVersion) + require.NoError(t, err) + assert.Equal(t, original, after.PgEdgeImage) + }) +}