diff --git a/internal/command/generate.go b/internal/command/generate.go index 5ca6c00..d0697ab 100644 --- a/internal/command/generate.go +++ b/internal/command/generate.go @@ -166,6 +166,10 @@ arguments. return fmt.Errorf("failed to convert '%s' to structure: %w", workloadName, err) } + if err := loader.Validate(&out); err != nil { + return fmt.Errorf("validation errors in workload '%s': %w", workloadName, err) + } + // Gather container build contexts, these will be stored and added to the generated compose output later containerBuildContexts := make(map[string]types.BuildConfig) if v, _ := cmd.Flags().GetStringArray(generateCmdBuildFlag); len(v) > 0 { @@ -537,3 +541,4 @@ func injectWaitService(p *types.Project) (string, bool) { p.Services[newService.Name] = newService return newService.Name, true } + diff --git a/internal/command/generate_test.go b/internal/command/generate_test.go index 9f8aef2..fee3231 100644 --- a/internal/command/generate_test.go +++ b/internal/command/generate_test.go @@ -453,6 +453,76 @@ resources: }) } +func TestInitAndGenerate_with_before_ordering(t *testing.T) { + td := changeToTempDir(t) + stdout, _, err := executeAndResetCommand(context.Background(), rootCmd, []string{"init"}) + assert.NoError(t, err) + assert.Equal(t, "", stdout) + + assert.NoError(t, os.WriteFile("score.yaml", []byte(` +apiVersion: score.dev/v1b1 +metadata: + name: example +containers: + init: + image: busybox + before: + main: + ready: complete + main: + image: nginx +`), 0644)) + // generate + stdout, _, err = executeAndResetCommand(context.Background(), rootCmd, []string{"generate", "score.yaml"}) + assert.NoError(t, err) + assert.Equal(t, "", stdout) + raw, err := os.ReadFile(filepath.Join(td, "compose.yaml")) + assert.NoError(t, err) + assert.Contains(t, string(raw), "depends_on") + assert.Contains(t, string(raw), "service_completed_successfully") + + t.Run("validate compose spec", func(t *testing.T) { + if os.Getenv("NO_DOCKER") != "" { + t.Skip("NO_DOCKER is set") + return + } + dockerCmd, err := exec.LookPath("docker") + require.NoError(t, err) + cmd := exec.Command(dockerCmd, "compose", "-f", "compose.yaml", "config", "--quiet", "--dry-run") + cmd.Dir = td + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + assert.NoError(t, cmd.Run()) + }) +} + +func TestInitAndGenerate_with_before_cycle(t *testing.T) { + _ = changeToTempDir(t) + stdout, _, err := executeAndResetCommand(context.Background(), rootCmd, []string{"init"}) + assert.NoError(t, err) + assert.Equal(t, "", stdout) + + assert.NoError(t, os.WriteFile("score.yaml", []byte(` +apiVersion: score.dev/v1b1 +metadata: + name: example +containers: + app: + image: busybox + before: + backend: + ready: started + backend: + image: busybox + before: + app: + ready: started +`), 0644)) + _, _, err = executeAndResetCommand(context.Background(), rootCmd, []string{"generate", "score.yaml"}) + assert.Error(t, err) + assert.Contains(t, err.Error(), "cycle") +} + func TestGeneratePostgresResource(t *testing.T) { td := changeToTempDir(t) stdout, _, err := executeAndResetCommand(context.Background(), rootCmd, []string{"init"}) @@ -844,12 +914,12 @@ apiVersion: score.dev/v1b1 metadata: name: example annotations: - key.com/foo-bar: thing + foo-bar: thing containers: example: image: foo variables: - REF: ${metadata.annotations.key\.com/foo-bar} + REF: ${metadata.annotations.foo-bar} `), 0644)) // generate stdout, _, err = executeAndResetCommand(context.Background(), rootCmd, []string{"generate", "score.yaml"}) @@ -862,7 +932,7 @@ services: example-example: annotations: compose.score.dev/workload-name: example - key.com/foo-bar: thing + foo-bar: thing environment: REF: thing hostname: example diff --git a/internal/compose/convert.go b/internal/compose/convert.go index 2057acb..420d250 100644 --- a/internal/compose/convert.go +++ b/internal/compose/convert.go @@ -66,13 +66,25 @@ func ConvertSpec(state *project.State, spec *score.Workload) (*compose.Project, // When multiple containers are specified we need to identify one container as the "main" container which will own // the network and use the native workload name. All other containers in this workload will have the container - // name appended as a suffix. We use the natural sort order of the container names and pick the first one + // name appended as a suffix. We use the natural sort order of the container names and pick the first one that + // is not expected to exit (i.e. does not have all before entries with ready: complete). containerNames := make([]string, 0, len(spec.Containers)) for name := range spec.Containers { containerNames = append(containerNames, name) } sort.Strings(containerNames) + // Determine which container should own the network namespace. + // By definition (enforced by validation), there must be at least one container with no + // 'before' entries — otherwise the before relationships would form a cycle. + primaryContainer := containerNames[0] + for _, name := range containerNames { + if len(spec.Containers[name].Before) == 0 { + primaryContainer = name + break + } + } + variablesSubstitutor := framework.Substituter{ Replacer: deferredSubstitutionFunction, UnEscaper: func(s string) (string, error) { @@ -80,7 +92,6 @@ func ConvertSpec(state *project.State, spec *score.Workload) (*compose.Project, }, } - var firstService string for _, containerName := range containerNames { cSpec := spec.Containers[containerName] @@ -171,22 +182,76 @@ func ConvertSpec(state *project.State, spec *score.Workload) (*compose.Project, svc.Image = "" } - // if we are not the "first" service, then inherit the network from the first service - if firstService == "" { - firstService = svc.Name + // if we are not the primary container, then inherit the network from the primary service. + // However, skip network_mode for containers that are expected to exit (all their before + // entries have ready: complete) to avoid circular dependencies. + if containerName == primaryContainer { // We name the containers as (workload name)-(container name) but we want the name for the main network // interface for be (workload name). So we set the hostname itself. This means that workloads cannot have // the same name within the project. But that's already enforced elsewhere. svc.Hostname = workloadName - } else { + } else if !isInitContainer(spec.Containers[containerName]) { svc.Ports = nil - svc.NetworkMode = "service:" + firstService + svc.NetworkMode = "service:" + workloadName + "-" + primaryContainer } composeProject.Services[svc.Name] = svc } + + // Invert before -> depends_on: if container A declares before: {B: {ready: complete}}, + // then service B depends_on A with the appropriate condition. + for _, containerName := range containerNames { + cSpec := spec.Containers[containerName] + for targetContainerName, entry := range cSpec.Before { + // Determine the compose condition from the ready field + var condition string + switch entry.Ready { + case score.ContainerBeforeReadyComplete: + condition = "service_completed_successfully" + case score.ContainerBeforeReadyHealthy: + condition = "service_healthy" + case score.ContainerBeforeReadyStarted: + condition = "service_started" + default: + return nil, fmt.Errorf("containers.%s.before.%s: unknown ready condition %q", containerName, targetContainerName, entry.Ready) + } + + if entry.Ready == score.ContainerBeforeReadyHealthy && cSpec.ReadinessProbe == nil && cSpec.LivenessProbe == nil { + return nil, fmt.Errorf("containers.%s.before: ready '%s' requires a readiness or liveness probe to be defined", containerName, score.ContainerBeforeReadyHealthy) + } + + sourceServiceName := workloadName + "-" + containerName + targetServiceName := workloadName + "-" + targetContainerName + + // Add depends_on to the target service + svc := composeProject.Services[targetServiceName] + if svc.DependsOn == nil { + svc.DependsOn = make(compose.DependsOnConfig) + } + svc.DependsOn[sourceServiceName] = compose.ServiceDependency{ + Condition: condition, + Required: true, + } + composeProject.Services[targetServiceName] = svc + } + } + return &composeProject, nil } +// isInitContainer returns true if the container is expected to exit. This is determined +// by checking if all its before entries specify ready: complete. +func isInitContainer(c score.Container) bool { + if len(c.Before) == 0 { + return false + } + for _, entry := range c.Before { + if entry.Ready != score.ContainerBeforeReadyComplete { + return false + } + } + return true +} + // buildWorkloadAnnotations returns an annotation set for the workload service. func buildWorkloadAnnotations(name string, spec *score.Workload) map[string]string { var out map[string]string diff --git a/internal/compose/convert_test.go b/internal/compose/convert_test.go index e68347c..9bd54bb 100644 --- a/internal/compose/convert_test.go +++ b/internal/compose/convert_test.go @@ -468,6 +468,205 @@ func TestScoreConvert(t *testing.T) { }, Error: errors.New("containers.test.volumes[/mnt/data]: resource 'thing.default#test.data' has no 'type' output"), }, + + // Before -> depends_on tests + // + { + Name: "Should convert before with ready complete to depends_on", + Source: &score.Workload{ + Metadata: score.WorkloadMetadata{ + "name": "test", + }, + Containers: score.WorkloadContainers{ + "init": score.Container{ + Image: "busybox", + Before: score.ContainerBefore{ + "main": score.ContainerBeforeEntry{ + Ready: score.ContainerBeforeReadyComplete, + }, + }, + }, + "main": score.Container{ + Image: "nginx", + }, + }, + }, + Project: &compose.Project{ + Services: compose.Services{ + "test-init": { + Name: "test-init", + Annotations: map[string]string{ + "compose.score.dev/workload-name": "test", + }, + Image: "busybox", + Environment: compose.MappingWithEquals{}, + }, + "test-main": { + Name: "test-main", + Annotations: map[string]string{ + "compose.score.dev/workload-name": "test", + }, + Hostname: "test", + Image: "nginx", + Environment: compose.MappingWithEquals{}, + DependsOn: compose.DependsOnConfig{ + "test-init": {Condition: "service_completed_successfully", Required: true}, + }, + }, + }, + }, + }, + { + Name: "Should convert chained before entries", + Source: &score.Workload{ + Metadata: score.WorkloadMetadata{ + "name": "test", + }, + Containers: score.WorkloadContainers{ + "init-one": score.Container{ + Image: "busybox", + Before: score.ContainerBefore{ + "init-two": score.ContainerBeforeEntry{ + Ready: score.ContainerBeforeReadyStarted, + }, + }, + }, + "init-two": score.Container{ + Image: "busybox", + Before: score.ContainerBefore{ + "main": score.ContainerBeforeEntry{ + Ready: score.ContainerBeforeReadyStarted, + }, + }, + }, + "main": score.Container{ + Image: "nginx", + }, + }, + }, + Project: &compose.Project{ + Services: compose.Services{ + "test-init-one": { + Name: "test-init-one", + Annotations: map[string]string{ + "compose.score.dev/workload-name": "test", + }, + Image: "busybox", + NetworkMode: "service:test-main", + Environment: compose.MappingWithEquals{}, + }, + "test-init-two": { + Name: "test-init-two", + Annotations: map[string]string{ + "compose.score.dev/workload-name": "test", + }, + Image: "busybox", + NetworkMode: "service:test-main", + Environment: compose.MappingWithEquals{}, + DependsOn: compose.DependsOnConfig{ + "test-init-one": {Condition: "service_started", Required: true}, + }, + }, + "test-main": { + Name: "test-main", + Annotations: map[string]string{ + "compose.score.dev/workload-name": "test", + }, + Hostname: "test", + Image: "nginx", + Environment: compose.MappingWithEquals{}, + DependsOn: compose.DependsOnConfig{ + "test-init-two": {Condition: "service_started", Required: true}, + }, + }, + }, + }, + }, + { + Name: "Should convert before with ready started and complete mixed", + Source: &score.Workload{ + Metadata: score.WorkloadMetadata{ + "name": "test", + }, + Containers: score.WorkloadContainers{ + "init": score.Container{ + Image: "busybox", + Before: score.ContainerBefore{ + "main": score.ContainerBeforeEntry{ + Ready: score.ContainerBeforeReadyComplete, + }, + }, + }, + "main": score.Container{ + Image: "nginx", + }, + "sidecar": score.Container{ + Image: "envoy", + Before: score.ContainerBefore{ + "main": score.ContainerBeforeEntry{ + Ready: score.ContainerBeforeReadyStarted, + }, + }, + }, + }, + }, + Project: &compose.Project{ + Services: compose.Services{ + "test-init": { + Name: "test-init", + Annotations: map[string]string{ + "compose.score.dev/workload-name": "test", + }, + Image: "busybox", + Environment: compose.MappingWithEquals{}, + }, + "test-main": { + Name: "test-main", + Annotations: map[string]string{ + "compose.score.dev/workload-name": "test", + }, + Hostname: "test", + Image: "nginx", + Environment: compose.MappingWithEquals{}, + DependsOn: compose.DependsOnConfig{ + "test-init": {Condition: "service_completed_successfully", Required: true}, + "test-sidecar": {Condition: "service_started", Required: true}, + }, + }, + "test-sidecar": { + Name: "test-sidecar", + Annotations: map[string]string{ + "compose.score.dev/workload-name": "test", + }, + Image: "envoy", + NetworkMode: "service:test-main", + Environment: compose.MappingWithEquals{}, + }, + }, + }, + }, + { + Name: "Should fail when ready healthy is used without a healthcheck probe", + Source: &score.Workload{ + Metadata: score.WorkloadMetadata{ + "name": "test", + }, + Containers: score.WorkloadContainers{ + "init": score.Container{ + Image: "busybox", + Before: score.ContainerBefore{ + "main": score.ContainerBeforeEntry{ + Ready: score.ContainerBeforeReadyHealthy, + }, + }, + }, + "main": score.Container{ + Image: "nginx", + }, + }, + }, + Error: errors.New("containers.init.before: ready 'healthy' requires a readiness or liveness probe to be defined"), + }, } for _, tt := range tests {