chore(volumes): map volume specs to container config, diff specs with volumes

internal/machine/docker/server.go

@@ -9,6 +9,7 @@ import (
 	"io"
 	"log/slog"
 	"regexp"
+	"slices"
 	"strconv"
 	"strings"
 
@@ -17,6 +18,7 @@ import (
 	"github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/api/types/filters"
 	"github.com/docker/docker/api/types/image"
+	"github.com/docker/docker/api/types/mount"
 	"github.com/docker/docker/api/types/network"
 	"github.com/docker/docker/api/types/volume"
 	"github.com/docker/docker/client"
@@ -461,6 +463,14 @@ func (s *Server) CreateServiceContainer(
 		config.Labels[api.LabelServicePorts] = strings.Join(encodedPorts, ",")
 	}
 
+	mounts, err := toDockerMounts(spec.Volumes, spec.Container.VolumeMounts)
+	if err != nil {
+		return nil, err
+	}
+	if err = s.verifyDockerVolumesExist(ctx, mounts); err != nil {
+		return nil, err
+	}
+
 	portBindings := make(nat.PortMap)
 	for _, p := range spec.Ports {
 		if p.Mode != api.PortModeHost {
@@ -479,6 +489,7 @@ func (s *Server) CreateServiceContainer(
 	hostConfig := &container.HostConfig{
 		Binds:        spec.Container.Volumes,
 		Init:         spec.Container.Init,
+		Mounts:       mounts,
 		PortBindings: portBindings,
 		// Always restart service containers if they exit or a machine restarts.
 		// For one-off containers and batch jobs we plan to use a different service type/mode.
@@ -526,54 +537,100 @@ func (s *Server) CreateServiceContainer(
 	return &pb.CreateContainerResponse{Response: respBytes}, nil
 }
 
-//func toMounts(volumes []api.VolumeSpec) ([]mount.Mount, error) {
+func toDockerMounts(volumes []api.VolumeSpec, mounts []api.VolumeMount) ([]mount.Mount, error) {
-//	mounts := make([]mount.Mount, 0, len(volumes))
+	dockerMounts := make([]mount.Mount, 0, len(mounts))
-//	for _, vol := range volumes {
+	for _, m := range mounts {
-//		m := mount.Mount{
+		idx := slices.IndexFunc(volumes, func(v api.VolumeSpec) bool {
-//			Type:     mount.Type(vol.Type),
+			return v.Name == m.VolumeName
-//			Source:   vol.Source,
+		})
-//			Target:   vol.Target,
+		if idx == -1 {
-//			ReadOnly: vol.ReadOnly,
+			return nil, fmt.Errorf("volume mount references a volume that doesn't exist in the volumes spec: '%s'",
-//		}
+				m.VolumeName)
-//
+		}
-//		// Set type-specific options.
+
-//		switch vol.Type {
+		vol := volumes[idx]
-//		case api.VolumeTypeBind:
+		if err := vol.Validate(); err != nil {
-//			if vol.BindOptions != nil {
+			return nil, fmt.Errorf("invalid volume: %w", err)
-//				m.BindOptions = &mount.BindOptions{
+		}
-//					Propagation:  vol.BindOptions.Propagation,
+
-//					NonRecursive: false,
+		dm := mount.Mount{
-//				}
+			Type:     mount.Type(vol.Type),
-//				if vol.BindOptions.CreateHostPath {
+			Target:   m.ContainerPath,
-//					m.BindOptions.CreateMountpoint = true
+			ReadOnly: m.ReadOnly,
-//				}
+		}
-//				// Handle SELinux options if specified
+
-//				if vol.BindOptions.SELinux == api.SELinuxShared {
+		switch vol.Type {
-//					m.BindOptions.Propagation = mount.PropagationShared
+		case api.VolumeTypeBind:
-//				} else if vol.BindOptions.SELinux == api.SELinuxUnshared {
+			dm.Source = vol.BindOptions.HostPath
-//					m.BindOptions.Propagation = mount.PropagationPrivate
+			dm.BindOptions = toDockerBindOptions(vol.BindOptions)
-//				}
+		case api.VolumeTypeVolume:
-//			}
+			dm.Source = vol.Name
-//		case api.VolumeTypeVolume:
+
-//			if vol.VolumeOptions != nil {
+			if vol.VolumeOptions != nil {
-//				m.VolumeOptions = &mount.VolumeOptions{
+				dm.VolumeOptions = &mount.VolumeOptions{
-//					NoCopy:       vol.VolumeOptions.NoCopy,
+					NoCopy:       vol.VolumeOptions.NoCopy,
-//					Labels:       vol.VolumeOptions.Labels,
+					Labels:       vol.VolumeOptions.Labels,
-//					DriverConfig: vol.VolumeOptions.Driver,
+					Subpath:      vol.VolumeOptions.SubPath,
-//					Subpath:      vol.VolumeOptions.Subpath,
+					DriverConfig: vol.VolumeOptions.Driver,
-//				}
+				}
-//			}
+
-//		case api.VolumeTypeTmpfs:
+				if vol.VolumeOptions.Name != "" {
-//			m.TmpfsOptions = vol.TmpfsOptions
+					dm.Source = vol.VolumeOptions.Name
-//		default:
+				}
-//			return nil, fmt.Errorf("invalid volume type: '%s' (must be one of %s, %s, %s)",
+			}
-//				vol.Type, api.VolumeTypeBind, api.VolumeTypeVolume, api.VolumeTypeTmpfs)
+		case api.VolumeTypeTmpfs:
-//		}
+			dm.TmpfsOptions = vol.TmpfsOptions
-//
+		default:
-//		mounts = append(mounts, m)
+			return nil, fmt.Errorf("unsupported volume type: '%s'", vol.Type)
-//	}
+		}
-//	return mounts
+
-//}
+		dockerMounts = append(dockerMounts, dm)
+	}
+
+	return dockerMounts, nil
+}
+
+func toDockerBindOptions(opts *api.BindOptions) *mount.BindOptions {
+	if opts == nil {
+		return nil
+	}
+
+	dockerOpts := &mount.BindOptions{
+		Propagation:      opts.Propagation,
+		CreateMountpoint: opts.CreateHostPath,
+	}
+
+	switch opts.Recursive {
+	case "disabled":
+		dockerOpts.NonRecursive = true
+	case "writable":
+		dockerOpts.ReadOnlyNonRecursive = true
+	case "readonly":
+		dockerOpts.ReadOnlyForceRecursive = true
+	}
+
+	return dockerOpts
+}
+
+// verifyDockerVolumesExist checks if the Docker named volumes referenced in the mounts exist on the machine.
+func (s *Server) verifyDockerVolumesExist(ctx context.Context, mounts []mount.Mount) error {
+	for _, m := range mounts {
+		if m.Type != mount.TypeVolume {
+			continue
+		}
+
+		// TODO: non-local volume drivers should likely be handled differently (needs proper investigation).
+		if _, err := s.client.VolumeInspect(ctx, m.Source); err != nil {
+			if client.IsErrNotFound(err) {
+				return status.Errorf(codes.NotFound, "volume '%s' not found", m.Source)
+			}
+			return status.Errorf(codes.Internal, "inspect volume '%s': %v", m.Source, err.Error())
+		}
+		// TODO: check if the volume driver and options are the same as in the mount and fail if not.
+	}
+
+	return nil
+}
 
 // InspectServiceContainer returns the container information and service specification that was used to create the
 // container with the given ID.

pkg/api/service.go

@@ -62,6 +62,10 @@ func (s *ServiceSpec) SetDefaults() ServiceSpec {
 	}
 	spec.Container = spec.Container.SetDefaults()
 
+	for i, v := range spec.Volumes {
+		spec.Volumes[i] = v.SetDefaults()
+	}
+
 	return spec
 }
 
@@ -87,11 +91,17 @@ func (s *ServiceSpec) Validate() error {
 
 	// TODO: validate there is no conflict between ports.
 
+	volumeNames := make(map[string]struct{})
 	for _, v := range s.Volumes {
 		if err := v.Validate(); err != nil {
 			return fmt.Errorf("invalid volume: %w", err)
 		}
+		if _, ok := volumeNames[v.Name]; ok {
+			return fmt.Errorf("duplicate volume name: '%s'", v.Name)
 		}
+		volumeNames[v.Name] = struct{}{}
+	}
+
 	for _, m := range s.Container.VolumeMounts {
 		if !slices.ContainsFunc(s.Volumes, func(v VolumeSpec) bool {
 			return v.Name == m.VolumeName
@@ -113,6 +123,13 @@ func (s *ServiceSpec) Clone() ServiceSpec {
 	}
 	spec.Container = s.Container.Clone()
 
+	if s.Volumes != nil {
+		spec.Volumes = make([]VolumeSpec, len(s.Volumes))
+		for i, v := range s.Volumes {
+			spec.Volumes[i] = v.Clone()
+		}
+	}
+
 	return spec
 }
 
@@ -170,6 +187,9 @@ func (s *ContainerSpec) Equals(spec ContainerSpec) bool {
 	slices.Sort(orig.Volumes)
 	slices.Sort(spec.Volumes)
 
+	sortVolumeMounts(orig.VolumeMounts)
+	sortVolumeMounts(spec.VolumeMounts)
+
 	return reflect.DeepEqual(orig, spec)
 }
 
@@ -188,6 +208,10 @@ func (s *ContainerSpec) Clone() ContainerSpec {
 		spec.Volumes = make([]string, len(s.Volumes))
 		copy(spec.Volumes, s.Volumes)
 	}
+	if s.VolumeMounts != nil {
+		spec.VolumeMounts = make([]VolumeMount, len(s.VolumeMounts))
+		copy(spec.VolumeMounts, s.VolumeMounts)
+	}
 
 	return spec
 }

pkg/api/volume.go

@@ -2,6 +2,8 @@ package api
 
 import (
 	"fmt"
+	"reflect"
+	"sort"
 	"strings"
 
 	"github.com/docker/docker/api/types/mount"
@@ -14,14 +16,10 @@ const (
 	VolumeTypeVolume = "volume"
 	// VolumeTypeTmpfs is the type for mounting a temporary file system stored in the host memory.
 	VolumeTypeTmpfs = "tmpfs"
-
-	// SELinuxShared share the volume content.
-	SELinuxShared = "z"
-	// SELinuxUnshared label content as private unshared.
-	SELinuxUnshared = "Z"
 )
 
-// VolumeSpec defines a volume mount specification.
+// VolumeSpec defines a volume mount specification. As of April 2025, the volume must be created before deploying
+// a service using it.
 type VolumeSpec struct {
 	// Name is the volume name used to reference this volume in container mounts.
 	Name          string
@@ -35,19 +33,16 @@ type VolumeSpec struct {
 type BindOptions struct {
 	// HostPath is the absolute path on the host filesystem.
 	HostPath string
-	// AutoCreate indicates whether the host path should be created if it doesn't exist.
+	// CreateHostPath indicates whether the host path should be created if it doesn't exist.
 	// If false, deployment will fail if the path doesn't exist.
-	AutoCreate  bool              `json:",omitempty"`
+	CreateHostPath bool              `json:",omitempty"`
 	Propagation    mount.Propagation `json:",omitempty"`
-	SELinux     string            `json:",omitempty"`
+	Recursive      string            `json:",omitempty"`
 }
 
 // VolumeOptions represents options for a managed volume.
 type VolumeOptions struct {
-	// AutoCreate indicates whether the volume should be created if it doesn't exist.
+	// Driver specifies the volume driver and its options for volume creation.
-	// If false, deployment will fail if the volume doesn't exist.
-	AutoCreate bool `json:",omitempty"`
-	// Driver specifies the volume driver and its options for volume creation (AutoCreate is true).
 	Driver *mount.Driver `json:",omitempty"`
 	// Labels are key-value metadata to apply to the volume if creating a new volume.
 	Labels map[string]string `json:",omitempty"`
@@ -59,13 +54,36 @@ type VolumeOptions struct {
 	SubPath string `json:",omitempty"`
 }
 
+func (v *VolumeSpec) SetDefaults() VolumeSpec {
+	spec := v.Clone()
+
+	if spec.Type == VolumeTypeVolume {
+		if spec.VolumeOptions == nil {
+			spec.VolumeOptions = &VolumeOptions{}
+		}
+		if spec.VolumeOptions.Driver == nil {
+			spec.VolumeOptions.Driver = &mount.Driver{Name: "local"}
+		}
+		if spec.VolumeOptions.Name == "" {
+			spec.VolumeOptions.Name = spec.Name
+		}
+	}
+	// TODO: set explicit default values for Propagation and Recursive for bind mounts?
+
+	return spec
+}
+
 func (v *VolumeSpec) Validate() error {
 	if v.Name == "" {
 		return fmt.Errorf("volume name must not be empty")
 	}
 
 	switch v.Type {
-	case VolumeTypeBind, VolumeTypeVolume, VolumeTypeTmpfs:
+	case VolumeTypeBind:
+		if v.BindOptions == nil {
+			return fmt.Errorf("bind volume must have bind options")
+		}
+	case VolumeTypeVolume, VolumeTypeTmpfs:
 	default:
 		return fmt.Errorf("invalid volume type: '%s', must be one of '%s', '%s', '%s')",
 			v.Type, VolumeTypeBind, VolumeTypeVolume, VolumeTypeTmpfs)
@@ -74,6 +92,57 @@ func (v *VolumeSpec) Validate() error {
 	return nil
 }
 
+func (v *VolumeSpec) Equals(other VolumeSpec) bool {
+	vol := v.SetDefaults()
+	other = other.SetDefaults()
+
+	return reflect.DeepEqual(vol, other)
+}
+
+func (v *VolumeSpec) Clone() VolumeSpec {
+	spec := *v
+
+	if v.BindOptions != nil {
+		opts := *v.BindOptions
+		spec.BindOptions = &opts
+	}
+
+	if v.VolumeOptions != nil {
+		opts := *v.VolumeOptions
+		if v.VolumeOptions.Driver != nil {
+			driver := *v.VolumeOptions.Driver
+			if driver.Options != nil {
+				driver.Options = make(map[string]string, len(v.VolumeOptions.Driver.Options))
+				for k, val := range v.VolumeOptions.Driver.Options {
+					driver.Options[k] = val
+				}
+			}
+			opts.Driver = &driver
+		}
+
+		if opts.Labels != nil {
+			opts.Labels = make(map[string]string, len(v.VolumeOptions.Labels))
+			for k, val := range v.VolumeOptions.Labels {
+				opts.Labels[k] = val
+			}
+		}
+
+		spec.VolumeOptions = &opts
+	}
+
+	if v.TmpfsOptions != nil {
+		opts := *v.TmpfsOptions
+		opts.Options = make([][]string, len(v.TmpfsOptions.Options))
+		for i, opt := range v.TmpfsOptions.Options {
+			opts.Options[i] = make([]string, len(opt))
+			copy(opts.Options[i], opt)
+		}
+		spec.TmpfsOptions = &opts
+	}
+
+	return spec
+}
+
 // VolumeMount defines how a volume is mounted into a container.
 type VolumeMount struct {
 	// VolumeName references a volume defined in ServiceSpec.Volumes by its Name field.
@@ -96,3 +165,20 @@ func (m *VolumeMount) Validate() error {
 
 	return nil
 }
+
+func sortVolumeMounts(mounts []VolumeMount) {
+	sort.Slice(mounts, func(i, j int) bool {
+		if mounts[i].VolumeName != mounts[j].VolumeName {
+			return mounts[i].VolumeName < mounts[j].VolumeName
+		}
+		if mounts[i].ContainerPath != mounts[j].ContainerPath {
+			return mounts[i].ContainerPath < mounts[j].ContainerPath
+		}
+		if mounts[i].ReadOnly != mounts[j].ReadOnly {
+			if !mounts[i].ReadOnly {
+				return true
+			}
+		}
+		return false
+	})
+}

pkg/client/deploy/container.go

@@ -1,6 +1,8 @@
 package deploy
 
 import (
+	"sort"
+
 	"github.com/psviderski/uncloud/pkg/api"
 )
 
@@ -35,5 +37,27 @@ func EvalContainerSpecChange(current api.ServiceSpec, new api.ServiceSpec) Conta
 		return ContainerNeedsRecreate
 	}
 
+	// Compare volumes.
+	if len(current.Volumes) != len(new.Volumes) {
+		return ContainerNeedsRecreate
+	}
+	sortVolumes(current.Volumes)
+	sortVolumes(new.Volumes)
+	for i := range current.Volumes {
+		if !current.Volumes[i].Equals(new.Volumes[i]) {
+			// TODO: require only spec update for cases (see TODOs in tests):
+			//  * bind volumes with different CreateHostPath
+			//  * changed reference name in spec but preserved original volume name
+			// TODO: should defined but not used (no corresponding mounts) volumes be simply ignored?
+			return ContainerNeedsRecreate
+		}
+	}
+
 	return ContainerUpToDate
 }
+
+func sortVolumes(volumes []api.VolumeSpec) {
+	sort.Slice(volumes, func(i, j int) bool {
+		return volumes[i].Name < volumes[j].Name
+	})
+}

pkg/client/deploy/container_test.go

@@ -0,0 +1,770 @@
+package deploy
+
+import (
+	"testing"
+
+	"github.com/docker/docker/api/types/mount"
+	"github.com/psviderski/uncloud/pkg/api"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestEvalContainerSpecChange_Volumes(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name     string
+		current  api.ServiceSpec
+		new      api.ServiceSpec
+		expected ContainerSpecStatus
+	}{
+		// TODO: should all volumes that are defined but not used (no corresponding mounts) be simply ignored?
+		{
+			name: "identical volumes",
+			current: api.ServiceSpec{
+				Volumes: []api.VolumeSpec{
+					{
+						Name: "data",
+						Type: api.VolumeTypeVolume,
+					},
+				},
+			},
+			new: api.ServiceSpec{
+				Volumes: []api.VolumeSpec{
+					{
+						Name: "data",
+						Type: api.VolumeTypeVolume,
+					},
+				},
+			},
+			expected: ContainerUpToDate,
+		},
+		{
+			name: "volumes in different order but identical",
+			current: api.ServiceSpec{
+				Volumes: []api.VolumeSpec{
+					{
+						Name: "data1",
+						Type: api.VolumeTypeVolume,
+					},
+					{
+						Name: "data2",
+						Type: api.VolumeTypeVolume,
+					},
+				},
+			},
+			new: api.ServiceSpec{
+				Volumes: []api.VolumeSpec{
+					{
+						Name: "data2",
+						Type: api.VolumeTypeVolume,
+					},
+					{
+						Name: "data1",
+						Type: api.VolumeTypeVolume,
+					},
+				},
+			},
+			expected: ContainerUpToDate,
+		},
+		{
+			name: "different number of volumes",
+			current: api.ServiceSpec{
+				Volumes: []api.VolumeSpec{
+					{
+						Name: "data",
+						Type: api.VolumeTypeVolume,
+					},
+				},
+			},
+			new: api.ServiceSpec{
+				Volumes: []api.VolumeSpec{
+					{
+						Name: "data",
+						Type: api.VolumeTypeVolume,
+					},
+					{
+						Name: "config",
+						Type: api.VolumeTypeVolume,
+					},
+				},
+			},
+			expected: ContainerNeedsRecreate,
+		},
+		{
+			name: "no volumes to one volume",
+			current: api.ServiceSpec{
+				Volumes: []api.VolumeSpec{},
+			},
+			new: api.ServiceSpec{
+				Volumes: []api.VolumeSpec{
+					{
+						Name: "data",
+						Type: api.VolumeTypeVolume,
+					},
+				},
+			},
+			expected: ContainerNeedsRecreate,
+		},
+		{
+			name: "one volume to no volumes",
+			current: api.ServiceSpec{
+				Volumes: []api.VolumeSpec{
+					{
+						Name: "data",
+						Type: api.VolumeTypeVolume,
+					},
+				},
+			},
+			new: api.ServiceSpec{
+				Volumes: []api.VolumeSpec{},
+			},
+			expected: ContainerNeedsRecreate,
+		},
+		{
+			name: "change volume type",
+			current: api.ServiceSpec{
+				Volumes: []api.VolumeSpec{
+					{
+						Name: "data",
+						Type: api.VolumeTypeVolume,
+					},
+				},
+			},
+			new: api.ServiceSpec{
+				Volumes: []api.VolumeSpec{
+					{
+						Name: "data",
+						Type: api.VolumeTypeBind,
+						BindOptions: &api.BindOptions{
+							HostPath: "/host/path",
+						},
+					},
+				},
+			},
+			expected: ContainerNeedsRecreate,
+		},
+		{
+			name: "change volume options",
+			current: api.ServiceSpec{
+				Volumes: []api.VolumeSpec{
+					{
+						Name: "data",
+						Type: api.VolumeTypeVolume,
+						VolumeOptions: &api.VolumeOptions{
+							NoCopy: false,
+						},
+					},
+				},
+			},
+			new: api.ServiceSpec{
+				Volumes: []api.VolumeSpec{
+					{
+						Name: "data",
+						Type: api.VolumeTypeVolume,
+						VolumeOptions: &api.VolumeOptions{
+							NoCopy: true,
+						},
+					},
+				},
+			},
+			expected: ContainerNeedsRecreate,
+		},
+		{
+			name: "change volume options to defaults",
+			current: api.ServiceSpec{
+				Volumes: []api.VolumeSpec{
+					{
+						Name: "data",
+						Type: api.VolumeTypeVolume,
+					},
+				},
+			},
+			new: api.ServiceSpec{
+				Volumes: []api.VolumeSpec{
+					{
+						Name: "data",
+						Type: api.VolumeTypeVolume,
+						VolumeOptions: &api.VolumeOptions{
+							Name: "data",
+							Driver: &mount.Driver{
+								Name: "local",
+							},
+						},
+					},
+				},
+			},
+			expected: ContainerUpToDate,
+		},
+		{
+			name: "change bind option CreateHostPath",
+			current: api.ServiceSpec{
+				Volumes: []api.VolumeSpec{
+					{
+						Name: "config",
+						Type: api.VolumeTypeBind,
+						BindOptions: &api.BindOptions{
+							HostPath: "/host/path",
+						},
+					},
+				},
+			},
+			new: api.ServiceSpec{
+				Volumes: []api.VolumeSpec{
+					{
+						Name: "config",
+						Type: api.VolumeTypeBind,
+						BindOptions: &api.BindOptions{
+							HostPath:       "/host/path",
+							CreateHostPath: true,
+						},
+					},
+				},
+			},
+			// TODO: this doesn't really require a recreate, only a spec update would be sufficient.
+			expected: ContainerNeedsRecreate,
+		},
+		{
+			name: "change bind option propagation",
+			current: api.ServiceSpec{
+				Volumes: []api.VolumeSpec{
+					{
+						Name: "config",
+						Type: api.VolumeTypeBind,
+						BindOptions: &api.BindOptions{
+							HostPath: "/host/path",
+						},
+					},
+				},
+			},
+			new: api.ServiceSpec{
+				Volumes: []api.VolumeSpec{
+					{
+						Name: "config",
+						Type: api.VolumeTypeBind,
+						BindOptions: &api.BindOptions{
+							HostPath:    "/host/path",
+							Propagation: mount.PropagationRPrivate,
+						},
+					},
+				},
+			},
+			// TODO: should we handle rprivate the same as empty propagation, hence up-to-date?
+			expected: ContainerNeedsRecreate,
+		},
+		{
+			name: "change tmpfs options",
+			current: api.ServiceSpec{
+				Volumes: []api.VolumeSpec{
+					{
+						Name: "temp",
+						Type: api.VolumeTypeTmpfs,
+						TmpfsOptions: &mount.TmpfsOptions{
+							SizeBytes: 1024 * 1024,
+						},
+					},
+				},
+			},
+			new: api.ServiceSpec{
+				Volumes: []api.VolumeSpec{
+					{
+						Name: "temp",
+						Type: api.VolumeTypeTmpfs,
+						TmpfsOptions: &mount.TmpfsOptions{
+							SizeBytes: 2 * 1024 * 1024,
+						},
+					},
+				},
+			},
+			expected: ContainerNeedsRecreate,
+		},
+		{
+			name: "changing volume name",
+			current: api.ServiceSpec{
+				Volumes: []api.VolumeSpec{
+					{
+						Name: "data",
+						Type: api.VolumeTypeVolume,
+					},
+				},
+			},
+			new: api.ServiceSpec{
+				Volumes: []api.VolumeSpec{
+					{
+						Name: "data-new",
+						Type: api.VolumeTypeVolume,
+					},
+				},
+			},
+			expected: ContainerNeedsRecreate,
+		},
+		// Tests for volume mounts
+		{
+			name: "identical volume mounts",
+			current: api.ServiceSpec{
+				Volumes: []api.VolumeSpec{
+					{
+						Name: "data",
+						Type: api.VolumeTypeVolume,
+					},
+				},
+				Container: api.ContainerSpec{
+					VolumeMounts: []api.VolumeMount{
+						{
+							VolumeName:    "data",
+							ContainerPath: "/data",
+						},
+					},
+				},
+			},
+			new: api.ServiceSpec{
+				Volumes: []api.VolumeSpec{
+					{
+						Name: "data",
+						Type: api.VolumeTypeVolume,
+					},
+				},
+				Container: api.ContainerSpec{
+					VolumeMounts: []api.VolumeMount{
+						{
+							VolumeName:    "data",
+							ContainerPath: "/data",
+						},
+					},
+				},
+			},
+			expected: ContainerUpToDate,
+		},
+		{
+			name: "volume mounts in different order but identical",
+			current: api.ServiceSpec{
+				Volumes: []api.VolumeSpec{
+					{
+						Name: "data1",
+						Type: api.VolumeTypeVolume,
+					},
+					{
+						Name: "data2",
+						Type: api.VolumeTypeVolume,
+					},
+				},
+				Container: api.ContainerSpec{
+					VolumeMounts: []api.VolumeMount{
+						{
+							VolumeName:    "data1",
+							ContainerPath: "/data1",
+						},
+						{
+							VolumeName:    "data2",
+							ContainerPath: "/data2",
+						},
+					},
+				},
+			},
+			new: api.ServiceSpec{
+				Volumes: []api.VolumeSpec{
+					{
+						Name: "data1",
+						Type: api.VolumeTypeVolume,
+					},
+					{
+						Name: "data2",
+						Type: api.VolumeTypeVolume,
+					},
+				},
+				Container: api.ContainerSpec{
+					VolumeMounts: []api.VolumeMount{
+						{
+							VolumeName:    "data2",
+							ContainerPath: "/data2",
+						},
+						{
+							VolumeName:    "data1",
+							ContainerPath: "/data1",
+						},
+					},
+				},
+			},
+			expected: ContainerUpToDate,
+		},
+		{
+			name: "volumes and volume mounts in different order but identical",
+			current: api.ServiceSpec{
+				Volumes: []api.VolumeSpec{
+					{
+						Name: "data1",
+						Type: api.VolumeTypeVolume,
+					},
+					{
+						Name: "data2",
+						Type: api.VolumeTypeVolume,
+					},
+				},
+				Container: api.ContainerSpec{
+					VolumeMounts: []api.VolumeMount{
+						{
+							VolumeName:    "data1",
+							ContainerPath: "/data1",
+						},
+						{
+							VolumeName:    "data2",
+							ContainerPath: "/data2",
+						},
+					},
+				},
+			},
+			new: api.ServiceSpec{
+				Volumes: []api.VolumeSpec{
+					{
+						Name: "data2",
+						Type: api.VolumeTypeVolume,
+					},
+					{
+						Name: "data1",
+						Type: api.VolumeTypeVolume,
+					},
+				},
+				Container: api.ContainerSpec{
+					VolumeMounts: []api.VolumeMount{
+						{
+							VolumeName:    "data2",
+							ContainerPath: "/data2",
+						},
+						{
+							VolumeName:    "data1",
+							ContainerPath: "/data1",
+						},
+					},
+				},
+			},
+			expected: ContainerUpToDate,
+		},
+		{
+			name: "added volume mount",
+			current: api.ServiceSpec{
+				Volumes: []api.VolumeSpec{
+					{
+						Name: "data",
+						Type: api.VolumeTypeVolume,
+					},
+					{
+						Name: "config",
+						Type: api.VolumeTypeVolume,
+					},
+				},
+				Container: api.ContainerSpec{
+					VolumeMounts: []api.VolumeMount{
+						{
+							VolumeName:    "data",
+							ContainerPath: "/data",
+						},
+					},
+				},
+			},
+			new: api.ServiceSpec{
+				Volumes: []api.VolumeSpec{
+					{
+						Name: "data",
+						Type: api.VolumeTypeVolume,
+					},
+					{
+						Name: "config",
+						Type: api.VolumeTypeVolume,
+					},
+				},
+				Container: api.ContainerSpec{
+					VolumeMounts: []api.VolumeMount{
+						{
+							VolumeName:    "data",
+							ContainerPath: "/data",
+						},
+						{
+							VolumeName:    "config",
+							ContainerPath: "/config",
+						},
+					},
+				},
+			},
+			expected: ContainerNeedsRecreate,
+		},
+		{
+			name: "removed volume mount",
+			current: api.ServiceSpec{
+				Volumes: []api.VolumeSpec{
+					{
+						Name: "data",
+						Type: api.VolumeTypeVolume,
+					},
+					{
+						Name: "config",
+						Type: api.VolumeTypeVolume,
+					},
+				},
+				Container: api.ContainerSpec{
+					VolumeMounts: []api.VolumeMount{
+						{
+							VolumeName:    "data",
+							ContainerPath: "/data",
+						},
+						{
+							VolumeName:    "config",
+							ContainerPath: "/config",
+						},
+					},
+				},
+			},
+			new: api.ServiceSpec{
+				Volumes: []api.VolumeSpec{
+					{
+						Name: "data",
+						Type: api.VolumeTypeVolume,
+					},
+					{
+						Name: "config",
+						Type: api.VolumeTypeVolume,
+					},
+				},
+				Container: api.ContainerSpec{
+					VolumeMounts: []api.VolumeMount{
+						{
+							VolumeName:    "data",
+							ContainerPath: "/data",
+						},
+					},
+				},
+			},
+			expected: ContainerNeedsRecreate,
+		},
+		{
+			name: "changed volume mount container path",
+			current: api.ServiceSpec{
+				Volumes: []api.VolumeSpec{
+					{
+						Name: "data",
+						Type: api.VolumeTypeVolume,
+					},
+				},
+				Container: api.ContainerSpec{
+					VolumeMounts: []api.VolumeMount{
+						{
+							VolumeName:    "data",
+							ContainerPath: "/data",
+						},
+					},
+				},
+			},
+			new: api.ServiceSpec{
+				Volumes: []api.VolumeSpec{
+					{
+						Name: "data",
+						Type: api.VolumeTypeVolume,
+					},
+				},
+				Container: api.ContainerSpec{
+					VolumeMounts: []api.VolumeMount{
+						{
+							VolumeName:    "data",
+							ContainerPath: "/new/data/path",
+						},
+					},
+				},
+			},
+			expected: ContainerNeedsRecreate,
+		},
+		{
+			name: "changed volume mount read-only flag",
+			current: api.ServiceSpec{
+				Volumes: []api.VolumeSpec{
+					{
+						Name: "data",
+						Type: api.VolumeTypeVolume,
+					},
+				},
+				Container: api.ContainerSpec{
+					VolumeMounts: []api.VolumeMount{
+						{
+							VolumeName:    "data",
+							ContainerPath: "/data",
+							ReadOnly:      false,
+						},
+					},
+				},
+			},
+			new: api.ServiceSpec{
+				Volumes: []api.VolumeSpec{
+					{
+						Name: "data",
+						Type: api.VolumeTypeVolume,
+					},
+				},
+				Container: api.ContainerSpec{
+					VolumeMounts: []api.VolumeMount{
+						{
+							VolumeName:    "data",
+							ContainerPath: "/data",
+							ReadOnly:      true,
+						},
+					},
+				},
+			},
+			expected: ContainerNeedsRecreate,
+		},
+		{
+			name: "changed mounted volume type",
+			current: api.ServiceSpec{
+				Volumes: []api.VolumeSpec{
+					{
+						Name: "data",
+						Type: api.VolumeTypeVolume,
+					},
+				},
+				Container: api.ContainerSpec{
+					VolumeMounts: []api.VolumeMount{
+						{
+							VolumeName:    "data",
+							ContainerPath: "/data",
+						},
+					},
+				},
+			},
+			new: api.ServiceSpec{
+				Volumes: []api.VolumeSpec{
+					{
+						Name: "data",
+						Type: api.VolumeTypeBind,
+						BindOptions: &api.BindOptions{
+							HostPath: "/host/path",
+						},
+					},
+				},
+				Container: api.ContainerSpec{
+					VolumeMounts: []api.VolumeMount{
+						{
+							VolumeName:    "data",
+							ContainerPath: "/data",
+						},
+					},
+				},
+			},
+			expected: ContainerNeedsRecreate,
+		},
+		{
+			name: "changed reference name in spec but preserved original volume name",
+			current: api.ServiceSpec{
+				Volumes: []api.VolumeSpec{
+					{
+						Name: "data",
+						Type: api.VolumeTypeVolume,
+					},
+				},
+				Container: api.ContainerSpec{
+					VolumeMounts: []api.VolumeMount{
+						{
+							VolumeName:    "data",
+							ContainerPath: "/data",
+						},
+					},
+				},
+			},
+			new: api.ServiceSpec{
+				Volumes: []api.VolumeSpec{
+					{
+						Name: "new_name",
+						Type: api.VolumeTypeVolume,
+						VolumeOptions: &api.VolumeOptions{
+							Name: "data",
+						},
+					},
+				},
+				Container: api.ContainerSpec{
+					VolumeMounts: []api.VolumeMount{
+						{
+							VolumeName:    "new_name",
+							ContainerPath: "/data",
+						},
+					},
+				},
+			},
+			// TODO: this doesn't really require a recreate, only a spec update would be sufficient.
+			expected: ContainerNeedsRecreate,
+		},
+		{
+			name: "changed volumes, mounts and paths",
+			current: api.ServiceSpec{
+				Volumes: []api.VolumeSpec{
+					{
+						Name: "data",
+						Type: api.VolumeTypeVolume,
+					},
+					{
+						Name: "config",
+						Type: api.VolumeTypeBind,
+						BindOptions: &api.BindOptions{
+							HostPath: "/etc/app/config",
+						},
+					},
+				},
+				Container: api.ContainerSpec{
+					VolumeMounts: []api.VolumeMount{
+						{
+							VolumeName:    "data",
+							ContainerPath: "/data",
+						},
+						{
+							VolumeName:    "config",
+							ContainerPath: "/app/config",
+							ReadOnly:      true,
+						},
+					},
+				},
+			},
+			new: api.ServiceSpec{
+				Volumes: []api.VolumeSpec{
+					{
+						Name: "data",
+						Type: api.VolumeTypeVolume,
+						VolumeOptions: &api.VolumeOptions{
+							NoCopy: true,
+						},
+					},
+					{
+						Name: "config",
+						Type: api.VolumeTypeBind,
+						BindOptions: &api.BindOptions{
+							HostPath:       "/etc/app/new-config",
+							CreateHostPath: true,
+						},
+					},
+					{
+						Name: "logs",
+						Type: api.VolumeTypeVolume,
+					},
+				},
+				Container: api.ContainerSpec{
+					VolumeMounts: []api.VolumeMount{
+						{
+							VolumeName:    "data",
+							ContainerPath: "/var/lib/app/data",
+						},
+						{
+							VolumeName:    "config",
+							ContainerPath: "/app/config",
+							ReadOnly:      true,
+						},
+						{
+							VolumeName:    "logs",
+							ContainerPath: "/var/log/app",
+						},
+					},
+				},
+			},
+			expected: ContainerNeedsRecreate,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := EvalContainerSpecChange(tt.current, tt.new)
+			assert.Equal(t, tt.expected, result)
+		})
+	}
+}