resmgr: name to container ID mapping fixes - #727
Conversation
askervin
left a comment
askervin
left a comment
There was a problem hiding this comment.
Very nice... just one question and one nit.
46f06f1 to
420f454
Compare
|
@askervin Addressed review comments. Also the actual fix was missing to prevent re-allocation of resource for old instances of transient duplicates. Added it an updated the test case to verify that the internal post-allocated duplicate detection runs clean, never detecting any duplicate resource allocations. |
klihub
changed the title
90ccaf8 to
02898ed
Compare
There was a problem hiding this comment.
Yet I already put my approval on this, may I ask you to fix my mistake in the commit string?
I'd also propose removing "fixup: e2e:..." part from the commit message of:
"e2e: add test for duplicate disambiguation".
(Removes another spelling error: "ry" instead of "try", and we won't need that part in the commit message anyway.)
I think with these fixes this PR is good for merging.
02898ed to
fba75ec
Compare
Argh... that was just me fat-fingering the squashed commit message, unintentional. Thanks for spotting it. |
Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
Mark containers as exited during initial state synchronization with the runtime. This should prevent subsequent configuration updates from trying to allocate resources for both the old and the new instance, in case the old has not been removed yet. When trying to detect transient stale duplicate containers for overlapping container create/crash/stop then re-create events, try to be smarter about which instance is the stale one. For a CreateContainer event always choose the old one. Otherwise for an initial sync, use the CreatedAt timestamps if available. If not, then assume that a container in running state is stale if the other one is in created state. Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
- If DEBUG_LOGGERS is not specified, configure the same debug loggers as earlier, with one exception. - Drop "nri-resource-policy" logger as there is no such a thing. Signed-off-by: Antti Kervinen <antti.kervinen@intel.com>
Add an test case to trigger and test the handling of transient duplicate container disambiguation. Co-authored-by: Antti Kervinen <antti.kervinen@intel.com> Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
fba75ec to
c8342fa
Compare
2 participants
This patch series improves container name to ID mapping, used for transient duplicate detection.
In particular, we now mark containers as exited during initial state synchronization with the runtime. This should prevent subsequent configuration updates from trying to allocate resources for both the old and the new instance, in case the old has not been removed yet. Also during initial sync, remove unmapped (disambiguated older duplicate) instances from the list of containers to be (re)allocated. This should prevent double accounting of resources for transient duplicates.
Additionally, when trying to detect transient stale duplicate containers for overlapping container create/crash/stop then re-create events, try to be smarter about which instance is the stale one. For a CreateContainer event always choose the old one. Otherwise for an initial sync, use the CreatedAt timestamps if available. If not, then assume that a container in running state is stale if the other one is in created state.
Finally, add a new e2e test case to trigger and test the handling of transient dupicates.