diff --git a/internal/machine/docker/server.go b/internal/machine/docker/server.go index d36f3a37..a731b010 100644 --- a/internal/machine/docker/server.go +++ b/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) { -// mounts := make([]mount.Mount, 0, len(volumes)) -// for _, vol := range volumes { -// m := mount.Mount{ -// Type: mount.Type(vol.Type), -// Source: vol.Source, -// Target: vol.Target, -// ReadOnly: vol.ReadOnly, -// } -// -// // Set type-specific options. -// switch vol.Type { -// case api.VolumeTypeBind: -// if vol.BindOptions != nil { -// m.BindOptions = &mount.BindOptions{ -// Propagation: vol.BindOptions.Propagation, -// NonRecursive: false, -// } -// if vol.BindOptions.CreateHostPath { -// m.BindOptions.CreateMountpoint = true -// } -// // Handle SELinux options if specified -// if vol.BindOptions.SELinux == api.SELinuxShared { -// m.BindOptions.Propagation = mount.PropagationShared -// } else if vol.BindOptions.SELinux == api.SELinuxUnshared { -// m.BindOptions.Propagation = mount.PropagationPrivate -// } -// } -// case api.VolumeTypeVolume: -// if vol.VolumeOptions != nil { -// m.VolumeOptions = &mount.VolumeOptions{ -// NoCopy: vol.VolumeOptions.NoCopy, -// Labels: vol.VolumeOptions.Labels, -// DriverConfig: vol.VolumeOptions.Driver, -// Subpath: vol.VolumeOptions.Subpath, -// } -// } -// case api.VolumeTypeTmpfs: -// m.TmpfsOptions = vol.TmpfsOptions -// default: -// return nil, fmt.Errorf("invalid volume type: '%s' (must be one of %s, %s, %s)", -// vol.Type, api.VolumeTypeBind, api.VolumeTypeVolume, api.VolumeTypeTmpfs) -// } -// -// mounts = append(mounts, m) -// } -// return mounts -//} +func toDockerMounts(volumes []api.VolumeSpec, mounts []api.VolumeMount) ([]mount.Mount, error) { + dockerMounts := make([]mount.Mount, 0, len(mounts)) + for _, m := range mounts { + idx := slices.IndexFunc(volumes, func(v api.VolumeSpec) bool { + return v.Name == m.VolumeName + }) + if idx == -1 { + return nil, fmt.Errorf("volume mount references a volume that doesn't exist in the volumes spec: '%s'", + m.VolumeName) + } + + vol := volumes[idx] + if err := vol.Validate(); err != nil { + return nil, fmt.Errorf("invalid volume: %w", err) + } + + dm := mount.Mount{ + Type: mount.Type(vol.Type), + Target: m.ContainerPath, + ReadOnly: m.ReadOnly, + } + + switch vol.Type { + case api.VolumeTypeBind: + dm.Source = vol.BindOptions.HostPath + dm.BindOptions = toDockerBindOptions(vol.BindOptions) + case api.VolumeTypeVolume: + dm.Source = vol.Name + + if vol.VolumeOptions != nil { + dm.VolumeOptions = &mount.VolumeOptions{ + NoCopy: vol.VolumeOptions.NoCopy, + Labels: vol.VolumeOptions.Labels, + Subpath: vol.VolumeOptions.SubPath, + DriverConfig: vol.VolumeOptions.Driver, + } + + if vol.VolumeOptions.Name != "" { + dm.Source = vol.VolumeOptions.Name + } + } + case api.VolumeTypeTmpfs: + dm.TmpfsOptions = vol.TmpfsOptions + default: + return nil, fmt.Errorf("unsupported volume type: '%s'", vol.Type) + } + + 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. diff --git a/pkg/api/service.go b/pkg/api/service.go index 231a4f89..4c7dfc31 100644 --- a/pkg/api/service.go +++ b/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 } diff --git a/pkg/api/volume.go b/pkg/api/volume.go index df66fd6f..e52f79d6 100644 --- a/pkg/api/volume.go +++ b/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"` - Propagation mount.Propagation `json:",omitempty"` - SELinux string `json:",omitempty"` + CreateHostPath bool `json:",omitempty"` + Propagation mount.Propagation `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. - // 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 specifies the volume driver and its options for volume creation. 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 + }) +} diff --git a/pkg/client/deploy/container.go b/pkg/client/deploy/container.go index fd25af27..eae037a8 100644 --- a/pkg/client/deploy/container.go +++ b/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 + }) +} diff --git a/pkg/client/deploy/container_test.go b/pkg/client/deploy/container_test.go new file mode 100644 index 00000000..c56a42d0 --- /dev/null +++ b/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) + }) + } +}