refactor: postpone spec resolver initialisation when creating Deployment

This commit is contained in:
Pavel Sviderski
2025-03-24 21:56:30 +10:00
parent 2d923380a3
commit 58b51b974c
10 changed files with 48 additions and 90 deletions
+1 -1
View File
@@ -96,7 +96,7 @@ func runDeploy(ctx context.Context, uncli *cli.CLI, opts deployOptions) error {
filter = machineFilter(machines)
}
d, err := clusterClient.NewCaddyDeployment(ctx, opts.image, filter)
d, err := clusterClient.NewCaddyDeployment(opts.image, filter)
if err != nil {
return fmt.Errorf("create caddy deployment: %w", err)
}
+1 -1
View File
@@ -136,7 +136,7 @@ func add(ctx context.Context, uncli *cli.CLI, remoteMachine cli.RemoteMachine, o
}
}
d, err := machineClient.NewCaddyDeployment(ctx, caddyImage, filter)
d, err := machineClient.NewCaddyDeployment(caddyImage, filter)
if err != nil {
return fmt.Errorf("create caddy deployment: %w", err)
}
+1 -1
View File
@@ -132,7 +132,7 @@ func initCluster(ctx context.Context, uncli *cli.CLI, remoteMachine *cli.RemoteM
}
if !opts.noCaddy {
d, err := client.NewCaddyDeployment(ctx, "", nil)
d, err := client.NewCaddyDeployment("", nil)
if err != nil {
return fmt.Errorf("create caddy deployment: %w", err)
}
+1 -6
View File
@@ -88,12 +88,7 @@ func scale(ctx context.Context, uncli *cli.CLI, opts scaleOptions) error {
}
spec.Replicas = opts.replicas
deployment, err := clusterClient.NewDeployment(ctx, spec, nil)
if err != nil {
return fmt.Errorf("create deployment: %w", err)
}
deployment := clusterClient.NewDeployment(spec, nil)
plan, err := deployment.Plan(ctx)
if err != nil {
return fmt.Errorf("plan deployment: %w", err)
+2 -5
View File
@@ -1,7 +1,6 @@
package client
import (
"context"
"fmt"
"github.com/Masterminds/semver"
"github.com/distribution/reference"
@@ -23,9 +22,7 @@ var caddyImageTagRegex = regexp.MustCompile(`^2\.\d+\.\d+$`)
// NewCaddyDeployment creates a new deployment for a Caddy reverse proxy service.
// The service is deployed in global mode to all machines in the cluster. If the image is not provided, the latest
// version of the official Caddy Docker image is used.
func (cli *Client) NewCaddyDeployment(
ctx context.Context, image string, filter deploy.MachineFilter,
) (*deploy.Deployment, error) {
func (cli *Client) NewCaddyDeployment(image string, filter deploy.MachineFilter) (*deploy.Deployment, error) {
latest, err := LatestCaddyImage()
if err != nil {
return nil, fmt.Errorf("look up latest Caddy image: %w", err)
@@ -59,7 +56,7 @@ func (cli *Client) NewCaddyDeployment(
},
}
return cli.NewDeployment(ctx, spec, &deploy.RollingStrategy{MachineFilter: filter})
return cli.NewDeployment(spec, &deploy.RollingStrategy{MachineFilter: filter}), nil
}
// LatestCaddyImage returns the latest image of the official Caddy Docker image on Docker Hub.
+1 -1
View File
@@ -55,7 +55,7 @@ func (d *Deployment) Plan(ctx context.Context) (deploy.SequenceOperation, error)
}
// TODO: properly handle depends_on conditions in the service deployment plan as the first operation.
deployment, err := deploy.NewDeployment(ctx, d.Client, spec, nil)
deployment := deploy.NewDeployment(d.Client, spec, nil)
if err != nil {
return fmt.Errorf("create deployment for service '%s': %w", name, err)
}
+2 -5
View File
@@ -1,15 +1,12 @@
package client
import (
"context"
"github.com/psviderski/uncloud/pkg/api"
"github.com/psviderski/uncloud/pkg/client/deploy"
)
// NewDeployment creates a new deployment for the given service specification.
// If strategy is nil, a default deploy.RollingStrategy will be used.
func (cli *Client) NewDeployment(
ctx context.Context, spec api.ServiceSpec, strategy deploy.Strategy,
) (*deploy.Deployment, error) {
return deploy.NewDeployment(ctx, cli, spec, strategy)
func (cli *Client) NewDeployment(spec api.ServiceSpec, strategy deploy.Strategy) *deploy.Deployment {
return deploy.NewDeployment(cli, spec, strategy)
}
+21 -23
View File
@@ -18,12 +18,11 @@ type Client interface {
// Deployment manages the process of creating or updating a service to match a desired state.
// It coordinates the validation, planning, and execution of deployment operations.
type Deployment struct {
Service *api.Service
Spec api.ServiceSpec
Strategy Strategy
cli Client
specResolver *ServiceSpecResolver
plan *Plan
Service *api.Service
Spec api.ServiceSpec
Strategy Strategy
cli Client
plan *Plan
}
type Plan struct {
@@ -40,27 +39,16 @@ var ErrNoMatchingMachines = errors.New("no machines match the filter")
// NewDeployment creates a new deployment for the given service specification.
// If strategy is nil, a default RollingStrategy will be used.
func NewDeployment(ctx context.Context, cli Client, spec api.ServiceSpec, strategy Strategy) (*Deployment, error) {
func NewDeployment(cli Client, spec api.ServiceSpec, strategy Strategy) *Deployment {
if strategy == nil {
strategy = &RollingStrategy{}
}
clusterDomain, err := cli.GetDomain(ctx)
if err != nil && !errors.Is(err, api.ErrNotFound) {
return nil, fmt.Errorf("get cluster domain: %w", err)
}
specResolver := &ServiceSpecResolver{
// If the domain is not found (not reserved), an empty domain is used for the resolver.
ClusterDomain: clusterDomain,
}
return &Deployment{
Spec: spec,
Strategy: strategy,
cli: cli,
specResolver: specResolver,
}, nil
Spec: spec,
Strategy: strategy,
cli: cli,
}
}
// Plan returns a plan of operations to reconcile the service to the desired state.
@@ -75,7 +63,17 @@ func (d *Deployment) Plan(ctx context.Context) (Plan, error) {
return Plan{}, fmt.Errorf("invalid deployment: %w", err)
}
resolvedSpec, err := d.specResolver.Resolve(d.Spec)
clusterDomain, err := d.cli.GetDomain(ctx)
if err != nil && !errors.Is(err, api.ErrNotFound) {
return Plan{}, fmt.Errorf("get cluster domain: %w", err)
}
specResolver := &ServiceSpecResolver{
// If the domain is not found (not reserved), an empty domain is used for the resolver.
ClusterDomain: clusterDomain,
}
resolvedSpec, err := specResolver.Resolve(d.Spec)
if err != nil {
return Plan{}, fmt.Errorf("resolve service spec: %w", err)
}
+1 -5
View File
@@ -42,11 +42,7 @@ func (cli *Client) RunService(
}
}
deployment, err := cli.NewDeployment(ctx, spec, &deploy.RollingStrategy{MachineFilter: filter})
if err != nil {
return resp, fmt.Errorf("create deployment: %w", err)
}
deployment := cli.NewDeployment(spec, &deploy.RollingStrategy{MachineFilter: filter})
plan, err := deployment.Plan(ctx)
if err != nil {
return resp, fmt.Errorf("plan deployment: %w", err)
+17 -42
View File
@@ -57,8 +57,7 @@ func TestDeployment(t *testing.T) {
Image: "portainer/pause:latest",
},
}
deployment, err := cli.NewDeployment(ctx, spec, nil)
require.NoError(t, err)
deployment := cli.NewDeployment(spec, nil)
err = deployment.Validate(ctx)
require.NoError(t, err)
@@ -102,8 +101,7 @@ func TestDeployment(t *testing.T) {
},
},
}
deployment, err = cli.NewDeployment(ctx, specWithPort, nil)
require.NoError(t, err)
deployment = cli.NewDeployment(specWithPort, nil)
plan, err = deployment.Plan(ctx)
require.NoError(t, err)
@@ -143,8 +141,7 @@ func TestDeployment(t *testing.T) {
},
},
}
deployment, err = cli.NewDeployment(ctx, specWithPortAndInit, nil)
require.NoError(t, err)
deployment = cli.NewDeployment(specWithPortAndInit, nil)
plan, err = deployment.Plan(ctx)
require.NoError(t, err)
@@ -167,8 +164,7 @@ func TestDeployment(t *testing.T) {
// Deploying the same spec should be a no-op.
initialContainers = containers
deployment, err = cli.NewDeployment(ctx, specWithPortAndInit, nil)
require.NoError(t, err)
deployment = cli.NewDeployment(specWithPortAndInit, nil)
plan, err = deployment.Plan(ctx)
require.NoError(t, err)
@@ -203,9 +199,7 @@ func TestDeployment(t *testing.T) {
Image: "portainer/pause:latest",
},
}
deployment, err := cli.NewDeployment(ctx, spec, nil)
require.NoError(t, err)
deployment := cli.NewDeployment(spec, nil)
_, err = deployment.Run(ctx)
require.NoError(t, err)
@@ -229,9 +223,7 @@ func TestDeployment(t *testing.T) {
return m.Name == c.Machines[0].Name || m.Name == c.Machines[2].Name
}
strategy := &deploy.RollingStrategy{MachineFilter: filter}
deployment, err = cli.NewDeployment(ctx, specWithInit, strategy)
require.NoError(t, err)
deployment = cli.NewDeployment(specWithInit, strategy)
_, err = deployment.Run(ctx)
require.NoError(t, err)
@@ -277,10 +269,7 @@ func TestDeployment(t *testing.T) {
Mode: api.PortModeHost,
},
}
deployment, err = cli.NewDeployment(ctx, specWithPort, nil)
require.NoError(t, err)
deployment = cli.NewDeployment(specWithPort, nil)
_, err = deployment.Run(ctx)
require.NoError(t, err)
@@ -310,7 +299,7 @@ func TestDeployment(t *testing.T) {
}
})
deployment, err := cli.NewCaddyDeployment(ctx, "", nil)
deployment, err := cli.NewCaddyDeployment("", nil)
require.NoError(t, err)
_, err = deployment.Run(ctx)
@@ -362,7 +351,7 @@ func TestDeployment(t *testing.T) {
return m.Name == c.Machines[0].Name
}
deployment, err := cli.NewCaddyDeployment(ctx, "", filter)
deployment, err := cli.NewCaddyDeployment("", filter)
require.NoError(t, err)
image := deployment.Spec.Container.Image
@@ -382,8 +371,7 @@ func TestDeployment(t *testing.T) {
filter = func(m *pb.MachineInfo) bool {
return m.Name == c.Machines[0].Name || m.Name == c.Machines[2].Name
}
deployment, err = cli.NewCaddyDeployment(ctx, image, filter)
deployment, err = cli.NewCaddyDeployment(image, filter)
require.NoError(t, err)
_, err = deployment.Run(ctx)
@@ -427,9 +415,7 @@ func TestDeployment(t *testing.T) {
},
Replicas: 2,
}
deployment, err := cli.NewDeployment(ctx, spec, nil)
require.NoError(t, err)
deployment := cli.NewDeployment(spec, nil)
err = deployment.Validate(ctx)
require.NoError(t, err)
@@ -458,9 +444,7 @@ func TestDeployment(t *testing.T) {
init := true
updatedSpec := spec
updatedSpec.Container.Init = &init
deployment, err = cli.NewDeployment(ctx, updatedSpec, nil)
require.NoError(t, err)
deployment = cli.NewDeployment(updatedSpec, nil)
plan, err = deployment.Plan(ctx)
require.NoError(t, err)
@@ -487,9 +471,7 @@ func TestDeployment(t *testing.T) {
threeReplicaSpec := updatedSpec
threeReplicaSpec.Replicas = 3
deployment, err = cli.NewDeployment(ctx, threeReplicaSpec, nil)
require.NoError(t, err)
deployment = cli.NewDeployment(threeReplicaSpec, nil)
plan, err = deployment.Plan(ctx)
require.NoError(t, err)
@@ -514,9 +496,7 @@ func TestDeployment(t *testing.T) {
fourReplicaSpec := updatedSpec
fourReplicaSpec.Container.Command = []string{"updated"}
fourReplicaSpec.Replicas = 5
deployment, err = cli.NewDeployment(ctx, fourReplicaSpec, nil)
require.NoError(t, err)
deployment = cli.NewDeployment(fourReplicaSpec, nil)
plan, err = deployment.Plan(ctx)
require.NoError(t, err)
@@ -544,8 +524,7 @@ func TestDeployment(t *testing.T) {
// 5. Redeploy the exact same spec and verify it's a noop.
initialContainers = containers // Reset container tracking.
deployment, err = cli.NewDeployment(ctx, fourReplicaSpec, nil)
require.NoError(t, err)
deployment = cli.NewDeployment(fourReplicaSpec, nil)
plan, err = deployment.Plan(ctx)
require.NoError(t, err)
@@ -586,9 +565,7 @@ func TestDeployment(t *testing.T) {
return m.Name == c.Machines[0].Name || m.Name == c.Machines[1].Name
}
strategy := &deploy.RollingStrategy{MachineFilter: machine01Filter}
deployment, err := cli.NewDeployment(ctx, spec, strategy)
require.NoError(t, err)
deployment := cli.NewDeployment(spec, strategy)
_, err = deployment.Run(ctx)
require.NoError(t, err)
@@ -615,10 +592,8 @@ func TestDeployment(t *testing.T) {
machine2Filter := func(m *pb.MachineInfo) bool {
return m.Name == c.Machines[2].Name
}
strategy = &deploy.RollingStrategy{MachineFilter: machine2Filter}
deployment, err = cli.NewDeployment(ctx, spec, strategy)
require.NoError(t, err)
deployment = cli.NewDeployment(spec, strategy)
_, err = deployment.Run(ctx)
require.NoError(t, err)