refactor: cpu and memory under Resources struct

pkg/api/resources.go

@@ -1,14 +1,11 @@
 package api
 
-type CPUResources struct {
+type ContainerResources struct {
-	// Limit is the maximum amount of CPU nanocores (1000000000 = 1 CPU core) the container can use.
+	// CPU is the maximum amount of CPU nanocores (1000000000 = 1 CPU core) the container can use.
-	Limit int64
+	CPU int64
-}
+	// Memory is the maximum amount of memory (in bytes) the container can use.
-
+	Memory int64
-type MemoryResources struct {
+	// MemoryReservation is the minimum amount of memory (in bytes) the container needs to run efficiently.
-	// 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.
 	// TODO: implement a placement constraint that checks available memory on machines.
-	Reservation int64
+	MemoryReservation int64
 }

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.

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.
+	// Save mutable container resources that can be updated without recreation.
-	newCPU := new.Container.CPU
+	newResources := new.Container.Resources
-	newMemory := new.Container.Memory
+	// Temporarily set mutable container resources to current values to check if other properties changed.
-	// Temporarily set mutable container properties to current values to check if other properties changed.
+	new.Container.Resources = current.Container.Resources
-	new.Container.CPU = current.Container.CPU
-	new.Container.Memory = current.Container.Memory
 
 	// 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
 	}
 

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
+		current api.ContainerResources
-		new     api.CPUResources
+		new     api.ContainerResources
 		want    ContainerSpecStatus
 	}{
 		{
-			name:    "set limit",
+			name:    "empty",
-			current: api.CPUResources{},
+			current: api.ContainerResources{},
-			new: api.CPUResources{
+			new:     api.ContainerResources{},
-				Limit: 1000000000,
+			want:    ContainerUpToDate,
+		},
+
+		// CPU
+		{
+			name:    "set CPU",
+			current: api.ContainerResources{},
+			new: api.ContainerResources{
+				CPU: 1000000000,
 			},
 			want: ContainerNeedsUpdate,
 		},
 		{
-			name: "change limit",
+			name: "change CPU",
-			current: api.CPUResources{
+			current: api.ContainerResources{
-				Limit: 1000000000,
+				CPU: 1000000000,
 			},
-			new: api.CPUResources{
+			new: api.ContainerResources{
-				Limit: 2000000000,
+				CPU: 2000000000,
 			},
 			want: ContainerNeedsUpdate,
 		},
 		{
-			name: "unset limit",
+			name: "unset CPU",
-			current: api.CPUResources{
+			current: api.ContainerResources{
-				Limit: 1000000000,
+				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",
+					Image:     "nginx:latest",
-					CPU:   tt.current,
+					Resources: tt.current,
 				},
 			}
 			newSpec := api.ServiceSpec{
 				Container: api.ContainerSpec{
-					Image: "nginx:latest",
+					Image:     "nginx:latest",
-					CPU:   tt.new,
+					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{
+					Resources: api.ContainerResources{
-						Limit: 100 * 1024 * 1024,
+						Memory: 100 * 1024 * 1024,
 					},
 				},
 			},
 			new: api.ServiceSpec{
 				Container: api.ContainerSpec{
 					Image: "nginx:latest",
-					CPU: api.CPUResources{
+					Resources: api.ContainerResources{
-						Limit: 1000000000,
+						CPU:               1000000000,
-					},
+						Memory:            200 * 1024 * 1024,
-					Memory: api.MemoryResources{
+						MemoryReservation: 100 * 1024 * 1024,
-						Limit:       200 * 1024 * 1024,
-						Reservation: 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{
+					Resources: api.ContainerResources{
-						Limit: 1000000000,
+						CPU: 1000000000,
 					},
 					User: "root",
 				},