Checking whether CG_MEM_SWAP is enabled at the moment. #28634
Conversation
|
/cc @justincormack |
GordonTheTurtle
added
the
dco/no
ghostplant
force-pushed
the
check_cfg_memsw
branch
from
45c7d71
to
8025513
Compare
GordonTheTurtle
added
the
dco/no
ghostplant
force-pushed
the
check_cfg_memsw
branch
from
8025513
to
bff0d18
Compare
GordonTheTurtle
removed
the
dco/no
| if [ -e /sys/fs/cgroup/memory/memory.memsw.limit_in_bytes ]; then | ||
| echo " $(wrap_color '(cgroup swap accounting is currently enabled)' bold black)" | ||
| elif is_set MEMCG_SWAP && ! is_set MEMCG_SWAP_ENABLED; then | ||
| echo " $(wrap_color '(cgroup swap accounting is currently not enabled, you can enable it by setting boot option "swapaccount=1")' bold black)" |
There was a problem hiding this comment.
This could cause the script to fail, to be consistent.
There was a problem hiding this comment.
What kind of fail is reported?
There was a problem hiding this comment.
its just an exit code, 0 for ok, 1 for there is a problem you should fix. Lets you use it in CI tests.
There was a problem hiding this comment.
Yes, and I have updated the code whose root-cause is check_flags missing.
If the exit code is still not 0 after the fix, it must be caused by other checking flags missing, not relevant to this change. Thanks!
Signed-off-by: CUI Wei <ghostplant@qq.com>
ghostplant
force-pushed
the
check_cfg_memsw
branch
from
bff0d18
to
cd7496a
Compare
|
Any comments? |
|
I made it so that it passed if you had at least one storage driver. Lots of
people do not have zfs so failing would not be helpful.
…On 24 Nov 2016 7:56 a.m., "ghostplant" ***@***.***> wrote:
Any comments?
It seems that many cases such as 'no zfs' could also make EXITCODE=1
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#28634 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAdcPFk3TnO-qs0Ea50F8aRafHiUy2kfks5rBUMlgaJpZM4K3lUx>
.
|
| echo " $(wrap_color '(note that cgroup swap accounting is not enabled in your kernel config, you can enable it by setting boot option "swapaccount=1")' bold black)" | ||
| if [ -e /sys/fs/cgroup/memory/memory.memsw.limit_in_bytes ]; then | ||
| echo " $(wrap_color '(cgroup swap accounting is currently enabled)' bold black)" | ||
| EXITCODE=${CODE} |
| echo " $(wrap_color '(cgroup swap accounting is currently enabled)' bold black)" | ||
| EXITCODE=${CODE} | ||
| elif is_set MEMCG_SWAP && ! is_set MEMCG_SWAP_ENABLED; then | ||
| echo " $(wrap_color '(cgroup swap accounting is currently not enabled, you can enable it by setting boot option "swapaccount=1")' bold black)" |
There was a problem hiding this comment.
you can just set EXITCODE=1 here instead
There was a problem hiding this comment.
I found that Line-95 in this script wrap_bad "CONFIG_$1" 'missing' ; EXITCODE=1 indicates that EXITCODE=1 is set when OS misses this flag, so the script will exit with code 1 even if CG_MEM_SWAP flag is enabled manually.
The current change will fix this case said above, though it seems to a little strange but actually not because it is referenced from Line-284. : )
|
Any other concern? |
|
'check-config.sh' in Docker 1.12.x branch doesn't fail on Ubuntu >=16.04, but 'check-config.sh' in Docker 1.14 branch will always fail even if all required configs are enabled. |
|
ping @justincormack PTAL |
|
ping @justincormack |
|
Ok this is fine, it fixes that issue. However on a Ubuntu 16.04 machine I had on Digital Ocean, |
|
Happy to merge this anyway as it is correct. |
There was a problem hiding this comment.
ok let's merge this one @ghostplant can you do a follow-up for @justincormack's note?
LGTM
- What I did
Checking whether CG_MEM_SWAP is enabled at the current system session.
- How I did it
The original checking script only describes the method to enable CG_MEM_SWAP, while doesn't tell whether current system session already enables it or not.
- How to verify it
If current system session already enables CG_MEM_SWAP, the script should report 'currently enabled', otherwise report 'currently not enabled, you can enable it by ..'
Signed-off-by: CUI Wei ghostplant@qq.com