Skip to content

Commit ecb6a72

Browse files
feat: support ordered container start (#321) (#454)
Signed-off-by: Abhishek <abhishekup082@gmail.com> Co-authored-by: Mathieu Benoit <mathieu-benoit@hotmail.fr>
1 parent 962b998 commit ecb6a72

4 files changed

Lines changed: 349 additions & 10 deletions

File tree

internal/command/generate.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,10 @@ arguments.
166166
return fmt.Errorf("failed to convert '%s' to structure: %w", workloadName, err)
167167
}
168168

169+
if err := loader.Validate(&out); err != nil {
170+
return fmt.Errorf("validation errors in workload '%s': %w", workloadName, err)
171+
}
172+
169173
// Gather container build contexts, these will be stored and added to the generated compose output later
170174
containerBuildContexts := make(map[string]types.BuildConfig)
171175
if v, _ := cmd.Flags().GetStringArray(generateCmdBuildFlag); len(v) > 0 {
@@ -537,3 +541,4 @@ func injectWaitService(p *types.Project) (string, bool) {
537541
p.Services[newService.Name] = newService
538542
return newService.Name, true
539543
}
544+

internal/command/generate_test.go

Lines changed: 73 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -453,6 +453,76 @@ resources:
453453
})
454454
}
455455

456+
func TestInitAndGenerate_with_before_ordering(t *testing.T) {
457+
td := changeToTempDir(t)
458+
stdout, _, err := executeAndResetCommand(context.Background(), rootCmd, []string{"init"})
459+
assert.NoError(t, err)
460+
assert.Equal(t, "", stdout)
461+
462+
assert.NoError(t, os.WriteFile("score.yaml", []byte(`
463+
apiVersion: score.dev/v1b1
464+
metadata:
465+
name: example
466+
containers:
467+
init:
468+
image: busybox
469+
before:
470+
main:
471+
ready: complete
472+
main:
473+
image: nginx
474+
`), 0644))
475+
// generate
476+
stdout, _, err = executeAndResetCommand(context.Background(), rootCmd, []string{"generate", "score.yaml"})
477+
assert.NoError(t, err)
478+
assert.Equal(t, "", stdout)
479+
raw, err := os.ReadFile(filepath.Join(td, "compose.yaml"))
480+
assert.NoError(t, err)
481+
assert.Contains(t, string(raw), "depends_on")
482+
assert.Contains(t, string(raw), "service_completed_successfully")
483+
484+
t.Run("validate compose spec", func(t *testing.T) {
485+
if os.Getenv("NO_DOCKER") != "" {
486+
t.Skip("NO_DOCKER is set")
487+
return
488+
}
489+
dockerCmd, err := exec.LookPath("docker")
490+
require.NoError(t, err)
491+
cmd := exec.Command(dockerCmd, "compose", "-f", "compose.yaml", "config", "--quiet", "--dry-run")
492+
cmd.Dir = td
493+
cmd.Stdout = os.Stdout
494+
cmd.Stderr = os.Stderr
495+
assert.NoError(t, cmd.Run())
496+
})
497+
}
498+
499+
func TestInitAndGenerate_with_before_cycle(t *testing.T) {
500+
_ = changeToTempDir(t)
501+
stdout, _, err := executeAndResetCommand(context.Background(), rootCmd, []string{"init"})
502+
assert.NoError(t, err)
503+
assert.Equal(t, "", stdout)
504+
505+
assert.NoError(t, os.WriteFile("score.yaml", []byte(`
506+
apiVersion: score.dev/v1b1
507+
metadata:
508+
name: example
509+
containers:
510+
app:
511+
image: busybox
512+
before:
513+
backend:
514+
ready: started
515+
backend:
516+
image: busybox
517+
before:
518+
app:
519+
ready: started
520+
`), 0644))
521+
_, _, err = executeAndResetCommand(context.Background(), rootCmd, []string{"generate", "score.yaml"})
522+
assert.Error(t, err)
523+
assert.Contains(t, err.Error(), "cycle")
524+
}
525+
456526
func TestGeneratePostgresResource(t *testing.T) {
457527
td := changeToTempDir(t)
458528
stdout, _, err := executeAndResetCommand(context.Background(), rootCmd, []string{"init"})
@@ -844,12 +914,12 @@ apiVersion: score.dev/v1b1
844914
metadata:
845915
name: example
846916
annotations:
847-
key.com/foo-bar: thing
917+
foo-bar: thing
848918
containers:
849919
example:
850920
image: foo
851921
variables:
852-
REF: ${metadata.annotations.key\.com/foo-bar}
922+
REF: ${metadata.annotations.foo-bar}
853923
`), 0644))
854924
// generate
855925
stdout, _, err = executeAndResetCommand(context.Background(), rootCmd, []string{"generate", "score.yaml"})
@@ -862,7 +932,7 @@ services:
862932
example-example:
863933
annotations:
864934
compose.score.dev/workload-name: example
865-
key.com/foo-bar: thing
935+
foo-bar: thing
866936
environment:
867937
REF: thing
868938
hostname: example

internal/compose/convert.go

Lines changed: 72 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -66,21 +66,32 @@ func ConvertSpec(state *project.State, spec *score.Workload) (*compose.Project,
6666

6767
// When multiple containers are specified we need to identify one container as the "main" container which will own
6868
// the network and use the native workload name. All other containers in this workload will have the container
69-
// name appended as a suffix. We use the natural sort order of the container names and pick the first one
69+
// name appended as a suffix. We use the natural sort order of the container names and pick the first one that
70+
// is not expected to exit (i.e. does not have all before entries with ready: complete).
7071
containerNames := make([]string, 0, len(spec.Containers))
7172
for name := range spec.Containers {
7273
containerNames = append(containerNames, name)
7374
}
7475
sort.Strings(containerNames)
7576

77+
// Determine which container should own the network namespace.
78+
// By definition (enforced by validation), there must be at least one container with no
79+
// 'before' entries — otherwise the before relationships would form a cycle.
80+
primaryContainer := containerNames[0]
81+
for _, name := range containerNames {
82+
if len(spec.Containers[name].Before) == 0 {
83+
primaryContainer = name
84+
break
85+
}
86+
}
87+
7688
variablesSubstitutor := framework.Substituter{
7789
Replacer: deferredSubstitutionFunction,
7890
UnEscaper: func(s string) (string, error) {
7991
return s, nil
8092
},
8193
}
8294

83-
var firstService string
8495
for _, containerName := range containerNames {
8596
cSpec := spec.Containers[containerName]
8697

@@ -171,22 +182,76 @@ func ConvertSpec(state *project.State, spec *score.Workload) (*compose.Project,
171182
svc.Image = ""
172183
}
173184

174-
// if we are not the "first" service, then inherit the network from the first service
175-
if firstService == "" {
176-
firstService = svc.Name
185+
// if we are not the primary container, then inherit the network from the primary service.
186+
// However, skip network_mode for containers that are expected to exit (all their before
187+
// entries have ready: complete) to avoid circular dependencies.
188+
if containerName == primaryContainer {
177189
// We name the containers as (workload name)-(container name) but we want the name for the main network
178190
// interface for be (workload name). So we set the hostname itself. This means that workloads cannot have
179191
// the same name within the project. But that's already enforced elsewhere.
180192
svc.Hostname = workloadName
181-
} else {
193+
} else if !isInitContainer(spec.Containers[containerName]) {
182194
svc.Ports = nil
183-
svc.NetworkMode = "service:" + firstService
195+
svc.NetworkMode = "service:" + workloadName + "-" + primaryContainer
184196
}
185197
composeProject.Services[svc.Name] = svc
186198
}
199+
200+
// Invert before -> depends_on: if container A declares before: {B: {ready: complete}},
201+
// then service B depends_on A with the appropriate condition.
202+
for _, containerName := range containerNames {
203+
cSpec := spec.Containers[containerName]
204+
for targetContainerName, entry := range cSpec.Before {
205+
// Determine the compose condition from the ready field
206+
var condition string
207+
switch entry.Ready {
208+
case score.ContainerBeforeReadyComplete:
209+
condition = "service_completed_successfully"
210+
case score.ContainerBeforeReadyHealthy:
211+
condition = "service_healthy"
212+
case score.ContainerBeforeReadyStarted:
213+
condition = "service_started"
214+
default:
215+
return nil, fmt.Errorf("containers.%s.before.%s: unknown ready condition %q", containerName, targetContainerName, entry.Ready)
216+
}
217+
218+
if entry.Ready == score.ContainerBeforeReadyHealthy && cSpec.ReadinessProbe == nil && cSpec.LivenessProbe == nil {
219+
return nil, fmt.Errorf("containers.%s.before: ready '%s' requires a readiness or liveness probe to be defined", containerName, score.ContainerBeforeReadyHealthy)
220+
}
221+
222+
sourceServiceName := workloadName + "-" + containerName
223+
targetServiceName := workloadName + "-" + targetContainerName
224+
225+
// Add depends_on to the target service
226+
svc := composeProject.Services[targetServiceName]
227+
if svc.DependsOn == nil {
228+
svc.DependsOn = make(compose.DependsOnConfig)
229+
}
230+
svc.DependsOn[sourceServiceName] = compose.ServiceDependency{
231+
Condition: condition,
232+
Required: true,
233+
}
234+
composeProject.Services[targetServiceName] = svc
235+
}
236+
}
237+
187238
return &composeProject, nil
188239
}
189240

241+
// isInitContainer returns true if the container is expected to exit. This is determined
242+
// by checking if all its before entries specify ready: complete.
243+
func isInitContainer(c score.Container) bool {
244+
if len(c.Before) == 0 {
245+
return false
246+
}
247+
for _, entry := range c.Before {
248+
if entry.Ready != score.ContainerBeforeReadyComplete {
249+
return false
250+
}
251+
}
252+
return true
253+
}
254+
190255
// buildWorkloadAnnotations returns an annotation set for the workload service.
191256
func buildWorkloadAnnotations(name string, spec *score.Workload) map[string]string {
192257
var out map[string]string

0 commit comments

Comments
 (0)