Skip to content

Commit

Permalink
fix: support subpath on docker or bind volumes (#169)
Browse files Browse the repository at this point in the history
Signed-off-by: Ben Meier <[email protected]>
  • Loading branch information
astromechza authored Jul 16, 2024
1 parent 02696db commit a2b08b8
Show file tree
Hide file tree
Showing 5 changed files with 76 additions and 48 deletions.
1 change: 0 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
|---------------------------------------------------------------------|---------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| `containers.*.resources.limits` / `containers.*.resources.requests` | none | **Limits will be validated but ignored.** While the compose specification has some support for this, it is requires particular Docker versions that cannot be relied on. *This should have no impact on Workload execution*. |
| `containers.*.livenessProbe` / `containers.*.readinessProbe` | none | **Probes will be validated but ignored.** The Score specification only details K8s-like HTTP probes, but the compose specification only supports direct command execution. We cannot convert between the two reliably. *This should have no impact on Workload execution*. Tracked in [#86](https://github.com/score-spec/score-compose/issues/86). |
| `containers.*.volumes[*].path` | none | **Volume mounts with `path` will be rejected.** Docker compose doesn't support sub-path mounts for Docker volumes. |

## Resource support

Expand Down
18 changes: 17 additions & 1 deletion internal/command/generate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1030,6 +1030,11 @@ func TestInitAndGenerate_with_volume_types(t *testing.T) {

// write custom providers
assert.NoError(t, os.WriteFile(filepath.Join(td, ".score-compose", "00-custom.provisioners.yaml"), []byte(`
- uri: template://docker-volume
type: volume
outputs: |
type: volume
source: named-volume
- uri: template://tmpfs-volume
type: tmp-volume
outputs: |
Expand Down Expand Up @@ -1058,11 +1063,17 @@ containers:
source: ${resources.v1}
- target: /mnt/v2
source: ${resources.v2}
path: thing
- target: /mnt/v3
source: ${resources.v3}
path: other/thing
resources:
v1:
type: tmp-volume
v2:
type: bind-volume
v3:
type: volume
`), 0644))

// generate
Expand All @@ -1080,10 +1091,15 @@ services:
image: busybox
volumes:
- type: bind
source: /dev/something
source: /dev/something/thing
target: /mnt/v2
bind:
create_host_path: true
- type: volume
source: named-volume
target: /mnt/v3
volume:
subpath: other/thing
- type: tmpfs
source: tmp-volume.default#example.v1
target: /mnt/v1
Expand Down
20 changes: 0 additions & 20 deletions internal/command/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -251,26 +251,6 @@ func TestExample_invalid_spec(t *testing.T) {
assert.NotEqual(t, "", strings.TrimSpace(stderr))
}

func TestVolumeSubPathNotSupported(t *testing.T) {
td := t.TempDir()
assert.NoError(t, os.WriteFile(filepath.Join(td, "score.yaml"), []byte(`
apiVersion: score.dev/v1b1
metadata:
name: example-workload-name123
containers:
container-one1:
image: localhost:4000/repo/my-image:tag
volumes:
- source: volume-name
target: /mnt/something
path: /sub/path
`), 0600))
stdout, stderr, err := executeAndResetCommand(context.Background(), rootCmd, []string{"run", "--file", filepath.Join(td, "score.yaml"), "--output", filepath.Join(td, "compose.yaml")})
assert.EqualError(t, err, "building docker-compose configuration: containers.container-one1.volumes[0].path: can't mount named volume with sub path '/sub/path': not supported")
assert.Equal(t, "", stdout)
assert.NotEqual(t, "", strings.TrimSpace(stderr))
}

func TestFilesNotSupported(t *testing.T) {
td := t.TempDir()
assert.NoError(t, os.WriteFile(filepath.Join(td, "score.yaml"), []byte(`
Expand Down
19 changes: 14 additions & 5 deletions internal/compose/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,13 +95,9 @@ func ConvertSpec(state *project.State, spec *score.Workload) (*compose.Project,
if len(cSpec.Volumes) > 0 {
volumes = make([]compose.ServiceVolumeConfig, len(cSpec.Volumes))
for idx, vol := range cSpec.Volumes {
if vol.Path != nil && *vol.Path != "" {
return nil, fmt.Errorf("containers.%s.volumes[%d].path: can't mount named volume with sub path '%s': not supported", containerName, idx, *vol.Path)
}

cfg, err := convertVolumeSourceIntoVolume(state, deferredSubstitutionFunction, workloadName, vol)
if err != nil {
return nil, fmt.Errorf("containers.%s.volumes[%d].source: %w", containerName, idx, err)
return nil, fmt.Errorf("containers.%s.volumes[%d]: %w", containerName, idx, err)
}
volumes[idx] = *cfg
}
Expand Down Expand Up @@ -293,6 +289,7 @@ func convertVolumeSourceIntoVolume(state *project.State, substitutionFunction fu
}

// now if the resolves source is a volume we can check the outputs or throw an error

res, ok := state.Resources[framework.ResourceUid(resolvedVolumeSource)]
if ok {
volType, ok := res.Outputs["type"].(string)
Expand All @@ -315,6 +312,12 @@ func convertVolumeSourceIntoVolume(state *project.State, substitutionFunction fu
}
outputVolume.Source = s.Source
outputVolume.Volume = s.Volume
if vol.Path != nil && *vol.Path != "" {
if outputVolume.Volume == nil {
outputVolume.Volume = &compose.ServiceVolumeVolume{}
}
outputVolume.Volume.Subpath = filepath.Join(outputVolume.Volume.Subpath, *vol.Path)
}
case "tmpfs":
s := struct {
Type string `json:"type"`
Expand All @@ -324,6 +327,9 @@ func convertVolumeSourceIntoVolume(state *project.State, substitutionFunction fu
return nil, fmt.Errorf("resource '%s' outputs cannot decode for tmpfs: %w", resolvedVolumeSource, err)
}
outputVolume.Tmpfs = s.Tmpfs
if vol.Path != nil && *vol.Path != "" {
return nil, fmt.Errorf("can't mount named tmpfs volume with sub path '%s': not supported", *vol.Path)
}
case "bind":
s := struct {
Type string `json:"type"`
Expand All @@ -334,6 +340,9 @@ func convertVolumeSourceIntoVolume(state *project.State, substitutionFunction fu
return nil, fmt.Errorf("resource '%s' outputs cannot decode for bind: %w", resolvedVolumeSource, err)
}
outputVolume.Source = s.Source
if vol.Path != nil && *vol.Path != "" {
outputVolume.Source = filepath.Join(outputVolume.Source, *vol.Path)
}
outputVolume.Bind = s.Bind
default:
return nil, fmt.Errorf("resource '%s' has invalid type '%s'", resolvedVolumeSource, volType)
Expand Down
66 changes: 45 additions & 21 deletions internal/compose/convert_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ func TestScoreConvert(t *testing.T) {
},
Volumes: []score.ContainerVolumesElem{
{
Source: "data",
Source: "${resources.data}",
Target: "/mnt/data",
ReadOnly: util.Ref(true),
},
Expand All @@ -135,7 +135,7 @@ func TestScoreConvert(t *testing.T) {
Type: "environment",
},
"app-db": {
Type: "postgress",
Type: "mysql",
},
"some-dns": {
Type: "dns",
Expand Down Expand Up @@ -163,7 +163,7 @@ func TestScoreConvert(t *testing.T) {
Volumes: []compose.ServiceVolumeConfig{
{
Type: "volume",
Source: "data",
Source: "example",
Target: "/mnt/data",
ReadOnly: true,
},
Expand Down Expand Up @@ -196,7 +196,7 @@ func TestScoreConvert(t *testing.T) {
},
Volumes: []score.ContainerVolumesElem{
{
Source: "data",
Source: "${resources.data}",
Target: "/mnt/data",
ReadOnly: util.Ref(true),
},
Expand Down Expand Up @@ -236,7 +236,7 @@ func TestScoreConvert(t *testing.T) {
Volumes: []compose.ServiceVolumeConfig{
{
Type: "volume",
Source: "data",
Source: "example",
Target: "/mnt/data",
ReadOnly: true,
},
Expand Down Expand Up @@ -354,10 +354,8 @@ func TestScoreConvert(t *testing.T) {
},
},

// Errors handling
//
{
Name: "Should report an error for volumes with sub path (not supported)",
Name: "Volume with sub-path should succeed",
Source: &score.Workload{
Metadata: score.WorkloadMetadata{
"name": "test",
Expand All @@ -367,7 +365,7 @@ func TestScoreConvert(t *testing.T) {
Image: "busybox",
Volumes: []score.ContainerVolumesElem{
{
Source: "data",
Source: "${resources.data}",
Target: "/mnt/data",
Path: util.Ref("sub/path"),
ReadOnly: util.Ref(true),
Expand All @@ -381,9 +379,34 @@ func TestScoreConvert(t *testing.T) {
},
},
},
Error: errors.New("not supported"),
Project: &compose.Project{
Services: compose.Services{
"test-backend": {
Name: "test-backend",
Annotations: map[string]string{
"compose.score.dev/workload-name": "test",
},
Image: "busybox",
Hostname: "test",
Environment: compose.MappingWithEquals{},
Volumes: []compose.ServiceVolumeConfig{
{
Type: "volume",
Source: "example",
Target: "/mnt/data",
ReadOnly: true,
Volume: &compose.ServiceVolumeVolume{
Subpath: "sub/path",
},
},
},
},
},
},
},

// Errors handling
//
{
Name: "Should report an error for volume that doesn't exist in resources",
Source: &score.Workload{
Expand All @@ -395,7 +418,7 @@ func TestScoreConvert(t *testing.T) {
},
},
},
Error: errors.New("containers.test.volumes[0].source: resource 'data' does not exist"),
Error: errors.New("containers.test.volumes[0]: resource 'data' does not exist"),
},

{
Expand All @@ -410,29 +433,30 @@ func TestScoreConvert(t *testing.T) {
},
Resources: map[string]score.Resource{"data": {Type: "thing"}},
},
Error: errors.New("containers.test.volumes[0].source: resource 'data' does not exist"),
Error: errors.New("containers.test.volumes[0]: resource 'thing.default#test.data' has no 'type' output"),
},
}

for _, tt := range tests {
t.Run(tt.Name, func(t *testing.T) {

state := &project.State{
Workloads: map[string]framework.ScoreWorkloadState[project.WorkloadExtras]{
"test": {Spec: score.Workload{Resources: map[string]score.Resource{
"env": {Type: "environment"},
"app-db": {Type: "thing"},
"some-dns": {Type: "thing"},
}}},
},
Workloads: map[string]framework.ScoreWorkloadState[project.WorkloadExtras]{},
Resources: map[framework.ResourceUid]framework.ScoreResourceState[framework.NoExtras]{},
}
state, _ = state.WithWorkload(tt.Source, nil, project.WorkloadExtras{})
state, _ = state.WithPrimedResources()

evt := new(envprov.Provisioner)
state.Resources["environment.default#test.env"] = framework.ScoreResourceState[framework.NoExtras]{OutputLookupFunc: evt.LookupOutput}
po, _ := evt.GenerateSubProvisioner("app-db", "").Provision(nil, nil)
state.Resources["thing.default#test.app-db"] = framework.ScoreResourceState[framework.NoExtras]{OutputLookupFunc: po.OutputLookupFunc}
state.Resources["mysql.default#test.app-db"] = framework.ScoreResourceState[framework.NoExtras]{OutputLookupFunc: po.OutputLookupFunc}
po, _ = evt.GenerateSubProvisioner("some-dns", "").Provision(nil, nil)
state.Resources["thing.default#test.some-dns"] = framework.ScoreResourceState[framework.NoExtras]{OutputLookupFunc: po.OutputLookupFunc}
state.Resources["dns.default#test.some-dns"] = framework.ScoreResourceState[framework.NoExtras]{OutputLookupFunc: po.OutputLookupFunc}
state.Resources["volume.default#test.data"] = framework.ScoreResourceState[framework.NoExtras]{Outputs: map[string]interface{}{
"type": "volume",
"source": "example",
}}

proj, err := ConvertSpec(state, tt.Source)

Expand Down

0 comments on commit a2b08b8

Please sign in to comment.