Skip to content

Commit

Permalink
Check container symlink when a new source is scheduled (#28712)
Browse files Browse the repository at this point in the history
  • Loading branch information
gh123man authored Aug 26, 2024
1 parent dd00b11 commit 207b216
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 7 deletions.
4 changes: 4 additions & 0 deletions pkg/logs/launchers/file/launcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,10 @@ func (s *Launcher) launchTailers(source *sources.LogSource) {
if s.tailers.Count() >= s.tailingLimit {
return
}

if fileprovider.ShouldIgnore(s.validatePodContainerID, file) {
continue
}
if tailer, isTailed := s.tailers.Get(file.GetScanKey()); isTailed {
// the file is already tailed, update the existing tailer's source so that the tailer
// uses this new source going forward
Expand Down
6 changes: 3 additions & 3 deletions pkg/logs/launchers/file/provider/file_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ func (p *FileProvider) addFilesToTailList(validatePodContainerID bool, inputFile
// Add each file one by one up to the limit
for j := 0; j < len(inputFiles) && len(filesToTail) < p.filesLimit; j++ {
file := inputFiles[j]
if shouldIgnore(validatePodContainerID, file) {
if ShouldIgnore(validatePodContainerID, file) {
continue
}
filesToTail = append(filesToTail, file)
Expand Down Expand Up @@ -348,7 +348,7 @@ func (p *FileProvider) applyOrdering(files []*tailer.File) {
}
}

// shouldIgnore resolves symlinks in /var/log/containers in order to use that redirection
// ShouldIgnore resolves symlinks in /var/log/containers in order to use that redirection
// to validate that we will be reading a file for the correct container.
//
// We have to make sure that the file we just detected is tagged with the correct
Expand All @@ -361,7 +361,7 @@ func (p *FileProvider) applyOrdering(files []*tailer.File) {
// See these links for more info:
// - https://github.com/kubernetes/kubernetes/issues/58638
// - https://github.com/fabric8io/fluent-plugin-kubernetes_metadata_filter/issues/105
func shouldIgnore(validatePodContainerID bool, file *tailer.File) bool {
func ShouldIgnore(validatePodContainerID bool, file *tailer.File) bool {
// this method needs a source config to detect whether we should ignore that file or not
if file == nil || file.Source == nil || file.Source.Config() == nil {
return false
Expand Down
8 changes: 4 additions & 4 deletions pkg/logs/launchers/file/provider/file_provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -679,21 +679,21 @@ func TestContainerIDInContainerLogFile(t *testing.T) {
}

// we've found a symlink validating that the file we have just scanned is concerning the container we're currently processing for this source
assert.False(shouldIgnore(true, &file), "the file existing in ContainersLogsDir is pointing to the same container, scanned file should be tailed")
assert.False(ShouldIgnore(true, &file), "the file existing in ContainersLogsDir is pointing to the same container, scanned file should be tailed")

// now, let's change the container for which we are trying to scan files,
// because the symlink is pointing from another container, we should ignore
// that log file
file.Source.Config().Identifier = "1234123412341234123412341234123412341234123412341234123412341234"
assert.True(shouldIgnore(true, &file), "the file existing in ContainersLogsDir is not pointing to the same container, scanned file should be ignored")
assert.True(ShouldIgnore(true, &file), "the file existing in ContainersLogsDir is not pointing to the same container, scanned file should be ignored")

// in this scenario, no link is found in /var/log/containers, thus, we should not ignore the file
os.Remove("/tmp/myapp_my-namespace_myapp-abcdefabcdefabcdabcdefabcdefabcdabcdefabcdefabcdabcdefabcdefabcd.log")
assert.False(shouldIgnore(true, &file), "no files existing in ContainersLogsDir, we should not ignore the file we have just scanned")
assert.False(ShouldIgnore(true, &file), "no files existing in ContainersLogsDir, we should not ignore the file we have just scanned")

// in this scenario, the file we've found doesn't look like a container ID
os.Symlink("/var/log/pods/file-uuid-foo-bar.log", "/tmp/myapp_my-namespace_myapp-thisisnotacontainerIDevenifthisispointingtothecorrectfile.log")
assert.False(shouldIgnore(true, &file), "no container ID found, we don't want to ignore this scanned file")
assert.False(ShouldIgnore(true, &file), "no container ID found, we don't want to ignore this scanned file")
}

func TestContainerPathsAreCorrectlyIgnored(t *testing.T) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# Each section from every release note are combined when the
# CHANGELOG.rst is rendered. So the text needs to be worded so that
# it does not depend on any information only available in another
# section. This may mean repeating some details, but each section
# must be readable independently of the other.
#
# Each section note must be formatted as reStructuredText.
---
fixes:
- |
Fixed a rare issue where short-lived containers could cause
logs to be sent with the wrong container ID.

0 comments on commit 207b216

Please sign in to comment.