Repository: stern/stern
Pull request #221 (2023-02-02 09:22:28)

Improve handling of container termination
Fixes https://github.com/stern/stern/issues/217

This PR improves the handling of container termination. As discussed in #217, stern currently does not handle container termination well. [The steps in the comment](https://github.com/stern/stern/issues/217#issuecomment-1411656570) can reproduce this issue.

This PR makes the following changes to improve.  (sorry for the big change)

- Manage the last shown Container IDs
    - Start tailing logs only when Container IDs are changed
    - It prevents the same log lines are unnecessarily shown
    - The logic to choose an ID is based on [Kubelet's validateContainerLogStatus](https://github.com/kubernetes/kubernetes/blob/v1.26.1/pkg/kubelet/kubelet_pods.go#L1246).
- The container states filter is applied only to containers that have existed before stern starts
    - It allows stern to show a pod that immediately completes that skips a "running" state.
- Do not cancel tailing when containers are terminated or removed
    - It prevents the last logs from being omitted by canceling.
    - Tailing will eventually stop by EOF or other errors
- Retry tailing if it gets an error
    - Before this change, retrying would happen when pod events fire
    - After this change, we need explicit retrying because we ignore pod events when Container IDs are not changed

This change is significant and hard to debug, so I would like to add debug logs. It also helps us to understand container state transitions. Here is an example of debug logs for [a restarted pod](https://github.com/stern/stern/issues/217#issuecomment-1411656570).

```
$ dist/stern --verbosity 7 restart 2> >(grep -E 'target.go|›')
I0202 21:21:21.051218   26997 target.go:139] "Container ID is empty" state="waiting" target="default-restart-busybox"
I0202 21:21:21.676584   26997 target.go:158] "Container ID was changed" state="terminated" target="default-restart-busybox" container="containerd://2a2a0574f0fdfbce8dddc647a1e3469e5b2ccd4e5fb8c95225e778c8f020ab04" last=""
+ restart › busybox
restart busybox 21567
- restart › busybox
I0202 21:21:22.679503   26997 target.go:158] "Container ID was changed" state="terminated" target="default-restart-busybox" container="containerd://fac6a6ad2e8029b97b9eb4a65a9168c2bea9fb063abe823749520952216a9bf0" last="containerd://2a2a0574f0fdfbce8dddc647a1e3469e5b2ccd4e5fb8c95225e778c8f020ab04"
+ restart › busybox
restart busybox 601
- restart › busybox
I0202 21:21:23.686007   26997 target.go:153] "Container ID is the same" state="waiting" target="default-restart-busybox" container="containerd://fac6a6ad2e8029b97b9eb4a65a9168c2bea9fb063abe823749520952216a9bf0"
I0202 21:21:38.713079   26997 target.go:158] "Container ID was changed" state="terminated" target="default-restart-busybox" container="containerd://d7e9e5fbc788cb67081b81697b9817ac2128a2d45e09c8e55c5095d2bf0f1128" last="containerd://fac6a6ad2e8029b97b9eb4a65a9168c2bea9fb063abe823749520952216a9bf0"
+ restart › busybox
restart busybox 19817
- restart › busybox
```


tksm: 📝 This change will also allow users to use stern to debug CrashLoopBackoff containers.

superbrothers: Thanks. I will check this out this weekend!
tksm: I'll rebase and squash after the review.
tksm: Thank you for the review. I've rebased on master in https://github.com/stern/stern/commit/a98c0b4eded10629b6308e8de71c26ca10b3e983.


Patch:
diff --git a/go.mod b/go.mod
index 095d0fd..a5e401a 100644
--- a/go.mod
+++ b/go.mod
@@ -9,6 +9,7 @@ require (
 	github.com/spf13/cobra v1.6.1
 	github.com/spf13/pflag v1.0.5
 	golang.org/x/sync v0.1.0
+	golang.org/x/time v0.3.0
 	k8s.io/api v0.26.0
 	k8s.io/apimachinery v0.26.0
 	k8s.io/cli-runtime v0.26.0
@@ -56,7 +57,6 @@ require (
 	golang.org/x/sys v0.4.0 // indirect
 	golang.org/x/term v0.4.0 // indirect
 	golang.org/x/text v0.6.0 // indirect
-	golang.org/x/time v0.3.0 // indirect
 	google.golang.org/appengine v1.6.7 // indirect
 	google.golang.org/protobuf v1.28.1 // indirect
 	gopkg.in/inf.v0 v0.9.1 // indirect
diff --git a/stern/list.go b/stern/list.go
index 1efbf22..7bae6f4 100644
--- a/stern/list.go
+++ b/stern/list.go
@@ -98,10 +98,8 @@ func ListTargets(ctx context.Context, i corev1client.PodInterface, labelSelector
 	}
 	var targets []*Target
 	for i := range list.Items {
-		filter.visit(&list.Items[i], func(t *Target, containerStateMatched bool) {
-			if containerStateMatched {
-				targets = append(targets, t)
-			}
+		filter.visit(&list.Items[i], func(t *Target) {
+			targets = append(targets, t)
 		})
 	}
 	return targets, nil
diff --git a/stern/stern.go b/stern/stern.go
index baaf44a..f36aec0 100644
--- a/stern/stern.go
+++ b/stern/stern.go
@@ -19,12 +19,13 @@ import (
 	"fmt"
 	"regexp"
 	"strings"
-	"sync"
+	"time"
 
 	"github.com/pkg/errors"
 
 	"github.com/stern/stern/kubernetes"
 	"golang.org/x/sync/errgroup"
+	"golang.org/x/time/rate"
 
 	"k8s.io/apimachinery/pkg/labels"
 
@@ -32,28 +33,6 @@ import (
 	clientset "k8s.io/client-go/kubernetes"
 )
 
-var tails = make(map[string]*Tail)
-var tailLock sync.RWMutex
-
-func getTail(targetID string) (*Tail, bool) {
-	tailLock.RLock()
-	defer tailLock.RUnlock()
-	tail, ok := tails[targetID]
-	return tail, ok
-}
-
-func setTail(targetID string, tail *Tail) {
-	tailLock.Lock()
-	defer tailLock.Unlock()
-	tails[targetID] = tail
-}
-
-func clearTail(targetID string) {
-	tailLock.Lock()
-	defer tailLock.Unlock()
-	delete(tails, targetID)
-}
-
 // Run starts the main run loop
 func Run(ctx context.Context, config *Config) error {
 	clientConfig := kubernetes.NewClientConfig(config.KubeConfig, config.ContextName)
@@ -103,7 +82,7 @@ func Run(ctx context.Context, config *Config) error {
 		}
 	}
 
-	filter := &targetFilter{
+	filter := newTargetFilter(targetFilterConfig{
 		podFilter:              config.PodQuery,
 		excludePodFilter:       config.ExcludePodQuery,
 		containerFilter:        config.ContainerQuery,
@@ -111,7 +90,7 @@ func Run(ctx context.Context, config *Config) error {
 		initContainers:         config.InitContainers,
 		ephemeralContainers:    config.EphemeralContainers,
 		containerStates:        config.ContainerStates,
-	}
+	})
 	newTail := func(t *Target) *Tail {
 		return NewTail(client.CoreV1(), t.Node, t.Namespace, t.Pod, t.Container, config.Template, config.Out, config.ErrOut, &TailOptions{
 			Timestamps:   config.Timestamps,
@@ -155,11 +134,9 @@ func Run(ctx context.Context, config *Config) error {
 	}
 
 	added := make(chan *Target)
-	removed := make(chan *Target)
 	errCh := make(chan error)
 
 	defer close(added)
-	defer close(removed)
 	defer close(errCh)
 
 	for _, n := range namespaces {
@@ -167,7 +144,7 @@ func Run(ctx context.Context, config *Config) error {
 		if err != nil {
 			return err
 		}
-		a, r, err := WatchTargets(ctx,
+		a, err := WatchTargets(ctx,
 			client.CoreV1().Pods(n),
 			selector,
 			config.FieldSelector,
@@ -186,12 +163,6 @@ func Run(ctx context.Context, config *Config) error {
 						return
 					}
 					added <- v
-				case v, ok := <-r:
-					if !ok {
-						errCh <- fmt.Errorf("lost watch connection")
-						return
-					}
-					removed <- v
 				case <-ctx.Done():
 					return
 				}
@@ -199,37 +170,33 @@ func Run(ctx context.Context, config *Config) error {
 		}()
 	}
 
-	go func() {
-		for p := range added {
-			targetID := p.GetID()
-
-			if tail, ok := getTail(targetID); ok {
-				if tail.isActive() {
-					continue
-				} else {
-					tail.Close()
-					clearTail(targetID)
-				}
+	addTarget := func(ctx context.Context, target *Target) {
+		// We use a rate limiter to prevent a burst of retries.
+		// It also enables us to retry immediately, in most cases,
+		// when it is disconnected on the way.
+		limiter := rate.NewLimiter(rate.Every(time.Second*10), 3)
+		for {
+			if err := limiter.Wait(ctx); err != nil {
+				fmt.Fprintf(config.ErrOut, "failed to retry: %v\n", err)
+				return
 			}
-
-			tail := newTail(p)
-			setTail(targetID, tail)
-
-			go func(tail *Tail) {
-				if err := tail.Start(ctx); err != nil {
-					fmt.Fprintf(config.ErrOut, "unexpected error: %v\n", err)
-				}
-			}(tail)
+			tail := newTail(target)
+			err := tail.Start(ctx)
+			tail.Close()
+			if err == nil {
+				return
+			}
+			if !filter.isActive(target) {
+				fmt.Fprintf(config.ErrOut, "failed to tail: %v\n", err)
+				return
+			}
+			fmt.Fprintf(config.ErrOut, "failed to tail: %v, will retry\n", err)
 		}
-	}()
+	}
 
 	go func() {
-		for p := range removed {
-			targetID := p.GetID()
-			if tail, ok := getTail(targetID); ok {
-				tail.Close()
-				clearTail(targetID)
-			}
+		for target := range added {
+			go addTarget(ctx, target)
 		}
 	}()
 
diff --git a/stern/tail.go b/stern/tail.go
index c9644eb..ab380e0 100644
--- a/stern/tail.go
+++ b/stern/tail.go
@@ -45,7 +45,6 @@ type Tail struct {
 	podColor       *color.Color
 	containerColor *color.Color
 	tmpl           *template.Template
-	active         bool
 	out            io.Writer
 	errOut         io.Writer
 }
@@ -119,7 +118,6 @@ func NewTail(clientset corev1client.CoreV1Interface, nodeName, namespace, podNam
 		Options:        options,
 		closed:         make(chan struct{}),
 		tmpl:           tmpl,
-		active:         true,
 		podColor:       podColor,
 		containerColor: containerColor,
 
@@ -165,7 +163,6 @@ func (t *Tail) Start(ctx context.Context) error {
 	})
 
 	err := t.ConsumeRequest(ctx, req)
-	t.active = false
 
 	if errors.Is(err, context.Canceled) {
 		return nil
@@ -267,11 +264,6 @@ func (t *Tail) Print(msg string) {
 	fmt.Fprint(t.out, buf.String())
 }
 
-// isActive returns false if the log stream is closed.
-func (t *Tail) isActive() bool {
-	return t.active
-}
-
 // Log is the object which will be used together with the template to generate
 // the output.
 type Log struct {
diff --git a/stern/target.go b/stern/target.go
index eac18e5..4a303be 100644
--- a/stern/target.go
+++ b/stern/target.go
@@ -17,8 +17,10 @@ package stern
 import (
 	"fmt"
 	"regexp"
+	"sync"
 
 	corev1 "k8s.io/api/core/v1"
+	"k8s.io/klog/v2"
 )
 
 // Target is a target to watch
@@ -34,8 +36,20 @@ func (t *Target) GetID() string {
 	return fmt.Sprintf("%s-%s-%s", t.Namespace, t.Pod, t.Container)
 }
 
+// targetState holds a last shown container ID
+type targetState struct {
+	podUID      string
+	containerID string
+}
+
 // targetFilter is a filter of Target
 type targetFilter struct {
+	c            targetFilterConfig
+	targetStates map[string]*targetState
+	mu           sync.RWMutex
+}
+
+type targetFilterConfig struct {
 	podFilter              *regexp.Regexp
 	excludePodFilter       []*regexp.Regexp
 	containerFilter        *regexp.Regexp
@@ -45,14 +59,21 @@ type targetFilter struct {
 	containerStates        []ContainerState
 }
 
+func newTargetFilter(c targetFilterConfig) *targetFilter {
+	return &targetFilter{
+		c:            c,
+		targetStates: make(map[string]*targetState),
+	}
+}
+
 // visit passes filtered Targets to the visitor function
-func (f *targetFilter) visit(pod *corev1.Pod, visitor func(t *Target, containerStateMatched bool)) {
+func (f *targetFilter) visit(pod *corev1.Pod, visitor func(t *Target)) {
 	// filter by pod
-	if !f.podFilter.MatchString(pod.Name) {
+	if !f.c.podFilter.MatchString(pod.Name) {
 		return
 	}
 
-	for _, re := range f.excludePodFilter {
+	for _, re := range f.c.excludePodFilter {
 		if re.MatchString(pod.Name) {
 			return
 		}
@@ -62,21 +83,21 @@ func (f *targetFilter) visit(pod *corev1.Pod, visitor func(t *Target, containerS
 	var statuses []corev1.ContainerStatus
 	statuses = append(statuses, pod.Status.ContainerStatuses...)
 
-	if f.initContainers {
+	if f.c.initContainers {
 		statuses = append(statuses, pod.Status.InitContainerStatuses...)
 	}
 
-	if f.ephemeralContainers {
+	if f.c.ephemeralContainers {
 		statuses = append(statuses, pod.Status.EphemeralContainerStatuses...)
 	}
 
 OUTER:
 	for _, c := range statuses {
-		if !f.containerFilter.MatchString(c.Name) {
+		if !f.c.containerFilter.MatchString(c.Name) {
 			continue
 		}
 
-		for _, re := range f.containerExcludeFilter {
+		for _, re := range f.c.containerExcludeFilter {
 			if re.MatchString(c.Name) {
 				continue OUTER
 			}
@@ -89,15 +110,101 @@ OUTER:
 			Container: c.Name,
 		}
 
-		visitor(t, f.matchContainerState(c.State))
+		if f.shouldAdd(t, string(pod.UID), c) {
+			visitor(t)
+		}
 	}
 }
 
 func (f *targetFilter) matchContainerState(state corev1.ContainerState) bool {
-	for _, containerState := range f.containerStates {
+	for _, containerState := range f.c.containerStates {
 		if containerState.Match(state) {
 			return true
 		}
 	}
 	return false
 }
+
+func (f *targetFilter) shouldAdd(t *Target, podUID string, cs corev1.ContainerStatus) bool {
+	state := stateToString(cs.State)
+	containerID := chooseContainerID(cs)
+
+	f.mu.Lock()
+	last := f.targetStates[t.GetID()]
+	f.targetStates[t.GetID()] = &targetState{podUID: podUID, containerID: containerID}
+	f.mu.Unlock()
+
+	if containerID == "" {
+		// does not have a container to retrieve logs
+		klog.V(7).InfoS("Container ID is empty", "state", state, "target", t.GetID())
+		return false
+	}
+
+	if last == nil {
+		// We filter out only containers that have existed before stern starts by container states.
+		// The container state transition skips the "running" when a pod immediately completes,
+		// so filtering by container states does not work as expected for newly created containers.
+		klog.V(7).InfoS("Container ID has existed before observation",
+			"state", state, "target", t.GetID(), "container", containerID)
+		return f.matchContainerState(cs.State)
+	}
+
+	if last.containerID == containerID {
+		klog.V(7).InfoS("Container ID is the same",
+			"state", state, "target", t.GetID(), "container", containerID)
+		return false
+	}
+	// add a container when the container ID is changed from the last time
+	klog.V(7).InfoS("Container ID was changed",
+		"state", state, "target", t.GetID(), "container", containerID, "last", last.containerID)
+	return true
+}
+
+func (f *targetFilter) forget(podUID string) {
+	f.mu.Lock()
+	defer f.mu.Unlock()
+	// delete target states belonging to the pod
+	for targetID, state := range f.targetStates {
+		if state.podUID == podUID {
+			klog.V(7).InfoS("Forget targetState", "target", targetID)
+			delete(f.targetStates, targetID)
+		}
+	}
+}
+
+func (f *targetFilter) isActive(t *Target) bool {
+	f.mu.RLock()
+	defer f.mu.RUnlock()
+	last := f.targetStates[t.GetID()]
+	return last != nil && last.containerID != ""
+}
+
+func chooseContainerID(cs corev1.ContainerStatus) string {
+	// This logic is based on kubelet's validateContainerLogStatus
+	// https://github.com/kubernetes/kubernetes/blob/v1.26.1/pkg/kubelet/kubelet_pods.go#L1246
+	switch {
+	case cs.State.Running != nil:
+		return cs.ContainerID
+	case cs.State.Terminated != nil:
+		if cs.State.Terminated.ContainerID != "" {
+			return cs.State.Terminated.ContainerID
+		}
+	}
+	lastTerminated := cs.LastTerminationState.Terminated
+	if lastTerminated != nil && lastTerminated.ContainerID != "" {
+		return lastTerminated.ContainerID
+	}
+	return ""
+}
+
+func stateToString(state corev1.ContainerState) string {
+	switch {
+	case state.Running != nil:
+		return "running"
+	case state.Terminated != nil:
+		return "terminated"
+	case state.Waiting != nil:
+		return "waiting"
+	}
+	return "unknown"
+}
diff --git a/stern/watch.go b/stern/watch.go
index a68c395..2f3aa83 100644
--- a/stern/watch.go
+++ b/stern/watch.go
@@ -30,9 +30,8 @@ import (
 )
 
 // Watch starts listening to Kubernetes events and emits modified
-// containers/pods. The first result is targets added, the second is targets
-// removed
-func WatchTargets(ctx context.Context, i v1.PodInterface, labelSelector labels.Selector, fieldSelector fields.Selector, filter *targetFilter) (chan *Target, chan *Target, error) {
+// containers/pods. The result is targets added.
+func WatchTargets(ctx context.Context, i v1.PodInterface, labelSelector labels.Selector, fieldSelector fields.Selector, filter *targetFilter) (chan *Target, error) {
 	// RetryWatcher will make sure that in case the underlying watcher is
 	// closed (e.g. due to API timeout or etcd timeout) it will get restarted
 	// from the last point without the consumer even knowing about it.
@@ -42,12 +41,10 @@ func WatchTargets(ctx context.Context, i v1.PodInterface, labelSelector labels.S
 		},
 	})
 	if err != nil {
-		return nil, nil, errors.Wrap(err, "failed to create a watcher")
+		return nil, errors.Wrap(err, "failed to create a watcher")
 	}
 
 	added := make(chan *Target)
-	removed := make(chan *Target)
-
 	go func() {
 		for {
 			select {
@@ -55,7 +52,6 @@ func WatchTargets(ctx context.Context, i v1.PodInterface, labelSelector labels.S
 				if e.Object == nil {
 					// Closed because of error
 					close(added)
-					close(removed)
 					return
 				}
 
@@ -66,26 +62,19 @@ func WatchTargets(ctx context.Context, i v1.PodInterface, labelSelector labels.S
 
 				switch e.Type {
 				case watch.Added, watch.Modified:
-					filter.visit(pod, func(t *Target, containerStateMatched bool) {
-						if containerStateMatched {
-							added <- t
-						} else {
-							removed <- t
-						}
+					filter.visit(pod, func(t *Target) {
+						added <- t
 					})
 				case watch.Deleted:
-					filter.visit(pod, func(t *Target, containerStateMatched bool) {
-						removed <- t
-					})
+					filter.forget(string(pod.UID))
 				}
 			case <-ctx.Done():
 				watcher.Stop()
 				close(added)
-				close(removed)
 				return
 			}
 		}
 	}()
 
-	return added, removed, nil
+	return added, nil
 }


Test patch:
diff --git a/stern/target_test.go b/stern/target_test.go
index 820970e..e064e81 100644
--- a/stern/target_test.go
+++ b/stern/target_test.go
@@ -11,7 +11,7 @@ import (
 
 func TestTargetFilter(t *testing.T) {
 	running := corev1.ContainerState{Running: &corev1.ContainerStateRunning{}}
-	terminated := corev1.ContainerState{Terminated: &corev1.ContainerStateTerminated{}}
+	terminated := corev1.ContainerState{Terminated: &corev1.ContainerStateTerminated{ContainerID: "dummy"}}
 	waiting := corev1.ContainerState{Waiting: &corev1.ContainerStateWaiting{}}
 
 	createPod := func(node, pod string) *corev1.Pod {
@@ -25,19 +25,19 @@ func TestTargetFilter(t *testing.T) {
 			},
 			Status: corev1.PodStatus{
 				ContainerStatuses: []corev1.ContainerStatus{
-					{Name: "container1-running", State: running},
+					{Name: "container1-running", State: running, ContainerID: "dummy"},
 					{Name: "container2-terminated", State: terminated},
-					{Name: "container3-waiting", State: waiting},
+					{Name: "container3-waiting", State: waiting, LastTerminationState: terminated},
 				},
 				InitContainerStatuses: []corev1.ContainerStatus{
-					{Name: "init-container1-running", State: running},
+					{Name: "init-container1-running", State: running, ContainerID: "dummy"},
 					{Name: "init-container2-terminated", State: terminated},
-					{Name: "init-container3-waiting", State: waiting},
+					{Name: "init-container3-waiting", State: waiting, LastTerminationState: terminated},
 				},
 				EphemeralContainerStatuses: []corev1.ContainerStatus{
-					{Name: "ephemeral-container1-running", State: running},
+					{Name: "ephemeral-container1-running", State: running, ContainerID: "dummy"},
 					{Name: "ephemeral-container2-terminated", State: terminated},
-					{Name: "ephemeral-container3-waiting", State: waiting},
+					{Name: "ephemeral-container3-waiting", State: waiting, LastTerminationState: terminated},
 				},
 			},
 		}
@@ -48,30 +48,23 @@ func TestTargetFilter(t *testing.T) {
 		createPod("node2", "pod2"),
 	}
 
-	type targetMatch struct {
-		target                Target
-		containerStateMatched bool
-	}
-	genTargetMatch := func(node, pod, container string, matched bool) targetMatch {
-		return targetMatch{
-			target: Target{
-				Namespace: "ns1",
-				Node:      node,
-				Pod:       pod,
-				Container: container,
-			},
-			containerStateMatched: matched,
+	genTarget := func(node, pod, container string) Target {
+		return Target{
+			Namespace: "ns1",
+			Node:      node,
+			Pod:       pod,
+			Container: container,
 		}
 	}
 
 	tests := []struct {
 		name     string
-		filter   targetFilter
-		expected []targetMatch
+		config   targetFilterConfig
+		expected []Target
 	}{
 		{
 			name: "match all",
-			filter: targetFilter{
+			config: targetFilterConfig{
 				podFilter:              regexp.MustCompile(`.*`),
 				excludePodFilter:       nil,
 				containerFilter:        regexp.MustCompile(`.*`),
@@ -80,30 +73,30 @@ func TestTargetFilter(t *testing.T) {
 				ephemeralContainers:    true,
 				containerStates:        []ContainerState{RUNNING, TERMINATED, WAITING},
 			},
-			expected: []targetMatch{
-				genTargetMatch("node1", "pod1", "container1-running", true),
-				genTargetMatch("node1", "pod1", "container2-terminated", true),
-				genTargetMatch("node1", "pod1", "container3-waiting", true),
-				genTargetMatch("node1", "pod1", "init-container1-running", true),
-				genTargetMatch("node1", "pod1", "init-container2-terminated", true),
-				genTargetMatch("node1", "pod1", "init-container3-waiting", true),
-				genTargetMatch("node1", "pod1", "ephemeral-container1-running", true),
-				genTargetMatch("node1", "pod1", "ephemeral-container2-terminated", true),
-				genTargetMatch("node1", "pod1", "ephemeral-container3-waiting", true),
-				genTargetMatch("node2", "pod2", "container1-running", true),
-				genTargetMatch("node2", "pod2", "container2-terminated", true),
-				genTargetMatch("node2", "pod2", "container3-waiting", true),
-				genTargetMatch("node2", "pod2", "init-container1-running", true),
-				genTargetMatch("node2", "pod2", "init-container2-terminated", true),
-				genTargetMatch("node2", "pod2", "init-container3-waiting", true),
-				genTargetMatch("node2", "pod2", "ephemeral-container1-running", true),
-				genTargetMatch("node2", "pod2", "ephemeral-container2-terminated", true),
-				genTargetMatch("node2", "pod2", "ephemeral-container3-waiting", true),
+			expected: []Target{
+				genTarget("node1", "pod1", "container1-running"),
+				genTarget("node1", "pod1", "container2-terminated"),
+				genTarget("node1", "pod1", "container3-waiting"),
+				genTarget("node1", "pod1", "init-container1-running"),
+				genTarget("node1", "pod1", "init-container2-terminated"),
+				genTarget("node1", "pod1", "init-container3-waiting"),
+				genTarget("node1", "pod1", "ephemeral-container1-running"),
+				genTarget("node1", "pod1", "ephemeral-container2-terminated"),
+				genTarget("node1", "pod1", "ephemeral-container3-waiting"),
+				genTarget("node2", "pod2", "container1-running"),
+				genTarget("node2", "pod2", "container2-terminated"),
+				genTarget("node2", "pod2", "container3-waiting"),
+				genTarget("node2", "pod2", "init-container1-running"),
+				genTarget("node2", "pod2", "init-container2-terminated"),
+				genTarget("node2", "pod2", "init-container3-waiting"),
+				genTarget("node2", "pod2", "ephemeral-container1-running"),
+				genTarget("node2", "pod2", "ephemeral-container2-terminated"),
+				genTarget("node2", "pod2", "ephemeral-container3-waiting"),
 			},
 		},
 		{
 			name: "filter by podFilter",
-			filter: targetFilter{
+			config: targetFilterConfig{
 				podFilter:              regexp.MustCompile(`not-matched`),
 				excludePodFilter:       nil,
 				containerFilter:        regexp.MustCompile(`.*`),
@@ -112,11 +105,11 @@ func TestTargetFilter(t *testing.T) {
 				ephemeralContainers:    true,
 				containerStates:        []ContainerState{RUNNING, TERMINATED, WAITING},
 			},
-			expected: []targetMatch{},
+			expected: []Target{},
 		},
 		{
 			name: "filter by excludePodFilter",
-			filter: targetFilter{
+			config: targetFilterConfig{
 				podFilter:              regexp.MustCompile(``),
 				excludePodFilter:       []*regexp.Regexp{regexp.MustCompile(`pod1`)},
 				containerFilter:        regexp.MustCompile(`.*`),
@@ -125,21 +118,21 @@ func TestTargetFilter(t *testing.T) {
 				ephemeralContainers:    true,
 				containerStates:        []ContainerState{RUNNING, TERMINATED, WAITING},
 			},
-			expected: []targetMatch{
-				genTargetMatch("node2", "pod2", "container1-running", true),
-				genTargetMatch("node2", "pod2", "container2-terminated", true),
-				genTargetMatch("node2", "pod2", "container3-waiting", true),
-				genTargetMatch("node2", "pod2", "init-container1-running", true),
-				genTargetMatch("node2", "pod2", "init-container2-terminated", true),
-				genTargetMatch("node2", "pod2", "init-container3-waiting", true),
-				genTargetMatch("node2", "pod2", "ephemeral-container1-running", true),
-				genTargetMatch("node2", "pod2", "ephemeral-container2-terminated", true),
-				genTargetMatch("node2", "pod2", "ephemeral-container3-waiting", true),
+			expected: []Target{
+				genTarget("node2", "pod2", "container1-running"),
+				genTarget("node2", "pod2", "container2-terminated"),
+				genTarget("node2", "pod2", "container3-waiting"),
+				genTarget("node2", "pod2", "init-container1-running"),
+				genTarget("node2", "pod2", "init-container2-terminated"),
+				genTarget("node2", "pod2", "init-container3-waiting"),
+				genTarget("node2", "pod2", "ephemeral-container1-running"),
+				genTarget("node2", "pod2", "ephemeral-container2-terminated"),
+				genTarget("node2", "pod2", "ephemeral-container3-waiting"),
 			},
 		},
 		{
 			name: "filter by multiple excludePodFilter",
-			filter: targetFilter{
+			config: targetFilterConfig{
 				podFilter: regexp.MustCompile(``),
 				excludePodFilter: []*regexp.Regexp{
 					regexp.MustCompile(`not-matched`),
@@ -151,21 +144,21 @@ func TestTargetFilter(t *testing.T) {
 				ephemeralContainers:    true,
 				containerStates:        []ContainerState{RUNNING, TERMINATED, WAITING},
 			},
-			expected: []targetMatch{
-				genTargetMatch("node1", "pod1", "container1-running", true),
-				genTargetMatch("node1", "pod1", "container2-terminated", true),
-				genTargetMatch("node1", "pod1", "container3-waiting", true),
-				genTargetMatch("node1", "pod1", "init-container1-running", true),
-				genTargetMatch("node1", "pod1", "init-container2-terminated", true),
-				genTargetMatch("node1", "pod1", "init-container3-waiting", true),
-				genTargetMatch("node1", "pod1", "ephemeral-container1-running", true),
-				genTargetMatch("node1", "pod1", "ephemeral-container2-terminated", true),
-				genTargetMatch("node1", "pod1", "ephemeral-container3-waiting", true),
+			expected: []Target{
+				genTarget("node1", "pod1", "container1-running"),
+				genTarget("node1", "pod1", "container2-terminated"),
+				genTarget("node1", "pod1", "container3-waiting"),
+				genTarget("node1", "pod1", "init-container1-running"),
+				genTarget("node1", "pod1", "init-container2-terminated"),
+				genTarget("node1", "pod1", "init-container3-waiting"),
+				genTarget("node1", "pod1", "ephemeral-container1-running"),
+				genTarget("node1", "pod1", "ephemeral-container2-terminated"),
+				genTarget("node1", "pod1", "ephemeral-container3-waiting"),
 			},
 		},
 		{
 			name: "filter by containerFilter",
-			filter: targetFilter{
+			config: targetFilterConfig{
 				podFilter:              regexp.MustCompile(`.*`),
 				excludePodFilter:       nil,
 				containerFilter:        regexp.MustCompile(`.*container1.*`),
@@ -174,18 +167,18 @@ func TestTargetFilter(t *testing.T) {
 				ephemeralContainers:    true,
 				containerStates:        []ContainerState{RUNNING, TERMINATED, WAITING},
 			},
-			expected: []targetMatch{
-				genTargetMatch("node1", "pod1", "container1-running", true),
-				genTargetMatch("node1", "pod1", "init-container1-running", true),
-				genTargetMatch("node1", "pod1", "ephemeral-container1-running", true),
-				genTargetMatch("node2", "pod2", "container1-running", true),
-				genTargetMatch("node2", "pod2", "init-container1-running", true),
-				genTargetMatch("node2", "pod2", "ephemeral-container1-running", true),
+			expected: []Target{
+				genTarget("node1", "pod1", "container1-running"),
+				genTarget("node1", "pod1", "init-container1-running"),
+				genTarget("node1", "pod1", "ephemeral-container1-running"),
+				genTarget("node2", "pod2", "container1-running"),
+				genTarget("node2", "pod2", "init-container1-running"),
+				genTarget("node2", "pod2", "ephemeral-container1-running"),
 			},
 		},
 		{
 			name: "filter by containerExcludeFilter",
-			filter: targetFilter{
+			config: targetFilterConfig{
 				podFilter:              regexp.MustCompile(`.*`),
 				excludePodFilter:       nil,
 				containerFilter:        regexp.MustCompile(`.*`),
@@ -194,24 +187,24 @@ func TestTargetFilter(t *testing.T) {
 				ephemeralContainers:    true,
 				containerStates:        []ContainerState{RUNNING, TERMINATED, WAITING},
 			},
-			expected: []targetMatch{
-				genTargetMatch("node1", "pod1", "container2-terminated", true),
-				genTargetMatch("node1", "pod1", "container3-waiting", true),
-				genTargetMatch("node1", "pod1", "init-container2-terminated", true),
-				genTargetMatch("node1", "pod1", "init-container3-waiting", true),
-				genTargetMatch("node1", "pod1", "ephemeral-container2-terminated", true),
-				genTargetMatch("node1", "pod1", "ephemeral-container3-waiting", true),
-				genTargetMatch("node2", "pod2", "container2-terminated", true),
-				genTargetMatch("node2", "pod2", "container3-waiting", true),
-				genTargetMatch("node2", "pod2", "init-container2-terminated", true),
-				genTargetMatch("node2", "pod2", "init-container3-waiting", true),
-				genTargetMatch("node2", "pod2", "ephemeral-container2-terminated", true),
-				genTargetMatch("node2", "pod2", "ephemeral-container3-waiting", true),
+			expected: []Target{
+				genTarget("node1", "pod1", "container2-terminated"),
+				genTarget("node1", "pod1", "container3-waiting"),
+				genTarget("node1", "pod1", "init-container2-terminated"),
+				genTarget("node1", "pod1", "init-container3-waiting"),
+				genTarget("node1", "pod1", "ephemeral-container2-terminated"),
+				genTarget("node1", "pod1", "ephemeral-container3-waiting"),
+				genTarget("node2", "pod2", "container2-terminated"),
+				genTarget("node2", "pod2", "container3-waiting"),
+				genTarget("node2", "pod2", "init-container2-terminated"),
+				genTarget("node2", "pod2", "init-container3-waiting"),
+				genTarget("node2", "pod2", "ephemeral-container2-terminated"),
+				genTarget("node2", "pod2", "ephemeral-container3-waiting"),
 			},
 		},
 		{
 			name: "filter by multiple containerExcludeFilter",
-			filter: targetFilter{
+			config: targetFilterConfig{
 				podFilter:        regexp.MustCompile(`.*`),
 				excludePodFilter: nil,
 				containerFilter:  regexp.MustCompile(`.*`),
@@ -223,22 +216,22 @@ func TestTargetFilter(t *testing.T) {
 				ephemeralContainers: true,
 				containerStates:     []ContainerState{RUNNING, TERMINATED, WAITING},
 			},
-			expected: []targetMatch{
-				genTargetMatch("node1", "pod1", "container2-terminated", true),
-				genTargetMatch("node1", "pod1", "container3-waiting", true),
-				genTargetMatch("node1", "pod1", "init-container3-waiting", true),
-				genTargetMatch("node1", "pod1", "ephemeral-container2-terminated", true),
-				genTargetMatch("node1", "pod1", "ephemeral-container3-waiting", true),
-				genTargetMatch("node2", "pod2", "container2-terminated", true),
-				genTargetMatch("node2", "pod2", "container3-waiting", true),
-				genTargetMatch("node2", "pod2", "init-container3-waiting", true),
-				genTargetMatch("node2", "pod2", "ephemeral-container2-terminated", true),
-				genTargetMatch("node2", "pod2", "ephemeral-container3-waiting", true),
+			expected: []Target{
+				genTarget("node1", "pod1", "container2-terminated"),
+				genTarget("node1", "pod1", "container3-waiting"),
+				genTarget("node1", "pod1", "init-container3-waiting"),
+				genTarget("node1", "pod1", "ephemeral-container2-terminated"),
+				genTarget("node1", "pod1", "ephemeral-container3-waiting"),
+				genTarget("node2", "pod2", "container2-terminated"),
+				genTarget("node2", "pod2", "container3-waiting"),
+				genTarget("node2", "pod2", "init-container3-waiting"),
+				genTarget("node2", "pod2", "ephemeral-container2-terminated"),
+				genTarget("node2", "pod2", "ephemeral-container3-waiting"),
 			},
 		},
 		{
 			name: "dot not include initContainers",
-			filter: targetFilter{
+			config: targetFilterConfig{
 				podFilter:              regexp.MustCompile(`.*`),
 				excludePodFilter:       nil,
 				containerFilter:        regexp.MustCompile(`.*`),
@@ -247,24 +240,24 @@ func TestTargetFilter(t *testing.T) {
 				ephemeralContainers:    true,
 				containerStates:        []ContainerState{RUNNING, TERMINATED, WAITING},
 			},
-			expected: []targetMatch{
-				genTargetMatch("node1", "pod1", "container1-running", true),
-				genTargetMatch("node1", "pod1", "container2-terminated", true),
-				genTargetMatch("node1", "pod1", "container3-waiting", true),
-				genTargetMatch("node1", "pod1", "ephemeral-container1-running", true),
-				genTargetMatch("node1", "pod1", "ephemeral-container2-terminated", true),
-				genTargetMatch("node1", "pod1", "ephemeral-container3-waiting", true),
-				genTargetMatch("node2", "pod2", "container1-running", true),
-				genTargetMatch("node2", "pod2", "container2-terminated", true),
-				genTargetMatch("node2", "pod2", "container3-waiting", true),
-				genTargetMatch("node2", "pod2", "ephemeral-container1-running", true),
-				genTargetMatch("node2", "pod2", "ephemeral-container2-terminated", true),
-				genTargetMatch("node2", "pod2", "ephemeral-container3-waiting", true),
+			expected: []Target{
+				genTarget("node1", "pod1", "container1-running"),
+				genTarget("node1", "pod1", "container2-terminated"),
+				genTarget("node1", "pod1", "container3-waiting"),
+				genTarget("node1", "pod1", "ephemeral-container1-running"),
+				genTarget("node1", "pod1", "ephemeral-container2-terminated"),
+				genTarget("node1", "pod1", "ephemeral-container3-waiting"),
+				genTarget("node2", "pod2", "container1-running"),
+				genTarget("node2", "pod2", "container2-terminated"),
+				genTarget("node2", "pod2", "container3-waiting"),
+				genTarget("node2", "pod2", "ephemeral-container1-running"),
+				genTarget("node2", "pod2", "ephemeral-container2-terminated"),
+				genTarget("node2", "pod2", "ephemeral-container3-waiting"),
 			},
 		},
 		{
 			name: "dot not include ephemeralContainers",
-			filter: targetFilter{
+			config: targetFilterConfig{
 				podFilter:              regexp.MustCompile(`.*`),
 				excludePodFilter:       nil,
 				containerFilter:        regexp.MustCompile(`.*`),
@@ -273,24 +266,24 @@ func TestTargetFilter(t *testing.T) {
 				ephemeralContainers:    false,
 				containerStates:        []ContainerState{RUNNING, TERMINATED, WAITING},
 			},
-			expected: []targetMatch{
-				genTargetMatch("node1", "pod1", "container1-running", true),
-				genTargetMatch("node1", "pod1", "container2-terminated", true),
-				genTargetMatch("node1", "pod1", "container3-waiting", true),
-				genTargetMatch("node1", "pod1", "init-container1-running", true),
-				genTargetMatch("node1", "pod1", "init-container2-terminated", true),
-				genTargetMatch("node1", "pod1", "init-container3-waiting", true),
-				genTargetMatch("node2", "pod2", "container1-running", true),
-				genTargetMatch("node2", "pod2", "container2-terminated", true),
-				genTargetMatch("node2", "pod2", "container3-waiting", true),
-				genTargetMatch("node2", "pod2", "init-container1-running", true),
-				genTargetMatch("node2", "pod2", "init-container2-terminated", true),
-				genTargetMatch("node2", "pod2", "init-container3-waiting", true),
+			expected: []Target{
+				genTarget("node1", "pod1", "container1-running"),
+				genTarget("node1", "pod1", "container2-terminated"),
+				genTarget("node1", "pod1", "container3-waiting"),
+				genTarget("node1", "pod1", "init-container1-running"),
+				genTarget("node1", "pod1", "init-container2-terminated"),
+				genTarget("node1", "pod1", "init-container3-waiting"),
+				genTarget("node2", "pod2", "container1-running"),
+				genTarget("node2", "pod2", "container2-terminated"),
+				genTarget("node2", "pod2", "container3-waiting"),
+				genTarget("node2", "pod2", "init-container1-running"),
+				genTarget("node2", "pod2", "init-container2-terminated"),
+				genTarget("node2", "pod2", "init-container3-waiting"),
 			},
 		},
 		{
 			name: "match running states",
-			filter: targetFilter{
+			config: targetFilterConfig{
 				podFilter:              regexp.MustCompile(`.*`),
 				excludePodFilter:       nil,
 				containerFilter:        regexp.MustCompile(`.*`),
@@ -299,35 +292,24 @@ func TestTargetFilter(t *testing.T) {
 				ephemeralContainers:    true,
 				containerStates:        []ContainerState{RUNNING},
 			},
-			expected: []targetMatch{
-				genTargetMatch("node1", "pod1", "container1-running", true),
-				genTargetMatch("node1", "pod1", "container2-terminated", false),
-				genTargetMatch("node1", "pod1", "container3-waiting", false),
-				genTargetMatch("node1", "pod1", "init-container1-running", true),
-				genTargetMatch("node1", "pod1", "init-container2-terminated", false),
-				genTargetMatch("node1", "pod1", "init-container3-waiting", false),
-				genTargetMatch("node1", "pod1", "ephemeral-container1-running", true),
-				genTargetMatch("node1", "pod1", "ephemeral-container2-terminated", false),
-				genTargetMatch("node1", "pod1", "ephemeral-container3-waiting", false),
-				genTargetMatch("node2", "pod2", "container1-running", true),
-				genTargetMatch("node2", "pod2", "container2-terminated", false),
-				genTargetMatch("node2", "pod2", "container3-waiting", false),
-				genTargetMatch("node2", "pod2", "init-container1-running", true),
-				genTargetMatch("node2", "pod2", "init-container2-terminated", false),
-				genTargetMatch("node2", "pod2", "init-container3-waiting", false),
-				genTargetMatch("node2", "pod2", "ephemeral-container1-running", true),
-				genTargetMatch("node2", "pod2", "ephemeral-container2-terminated", false),
-				genTargetMatch("node2", "pod2", "ephemeral-container3-waiting", false),
+			expected: []Target{
+				genTarget("node1", "pod1", "container1-running"),
+				genTarget("node1", "pod1", "init-container1-running"),
+				genTarget("node1", "pod1", "ephemeral-container1-running"),
+				genTarget("node2", "pod2", "container1-running"),
+				genTarget("node2", "pod2", "init-container1-running"),
+				genTarget("node2", "pod2", "ephemeral-container1-running"),
 			},
 		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			actual := []targetMatch{}
+			actual := []Target{}
 			for _, pod := range pods {
-				tt.filter.visit(pod, func(target *Target, containerStateMatched bool) {
-					actual = append(actual, targetMatch{target: *target, containerStateMatched: containerStateMatched})
+				filter := newTargetFilter(tt.config)
+				filter.visit(pod, func(target *Target) {
+					actual = append(actual, *target)
 				})
 			}
 
@@ -337,3 +319,220 @@ func TestTargetFilter(t *testing.T) {
 		})
 	}
 }
+
+func TestTargetFilterShouldAdd(t *testing.T) {
+	filter := newTargetFilter(targetFilterConfig{
+		// matches all
+		podFilter:              regexp.MustCompile(`.*`),
+		excludePodFilter:       nil,
+		containerFilter:        regexp.MustCompile(`.*`),
+		containerExcludeFilter: nil,
+		initContainers:         true,
+		ephemeralContainers:    true,
+		containerStates:        []ContainerState{RUNNING, TERMINATED, WAITING},
+	})
+	createPod := func(cs corev1.ContainerStatus) *corev1.Pod {
+		return &corev1.Pod{
+			ObjectMeta: metav1.ObjectMeta{
+				Namespace: "ns1",
+				Name:      "pod1",
+				UID:       "uid1",
+			},
+			Spec: corev1.PodSpec{
+				NodeName: "node1",
+			},
+			Status: corev1.PodStatus{
+				ContainerStatuses: []corev1.ContainerStatus{cs},
+			},
+		}
+	}
+	genTarget := func(container string) Target {
+		return Target{
+			Namespace: "ns1",
+			Node:      "node1",
+			Pod:       "pod1",
+			Container: container,
+		}
+	}
+	tests := []struct {
+		name     string
+		forget   bool
+		cs       corev1.ContainerStatus
+		expected []Target
+	}{
+		{
+			name:     "empty state should be ignored",
+			cs:       corev1.ContainerStatus{Name: "c1"},
+			expected: []Target{},
+		},
+		{
+			name: "running container observed the first time",
+			cs: corev1.ContainerStatus{
+				Name:        "c1",
+				ContainerID: "cid1",
+				State: corev1.ContainerState{
+					Running: &corev1.ContainerStateRunning{},
+				},
+			},
+			expected: []Target{genTarget("c1")},
+		},
+		{
+			name: "same container ID should be ignored",
+			cs: corev1.ContainerStatus{
+				Name:        "c1",
+				ContainerID: "cid1",
+				State: corev1.ContainerState{
+					Running: &corev1.ContainerStateRunning{},
+				},
+			},
+			expected: []Target{},
+		},
+		{
+			name: "different container ID can be added",
+			cs: corev1.ContainerStatus{
+				Name:        "c1",
+				ContainerID: "cid2", // changed
+				State: corev1.ContainerState{
+					Running: &corev1.ContainerStateRunning{},
+				},
+			},
+			expected: []Target{genTarget("c1")},
+		},
+		{
+			name:   "forget() allows the same ID ",
+			forget: true,
+			cs: corev1.ContainerStatus{
+				Name:        "c1",
+				ContainerID: "cid2",
+				State: corev1.ContainerState{
+					Running: &corev1.ContainerStateRunning{},
+				},
+			},
+			expected: []Target{genTarget("c1")},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if tt.forget {
+				filter.forget("uid1")
+			}
+			actual := []Target{}
+			filter.visit(createPod(tt.cs), func(target *Target) {
+				actual = append(actual, *target)
+			})
+			if !reflect.DeepEqual(tt.expected, actual) {
+				t.Errorf("expected %v, but actual %v", tt.expected, actual)
+			}
+		})
+	}
+}
+
+func TestChooseContainerID(t *testing.T) {
+	lastState := corev1.ContainerState{
+		Terminated: &corev1.ContainerStateTerminated{
+			ContainerID: "last",
+		},
+	}
+	tests := []struct {
+		name     string
+		cs       corev1.ContainerStatus
+		expected string
+	}{
+		{
+			name: "running",
+			cs: corev1.ContainerStatus{
+				ContainerID:          "current",
+				LastTerminationState: lastState,
+				State: corev1.ContainerState{
+					Running: &corev1.ContainerStateRunning{},
+				},
+			},
+			expected: "current",
+		},
+		{
+			name: "running (empty)",
+			cs: corev1.ContainerStatus{
+				LastTerminationState: lastState,
+				State: corev1.ContainerState{
+					Running: &corev1.ContainerStateRunning{},
+				},
+			},
+			expected: "",
+		},
+		{
+			name: "terminated (current terminated container)",
+			cs: corev1.ContainerStatus{
+				ContainerID:          "current",
+				LastTerminationState: lastState,
+				State: corev1.ContainerState{
+					Terminated: &corev1.ContainerStateTerminated{
+						ContainerID: "terminated",
+					},
+				},
+			},
+			expected: "terminated",
+		},
+		{
+			name: "terminated (last terminated container)",
+			cs: corev1.ContainerStatus{
+				ContainerID:          "current",
+				LastTerminationState: lastState,
+				State: corev1.ContainerState{
+					Terminated: &corev1.ContainerStateTerminated{},
+				},
+			},
+			expected: "last",
+		},
+		{
+			name: "terminated (empty)",
+			cs: corev1.ContainerStatus{
+				State: corev1.ContainerState{
+					Terminated: &corev1.ContainerStateTerminated{},
+				},
+			},
+			expected: "",
+		},
+		{
+			name: "waiting",
+			cs: corev1.ContainerStatus{
+				ContainerID:          "current",
+				LastTerminationState: lastState,
+				State: corev1.ContainerState{
+					Waiting: &corev1.ContainerStateWaiting{},
+				},
+			},
+			expected: "last",
+		},
+		{
+			name: "waiting (empty)",
+			cs: corev1.ContainerStatus{
+				ContainerID: "current", // should be ignored
+				State: corev1.ContainerState{
+					Waiting: &corev1.ContainerStateWaiting{},
+				},
+			},
+			expected: "",
+		},
+		{
+			name: "no current state with last state",
+			cs: corev1.ContainerStatus{
+				ContainerID:          "current",
+				LastTerminationState: lastState,
+			},
+			expected: "last",
+		},
+		{
+			name:     "empty state",
+			cs:       corev1.ContainerStatus{},
+			expected: "",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			actual := chooseContainerID(tt.cs)
+			if tt.expected != actual {
+				t.Errorf("expected %v, but actual %v", tt.expected, actual)
+			}
+		})
+	}
+}