Skip to content

Commit 9802bfc

Browse files
cheftakocheftako
committed
Add code to fix kubelet/metrics memory issue.
Bucketing url paths based on concept/handling. Bucketing code placed by handling code to encourage usage. Added unit tests. Fix format.
1 parent 6a92f19 commit 9802bfc

File tree

2 files changed

+105
-5
lines changed

2 files changed

+105
-5
lines changed

‎pkg/kubelet/server/server.go‎

Lines changed: 52 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ type Server struct {
8989
auth AuthInterface
9090
host HostInterface
9191
restfulCont containerInterface
92+
metricsBuckets map[string]bool
9293
resourceAnalyzer stats.ResourceAnalyzer
9394
redirectContainerStreaming bool
9495
}
@@ -225,6 +226,7 @@ func NewServer(
225226
resourceAnalyzer: resourceAnalyzer,
226227
auth: auth,
227228
restfulCont: &filteringContainer{Container: restful.NewContainer()},
229+
metricsBuckets: make(map[string]bool),
228230
redirectContainerStreaming: redirectContainerStreaming,
229231
}
230232
if auth != nil {
@@ -280,14 +282,32 @@ func (s *Server) InstallAuthFilter() {
280282
})
281283
}
282284

285+
// addMetricsBucketMatcher adds a regexp matcher and the relevant bucket to use when
286+
// it matches. Please be aware this is not thread safe and should not be used dynamically
287+
func (s *Server) addMetricsBucketMatcher(bucket string) {
288+
s.metricsBuckets[bucket] = true
289+
}
290+
291+
// getMetricBucket find the appropriate metrics reporting bucket for the given path
292+
func (s *Server) getMetricBucket(path string) string {
293+
root := getURLRootPath(path)
294+
if s.metricsBuckets[root] == true {
295+
return root
296+
}
297+
return "Invalid path"
298+
}
299+
283300
// InstallDefaultHandlers registers the default set of supported HTTP request
284301
// patterns with the restful Container.
285302
func (s *Server) InstallDefaultHandlers(enableCAdvisorJSONEndpoints bool) {
303+
s.addMetricsBucketMatcher("healthz")
286304
healthz.InstallHandler(s.restfulCont,
287305
healthz.PingHealthz,
288306
healthz.LogHealthz,
289307
healthz.NamedCheck("syncloop", s.syncLoopHealthCheck),
290308
)
309+
310+
s.addMetricsBucketMatcher("pods")
291311
ws := new(restful.WebService)
292312
ws.
293313
Path("/pods").
@@ -297,7 +317,14 @@ func (s *Server) InstallDefaultHandlers(enableCAdvisorJSONEndpoints bool) {
297317
Operation("getPods"))
298318
s.restfulCont.Add(ws)
299319

320+
s.addMetricsBucketMatcher("stats")
300321
s.restfulCont.Add(stats.CreateHandlers(statsPath, s.host, s.resourceAnalyzer, enableCAdvisorJSONEndpoints))
322+
323+
s.addMetricsBucketMatcher("metrics")
324+
s.addMetricsBucketMatcher("metrics/cadvisor")
325+
s.addMetricsBucketMatcher("metrics/probes")
326+
s.addMetricsBucketMatcher("metrics/resource/v1alpha1")
327+
s.addMetricsBucketMatcher("metrics/resource")
301328
//lint:ignore SA1019 https://github.com/kubernetes/enhancements/issues/1206
302329
s.restfulCont.Handle(metricsPath, legacyregistry.Handler())
303330

@@ -321,12 +348,14 @@ func (s *Server) InstallDefaultHandlers(enableCAdvisorJSONEndpoints bool) {
321348
)
322349

323350
// deprecated endpoint which will be removed in release 1.20.0+.
351+
s.addMetricsBucketMatcher("metrics/resource/v1alpha1")
324352
v1alpha1ResourceRegistry := compbasemetrics.NewKubeRegistry()
325353
v1alpha1ResourceRegistry.CustomMustRegister(stats.NewPrometheusResourceMetricCollector(s.resourceAnalyzer, v1alpha1.Config()))
326354
s.restfulCont.Handle(path.Join(resourceMetricsPath, v1alpha1.Version),
327355
compbasemetrics.HandlerFor(v1alpha1ResourceRegistry, compbasemetrics.HandlerOpts{ErrorHandling: compbasemetrics.ContinueOnError}),
328356
)
329357

358+
s.addMetricsBucketMatcher("metrics/resource")
330359
resourceRegistry := compbasemetrics.NewKubeRegistry()
331360
resourceRegistry.CustomMustRegister(collectors.NewResourceMetricsCollector(s.resourceAnalyzer))
332361
s.restfulCont.Handle(resourceMetricsPath,
@@ -335,13 +364,15 @@ func (s *Server) InstallDefaultHandlers(enableCAdvisorJSONEndpoints bool) {
335364

336365
// prober metrics are exposed under a different endpoint
337366

367+
s.addMetricsBucketMatcher("metrics/probes")
338368
p := compbasemetrics.NewKubeRegistry()
339369
_ = compbasemetrics.RegisterProcessStartTime(p.Register)
340370
p.MustRegister(prober.ProberResults)
341371
s.restfulCont.Handle(proberMetricsPath,
342372
compbasemetrics.HandlerFor(p, compbasemetrics.HandlerOpts{ErrorHandling: compbasemetrics.ContinueOnError}),
343373
)
344374

375+
s.addMetricsBucketMatcher("spec")
345376
if enableCAdvisorJSONEndpoints {
346377
ws := new(restful.WebService)
347378
ws.
@@ -361,6 +392,7 @@ const pprofBasePath = "/debug/pprof/"
361392
func (s *Server) InstallDebuggingHandlers(criHandler http.Handler) {
362393
klog.Infof("Adding debug handlers to kubelet server.")
363394

395+
s.addMetricsBucketMatcher("run")
364396
ws := new(restful.WebService)
365397
ws.
366398
Path("/run")
@@ -372,6 +404,7 @@ func (s *Server) InstallDebuggingHandlers(criHandler http.Handler) {
372404
Operation("getRun"))
373405
s.restfulCont.Add(ws)
374406

407+
s.addMetricsBucketMatcher("exec")
375408
ws = new(restful.WebService)
376409
ws.
377410
Path("/exec")
@@ -389,6 +422,7 @@ func (s *Server) InstallDebuggingHandlers(criHandler http.Handler) {
389422
Operation("getExec"))
390423
s.restfulCont.Add(ws)
391424

425+
s.addMetricsBucketMatcher("attach")
392426
ws = new(restful.WebService)
393427
ws.
394428
Path("/attach")
@@ -406,6 +440,7 @@ func (s *Server) InstallDebuggingHandlers(criHandler http.Handler) {
406440
Operation("getAttach"))
407441
s.restfulCont.Add(ws)
408442

443+
s.addMetricsBucketMatcher("portForward")
409444
ws = new(restful.WebService)
410445
ws.
411446
Path("/portForward")
@@ -423,6 +458,7 @@ func (s *Server) InstallDebuggingHandlers(criHandler http.Handler) {
423458
Operation("getPortForward"))
424459
s.restfulCont.Add(ws)
425460

461+
s.addMetricsBucketMatcher("logs")
426462
ws = new(restful.WebService)
427463
ws.
428464
Path(logsPath)
@@ -435,6 +471,7 @@ func (s *Server) InstallDebuggingHandlers(criHandler http.Handler) {
435471
Param(ws.PathParameter("logpath", "path to the log").DataType("string")))
436472
s.restfulCont.Add(ws)
437473

474+
s.addMetricsBucketMatcher("containerLogs")
438475
ws = new(restful.WebService)
439476
ws.
440477
Path("/containerLogs")
@@ -443,8 +480,10 @@ func (s *Server) InstallDebuggingHandlers(criHandler http.Handler) {
443480
Operation("getContainerLogs"))
444481
s.restfulCont.Add(ws)
445482

483+
s.addMetricsBucketMatcher("configz")
446484
configz.InstallHandler(s.restfulCont)
447485

486+
s.addMetricsBucketMatcher("debug")
448487
handlePprofEndpoint := func(req *restful.Request, resp *restful.Response) {
449488
name := strings.TrimPrefix(req.Request.URL.Path, pprofBasePath)
450489
switch name {
@@ -460,7 +499,6 @@ func (s *Server) InstallDebuggingHandlers(criHandler http.Handler) {
460499
pprof.Index(resp, req.Request)
461500
}
462501
}
463-
464502
// Setup pprof handlers.
465503
ws = new(restful.WebService).Path(pprofBasePath)
466504
ws.Route(ws.GET("/{subpath:*}").To(func(req *restful.Request, resp *restful.Response) {
@@ -473,6 +511,7 @@ func (s *Server) InstallDebuggingHandlers(criHandler http.Handler) {
473511
s.restfulCont.Handle("/debug/flags/v", routes.StringFlagPutHandler(logs.GlogSetter))
474512

475513
// The /runningpods endpoint is used for testing only.
514+
s.addMetricsBucketMatcher("runningpods")
476515
ws = new(restful.WebService)
477516
ws.
478517
Path("/runningpods/").
@@ -482,6 +521,7 @@ func (s *Server) InstallDebuggingHandlers(criHandler http.Handler) {
482521
Operation("getRunningPods"))
483522
s.restfulCont.Add(ws)
484523

524+
s.addMetricsBucketMatcher("cri")
485525
if criHandler != nil {
486526
s.restfulCont.Handle("/cri/", criHandler)
487527
}
@@ -493,6 +533,14 @@ func (s *Server) InstallDebuggingDisabledHandlers() {
493533
http.Error(w, "Debug endpoints are disabled.", http.StatusMethodNotAllowed)
494534
})
495535

536+
s.addMetricsBucketMatcher("run")
537+
s.addMetricsBucketMatcher("exec")
538+
s.addMetricsBucketMatcher("attach")
539+
s.addMetricsBucketMatcher("portForward")
540+
s.addMetricsBucketMatcher("containerLogs")
541+
s.addMetricsBucketMatcher("runningpods")
542+
s.addMetricsBucketMatcher("pprof")
543+
s.addMetricsBucketMatcher("logs")
496544
paths := []string{
497545
"/run/", "/exec/", "/attach/", "/portForward/", "/containerLogs/",
498546
"/runningpods/", pprofBasePath, logsPath}
@@ -809,10 +857,10 @@ func (s *Server) getPortForward(request *restful.Request, response *restful.Resp
809857
proxyStream(response.ResponseWriter, request.Request, url)
810858
}
811859

812-
// trimURLPath trims a URL path.
860+
// getURLRootPath trims a URL path.
813861
// For paths in the format of "/metrics/xxx", "metrics/xxx" is returned;
814862
// For all other paths, the first part of the path is returned.
815-
func trimURLPath(path string) string {
863+
func getURLRootPath(path string) string {
816864
parts := strings.SplitN(strings.TrimPrefix(path, "/"), "/", 3)
817865
if len(parts) == 0 {
818866
return path
@@ -860,7 +908,7 @@ func (s *Server) ServeHTTP(w http.ResponseWriter, req *http.Request) {
860908
serverType = "readwrite"
861909
}
862910

863-
method, path := req.Method, trimURLPath(req.URL.Path)
911+
method, path := req.Method, s.getMetricBucket(req.URL.Path)
864912

865913
longRunning := strconv.FormatBool(isLongRunningRequest(path))
866914

‎pkg/kubelet/server/server_test.go‎

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1471,6 +1471,58 @@ func TestCRIHandler(t *testing.T) {
14711471
assert.Equal(t, query, fw.criHandler.RequestReceived.URL.RawQuery)
14721472
}
14731473

1474+
func TestMetricBuckets(t *testing.T) {
1475+
tests := map[string]struct {
1476+
url string
1477+
bucket string
1478+
}{
1479+
"healthz endpoint": {url: "/healthz", bucket: "healthz"},
1480+
"attach": {url: "/attach/podNamespace/podID/containerName", bucket: "attach"},
1481+
"attach with uid": {url: "/attach/podNamespace/podID/uid/containerName", bucket: "attach"},
1482+
"configz": {url: "/configz", bucket: "configz"},
1483+
"containerLogs": {url: "/containerLogs/podNamespace/podID/containerName", bucket: "containerLogs"},
1484+
"cri": {url: "/cri/", bucket: "cri"},
1485+
"cri with sub": {url: "/cri/foo", bucket: "cri"},
1486+
"debug v flags": {url: "/debug/flags/v", bucket: "debug"},
1487+
"pprof with sub": {url: "/debug/pprof/subpath", bucket: "debug"},
1488+
"exec": {url: "/exec/podNamespace/podID/containerName", bucket: "exec"},
1489+
"exec with uid": {url: "/exec/podNamespace/podID/uid/containerName", bucket: "exec"},
1490+
"healthz": {url: "/healthz/", bucket: "healthz"},
1491+
"healthz log sub": {url: "/healthz/log", bucket: "healthz"},
1492+
"healthz ping": {url: "/healthz/ping", bucket: "healthz"},
1493+
"healthz sync loop": {url: "/healthz/syncloop", bucket: "healthz"},
1494+
"logs": {url: "/logs/", bucket: "logs"},
1495+
"logs with path": {url: "/logs/logpath", bucket: "logs"},
1496+
"metrics": {url: "/metrics", bucket: "metrics"},
1497+
"metrics cadvisor sub": {url: "/metrics/cadvisor", bucket: "metrics/cadvisor"},
1498+
"metrics probes sub": {url: "/metrics/probes", bucket: "metrics/probes"},
1499+
"metrics resource v1alpha1": {url: "/metrics/resource/v1alpha1", bucket: "metrics/resource"},
1500+
"metrics resource sub": {url: "/metrics/resource", bucket: "metrics/resource"},
1501+
"pods": {url: "/pods/", bucket: "pods"},
1502+
"portForward": {url: "/portForward/podNamespace/podID", bucket: "portForward"},
1503+
"portForward with uid": {url: "/portForward/podNamespace/podID/uid", bucket: "portForward"},
1504+
"run": {url: "/run/podNamespace/podID/containerName", bucket: "run"},
1505+
"run with uid": {url: "/run/podNamespace/podID/uid/containerName", bucket: "run"},
1506+
"runningpods": {url: "/runningpods/", bucket: "runningpods"},
1507+
"spec": {url: "/spec/", bucket: "spec"},
1508+
"stats": {url: "/stats/", bucket: "stats"},
1509+
"stats container sub": {url: "/stats/container", bucket: "stats"},
1510+
"stats summary sub": {url: "/stats/summary", bucket: "stats"},
1511+
"stats containerName with uid": {url: "/stats/namespace/podName/uid/containerName", bucket: "stats"},
1512+
"stats containerName": {url: "/stats/podName/containerName", bucket: "stats"},
1513+
"invalid path": {url: "/junk", bucket: "Invalid path"},
1514+
"invalid path starting with good": {url: "/healthzjunk", bucket: "Invalid path"},
1515+
}
1516+
fw := newServerTest()
1517+
defer fw.testHTTPServer.Close()
1518+
1519+
for _, test := range tests {
1520+
path := test.url
1521+
bucket := test.bucket
1522+
require.Equal(t, fw.serverUnderTest.getMetricBucket(path), bucket)
1523+
}
1524+
}
1525+
14741526
func TestDebuggingDisabledHandlers(t *testing.T) {
14751527
fw := newServerTestWithDebug(false, false, nil)
14761528
defer fw.testHTTPServer.Close()
@@ -1544,6 +1596,6 @@ func TestTrimURLPath(t *testing.T) {
15441596
}
15451597

15461598
for _, test := range tests {
1547-
assert.Equal(t, test.expected, trimURLPath(test.path), fmt.Sprintf("path is: %s", test.path))
1599+
assert.Equal(t, test.expected, getURLRootPath(test.path), fmt.Sprintf("path is: %s", test.path))
15481600
}
15491601
}

0 commit comments

Comments
 (0)