Skip to content

Commit 6da73aa

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes#93333 from loburm/fix-logrotate
Fix an issue when rotated logs of dead containers are not removed.
2 parents d20d894 + 5d1b3e2 commit 6da73aa

12 files changed

+235
-44
lines changed

‎pkg/kubelet/container/os.go‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,9 @@ type OSInterface interface {
3838
Pipe() (r *os.File, w *os.File, err error)
3939
ReadDir(dirname string) ([]os.FileInfo, error)
4040
Glob(pattern string) ([]string, error)
41+
Open(name string) (*os.File, error)
42+
OpenFile(name string, flag int, perm os.FileMode) (*os.File, error)
43+
Rename(oldpath, newpath string) error
4144
}
4245

4346
// RealOS is used to dispatch the real system level operations.
@@ -105,3 +108,18 @@ func (RealOS) ReadDir(dirname string) ([]os.FileInfo, error) {
105108
func (RealOS) Glob(pattern string) ([]string, error) {
106109
return filepath.Glob(pattern)
107110
}
111+
112+
// Open will call os.Open to return the file.
113+
func (RealOS) Open(name string) (*os.File, error) {
114+
return os.Open(name)
115+
}
116+
117+
// OpenFile will call os.OpenFile to return the file.
118+
func (RealOS) OpenFile(name string, flag int, perm os.FileMode) (*os.File, error) {
119+
return os.OpenFile(name, flag, perm)
120+
}
121+
122+
// Rename will call os.Rename to rename a file.
123+
func (RealOS) Rename(oldpath, newpath string) error {
124+
return os.Rename(oldpath, newpath)
125+
}

‎pkg/kubelet/container/testing/os.go‎

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ type FakeOS struct {
3030
ReadDirFn func(string) ([]os.FileInfo, error)
3131
MkdirAllFn func(string, os.FileMode) error
3232
SymlinkFn func(string, string) error
33+
GlobFn func(string, string) bool
3334
HostName string
3435
Removes []string
3536
Files map[string][]*os.FileInfo
@@ -78,8 +79,12 @@ func (f *FakeOS) RemoveAll(path string) error {
7879
return nil
7980
}
8081

81-
// Create is a fake call that returns nil.
82-
func (FakeOS) Create(path string) (*os.File, error) {
82+
// Create is a fake call that creates a virtual file and returns nil.
83+
func (f *FakeOS) Create(path string) (*os.File, error) {
84+
if f.Files == nil {
85+
f.Files = make(map[string][]*os.FileInfo)
86+
}
87+
f.Files[path] = []*os.FileInfo{}
8388
return nil, nil
8489
}
8590

@@ -111,7 +116,31 @@ func (f *FakeOS) ReadDir(dirname string) ([]os.FileInfo, error) {
111116
return nil, nil
112117
}
113118

114-
// Glob is a fake call that returns nil.
119+
// Glob is a fake call that returns list of virtual files matching a pattern.
115120
func (f *FakeOS) Glob(pattern string) ([]string, error) {
121+
if f.GlobFn != nil {
122+
var res []string
123+
for k := range f.Files {
124+
if f.GlobFn(pattern, k) {
125+
res = append(res, k)
126+
}
127+
}
128+
return res, nil
129+
}
130+
return nil, nil
131+
}
132+
133+
// Open is a fake call that returns nil.
134+
func (FakeOS) Open(name string) (*os.File, error) {
116135
return nil, nil
117136
}
137+
138+
// OpenFile is a fake call that return nil.
139+
func (FakeOS) OpenFile(name string, flag int, perm os.FileMode) (*os.File, error) {
140+
return nil, nil
141+
}
142+
143+
// Rename is a fake call that return nil.
144+
func (FakeOS) Rename(oldpath, newpath string) error {
145+
return nil
146+
}

‎pkg/kubelet/kubelet.go‎

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -585,6 +585,22 @@ func NewMainKubelet(kubeCfg *kubeletconfiginternal.KubeletConfiguration,
585585
klet.runtimeClassManager = runtimeclass.NewManager(kubeDeps.KubeClient)
586586
}
587587

588+
if containerRuntime == kubetypes.RemoteContainerRuntime && utilfeature.DefaultFeatureGate.Enabled(features.CRIContainerLogRotation) {
589+
// setup containerLogManager for CRI container runtime
590+
containerLogManager, err := logs.NewContainerLogManager(
591+
klet.runtimeService,
592+
kubeDeps.OSInterface,
593+
kubeCfg.ContainerLogMaxSize,
594+
int(kubeCfg.ContainerLogMaxFiles),
595+
)
596+
if err != nil {
597+
return nil, fmt.Errorf("failed to initialize container log manager: %v", err)
598+
}
599+
klet.containerLogManager = containerLogManager
600+
} else {
601+
klet.containerLogManager = logs.NewStubContainerLogManager()
602+
}
603+
588604
runtime, err := kuberuntime.NewKubeGenericRuntimeManager(
589605
kubecontainer.FilterEventRecorder(kubeDeps.Recorder),
590606
klet.livenessManager,
@@ -605,6 +621,7 @@ func NewMainKubelet(kubeCfg *kubeletconfiginternal.KubeletConfiguration,
605621
kubeDeps.RemoteImageService,
606622
kubeDeps.ContainerManager.InternalContainerLifecycle(),
607623
kubeDeps.dockerLegacyService,
624+
klet.containerLogManager,
608625
klet.runtimeClassManager,
609626
)
610627
if err != nil {
@@ -662,21 +679,6 @@ func NewMainKubelet(kubeCfg *kubeletconfiginternal.KubeletConfiguration,
662679
}
663680
klet.imageManager = imageManager
664681

665-
if containerRuntime == kubetypes.RemoteContainerRuntime && utilfeature.DefaultFeatureGate.Enabled(features.CRIContainerLogRotation) {
666-
// setup containerLogManager for CRI container runtime
667-
containerLogManager, err := logs.NewContainerLogManager(
668-
klet.runtimeService,
669-
kubeCfg.ContainerLogMaxSize,
670-
int(kubeCfg.ContainerLogMaxFiles),
671-
)
672-
if err != nil {
673-
return nil, fmt.Errorf("failed to initialize container log manager: %v", err)
674-
}
675-
klet.containerLogManager = containerLogManager
676-
} else {
677-
klet.containerLogManager = logs.NewStubContainerLogManager()
678-
}
679-
680682
if kubeCfg.ServerTLSBootstrap && kubeDeps.TLSOptions != nil && utilfeature.DefaultFeatureGate.Enabled(features.RotateKubeletServerCertificate) {
681683
klet.serverCertificateManager, err = kubeletcertificate.NewKubeletServerCertificateManager(klet.kubeClient, kubeCfg, klet.nodeName, klet.getLastObservedNodeAddresses, certDirectory)
682684
if err != nil {

‎pkg/kubelet/kuberuntime/BUILD‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ go_library(
4141
"//pkg/kubelet/images:go_default_library",
4242
"//pkg/kubelet/kuberuntime/logs:go_default_library",
4343
"//pkg/kubelet/lifecycle:go_default_library",
44+
"//pkg/kubelet/logs:go_default_library",
4445
"//pkg/kubelet/metrics:go_default_library",
4546
"//pkg/kubelet/prober/results:go_default_library",
4647
"//pkg/kubelet/runtimeclass:go_default_library",

‎pkg/kubelet/kuberuntime/fake_kuberuntime_manager.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import (
3232
kubecontainer "k8s.io/kubernetes/pkg/kubelet/container"
3333
"k8s.io/kubernetes/pkg/kubelet/images"
3434
"k8s.io/kubernetes/pkg/kubelet/lifecycle"
35+
"k8s.io/kubernetes/pkg/kubelet/logs"
3536
proberesults "k8s.io/kubernetes/pkg/kubelet/prober/results"
3637
)
3738

@@ -73,6 +74,10 @@ func (f *fakePodStateProvider) IsPodTerminated(uid types.UID) bool {
7374

7475
func newFakeKubeRuntimeManager(runtimeService internalapi.RuntimeService, imageService internalapi.ImageManagerService, machineInfo *cadvisorapi.MachineInfo, osInterface kubecontainer.OSInterface, runtimeHelper kubecontainer.RuntimeHelper, keyring credentialprovider.DockerKeyring) (*kubeGenericRuntimeManager, error) {
7576
recorder := &record.FakeRecorder{}
77+
logManager, err := logs.NewContainerLogManager(runtimeService, osInterface, "1", 2)
78+
if err != nil {
79+
return nil, err
80+
}
7681
kubeRuntimeManager := &kubeGenericRuntimeManager{
7782
recorder: recorder,
7883
cpuCFSQuota: false,
@@ -88,6 +93,7 @@ func newFakeKubeRuntimeManager(runtimeService internalapi.RuntimeService, imageS
8893
seccompProfileRoot: fakeSeccompProfileRoot,
8994
internalLifecycle: cm.NewFakeInternalContainerLifecycle(),
9095
logReduction: logreduction.NewLogReduction(identicalErrorDelay),
96+
logManager: logManager,
9197
}
9298

9399
typedVersion, err := runtimeService.Version(kubeRuntimeAPIVersion)

‎pkg/kubelet/kuberuntime/kuberuntime_container.go‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -855,19 +855,19 @@ func (m *kubeGenericRuntimeManager) removeContainer(containerID string) error {
855855

856856
// removeContainerLog removes the container log.
857857
func (m *kubeGenericRuntimeManager) removeContainerLog(containerID string) error {
858-
// Remove the container log.
858+
// Use log manager to remove rotated logs.
859+
err := m.logManager.Clean(containerID)
860+
if err != nil {
861+
return err
862+
}
863+
859864
status, err := m.runtimeService.ContainerStatus(containerID)
860865
if err != nil {
861866
return fmt.Errorf("failed to get container status %q: %v", containerID, err)
862867
}
863-
labeledInfo := getContainerInfoFromLabels(status.Labels)
864-
path := status.GetLogPath()
865-
if err := m.osInterface.Remove(path); err != nil && !os.IsNotExist(err) {
866-
return fmt.Errorf("failed to remove container %q log %q: %v", containerID, path, err)
867-
}
868-
869868
// Remove the legacy container log symlink.
870869
// TODO(random-liu): Remove this after cluster logging supports CRI container log path.
870+
labeledInfo := getContainerInfoFromLabels(status.Labels)
871871
legacySymlink := legacyLogSymlink(containerID, labeledInfo.ContainerName, labeledInfo.PodName,
872872
labeledInfo.PodNamespace)
873873
if err := m.osInterface.Remove(legacySymlink); err != nil && !os.IsNotExist(err) {

‎pkg/kubelet/kuberuntime/kuberuntime_container_test.go‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package kuberuntime
1919
import (
2020
"fmt"
2121
"path/filepath"
22+
"regexp"
2223
"strings"
2324
"testing"
2425
"time"
@@ -65,12 +66,22 @@ func TestRemoveContainer(t *testing.T) {
6566

6667
containerID := fakeContainers[0].Id
6768
fakeOS := m.osInterface.(*containertest.FakeOS)
69+
fakeOS.GlobFn = func(pattern, path string) bool {
70+
pattern = strings.Replace(pattern, "*", ".*", -1)
71+
return regexp.MustCompile(pattern).MatchString(path)
72+
}
73+
expectedContainerLogPath := filepath.Join(podLogsRootDirectory, "new_bar_12345678", "foo", "0.log")
74+
expectedContainerLogPathRotated := filepath.Join(podLogsRootDirectory, "new_bar_12345678", "foo", "0.log.20060102-150405")
75+
expectedContainerLogSymlink := legacyLogSymlink(containerID, "foo", "bar", "new")
76+
77+
fakeOS.Create(expectedContainerLogPath)
78+
fakeOS.Create(expectedContainerLogPathRotated)
79+
6880
err = m.removeContainer(containerID)
6981
assert.NoError(t, err)
7082
// Verify container log is removed
71-
expectedContainerLogPath := filepath.Join(podLogsRootDirectory, "new_bar_12345678", "foo", "0.log")
72-
expectedContainerLogSymlink := legacyLogSymlink(containerID, "foo", "bar", "new")
73-
assert.Equal(t, fakeOS.Removes, []string{expectedContainerLogPath, expectedContainerLogSymlink})
83+
84+
assert.Equal(t, []string{expectedContainerLogPath, expectedContainerLogPathRotated, expectedContainerLogSymlink}, fakeOS.Removes)
7485
// Verify container is removed
7586
assert.Contains(t, fakeRuntime.Called, "RemoveContainer")
7687
containers, err := fakeRuntime.ListContainers(&runtimeapi.ContainerFilter{Id: containerID})

‎pkg/kubelet/kuberuntime/kuberuntime_manager.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ import (
4646
"k8s.io/kubernetes/pkg/kubelet/events"
4747
"k8s.io/kubernetes/pkg/kubelet/images"
4848
"k8s.io/kubernetes/pkg/kubelet/lifecycle"
49+
"k8s.io/kubernetes/pkg/kubelet/logs"
4950
proberesults "k8s.io/kubernetes/pkg/kubelet/prober/results"
5051
"k8s.io/kubernetes/pkg/kubelet/runtimeclass"
5152
"k8s.io/kubernetes/pkg/kubelet/types"
@@ -127,6 +128,9 @@ type kubeGenericRuntimeManager struct {
127128
// A shim to legacy functions for backward compatibility.
128129
legacyLogProvider LegacyLogProvider
129130

131+
// Manage container logs.
132+
logManager logs.ContainerLogManager
133+
130134
// Manage RuntimeClass resources.
131135
runtimeClassManager *runtimeclass.Manager
132136

@@ -168,6 +172,7 @@ func NewKubeGenericRuntimeManager(
168172
imageService internalapi.ImageManagerService,
169173
internalLifecycle cm.InternalContainerLifecycle,
170174
legacyLogProvider LegacyLogProvider,
175+
logManager logs.ContainerLogManager,
171176
runtimeClassManager *runtimeclass.Manager,
172177
) (KubeGenericRuntime, error) {
173178
kubeRuntimeManager := &kubeGenericRuntimeManager{
@@ -185,6 +190,7 @@ func NewKubeGenericRuntimeManager(
185190
keyring: credentialprovider.NewDockerKeyring(),
186191
internalLifecycle: internalLifecycle,
187192
legacyLogProvider: legacyLogProvider,
193+
logManager: logManager,
188194
runtimeClassManager: runtimeClassManager,
189195
logReduction: logreduction.NewLogReduction(identicalErrorDelay),
190196
}

‎pkg/kubelet/logs/BUILD‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ go_library(
99
importpath = "k8s.io/kubernetes/pkg/kubelet/logs",
1010
visibility = ["//visibility:public"],
1111
deps = [
12+
"//pkg/kubelet/container:go_default_library",
1213
"//staging/src/k8s.io/apimachinery/pkg/api/resource:go_default_library",
1314
"//staging/src/k8s.io/apimachinery/pkg/util/clock:go_default_library",
1415
"//staging/src/k8s.io/apimachinery/pkg/util/wait:go_default_library",
@@ -23,6 +24,7 @@ go_test(
2324
srcs = ["container_log_manager_test.go"],
2425
embed = [":go_default_library"],
2526
deps = [
27+
"//pkg/kubelet/container:go_default_library",
2628
"//staging/src/k8s.io/apimachinery/pkg/util/clock:go_default_library",
2729
"//staging/src/k8s.io/cri-api/pkg/apis/runtime/v1alpha2:go_default_library",
2830
"//staging/src/k8s.io/cri-api/pkg/apis/testing:go_default_library",

0 commit comments

Comments
 (0)