From df43eb931e7798264704616b62b3db1b99d4d6a4 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 24 May 2018 01:11:49 +0200 Subject: [PATCH 1/2] Fix cpu/memory limits and reservations being reset on service update Before this change: ---------------------------------------------------- Create a service with reservations and limits for memory and cpu: docker service create --name test \ --limit-memory=100M --limit-cpu=1 \ --reserve-memory=100M --reserve-cpu=1 \ nginx:alpine Verify the configuration docker service inspect --format '{{json .Spec.TaskTemplate.Resources}}' test { "Limits": { "NanoCPUs": 1000000000, "MemoryBytes": 104857600 }, "Reservations": { "NanoCPUs": 1000000000, "MemoryBytes": 104857600 } } Update just CPU limit and reservation: docker service update --limit-cpu=2 --reserve-cpu=2 test Notice that the memory limit and reservation is not preserved: docker service inspect --format '{{json .Spec.TaskTemplate.Resources}}' test { "Limits": { "NanoCPUs": 2000000000 }, "Reservations": { "NanoCPUs": 2000000000 } } Update just Memory limit and reservation: docker service update --limit-memory=200M --reserve-memory=200M test Notice that the CPU limit and reservation is not preserved: docker service inspect --format '{{json .Spec.TaskTemplate.Resources}}' test { "Limits": { "MemoryBytes": 209715200 }, "Reservations": { "MemoryBytes": 209715200 } } After this change: ---------------------------------------------------- Create a service with reservations and limits for memory and cpu: docker service create --name test \ --limit-memory=100M --limit-cpu=1 \ --reserve-memory=100M --reserve-cpu=1 \ nginx:alpine Verify the configuration docker service inspect --format '{{json .Spec.TaskTemplate.Resources}}' test { "Limits": { "NanoCPUs": 1000000000, "MemoryBytes": 104857600 }, "Reservations": { "NanoCPUs": 1000000000, "MemoryBytes": 104857600 } } Update just CPU limit and reservation: docker service update --limit-cpu=2 --reserve-cpu=2 test Confirm that the CPU limits/reservations are updated, but memory limit and reservation are preserved: docker service inspect --format '{{json .Spec.TaskTemplate.Resources}}' test { "Limits": { "NanoCPUs": 2000000000, "MemoryBytes": 104857600 }, "Reservations": { "NanoCPUs": 2000000000, "MemoryBytes": 104857600 } } Update just Memory limit and reservation: docker service update --limit-memory=200M --reserve-memory=200M test Confirm that the Mempry limits/reservations are updated, but CPU limit and reservation are preserved: docker service inspect --format '{{json .Spec.TaskTemplate.Resources}}' test { "Limits": { "NanoCPUs": 2000000000, "MemoryBytes": 209715200 }, "Reservations": { "NanoCPUs": 2000000000, "MemoryBytes": 209715200 } } Signed-off-by: Sebastiaan van Stijn Signed-off-by: Sebastiaan van Stijn --- cli/command/service/update.go | 4 +-- cli/command/service/update_test.go | 44 ++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/cli/command/service/update.go b/cli/command/service/update.go index 0a808619be3e..1124d703e139 100644 --- a/cli/command/service/update.go +++ b/cli/command/service/update.go @@ -314,12 +314,12 @@ func updateService(ctx context.Context, apiClient client.NetworkAPIClient, flags } if flags.Changed(flagLimitCPU) || flags.Changed(flagLimitMemory) { - taskResources().Limits = &swarm.Resources{} + taskResources().Limits = spec.TaskTemplate.Resources.Limits updateInt64Value(flagLimitCPU, &task.Resources.Limits.NanoCPUs) updateInt64Value(flagLimitMemory, &task.Resources.Limits.MemoryBytes) } if flags.Changed(flagReserveCPU) || flags.Changed(flagReserveMemory) { - taskResources().Reservations = &swarm.Resources{} + taskResources().Reservations = spec.TaskTemplate.Resources.Reservations updateInt64Value(flagReserveCPU, &task.Resources.Reservations.NanoCPUs) updateInt64Value(flagReserveMemory, &task.Resources.Reservations.MemoryBytes) } diff --git a/cli/command/service/update_test.go b/cli/command/service/update_test.go index 8536dd3a7309..94ccb4963fa5 100644 --- a/cli/command/service/update_test.go +++ b/cli/command/service/update_test.go @@ -587,6 +587,50 @@ func TestUpdateIsolationValid(t *testing.T) { assert.Check(t, is.Equal(container.IsolationProcess, spec.TaskTemplate.ContainerSpec.Isolation)) } +// TestUpdateLimitsReservations tests that limits and reservations are updated, +// and that values are not updated are not reset to their default value +func TestUpdateLimitsReservations(t *testing.T) { + spec := swarm.ServiceSpec{ + TaskTemplate: swarm.TaskSpec{ + ContainerSpec: &swarm.ContainerSpec{}, + Resources: &swarm.ResourceRequirements{ + Limits: &swarm.Resources{ + NanoCPUs: 1000000000, + MemoryBytes: 104857600, + }, + Reservations: &swarm.Resources{ + NanoCPUs: 1000000000, + MemoryBytes: 104857600, + }, + }, + }, + } + + flags := newUpdateCommand(nil).Flags() + err := flags.Set(flagLimitCPU, "2") + assert.NilError(t, err) + err = flags.Set(flagReserveCPU, "2") + assert.NilError(t, err) + err = updateService(context.Background(), nil, flags, &spec) + assert.NilError(t, err) + assert.Check(t, is.Equal(spec.TaskTemplate.Resources.Limits.NanoCPUs, int64(2000000000))) + assert.Check(t, is.Equal(spec.TaskTemplate.Resources.Limits.MemoryBytes, int64(104857600))) + assert.Check(t, is.Equal(spec.TaskTemplate.Resources.Reservations.NanoCPUs, int64(2000000000))) + assert.Check(t, is.Equal(spec.TaskTemplate.Resources.Reservations.MemoryBytes, int64(104857600))) + + flags = newUpdateCommand(nil).Flags() + err = flags.Set(flagLimitMemory, "200M") + assert.NilError(t, err) + err = flags.Set(flagReserveMemory, "200M") + assert.NilError(t, err) + err = updateService(context.Background(), nil, flags, &spec) + assert.NilError(t, err) + assert.Check(t, is.Equal(spec.TaskTemplate.Resources.Limits.NanoCPUs, int64(2000000000))) + assert.Check(t, is.Equal(spec.TaskTemplate.Resources.Limits.MemoryBytes, int64(209715200))) + assert.Check(t, is.Equal(spec.TaskTemplate.Resources.Reservations.NanoCPUs, int64(2000000000))) + assert.Check(t, is.Equal(spec.TaskTemplate.Resources.Reservations.MemoryBytes, int64(209715200))) +} + func TestUpdateIsolationInvalid(t *testing.T) { // validation depends on daemon os / version so validation should be done on the daemon side flags := newUpdateCommand(nil).Flags() From df9a0c77975a1c0a3ccff57303caac94139bb533 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 24 May 2018 01:13:35 +0200 Subject: [PATCH 2/2] Minor refactor: use anyChanged() to detect changed flags Signed-off-by: Sebastiaan van Stijn --- cli/command/service/update.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/cli/command/service/update.go b/cli/command/service/update.go index 1124d703e139..66494fdbb9e3 100644 --- a/cli/command/service/update.go +++ b/cli/command/service/update.go @@ -313,12 +313,13 @@ func updateService(ctx context.Context, apiClient client.NetworkAPIClient, flags return err } - if flags.Changed(flagLimitCPU) || flags.Changed(flagLimitMemory) { + if anyChanged(flags, flagLimitCPU, flagLimitMemory) { taskResources().Limits = spec.TaskTemplate.Resources.Limits updateInt64Value(flagLimitCPU, &task.Resources.Limits.NanoCPUs) updateInt64Value(flagLimitMemory, &task.Resources.Limits.MemoryBytes) } - if flags.Changed(flagReserveCPU) || flags.Changed(flagReserveMemory) { + + if anyChanged(flags, flagReserveCPU, flagReserveMemory) { taskResources().Reservations = spec.TaskTemplate.Resources.Reservations updateInt64Value(flagReserveCPU, &task.Resources.Reservations.NanoCPUs) updateInt64Value(flagReserveMemory, &task.Resources.Reservations.MemoryBytes)