Use non-default directory for pod logs and limit path to ASCII characters

Signed-off-by: Maksym Pavlenko <pavlenko.maksym@gmail.com>
This commit is contained in:
Maksym Pavlenko 2023-02-20 11:59:43 -08:00
parent 19d9405a1c
commit ff4f2907c5
6 changed files with 25 additions and 14 deletions

View File

@ -752,6 +752,7 @@ func TestSetDefaultsKubeletConfiguration(t *testing.T) {
MemoryThrottlingFactor: utilpointer.Float64Ptr(DefaultMemoryThrottlingFactor),
RegisterNode: utilpointer.Bool(true),
LocalStorageCapacityIsolation: utilpointer.Bool(true),
PodLogsDir: DefaultPodLogsDir,
},
},
{
@ -843,6 +844,7 @@ func TestSetDefaultsKubeletConfiguration(t *testing.T) {
MemoryThrottlingFactor: utilpointer.Float64Ptr(DefaultMemoryThrottlingFactor),
RegisterNode: utilpointer.Bool(true),
LocalStorageCapacityIsolation: utilpointer.Bool(true),
PodLogsDir: DefaultPodLogsDir,
},
},
{

View File

@ -20,6 +20,7 @@ import (
"fmt"
"path/filepath"
"time"
"unicode"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
utilerrors "k8s.io/apimachinery/pkg/util/errors"
@ -300,5 +301,13 @@ func ValidateKubeletConfiguration(kc *kubeletconfig.KubeletConfiguration, featur
allErrors = append(allErrors, fmt.Errorf("invalid configuration: pod logs path %q must be normalized", kc.PodLogsDir))
}
// Since pod logs path is used in metrics, make sure it contains only ASCII characters.
for _, c := range kc.PodLogsDir {
if c > unicode.MaxASCII {
allErrors = append(allErrors, fmt.Errorf("invalid configuration: pod logs path %q mut contains ASCII characters only", kc.PodLogsDir))
break
}
}
return utilerrors.NewAggregate(allErrors)
}

View File

@ -569,13 +569,6 @@ func TestValidateKubeletConfiguration(t *testing.T) {
return conf
},
errMsg: "invalid configuration: containerLogMonitorInterval must be a positive time duration greater than or equal to 3s",
}, {
name: "invalid podLogsPath",
configure: func(conf *kubeletconfig.KubeletConfiguration) *kubeletconfig.KubeletConfiguration {
conf.PodLogsDir = ""
return conf
},
errMsg: "invalid configuration: podLogsPath was not specified",
}, {
name: "pod logs path must be absolute",
configure: func(config *kubeletconfig.KubeletConfiguration) *kubeletconfig.KubeletConfiguration {
@ -590,7 +583,15 @@ func TestValidateKubeletConfiguration(t *testing.T) {
return config
},
errMsg: `invalid configuration: pod logs path "/path/../" must be normalized`,
}}
}, {
name: "pod logs path is ascii only",
configure: func(config *kubeletconfig.KubeletConfiguration) *kubeletconfig.KubeletConfiguration {
config.PodLogsDir = "/🧪"
return config
},
errMsg: `invalid configuration: pod logs path "/🧪" mut contains ASCII characters only`,
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {

View File

@ -703,12 +703,11 @@ func TestCadvisorListPodStatsWhenContainerLogFound(t *testing.T) {
"/pod0-c1": getTestContainerInfo(seedPod0Container1, pName0, namespace0, cName01),
}
podLogsDirectory := "/var/log/pods"
containerLogStats0 := makeFakeLogStats(0)
containerLogStats1 := makeFakeLogStats(0)
fakeStats := map[string]*volume.Metrics{
kuberuntime.BuildContainerLogsDirectory(podLogsDirectory, prf0.Namespace, prf0.Name, types.UID(prf0.UID), cName00): containerLogStats0,
kuberuntime.BuildContainerLogsDirectory(podLogsDirectory, prf0.Namespace, prf0.Name, types.UID(prf0.UID), cName01): containerLogStats1,
kuberuntime.BuildContainerLogsDirectory(testPodLogDirectory, prf0.Namespace, prf0.Name, types.UID(prf0.UID), cName00): containerLogStats0,
kuberuntime.BuildContainerLogsDirectory(testPodLogDirectory, prf0.Namespace, prf0.Name, types.UID(prf0.UID), cName01): containerLogStats1,
}
fakeStatsSlice := []*volume.Metrics{containerLogStats0, containerLogStats1}
fakeOS := &containertest.FakeOS{}

View File

@ -89,7 +89,7 @@ const (
cName9 = "container9-name"
)
const testPodLogDirectory = "/var/log/pods/"
const testPodLogDirectory = "/var/log/kube/pods/" // Use non-default path to ensure stats are collected properly
func TestCRIListPodStats(t *testing.T) {
ctx := context.Background()

View File

@ -50,7 +50,7 @@ func NewFakeHostStatsProviderWithData(fakeStats map[string]*volume.Metrics, osIn
}
func (f *fakeHostStatsProvider) getPodLogStats(podNamespace, podName string, podUID types.UID, rootFsInfo *cadvisorapiv2.FsInfo) (*statsapi.FsStats, error) {
path := kuberuntime.BuildPodLogsDirectory("/var/log/pods", podNamespace, podName, podUID)
path := kuberuntime.BuildPodLogsDirectory("/var/log/kube/pods/", podNamespace, podName, podUID)
files, err := f.osInterface.ReadDir(path)
if err != nil {
return nil, err
@ -68,7 +68,7 @@ func (f *fakeHostStatsProvider) getPodLogStats(podNamespace, podName string, pod
}
func (f *fakeHostStatsProvider) getPodContainerLogStats(podNamespace, podName string, podUID types.UID, containerName string, rootFsInfo *cadvisorapiv2.FsInfo) (*statsapi.FsStats, error) {
path := kuberuntime.BuildContainerLogsDirectory("/var/log/pods", podNamespace, podName, podUID, containerName)
path := kuberuntime.BuildContainerLogsDirectory("/var/log/kube/pods/", podNamespace, podName, podUID, containerName)
metricsProvider := NewFakeMetricsDu(path, f.fakeStats[path])
return fakeMetricsProvidersToStats([]volume.MetricsProvider{metricsProvider}, rootFsInfo)
}