diff --git a/pkg/api/resources.go b/pkg/api/resources.go index 096585e8..db33456d 100644 --- a/pkg/api/resources.go +++ b/pkg/api/resources.go @@ -1,14 +1,11 @@ package api -type CPUResources struct { - // Limit is the maximum amount of CPU nanocores (1000000000 = 1 CPU core) the container can use. - Limit int64 -} - -type MemoryResources struct { - // Limit is the maximum amount of memory (in bytes) the container can use. - Limit int64 - // Reservation is the minimum amount of memory (in bytes) the container needs to run efficiently. +type ContainerResources struct { + // CPU is the maximum amount of CPU nanocores (1000000000 = 1 CPU core) the container can use. + CPU int64 + // Memory is the maximum amount of memory (in bytes) the container can use. + Memory int64 + // MemoryReservation is the minimum amount of memory (in bytes) the container needs to run efficiently. // TODO: implement a placement constraint that checks available memory on machines. - Reservation int64 + MemoryReservation int64 } diff --git a/pkg/api/service.go b/pkg/api/service.go index 7a50496d..ca21e5fe 100644 --- a/pkg/api/service.go +++ b/pkg/api/service.go @@ -162,8 +162,6 @@ func (s *ServiceSpec) Clone() ServiceSpec { type ContainerSpec struct { // Command overrides the default CMD of the image to be executed when running a container. Command []string - // CPU resource allocation for the container. - CPU CPUResources // Entrypoint overrides the default ENTRYPOINT of the image. Entrypoint []string // Env defines the environment variables to set inside the container. @@ -173,13 +171,13 @@ type ContainerSpec struct { Init *bool // LogDriver overrides the default logging driver for the container. Each Docker daemon can have its own default. LogDriver *LogDriver - // Memory resource allocation for the container. - Memory MemoryResources // Privileged gives extended privileges to the container. This is a security risk and should be used with caution. Privileged bool // PullPolicy determines when to pull the image from the registry or use the image already available in the cluster. // Default is PullPolicyMissing if empty. PullPolicy string + // Resource allocation for the container. + Resources ContainerResources // User overrides the default user of the image used to run the container. Format: user|UID[:group|GID]. User string // VolumeMounts specifies how volumes are mounted into the container filesystem. diff --git a/pkg/client/deploy/container.go b/pkg/client/deploy/container.go index 51464cef..994bff29 100644 --- a/pkg/client/deploy/container.go +++ b/pkg/client/deploy/container.go @@ -29,12 +29,10 @@ func EvalContainerSpecChange(current api.ServiceSpec, new api.ServiceSpec) Conta // Pull policy doesn't affect the container configuration. new.Container.PullPolicy = current.Container.PullPolicy - // Save mutable container properties that can be updated without recreation. - newCPU := new.Container.CPU - newMemory := new.Container.Memory - // Temporarily set mutable container properties to current values to check if other properties changed. - new.Container.CPU = current.Container.CPU - new.Container.Memory = current.Container.Memory + // Save mutable container resources that can be updated without recreation. + newResources := new.Container.Resources + // Temporarily set mutable container resources to current values to check if other properties changed. + new.Container.Resources = current.Container.Resources // Check if immutable container properties changed. if !current.Container.Equals(new.Container) { @@ -69,7 +67,7 @@ func EvalContainerSpecChange(current api.ServiceSpec, new api.ServiceSpec) Conta } // Check if any mutable properties changed. - if !reflect.DeepEqual(current.Container.CPU, newCPU) || !reflect.DeepEqual(current.Container.Memory, newMemory) { + if !reflect.DeepEqual(current.Container.Resources, newResources) { return ContainerNeedsUpdate } diff --git a/pkg/client/deploy/container_test.go b/pkg/client/deploy/container_test.go index 7510cb36..f806a4c6 100644 --- a/pkg/client/deploy/container_test.go +++ b/pkg/client/deploy/container_test.go @@ -8,39 +8,145 @@ import ( "github.com/stretchr/testify/assert" ) -func TestEvalContainerSpecChange_ContainerCPU(t *testing.T) { +func TestEvalContainerSpecChange_ContainerResources(t *testing.T) { t.Parallel() tests := []struct { name string - current api.CPUResources - new api.CPUResources + current api.ContainerResources + new api.ContainerResources want ContainerSpecStatus }{ { - name: "set limit", - current: api.CPUResources{}, - new: api.CPUResources{ - Limit: 1000000000, + name: "empty", + current: api.ContainerResources{}, + new: api.ContainerResources{}, + want: ContainerUpToDate, + }, + + // CPU + { + name: "set CPU", + current: api.ContainerResources{}, + new: api.ContainerResources{ + CPU: 1000000000, }, want: ContainerNeedsUpdate, }, { - name: "change limit", - current: api.CPUResources{ - Limit: 1000000000, + name: "change CPU", + current: api.ContainerResources{ + CPU: 1000000000, }, - new: api.CPUResources{ - Limit: 2000000000, + new: api.ContainerResources{ + CPU: 2000000000, }, want: ContainerNeedsUpdate, }, { - name: "unset limit", - current: api.CPUResources{ - Limit: 1000000000, + name: "unset CPU", + current: api.ContainerResources{ + CPU: 1000000000, + }, + new: api.ContainerResources{}, + want: ContainerNeedsUpdate, + }, + + // Memory + { + name: "equal memory", + current: api.ContainerResources{Memory: 100 * 1024 * 1024}, + new: api.ContainerResources{Memory: 100 * 1024 * 1024}, + want: ContainerUpToDate, + }, + { + name: "equal reservation", + current: api.ContainerResources{MemoryReservation: 50 * 1024 * 1024}, + new: api.ContainerResources{MemoryReservation: 50 * 1024 * 1024}, + want: ContainerUpToDate, + }, + { + name: "equal memory and reservation", + current: api.ContainerResources{Memory: 100 * 1024 * 1024, MemoryReservation: 50 * 1024 * 1024}, + new: api.ContainerResources{Memory: 100 * 1024 * 1024, MemoryReservation: 50 * 1024 * 1024}, + want: ContainerUpToDate, + }, + { + name: "set memory", + current: api.ContainerResources{}, + new: api.ContainerResources{Memory: 100 * 1024 * 1024}, + want: ContainerNeedsUpdate, + }, + { + name: "set reservation", + current: api.ContainerResources{}, + new: api.ContainerResources{MemoryReservation: 50 * 1024 * 1024}, + want: ContainerNeedsUpdate, + }, + { + name: "set memory and reservation", + current: api.ContainerResources{}, + new: api.ContainerResources{Memory: 100 * 1024 * 1024, MemoryReservation: 50 * 1024 * 1024}, + want: ContainerNeedsUpdate, + }, + { + name: "change memory", + current: api.ContainerResources{Memory: 100 * 1024 * 1024}, + new: api.ContainerResources{Memory: 200 * 1024 * 1024}, + want: ContainerNeedsUpdate, + }, + { + name: "change reservation", + current: api.ContainerResources{MemoryReservation: 50 * 1024 * 1024}, + new: api.ContainerResources{MemoryReservation: 100 * 1024 * 1024}, + want: ContainerNeedsUpdate, + }, + { + name: "change memory and reservation", + current: api.ContainerResources{Memory: 100 * 1024 * 1024, MemoryReservation: 50 * 1024 * 1024}, + new: api.ContainerResources{Memory: 200 * 1024 * 1024, MemoryReservation: 100 * 1024 * 1024}, + want: ContainerNeedsUpdate, + }, + { + name: "unset memory", + current: api.ContainerResources{Memory: 100 * 1024 * 1024, MemoryReservation: 50 * 1024 * 1024}, + new: api.ContainerResources{MemoryReservation: 50 * 1024 * 1024}, + want: ContainerNeedsUpdate, + }, + { + name: "unset reservation", + current: api.ContainerResources{Memory: 100 * 1024 * 1024, MemoryReservation: 50 * 1024 * 1024}, + new: api.ContainerResources{Memory: 100 * 1024 * 1024}, + want: ContainerNeedsUpdate, + }, + { + name: "unset memory and reservation", + current: api.ContainerResources{Memory: 100 * 1024 * 1024, MemoryReservation: 50 * 1024 * 1024}, + new: api.ContainerResources{}, + want: ContainerNeedsUpdate, + }, + + // Memory and CPU + { + name: "set CPU and memory", + current: api.ContainerResources{}, + new: api.ContainerResources{ + CPU: 1000000000, + Memory: 100 * 1024 * 1024, + }, + want: ContainerNeedsUpdate, + }, + { + name: "update CPU and memory", + current: api.ContainerResources{ + CPU: 1000000000, + Memory: 100 * 1024 * 1024, + }, + new: api.ContainerResources{ + CPU: 2000000000, + Memory: 200 * 1024 * 1024, + MemoryReservation: 100 * 1024 * 1024, }, - new: api.CPUResources{}, want: ContainerNeedsUpdate, }, } @@ -49,14 +155,14 @@ func TestEvalContainerSpecChange_ContainerCPU(t *testing.T) { t.Run(tt.name, func(t *testing.T) { currentSpec := api.ServiceSpec{ Container: api.ContainerSpec{ - Image: "nginx:latest", - CPU: tt.current, + Image: "nginx:latest", + Resources: tt.current, }, } newSpec := api.ServiceSpec{ Container: api.ContainerSpec{ - Image: "nginx:latest", - CPU: tt.new, + Image: "nginx:latest", + Resources: tt.new, }, } @@ -199,116 +305,6 @@ func TestEvalContainerSpecChange_ContainerLogDriver(t *testing.T) { } } -func TestEvalContainerSpecChange_ContainerMemory(t *testing.T) { - t.Parallel() - - tests := []struct { - name string - current api.MemoryResources - new api.MemoryResources - want ContainerSpecStatus - }{ - { - name: "empty", - current: api.MemoryResources{}, - new: api.MemoryResources{}, - want: ContainerUpToDate, - }, - { - name: "equal limit", - current: api.MemoryResources{Limit: 100 * 1024 * 1024}, - new: api.MemoryResources{Limit: 100 * 1024 * 1024}, - want: ContainerUpToDate, - }, - { - name: "equal reservation", - current: api.MemoryResources{Reservation: 50 * 1024 * 1024}, - new: api.MemoryResources{Reservation: 50 * 1024 * 1024}, - want: ContainerUpToDate, - }, - { - name: "equal limit and reservation", - current: api.MemoryResources{Limit: 100 * 1024 * 1024, Reservation: 50 * 1024 * 1024}, - new: api.MemoryResources{Limit: 100 * 1024 * 1024, Reservation: 50 * 1024 * 1024}, - want: ContainerUpToDate, - }, - { - name: "set limit", - current: api.MemoryResources{}, - new: api.MemoryResources{Limit: 100 * 1024 * 1024}, - want: ContainerNeedsUpdate, - }, - { - name: "set reservation", - current: api.MemoryResources{}, - new: api.MemoryResources{Reservation: 50 * 1024 * 1024}, - want: ContainerNeedsUpdate, - }, - { - name: "set limit and reservation", - current: api.MemoryResources{}, - new: api.MemoryResources{Limit: 100 * 1024 * 1024, Reservation: 50 * 1024 * 1024}, - want: ContainerNeedsUpdate, - }, - { - name: "change limit", - current: api.MemoryResources{Limit: 100 * 1024 * 1024}, - new: api.MemoryResources{Limit: 200 * 1024 * 1024}, - want: ContainerNeedsUpdate, - }, - { - name: "change reservation", - current: api.MemoryResources{Reservation: 50 * 1024 * 1024}, - new: api.MemoryResources{Reservation: 100 * 1024 * 1024}, - want: ContainerNeedsUpdate, - }, - { - name: "change limit and reservation", - current: api.MemoryResources{Limit: 100 * 1024 * 1024, Reservation: 50 * 1024 * 1024}, - new: api.MemoryResources{Limit: 200 * 1024 * 1024, Reservation: 100 * 1024 * 1024}, - want: ContainerNeedsUpdate, - }, - { - name: "unset limit", - current: api.MemoryResources{Limit: 100 * 1024 * 1024, Reservation: 50 * 1024 * 1024}, - new: api.MemoryResources{Reservation: 50 * 1024 * 1024}, - want: ContainerNeedsUpdate, - }, - { - name: "unset reservation", - current: api.MemoryResources{Limit: 100 * 1024 * 1024, Reservation: 50 * 1024 * 1024}, - new: api.MemoryResources{Limit: 100 * 1024 * 1024}, - want: ContainerNeedsUpdate, - }, - { - name: "unset limit and reservation", - current: api.MemoryResources{Limit: 100 * 1024 * 1024, Reservation: 50 * 1024 * 1024}, - new: api.MemoryResources{}, - want: ContainerNeedsUpdate, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - currentSpec := api.ServiceSpec{ - Container: api.ContainerSpec{ - Image: "nginx:latest", - Memory: tt.current, - }, - } - newSpec := api.ServiceSpec{ - Container: api.ContainerSpec{ - Image: "nginx:latest", - Memory: tt.new, - }, - } - - result := EvalContainerSpecChange(currentSpec, newSpec) - assert.Equal(t, tt.want, result) - }) - } -} - func TestEvalContainerSpecChange_ContainerPrivileged(t *testing.T) { t.Parallel() @@ -1321,20 +1317,18 @@ func TestEvalContainerSpecChange_Mixed(t *testing.T) { current: api.ServiceSpec{ Container: api.ContainerSpec{ Image: "nginx:latest", - Memory: api.MemoryResources{ - Limit: 100 * 1024 * 1024, + Resources: api.ContainerResources{ + Memory: 100 * 1024 * 1024, }, }, }, new: api.ServiceSpec{ Container: api.ContainerSpec{ Image: "nginx:latest", - CPU: api.CPUResources{ - Limit: 1000000000, - }, - Memory: api.MemoryResources{ - Limit: 200 * 1024 * 1024, - Reservation: 100 * 1024 * 1024, + Resources: api.ContainerResources{ + CPU: 1000000000, + Memory: 200 * 1024 * 1024, + MemoryReservation: 100 * 1024 * 1024, }, }, }, @@ -1350,8 +1344,8 @@ func TestEvalContainerSpecChange_Mixed(t *testing.T) { new: api.ServiceSpec{ Container: api.ContainerSpec{ Image: "nginx:latest", - CPU: api.CPUResources{ - Limit: 1000000000, + Resources: api.ContainerResources{ + CPU: 1000000000, }, User: "root", },