From c025861e0cd6dcef2790b6be907407ff5a4b017c Mon Sep 17 00:00:00 2001 From: Francesco Romani Date: Tue, 24 Sep 2024 09:29:57 +0200 Subject: [PATCH] node: metrics: add resource alignment metrics In order to improve the observability of the resource management in kubelet, cpu allocation and NUMA alignment, we add more metrics to report if resource alignment is in effect. The more precise reporting would probably be using pod status, but this would require more invasive and riskier changes, and possibly extra interactions to the APIServer. We start adding metrics to report if containers got their compute resources aligned. If metrics are growing, the assingment is working as expected; If metrics stay consistent, perhaps at zero, no resource alignment is done. Extra fixes brought by this work - retroactively add labels for existing tests - running metrics test demands precision accounting to avoid flakes; ensure the node state is restored pristine between each test, to minimize the aforementioned risk of flakes. - The test pod command line was wrong, with this the pod could not reach Running state. That gone unnoticed so far because no test using this utility function actually needed a pod in running state. Signed-off-by: Francesco Romani --- pkg/kubelet/cm/cpumanager/policy_static.go | 6 +++++ pkg/kubelet/cm/topologymanager/policy.go | 8 ++++++ .../cm/topologymanager/scope_container.go | 4 +++ pkg/kubelet/cm/topologymanager/scope_pod.go | 4 +++ pkg/kubelet/metrics/metrics.go | 22 ++++++++++++++++ test/e2e_node/cpu_manager_metrics_test.go | 26 +++++++++++++++++-- test/e2e_node/resource_metrics_test.go | 7 +++++ .../e2e_node/topology_manager_metrics_test.go | 21 +++++++++++++++ 8 files changed, 96 insertions(+), 2 deletions(-) diff --git a/pkg/kubelet/cm/cpumanager/policy_static.go b/pkg/kubelet/cm/cpumanager/policy_static.go index 333c5c3caaf..db8770f96f6 100644 --- a/pkg/kubelet/cm/cpumanager/policy_static.go +++ b/pkg/kubelet/cm/cpumanager/policy_static.go @@ -312,7 +312,13 @@ func (p *staticPolicy) Allocate(s state.State, pod *v1.Pod, container *v1.Contai defer func() { if rerr != nil { metrics.CPUManagerPinningErrorsTotal.Inc() + return } + if !p.options.FullPhysicalCPUsOnly { + // increment only if we know we allocate aligned resources + return + } + metrics.ContainerAlignedComputeResources.WithLabelValues(metrics.AlignScopeContainer, metrics.AlignedPhysicalCPU).Inc() }() if p.options.FullPhysicalCPUsOnly { diff --git a/pkg/kubelet/cm/topologymanager/policy.go b/pkg/kubelet/cm/topologymanager/policy.go index 09f1c7bc2cf..80e9292ae41 100644 --- a/pkg/kubelet/cm/topologymanager/policy.go +++ b/pkg/kubelet/cm/topologymanager/policy.go @@ -30,6 +30,14 @@ type Policy interface { Merge(providersHints []map[string][]TopologyHint) (TopologyHint, bool) } +// IsAlignmentGuaranteed return true if the given policy guarantees that either +// the compute resources will be allocated within a NUMA boundary, or the allocation will fail at all. +func IsAlignmentGuaranteed(p Policy) bool { + // We are abusing the name, but atm this matches almost 1:1 the policy name + // so we are not adding new fields for now. + return p.Name() == PolicySingleNumaNode +} + // Merge a TopologyHints permutation to a single hint by performing a bitwise-AND // of their affinity masks. The hint shall be preferred if all hits in the permutation // are preferred. diff --git a/pkg/kubelet/cm/topologymanager/scope_container.go b/pkg/kubelet/cm/topologymanager/scope_container.go index 857ac2e9ae0..61cef19df58 100644 --- a/pkg/kubelet/cm/topologymanager/scope_container.go +++ b/pkg/kubelet/cm/topologymanager/scope_container.go @@ -61,6 +61,10 @@ func (s *containerScope) Admit(pod *v1.Pod) lifecycle.PodAdmitResult { metrics.TopologyManagerAdmissionErrorsTotal.Inc() return admission.GetPodAdmitResult(err) } + + if IsAlignmentGuaranteed(s.policy) { + metrics.ContainerAlignedComputeResources.WithLabelValues(metrics.AlignScopeContainer, metrics.AlignedNUMANode).Inc() + } } return admission.GetPodAdmitResult(nil) } diff --git a/pkg/kubelet/cm/topologymanager/scope_pod.go b/pkg/kubelet/cm/topologymanager/scope_pod.go index 2dc1773fb3d..61d599ebc08 100644 --- a/pkg/kubelet/cm/topologymanager/scope_pod.go +++ b/pkg/kubelet/cm/topologymanager/scope_pod.go @@ -62,6 +62,10 @@ func (s *podScope) Admit(pod *v1.Pod) lifecycle.PodAdmitResult { return admission.GetPodAdmitResult(err) } } + if IsAlignmentGuaranteed(s.policy) { + // increment only if we know we allocate aligned resources. + metrics.ContainerAlignedComputeResources.WithLabelValues(metrics.AlignScopePod, metrics.AlignedNUMANode).Inc() + } return admission.GetPodAdmitResult(nil) } diff --git a/pkg/kubelet/metrics/metrics.go b/pkg/kubelet/metrics/metrics.go index a21e8c1484d..30a194a3a66 100644 --- a/pkg/kubelet/metrics/metrics.go +++ b/pkg/kubelet/metrics/metrics.go @@ -127,10 +127,21 @@ const ( // Metric for tracking garbage collected images ImageGarbageCollectedTotalKey = "image_garbage_collected_total" + // Metric for tracking aligment of compute resources + ContainerAlignedComputeResourcesNameKey = "container_aligned_compute_resources_count" + ContainerAlignedComputeResourcesScopeLabelKey = "scope" + ContainerAlignedComputeResourcesBoundaryLabelKey = "boundary" + // Values used in metric labels Container = "container" InitContainer = "init_container" EphemeralContainer = "ephemeral_container" + + AlignScopePod = "pod" + AlignScopeContainer = "container" + + AlignedPhysicalCPU = "physical_cpu" + AlignedNUMANode = "numa_node" ) type imageSizeBucket struct { @@ -762,6 +773,16 @@ var ( }, ) + ContainerAlignedComputeResources = metrics.NewCounterVec( + &metrics.CounterOpts{ + Subsystem: KubeletSubsystem, + Name: ContainerAlignedComputeResourcesNameKey, + Help: "Cumulative number of aligned compute resources allocated to containers by alignment type.", + StabilityLevel: metrics.ALPHA, + }, + []string{ContainerAlignedComputeResourcesScopeLabelKey, ContainerAlignedComputeResourcesBoundaryLabelKey}, + ) + // MemoryManagerPinningRequestTotal tracks the number of times the pod spec required the memory manager to pin memory pages MemoryManagerPinningRequestTotal = metrics.NewCounter( &metrics.CounterOpts{ @@ -985,6 +1006,7 @@ func Register(collectors ...metrics.StableCollector) { legacyregistry.MustRegister(RunPodSandboxErrors) legacyregistry.MustRegister(CPUManagerPinningRequestsTotal) legacyregistry.MustRegister(CPUManagerPinningErrorsTotal) + legacyregistry.MustRegister(ContainerAlignedComputeResources) if utilfeature.DefaultFeatureGate.Enabled(features.MemoryManager) { legacyregistry.MustRegister(MemoryManagerPinningRequestTotal) legacyregistry.MustRegister(MemoryManagerPinningErrorsTotal) diff --git a/test/e2e_node/cpu_manager_metrics_test.go b/test/e2e_node/cpu_manager_metrics_test.go index 7a760afb21f..aacde715cab 100644 --- a/test/e2e_node/cpu_manager_metrics_test.go +++ b/test/e2e_node/cpu_manager_metrics_test.go @@ -91,6 +91,7 @@ var _ = SIGDescribe("CPU Manager Metrics", framework.WithSerial(), feature.CPUMa ginkgo.AfterEach(func(ctx context.Context) { if testPod != nil { deletePodSyncByName(ctx, f, testPod.Name) + waitForContainerRemoval(ctx, testPod.Spec.Containers[0].Name, testPod.Name, testPod.Namespace) } updateKubeletConfig(ctx, f, oldCfg, true) }) @@ -160,11 +161,32 @@ var _ = SIGDescribe("CPU Manager Metrics", framework.WithSerial(), feature.CPUMa ginkgo.By("Ensuring the metrics match the expectations a few more times") gomega.Consistently(ctx, getKubeletMetrics, 1*time.Minute, 15*time.Second).Should(matchResourceMetrics) }) + + ginkgo.It("should return updated alignment counters when pod successfully run", func(ctx context.Context) { + ginkgo.By("Creating the test pod") + testPod = e2epod.NewPodClient(f).Create(ctx, makeGuaranteedCPUExclusiveSleeperPod("count-align-smt-ok", smtLevel)) + + // we updated the kubelet config in BeforeEach, so we can assume we start fresh. + // being [Serial], we can also assume noone else but us is running pods. + ginkgo.By("Checking the cpumanager metrics right after the kubelet restart, with pod should be admitted") + + idFn := makeCustomPairID("scope", "boundary") + matchAlignmentMetrics := gstruct.MatchKeys(gstruct.IgnoreExtras, gstruct.Keys{ + "kubelet_container_aligned_compute_resources_count": gstruct.MatchElements(idFn, gstruct.IgnoreExtras, gstruct.Elements{ + "container::physical_cpu": timelessSample(1), + }), + }) + + ginkgo.By("Giving the Kubelet time to update the alignment metrics") + gomega.Eventually(ctx, getKubeletMetrics, 1*time.Minute, 15*time.Second).Should(matchAlignmentMetrics) + ginkgo.By("Ensuring the metrics match the expectations about alignment metrics a few more times") + gomega.Consistently(ctx, getKubeletMetrics, 1*time.Minute, 15*time.Second).Should(matchAlignmentMetrics) + }) }) }) func getKubeletMetrics(ctx context.Context) (e2emetrics.KubeletMetrics, error) { - ginkgo.By("getting Kubelet metrics from the metrics API") + ginkgo.By("Getting Kubelet metrics from the metrics API") return e2emetrics.GrabKubeletMetricsWithoutProxy(ctx, nodeNameOrIP()+":10255", "/metrics") } @@ -189,7 +211,7 @@ func makeGuaranteedCPUExclusiveSleeperPod(name string, cpus int) *v1.Pod { v1.ResourceMemory: resource.MustParse("64Mi"), }, }, - Command: []string{"sh", "-c", "sleep", "1d"}, + Command: []string{"sh", "-c", "sleep 1d"}, }, }, }, diff --git a/test/e2e_node/resource_metrics_test.go b/test/e2e_node/resource_metrics_test.go index 0d6e3e725c5..28f1fed97fd 100644 --- a/test/e2e_node/resource_metrics_test.go +++ b/test/e2e_node/resource_metrics_test.go @@ -157,6 +157,13 @@ func containerID(element interface{}) string { return fmt.Sprintf("%s::%s::%s", el.Metric["namespace"], el.Metric["pod"], el.Metric["container"]) } +func makeCustomPairID(pri, sec string) func(interface{}) string { + return func(element interface{}) string { + el := element.(*model.Sample) + return fmt.Sprintf("%s::%s", el.Metric[model.LabelName(pri)], el.Metric[model.LabelName(sec)]) + } +} + func boundedSample(lower, upper interface{}) types.GomegaMatcher { return gstruct.PointTo(gstruct.MatchAllFields(gstruct.Fields{ // We already check Metric when matching the Id diff --git a/test/e2e_node/topology_manager_metrics_test.go b/test/e2e_node/topology_manager_metrics_test.go index 817a6beeec7..0fef30e82ec 100644 --- a/test/e2e_node/topology_manager_metrics_test.go +++ b/test/e2e_node/topology_manager_metrics_test.go @@ -28,6 +28,7 @@ import ( v1 "k8s.io/api/core/v1" kubeletconfig "k8s.io/kubernetes/pkg/kubelet/apis/config" "k8s.io/kubernetes/pkg/kubelet/cm/topologymanager" + "k8s.io/kubernetes/pkg/kubelet/metrics" "k8s.io/kubernetes/test/e2e/feature" "k8s.io/kubernetes/test/e2e/framework" e2epod "k8s.io/kubernetes/test/e2e/framework/pod" @@ -152,6 +153,26 @@ var _ = SIGDescribe("Topology Manager Metrics", framework.WithSerial(), feature. ginkgo.By("Ensuring the metrics match the expectations a few more times") gomega.Consistently(ctx, getKubeletMetrics, 2*time.Minute, 10*time.Second).Should(matchResourceMetrics) }) + + ginkgo.It("[alignment] should return updated alignment counters when pod successfully run", func(ctx context.Context) { + ginkgo.By("Creating the test pod") + testPod = e2epod.NewPodClient(f).Create(ctx, makeGuaranteedCPUExclusiveSleeperPod("count-align-numa-ok", cpusNumPerNUMA)) + + // we updated the kubelet config in BeforeEach, so we can assume we start fresh. + // being [Serial], we can also assume noone else but us is running pods. + ginkgo.By("Checking the cpumanager metrics right after the kubelet restart, with pod should be admitted") + + matchAlignmentMetrics := gstruct.MatchKeys(gstruct.IgnoreExtras, gstruct.Keys{ + "kubelet_container_aligned_compute_resources_count": gstruct.MatchAllElements(nodeID, gstruct.Elements{ + metrics.AlignedNUMANode: timelessSample(1), + }), + }) + + ginkgo.By("Giving the Kubelet time to update the alignment metrics") + gomega.Eventually(ctx, getKubeletMetrics, 1*time.Minute, 15*time.Second).Should(matchAlignmentMetrics) + ginkgo.By("Ensuring the metrics match the expectations about alignment metrics a few more times") + gomega.Consistently(ctx, getKubeletMetrics, 1*time.Minute, 15*time.Second).Should(matchAlignmentMetrics) + }) }) })