From db09dec85517af9534dc0857397618ad3ea104df Mon Sep 17 00:00:00 2001 From: Pasha Sviderski Date: Mon, 15 Dec 2025 18:12:12 +1000 Subject: [PATCH] chore: add more tests for MatchesDockerVolume and update error message when volume spec doesn't match --- pkg/api/volume_test.go | 74 +++++++++++++++++++++++++++ pkg/client/deploy/scheduler/volume.go | 5 +- 2 files changed, 78 insertions(+), 1 deletion(-) diff --git a/pkg/api/volume_test.go b/pkg/api/volume_test.go index 0e41befa..98d262af 100644 --- a/pkg/api/volume_test.go +++ b/pkg/api/volume_test.go @@ -9,6 +9,8 @@ import ( ) func TestVolumeSpec_MatchesDockerVolume(t *testing.T) { + t.Parallel() + tests := []struct { name string spec VolumeSpec @@ -57,6 +59,78 @@ func TestVolumeSpec_MatchesDockerVolume(t *testing.T) { }, expected: true, }, + { + name: "non-match with different driver options", + spec: VolumeSpec{ + Type: VolumeTypeVolume, + VolumeOptions: &VolumeOptions{ + Driver: &mount.Driver{ + Name: "local", + Options: map[string]string{ + "foo": "baz", + }, + }, + }, + }, + vol: volume.Volume{ + Driver: "local", + Options: map[string]string{ + "foo": "bar", + }, + }, + expected: false, + }, + { + name: "non-match with different driver name", + spec: VolumeSpec{ + Type: VolumeTypeVolume, + VolumeOptions: &VolumeOptions{ + Driver: &mount.Driver{ + Name: "custom", + Options: map[string]string{ + "foo": "bar", + }, + }, + }, + }, + vol: volume.Volume{ + Driver: "local", + Options: map[string]string{ + "foo": "bar", + }, + }, + expected: false, + }, + { + name: "match external volume without driver by name only", + spec: VolumeSpec{ + Name: "external", + Type: VolumeTypeVolume, + }, + vol: volume.Volume{ + Name: "external", + Driver: "local", + Options: map[string]string{ + "foo": "bar", + }, + }, + expected: true, + }, + { + name: "non-match external volume by name", + spec: VolumeSpec{ + Name: "unknown", + Type: VolumeTypeVolume, + }, + vol: volume.Volume{ + Name: "external", + Driver: "local", + Options: map[string]string{ + "foo": "bar", + }, + }, + expected: false, + }, } for _, tt := range tests { diff --git a/pkg/client/deploy/scheduler/volume.go b/pkg/client/deploy/scheduler/volume.go index 51927150..2f0019e9 100644 --- a/pkg/client/deploy/scheduler/volume.go +++ b/pkg/client/deploy/scheduler/volume.go @@ -83,7 +83,10 @@ func NewVolumeScheduler(state *ClusterState, specs []api.ServiceSpec) (*VolumeSc if spec, ok := volumeSpecs[vol.Name]; ok { if !spec.MatchesDockerVolume(vol) { return nil, fmt.Errorf("volume '%s' specification does not match the existing volume "+ - "on machine '%s'", vol.Name, machine.Info.Name) + "on machine '%s'. Use a different volume name or adjust the volume options to match "+ + "the existing volume. You can also remove the existing volume from the machine(s) with "+ + "'uc volume rm' (WARNING: the data will be lost) and run the deployment again to create "+ + "a new volume with the correct specification", vol.Name, machine.Info.Name) } if _, setInitialised := existingVolumeMachines[vol.Name]; !setInitialised {