diff --git a/cmd/uncloud/caddy/deploy.go b/cmd/uncloud/caddy/deploy.go index 0b9ce184..9ca40635 100644 --- a/cmd/uncloud/caddy/deploy.go +++ b/cmd/uncloud/caddy/deploy.go @@ -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) } diff --git a/cmd/uncloud/machine/add.go b/cmd/uncloud/machine/add.go index 351c81fc..4941269c 100644 --- a/cmd/uncloud/machine/add.go +++ b/cmd/uncloud/machine/add.go @@ -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) } diff --git a/cmd/uncloud/machine/init.go b/cmd/uncloud/machine/init.go index c3ae9f81..c6c19ac9 100644 --- a/cmd/uncloud/machine/init.go +++ b/cmd/uncloud/machine/init.go @@ -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) } diff --git a/cmd/uncloud/service/scale.go b/cmd/uncloud/service/scale.go index 0fcb512d..bd700ed3 100644 --- a/cmd/uncloud/service/scale.go +++ b/cmd/uncloud/service/scale.go @@ -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) diff --git a/pkg/client/caddy.go b/pkg/client/caddy.go index 07c66078..b9688998 100644 --- a/pkg/client/caddy.go +++ b/pkg/client/caddy.go @@ -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. diff --git a/pkg/client/compose/deploy.go b/pkg/client/compose/deploy.go index c73236b0..b4c95276 100644 --- a/pkg/client/compose/deploy.go +++ b/pkg/client/compose/deploy.go @@ -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) } diff --git a/pkg/client/deploy.go b/pkg/client/deploy.go index d593b1cf..ef30b745 100644 --- a/pkg/client/deploy.go +++ b/pkg/client/deploy.go @@ -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) } diff --git a/pkg/client/deploy/deploy.go b/pkg/client/deploy/deploy.go index c45cd1f5..1640f34a 100644 --- a/pkg/client/deploy/deploy.go +++ b/pkg/client/deploy/deploy.go @@ -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) } diff --git a/pkg/client/service.go b/pkg/client/service.go index 1603cece..eb7226e8 100644 --- a/pkg/client/service.go +++ b/pkg/client/service.go @@ -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) diff --git a/test/e2e/service_test.go b/test/e2e/service_test.go index e93b4ef3..875c7a80 100644 --- a/test/e2e/service_test.go +++ b/test/e2e/service_test.go @@ -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)