Add memory swap to swarm #6619
Add memory swap to swarm #6619
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
045f838 to
7fe1bc0
Compare
| }, | ||
| "memswap_limit": { | ||
| "type": "integer" | ||
| }, | ||
| "mem_swappiness": { | ||
| "type": "integer" |
There was a problem hiding this comment.
This probably needs a new version of the schema; to do so;
- add a commit that adds a straight copy of the existing schema and update the default version
- then apply this diff to the copy
Doing it in 2 commits helps making the diff reviewable
There was a problem hiding this comment.
See this commit (and PR) for the steps;
7fe1bc0 to
8fff7e8
Compare
8fff7e8 to
f81fff5
Compare
Signed-off-by: Sebastiaan van Stijn <[email protected]>
Adds support for setting memory swap settings on Swarm services * Adds flags `memory-swap` and `memory-swappiness` to `docker service create` and `docker service update` commands. * Adds compose fields `memswap_limit` and `mem_swappiness` for `docker stack` commands. Signed-off-by: Drew Erny <[email protected]> Signed-off-by: Sebastiaan van Stijn <[email protected]>
f81fff5 to
71828f2
Compare
|
|
||
| const ( | ||
| defaultVersion = "3.13" | ||
| defaultVersion = "3.14" |
There was a problem hiding this comment.
:-1 this isn't relevant anymore with Compose specification
There was a problem hiding this comment.
Yeah, the schema is optional in this implementation, but allows for downgrading to a specific version.
There was a problem hiding this comment.
allows for downgrading to a specific version
Please clarify. The reason compose-spec was made "version-less" is exactly because such "downgrade" doesn't bring any value. There's no place in the many versions where an attribute definition changed - some where removed (but re-introduced in the compose-spec schema) but none had definition updated that would make "downgrading to a specific version" bring any value
|
some thoughts:
|
Good call; let me update this; it didn't ship yet in a non-pre release, so we can update.
Noted. We had to make a call, and it was nearly 2AM 🙈 ; inclusion of this feature (at least in the API) was agreed on to be a release-blocker, but the CLI-side was not merged yet; I made preparations to split out the schema changes #6619 (comment), but didn't have time to remove the compose / stack changes separate, so we went ahead and brought this in for the RC. I can either revert all compose changes, and only keep the CLI side, or if changing the names to Let me at least start with those renames. |
| Limits *ResourceLimit `yaml:",omitempty" json:"limits,omitempty"` | ||
| Reservations *Resource `yaml:",omitempty" json:"reservations,omitempty"` | ||
| MemswapLimit *int64 `mapstructure:"memswap_limit" yaml:"memswap_limit,omitempty" json:"memswap_limit,omitempty"` | ||
| MemSwappiness *int64 `mapstructure:"mem_swappiness" yaml:"mem_swappiness,omitempty" json:"mem_swappiness,omitempty"` |
There was a problem hiding this comment.
Why introduce those, while service definition already defines memswap_limit and mem_swappiness?
This duplication will just bring more confusion to the compose syntax
|
On the compose-spec side, |
- What I did
Adds support for setting memory swap settings on Swarm services
memory-swapandmemory-swappinesstodocker service createanddocker service updatecommands.memswap_limitandmem_swappinessfordocker stackcommands.- How I did it
Ran through all the plumbing as one does any time such a flag is added.
- How to verify it
Includes tests.
- Human readable description for the release notes