From 041d51b74f42e9aea6b1af104bcd2d755e6889a7 Mon Sep 17 00:00:00 2001 From: Pavel Sviderski Date: Wed, 19 Mar 2025 16:28:17 +1000 Subject: [PATCH] refactor: compose deployment with pluggable spec resolver, basic test --- cmd/uncloud/deploy.go | 7 ++- internal/cli/client/compose.go | 74 ++++++++++++++++++++++++++-- internal/cli/client/operation.go | 3 ++ internal/cli/client/resolver.go | 25 ++++++++-- internal/cli/client/service.go | 9 ++-- internal/cli/client/strategy.go | 1 + internal/compose/project.go | 2 + internal/compose/service.go | 3 -- test/e2e/compose_test.go | 71 ++++++++++++++++++++++++++ test/e2e/fixtures/basic-compose.yaml | 5 ++ 10 files changed, 185 insertions(+), 15 deletions(-) create mode 100644 test/e2e/compose_test.go create mode 100644 test/e2e/fixtures/basic-compose.yaml diff --git a/cmd/uncloud/deploy.go b/cmd/uncloud/deploy.go index 2591a4ed..58d72e07 100644 --- a/cmd/uncloud/deploy.go +++ b/cmd/uncloud/deploy.go @@ -67,7 +67,12 @@ func deploy(ctx context.Context, uncli *cli.CLI, opts deployOptions) error { } defer clusterClient.Close() - plan, err := client.PlanComposeDeployment(ctx, project, clusterClient) + composeDeploy, err := clusterClient.NewComposeDeployment(ctx, project) + if err != nil { + return fmt.Errorf("create compose deployment: %w", err) + } + + plan, err := composeDeploy.Plan(ctx) if err != nil { return fmt.Errorf("plan deployment: %w", err) } diff --git a/internal/cli/client/compose.go b/internal/cli/client/compose.go index 911241d5..6e1752d5 100644 --- a/internal/cli/client/compose.go +++ b/internal/cli/client/compose.go @@ -2,26 +2,58 @@ package client import ( "context" + "errors" "fmt" "github.com/compose-spec/compose-go/v2/graph" "github.com/compose-spec/compose-go/v2/types" + "uncloud/internal/api" "uncloud/internal/compose" ) -func PlanComposeDeployment(ctx context.Context, project *types.Project, cli *Client) (SequenceOperation, error) { +func (cli *Client) NewComposeDeployment(ctx context.Context, project *types.Project) (*ComposeDeployment, error) { + domain, err := cli.GetDomain(ctx) + if err != nil && !errors.Is(err, ErrNotFound) { + return nil, fmt.Errorf("get cluster domain: %w", err) + } + + resolver := &ServiceSpecResolver{ + // If the domain is not found (not reserved), an empty domain is used for the resolver. + ClusterDomain: domain, + // TODO: provide an image resolver. + } + + return &ComposeDeployment{ + Client: cli, + Project: project, + SpecResolver: resolver, + }, nil +} + +type ComposeDeployment struct { + Client *Client + Project *types.Project + SpecResolver *ServiceSpecResolver + plan *SequenceOperation +} + +func (d *ComposeDeployment) Plan(ctx context.Context) (SequenceOperation, error) { + if d.plan != nil { + return *d.plan, nil + } + plan := SequenceOperation{} - err := graph.InDependencyOrder(ctx, project, + err := graph.InDependencyOrder(ctx, d.Project, func(ctx context.Context, name string, service types.ServiceConfig) error { spec, err := compose.ServiceSpecFromCompose(name, service) if err != nil { return fmt.Errorf("convert compose service '%s' to service spec: %w", name, err) } - if spec, err = cli.PrepareDeploymentSpec(ctx, spec); err != nil { + if spec, err = d.Client.PrepareDeploymentSpec(ctx, spec); err != nil { return fmt.Errorf("prepare service '%s' spec ready for deployment: %w", name, err) } // TODO: properly handle dependency conditions in the service deployment plan as the first operation. - deploy, err := cli.NewDeployment(spec, nil) + deploy, err := d.Client.NewDeployment(spec, nil) if err != nil { return fmt.Errorf("create deployment for service '%s': %w", name, err) } @@ -38,6 +70,40 @@ func PlanComposeDeployment(ctx context.Context, project *types.Project, cli *Cli return nil }) + if err != nil { + d.plan = &plan + } return plan, err } + +// ServiceSpec returns the service specification for the given compose service that is ready for deployment. +func (d *ComposeDeployment) ServiceSpec(name string) (api.ServiceSpec, error) { + service, err := d.Project.GetService(name) + if err != nil { + return api.ServiceSpec{}, fmt.Errorf("get config for compose service '%s': %w", name, err) + } + + spec, err := compose.ServiceSpecFromCompose(name, service) + if err != nil { + return spec, fmt.Errorf("convert compose service '%s' to service spec: %w", name, err) + } + + // TODO: resolve the image to a digest and supported platforms using an image resolver that broadcasts requests + // to all machines in the cluster. + // TODO: configure placement filter based on the supported platforms of the image. + if err = d.SpecResolver.Resolve(&spec); err != nil { + return spec, fmt.Errorf("resolve service spec '%s': %w", name, err) + } + + return spec, nil +} + +func (d *ComposeDeployment) Run(ctx context.Context) error { + plan, err := d.Plan(ctx) + if err != nil { + return fmt.Errorf("create plan: %w", err) + } + + return plan.Execute(ctx, d.Client) +} diff --git a/internal/cli/client/operation.go b/internal/cli/client/operation.go index 5db7b1ac..5dc3023f 100644 --- a/internal/cli/client/operation.go +++ b/internal/cli/client/operation.go @@ -11,6 +11,9 @@ import ( // Operation represents a single atomic operation in a deployment process. // Operations can be composed to form complex deployment strategies. type Operation interface { + // Execute performs the operation using the provided client. + // TODO: Encapsulate the client in the operation as otherwise it gives an impression that different clients + // can be provided. But in reality, the operation is tightly coupled with the client that was used to create it. Execute(ctx context.Context, cli *Client) error // Format returns a human-readable representation of the operation. Format(resolver NameResolver) string diff --git a/internal/cli/client/resolver.go b/internal/cli/client/resolver.go index 67f6a4b0..eb1cec46 100644 --- a/internal/cli/client/resolver.go +++ b/internal/cli/client/resolver.go @@ -8,13 +8,14 @@ import ( "uncloud/internal/secret" ) +type ImageDigestResolver interface { + Resolve(image string) (string, error) +} + // ServiceSpecResolver transforms user-provided service specs into deployment-ready form. type ServiceSpecResolver struct { ClusterDomain string -} - -func NewServiceSpecResolver(clusterDomain string) *ServiceSpecResolver { - return &ServiceSpecResolver{ClusterDomain: clusterDomain} + ImageResolver ImageDigestResolver } // Resolve transforms a service spec into its fully resolved form ready for deployment. @@ -26,6 +27,7 @@ func (r *ServiceSpecResolver) Resolve(spec *api.ServiceSpec) error { steps := []func(*api.ServiceSpec) error{ r.applyDefaults, r.resolveServiceName, + r.resolveImageDigest, r.expandIngressPorts, } @@ -76,6 +78,21 @@ func (r *ServiceSpecResolver) resolveServiceName(spec *api.ServiceSpec) error { return nil } +func (r *ServiceSpecResolver) resolveImageDigest(spec *api.ServiceSpec) error { + if r.ImageResolver == nil { + // Skip digest resolution when no resolver is provided. + return nil + } + + imageDigest, err := r.ImageResolver.Resolve(spec.Container.Image) + if err != nil { + return fmt.Errorf("resolve image digest: %w", err) + } + spec.Container.Image = imageDigest + + return nil +} + // expandIngressPorts processes HTTP(S) ingress ports in a service spec by: // 1. Setting a default hostname (service-name.cluster-domain) for ports without a hostname. // 2. Duplicating a port with a cluster domain hostname for ports with external domains. diff --git a/internal/cli/client/service.go b/internal/cli/client/service.go index f1de86e7..422cdb96 100644 --- a/internal/cli/client/service.go +++ b/internal/cli/client/service.go @@ -19,11 +19,14 @@ import ( func (cli *Client) PrepareDeploymentSpec(ctx context.Context, spec api.ServiceSpec) (api.ServiceSpec, error) { domain, err := cli.GetDomain(ctx) if err != nil && !errors.Is(err, ErrNotFound) { - return spec, fmt.Errorf("get domain: %w", err) + return spec, fmt.Errorf("get cluster domain: %w", err) } - // If the domain is not found (not reserved), an empty domain is used for the resolver. - resolver := NewServiceSpecResolver(domain) + resolver := ServiceSpecResolver{ + // If the domain is not found (not reserved), an empty domain is used for the resolver. + ClusterDomain: domain, + // TODO: provide an image resolver. + } if err = resolver.Resolve(&spec); err != nil { return spec, err diff --git a/internal/cli/client/strategy.go b/internal/cli/client/strategy.go index 0c0dd0bb..4b30f574 100644 --- a/internal/cli/client/strategy.go +++ b/internal/cli/client/strategy.go @@ -311,6 +311,7 @@ func reconcileGlobalContainer( } break } + // TODO: handle ContainerNeedsUpdate when update of mutable fields on a container is supported. } if upToDate { return ops, nil diff --git a/internal/compose/project.go b/internal/compose/project.go index 042145cc..71ef0b87 100644 --- a/internal/compose/project.go +++ b/internal/compose/project.go @@ -1,5 +1,7 @@ package compose +// TODO: make compose, cli, and api packages public. + import ( "context" "fmt" diff --git a/internal/compose/service.go b/internal/compose/service.go index 0413ecd9..0c18541f 100644 --- a/internal/compose/service.go +++ b/internal/compose/service.go @@ -7,9 +7,6 @@ import ( ) func ServiceSpecFromCompose(name string, service types.ServiceConfig) (api.ServiceSpec, error) { - // TODO: resolve the image to a digest and supported platforms using an image resolver that broadcasts requests - // to all machines in the cluster. - // TODO: configure placement filter based on the supported platforms of the image. spec := api.ServiceSpec{ Container: api.ContainerSpec{ Command: service.Command, diff --git a/test/e2e/compose_test.go b/test/e2e/compose_test.go new file mode 100644 index 00000000..177cc9ab --- /dev/null +++ b/test/e2e/compose_test.go @@ -0,0 +1,71 @@ +package e2e + +import ( + "context" + "errors" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "testing" + "uncloud/internal/api" + "uncloud/internal/cli/client" + "uncloud/internal/compose" + "uncloud/internal/ucind" +) + +func TestComposeDeployment(t *testing.T) { + t.Parallel() + + clusterName := "ucind-test.compose" + ctx := context.Background() + c, _ := createTestCluster(t, clusterName, ucind.CreateClusterOptions{Machines: 3}, true) + + cli, err := c.Machines[0].Connect(ctx) + require.NoError(t, err) + + t.Run("basic", func(t *testing.T) { + t.Parallel() + + name := "basic" + t.Cleanup(func() { + err := cli.RemoveService(ctx, name) + if !errors.Is(err, client.ErrNotFound) { + require.NoError(t, err) + } + }) + + project, err := compose.LoadProject(ctx, []string{"fixtures/basic-compose.yaml"}) + require.NoError(t, err) + + deploy, err := cli.NewComposeDeployment(ctx, project) + require.NoError(t, err) + + plan, err := deploy.Plan(ctx) + require.NoError(t, err) + assert.Len(t, plan.Operations, 1, "Expected 1 service to deploy") + + err = deploy.Run(ctx) + require.NoError(t, err) + + svc, err := cli.InspectService(ctx, name) + require.NoError(t, err) + + expectedSpec := api.ServiceSpec{ + Name: name, + Mode: api.ServiceModeReplicated, + Container: api.ContainerSpec{ + // TODO: resolve image digest and substitute the image with the image@digest. + Image: "portainer/pause:3.9", + }, + Ports: []api.PortSpec{ + { + Hostname: "basic.example.com", + ContainerPort: 80, + Protocol: api.ProtocolHTTPS, + Mode: api.PortModeIngress, + }, + }, + Replicas: 1, + } + assertServiceMatchesSpec(t, svc, expectedSpec) + }) +} diff --git a/test/e2e/fixtures/basic-compose.yaml b/test/e2e/fixtures/basic-compose.yaml new file mode 100644 index 00000000..7dde8649 --- /dev/null +++ b/test/e2e/fixtures/basic-compose.yaml @@ -0,0 +1,5 @@ +services: + basic: + image: portainer/pause:3.9 + x-ports: + - basic.example.com:80/https