Skip to content

Commit

Permalink
Better handle missing containerids
Browse files Browse the repository at this point in the history
There are cases when we are notified of a new container and there is a
ContainerStatus object that matches the container, but the container is
not running yet so it does not have a containerID property within it
yet. This messes up our state machine where we attempt to tail the wrong
file without a containerID as part of the file path.
  • Loading branch information
Mike Keesey authored and mkeesey committed Oct 18, 2024
1 parent d9ebd8c commit 79519c6
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 3 deletions.
6 changes: 4 additions & 2 deletions collector/logs/sources/tail/pod_discovery.go
Original file line number Diff line number Diff line change
Expand Up @@ -206,8 +206,10 @@ func (i *PodDiscovery) tombstoneTarget(existingCurrentTarget *currenttarget) {

// We don't want to just tell tailsource to stop tailing when the file reaches EOF because the file
// may be written to again in the "future" and perhaps even rotated.
logger.Infof("Tombstoning target %s", existingCurrentTarget.CurrentTarget.FilePath)
existingCurrentTarget.ExpireTime = i.nowFunc().Add(1 * time.Minute)
if existingCurrentTarget.ExpireTime.IsZero() {
logger.Infof("Tombstoning target %s", existingCurrentTarget.CurrentTarget.FilePath)
existingCurrentTarget.ExpireTime = i.nowFunc().Add(1 * time.Minute)
}
}

func (i *PodDiscovery) cleanTombstonedTargets() {
Expand Down
19 changes: 19 additions & 0 deletions collector/logs/sources/tail/pod_discovery_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,25 @@ func TestPodDiscoveryLifecycle(t *testing.T) {
require.Equal(t, 0, len(tailSource.targetRemoves))
})

t.Run("Add Pod Without ContainerId", func(t *testing.T) {
_, tailSource, podDiscovery := create()
defer tailSource.Close()
err := podDiscovery.Open(context.Background())
require.NoError(t, err)

pod := scrapedPod("pod1")
pod.Status.ContainerStatuses[0].ContainerID = ""

podDiscovery.OnAdd(pod, false)

cleanTombstonedTargets(podDiscovery)
err = podDiscovery.Close()
require.NoError(t, err)
// No containerID yet, so no target added.
require.Equal(t, 0, len(tailSource.targetAdds))
require.Equal(t, 0, len(tailSource.targetRemoves))
})

t.Run("Add Tolerant of Failure", func(t *testing.T) {
// Failure tailing first container, but ok to tail the second container
failureFunc := func(target FileTailTarget) error {
Expand Down
3 changes: 2 additions & 1 deletion collector/logs/sources/tail/pod_target.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,8 @@ func isTargetChanged(old, new FileTailTarget) bool {
func getContainerID(containerStatuses []v1.ContainerStatus, containerName string) (string, bool) {
for _, container := range containerStatuses {
if container.Name == containerName {
return container.ContainerID, true
containerIdPopulated := container.ContainerID != ""
return container.ContainerID, containerIdPopulated
}
}
return "", false
Expand Down

0 comments on commit 79519c6

Please sign in to comment.