resmgr: lifecycle overlap detection and workaround.

We recently discovered a problem with the generated stream of
container lifecycle events with some runtime versions. A side
effect of this is that we get Create/Stop events for multiple
container instances with seemingly overlapping lifecycle: the
latter instance get created before the former one is stopped.

When undetected, such a false overlap might cause overcommit
of resources, with both instances temporarily using the full
resource set of the container. As a workaround, we now track
containers also by fully qualified name ($namespace/pod/ctr)
and internally generate an event for releasing the resources
if the old instance whenever we notice that a creation event
would cause a duplicate instance for the same name.

Signed-off-by: Krisztian Litkey <[email protected]>

pkg/resmgr/nri.go

@@ -34,6 +34,7 @@ import (
 type nriPlugin struct {
 	stub   stub.Stub
 	resmgr *resmgr
+	byname map[string]cache.Container
 }
 
 var (
@@ -43,6 +44,7 @@ var (
 func newNRIPlugin(resmgr *resmgr) (*nriPlugin, error) {
 	p := &nriPlugin{
 		resmgr: resmgr,
+		byname: make(map[string]cache.Container),
 	}
 
 	nri.Info("creating plugin...")
@@ -114,6 +116,56 @@ func (p *nriPlugin) onClose() {
 	os.Exit(1)
 }
 
+func (p *nriPlugin) syncNamesToContainers(containers []cache.Container) []cache.Container {
+	unmapped := make([]cache.Container, 0, len(p.byname))
+
+	for _, ctr := range containers {
+		old := p.mapNameToContainer(ctr)
+		if old != nil && old.GetID() != ctr.GetID() {
+			unmapped = append(unmapped, old)
+		}
+	}
+
+	return unmapped
+}
+
+func (p *nriPlugin) mapNameToContainer(ctr cache.Container) cache.Container {
+	name := ctr.PrettyName()
+	old, ok := p.byname[name]
+
+	p.byname[name] = ctr
+	if ok {
+		nri.Info("%s: remapped container from %s to %s", name, old.GetID(), ctr.GetID())
+		return old
+	}
+
+	nri.Info("%s: mapped container to %s", name, ctr.GetID())
+	return nil
+}
+
+func (p *nriPlugin) unmapName(name string) (cache.Container, bool) {
+	old, ok := p.byname[name]
+	if ok {
+		delete(p.byname, name)
+		nri.Info("%s: unmapped container from %s", name, old.GetID())
+	}
+	return old, ok
+}
+
+func (p *nriPlugin) unmapContainer(ctr cache.Container) {
+	name := ctr.PrettyName()
+	old, ok := p.byname[name]
+	if ok {
+		if old == ctr {
+			delete(p.byname, name)
+			nri.Info("%s: unmapped container (%s)", name, ctr.GetID())
+		} else {
+			nri.Warn("%s: leaving container mapped, ID mismatch (%s != %s)", name,
+				old.GetID(), ctr.GetID())
+		}
+	}
+}
+
 func (p *nriPlugin) Configure(ctx context.Context, cfg, runtime, version string) (stub.EventMask, error) {
 	event := Configure
 
@@ -212,7 +264,8 @@ func (p *nriPlugin) Synchronize(ctx context.Context, pods []*api.PodSandbox, con
 		return nil, err
 	}
 
-	if err := m.policy.Sync(allocated, released); err != nil {
+	unmapped := p.syncNamesToContainers(allocated)
+	if err := m.policy.Sync(allocated, append(released, unmapped...)); err != nil {
 		return nil, fmt.Errorf("failed to sync policy %s: %w", m.policy.ActivePolicy(), err)
 	}
 
@@ -345,6 +398,14 @@ func (p *nriPlugin) CreateContainer(ctx context.Context, podSandbox *api.PodSand
 	}
 	c.UpdateState(cache.ContainerStateCreating)
 
+	if old, ok := p.unmapName(c.PrettyName()); ok {
+		nri.Info("%s: releasing stale instance %s", c.PrettyName(), old.GetID())
+		if err := m.policy.ReleaseResources(old); err != nil {
+			nri.Error("%s: failed to release stale instance %s", c.PrettyName(), old.GetID())
+		}
+		old.UpdateState(cache.ContainerStateExited)
+	}
+
 	if err := m.policy.AllocateResources(c); err != nil {
 		c.UpdateState(cache.ContainerStateStale)
 		return nil, nil, fmt.Errorf("failed to allocate resources: %w", err)
@@ -372,6 +433,8 @@ func (p *nriPlugin) CreateContainer(ctx context.Context, podSandbox *api.PodSand
 	adjust = p.getPendingAdjustment(container)
 	updates = p.getPendingUpdates(container)
 
+	p.mapNameToContainer(c)
+
 	return adjust, updates, nil
 }
 
@@ -509,6 +572,8 @@ func (p *nriPlugin) StopContainer(ctx context.Context, pod *api.PodSandbox, cont
 		return nil, nil
 	}
 
+	p.unmapContainer(c)
+
 	if err := m.policy.ReleaseResources(c); err != nil {
 		return nil, fmt.Errorf("failed to release resources: %w", err)
 	}