Skip to content

Fix lxc module to operate as per the docs for container memory configuration#42631

Merged
cachedout merged 6 commits intosaltstack:developfrom
jeduardo:lxc-consistency-fixes
Aug 1, 2017
Merged

Fix lxc module to operate as per the docs for container memory configuration#42631
cachedout merged 6 commits intosaltstack:developfrom
jeduardo:lxc-consistency-fixes

Conversation

@jeduardo
Copy link
Contributor

What does this PR do?

It changes the calculation logic for the property lxc.cgroup.memory.limit_in_bytes written to the lxc configuration of a given container to behave in a manner consistent to what is described in the Salt documentation.

What issues does this PR fix or reference?

It changes the behaviour of the lxc module to work as specified in the documentation.

Previous Behavior

The amount of memory that should be available to the container is multiplied by 1024 only once, being converted to KB instead of MB.

New Behavior

The amount of memory that should be available to the container is multiplied by 1024 twice, converting it first to KB and then to MB, allowing the module to work as specified in the configuration.

Tests written?

No

@ghost
Copy link

ghost commented Jul 29, 2017

@jeduardo, thanks for your PR! By analyzing the history of the files in this pull request, we identified @terminalmage, @kiorky and @mgwilliams to be potential reviewers.

@jeduardo jeduardo changed the title Fixing lxc module to receive the memory configuration in MB instead of KB Fix lxc module to receive the memory configuration in MB instead of KB Jul 29, 2017
@jeduardo jeduardo changed the title Fix lxc module to receive the memory configuration in MB instead of KB Fix lxc module to operate as per the docs for container memory configuration Jul 29, 2017
@cachedout
Copy link
Contributor

Go Go Jenkins!

@jeduardo
Copy link
Contributor Author

jeduardo commented Aug 1, 2017

@cachedout a lot of jenkinsing for such a simple change. :) Any idea why it's failing?

@cachedout cachedout merged commit 1b9cb26 into saltstack:develop Aug 1, 2017
@cachedout
Copy link
Contributor

@jeduardo Jenkins is having some issues. This looks fine though and passed the relevant tests.

@jeduardo
Copy link
Contributor Author

jeduardo commented Aug 1, 2017

@cachedout ah excellent. Thanks for merging it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants