-
Notifications
You must be signed in to change notification settings - Fork 2k
Fix cpu/memory limits and reservations being reset on service update #1079
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix cpu/memory limits and reservations being reset on service update #1079
Conversation
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 <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
22c9a9e to
df9a0c7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🦁
| 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() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: split it into 2 subtests?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mean: only change (eg) reservation so that we can catch regressions where changing a reservation would reset limits?
Actually thought about that, don't know why I didn't do that; let me know if you want that updated in this PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, in case of failure it points directly to the right sub-test. But it's only a small nit, no need to update it 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM

Formed in 2009, the Archive Team (not to be confused with the archive.org Archive-It Team) is a rogue archivist collective dedicated to saving copies of rapidly dying or deleted websites for the sake of history and digital heritage. The group is 100% composed of volunteers and interested parties, and has expanded into a large amount of related projects for saving online and digital history.

Fix cpu/memory limits and reservations being reset on service update
fixes moby/moby#37036
fixes moby/moby#37037
Before this change:
Create a service with reservations and limits for memory and cpu:
docker service create \ --limit-memory=100M \ --limit-cpu=1 \ --reserve-memory=100M \ --reserve-cpu=1 \ --name test \ nginx:alpineVerify the configuration
{ "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 testNotice that the memory limit and reservation is not preserved:
{ "Limits": { "NanoCPUs": 2000000000 }, "Reservations": { "NanoCPUs": 2000000000 } }Update just Memory limit and reservation:
docker service update --limit-memory=200M --reserve-memory=200M testNotice that the CPU limit and reservation is not preserved:
{ "Limits": { "MemoryBytes": 209715200 }, "Reservations": { "MemoryBytes": 209715200 } }After this change:
Create a service with reservations and limits for memory and cpu:
docker service create \ --limit-memory=100M \ --limit-cpu=1 \ --reserve-memory=100M \ --reserve-cpu=1 \ --name test \ nginx:alpineVerify the configuration
{ "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 testConfirm that the CPU limits/reservations are updated, but memory limit and reservation are preserved:
{ "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 testConfirm that the Mempry limits/reservations are updated, but CPU limit and reservation are preserved:
{ "Limits": { "NanoCPUs": 2000000000, "MemoryBytes": 209715200 }, "Reservations": { "NanoCPUs": 2000000000, "MemoryBytes": 209715200 } }