Makes Load function more lenient on subsystems' checking#63
Makes Load function more lenient on subsystems' checking#63estesp merged 2 commits intocontainerd:masterfrom
Conversation
ChrsMark
force-pushed
the
lenient-subsystems-checking
branch
2 times, most recently
from
8cfd7b4 to
99a20c9
Compare
|
Thanks, i'll try to get this reviewed today or tomorrow |
| return nil, err | ||
| } | ||
| // check the the subsystems still exist | ||
| for _, s := range pathers(subsystems) { |
There was a problem hiding this comment.
could we keep the for range the same and create a temporary var for valid subsystems? Maybe something like activeSubsystems and you append active ones instead of removing the ones that are not found from the current list? I think it would make the code simpler and explicit.
What do you think?
There was a problem hiding this comment.
Sure! I had the same thought but chose to go with the more "economical" way by keeping the same array.
|
Also, after you make the updates, do you think you could work up a test in a separate commit to verify that we handle omitting a subsystem? |
|
@crosbymichael yes, tests need to be added on this. Another commit is coming from my friend @MichaelKatsoulis adding the tests. |
Codecov Report
@@ Coverage Diff @@
## master #63 +/- ##
========================================
+ Coverage 13.6% 13.7% +0.1%
========================================
Files 23 23
Lines 4528 4530 +2
========================================
+ Hits 616 621 +5
+ Misses 3797 3795 -2
+ Partials 115 114 -1
Continue to review full report at Codecov.
|
ChrsMark
force-pushed
the
lenient-subsystems-checking
branch
from
c253b52 to
5115b1f
Compare
|
@ChrsMark Thanks! The code is looking great now! |
|
@ChrsMark when you update and add the test, can you rebase on master so that the CI runs successfully? |
ChrsMark
force-pushed
the
lenient-subsystems-checking
branch
from
5115b1f to
cc652b9
Compare
Currently when a cgroup path is not found under a subsystem's directory Load function will fail completely. This can be blocking when a subsystem is taking time to update its cgroups. This commit makes Load to by-pass any not-found subsystem returning a Cgroup with a list of the only-found subsystems. Signed-off-by: Chris Mark <[email protected]>
ChrsMark
force-pushed
the
lenient-subsystems-checking
branch
from
cc652b9 to
f6cbfb4
Compare
Signed-off-by: MichaelKatsoulis <[email protected]>
|
@crosbymichael branch rebased on top of upstream's master and the test added. Please could you have a look? |
|
LGTM Thanks @ChrsMark and @MichaelKatsoulis ! |
5 participants
closes: #58
Currently when a cgroup path is not found under a subsystem's directory
Load function will fail completely. This can be blocking when a subsystem
is taking time to update its cgroups. This commit makes Load to by-pass
any not-found subsystem returning a Cgroup containing a list of the
only-found subsystems.
cc: @crosbymichael