Skip to content

Commit 389dba0

Browse files
stttsbertinatto
authored andcommitted
UPSTREAM: <carry>: kube-apiserver: wire through isTerminating into handler chain
UPSTREAM: <carry>: use lifeCycleSignals for isTerminating OpenShift-Rebase-Source: a736659
1 parent bbfba10 commit 389dba0

File tree

9 files changed

+70
-16
lines changed

9 files changed

+70
-16
lines changed

Diff for: cmd/kube-scheduler/app/server.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -330,7 +330,7 @@ func buildHandlerChain(handler http.Handler, authn authenticator.Request, authz
330330
handler = genericapifilters.WithAuthentication(handler, authn, failedHandler, nil, nil)
331331
handler = genericapifilters.WithRequestInfo(handler, requestInfoResolver)
332332
handler = genericapifilters.WithCacheControl(handler)
333-
handler = genericfilters.WithHTTPLogging(handler)
333+
handler = genericfilters.WithHTTPLogging(handler, nil)
334334
handler = genericfilters.WithPanicRecovery(handler, requestInfoResolver)
335335

336336
return handler

Diff for: pkg/kubelet/server/server.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -1155,7 +1155,7 @@ var statusesNoTracePred = httplog.StatusIsNot(
11551155

11561156
// ServeHTTP responds to HTTP requests on the Kubelet.
11571157
func (s *Server) ServeHTTP(w http.ResponseWriter, req *http.Request) {
1158-
handler := httplog.WithLogging(s.restfulCont, statusesNoTracePred)
1158+
handler := httplog.WithLogging(s.restfulCont, statusesNoTracePred, nil)
11591159

11601160
// monitor http requests
11611161
var serverType string

Diff for: staging/src/k8s.io/apiserver/pkg/server/config.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -1073,7 +1073,7 @@ func DefaultBuildHandlerChain(apiHandler http.Handler, c *Config) http.Handler {
10731073
if c.ShutdownSendRetryAfter {
10741074
handler = genericfilters.WithRetryAfter(handler, c.lifecycleSignals.NotAcceptingNewRequest.Signaled())
10751075
}
1076-
handler = genericfilters.WithHTTPLogging(handler)
1076+
handler = genericfilters.WithHTTPLogging(handler, c.newIsTerminatingFunc())
10771077
handler = genericapifilters.WithLatencyTrackers(handler)
10781078
// WithRoutine will execute future handlers in a separate goroutine and serving
10791079
// handler in current goroutine to minimize the stack memory usage. It must be

Diff for: staging/src/k8s.io/apiserver/pkg/server/filters/timeout_test.go

+3
Original file line numberDiff line numberDiff line change
@@ -355,6 +355,9 @@ func TestTimeoutWithLogging(t *testing.T) {
355355
},
356356
),
357357
),
358+
func() bool {
359+
return false
360+
},
358361
),
359362
)
360363
defer ts.Close()

Diff for: staging/src/k8s.io/apiserver/pkg/server/filters/wrap.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,8 @@ func WithPanicRecovery(handler http.Handler, resolver request.RequestInfoResolve
5959
}
6060

6161
// WithHTTPLogging enables logging of incoming requests.
62-
func WithHTTPLogging(handler http.Handler) http.Handler {
63-
return httplog.WithLogging(handler, httplog.DefaultStacktracePred)
62+
func WithHTTPLogging(handler http.Handler, isTerminating func() bool) http.Handler {
63+
return httplog.WithLogging(handler, httplog.DefaultStacktracePred, isTerminating)
6464
}
6565

6666
func withPanicRecovery(handler http.Handler, crashHandler func(http.ResponseWriter, *http.Request, interface{})) http.Handler {

Diff for: staging/src/k8s.io/apiserver/pkg/server/httplog/httplog.go

+18-6
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ type respLogger struct {
6767
addedInfo strings.Builder
6868
addedKeyValuePairs []interface{}
6969
startTime time.Time
70+
isTerminating bool
7071

7172
captureErrorOutput bool
7273

@@ -100,13 +101,13 @@ func DefaultStacktracePred(status int) bool {
100101
const withLoggingLevel = 3
101102

102103
// WithLogging wraps the handler with logging.
103-
func WithLogging(handler http.Handler, pred StacktracePred) http.Handler {
104+
func WithLogging(handler http.Handler, pred StacktracePred, isTerminatingFn func() bool) http.Handler {
104105
return withLogging(handler, pred, func() bool {
105106
return klog.V(withLoggingLevel).Enabled()
106-
})
107+
}, isTerminatingFn)
107108
}
108109

109-
func withLogging(handler http.Handler, stackTracePred StacktracePred, shouldLogRequest ShouldLogRequestPred) http.Handler {
110+
func withLogging(handler http.Handler, stackTracePred StacktracePred, shouldLogRequest ShouldLogRequestPred, isTerminatingFn func() bool) http.Handler {
110111
return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
111112
if !shouldLogRequest() {
112113
handler.ServeHTTP(w, req)
@@ -117,14 +118,16 @@ func withLogging(handler http.Handler, stackTracePred StacktracePred, shouldLogR
117118
if old := respLoggerFromRequest(req); old != nil {
118119
panic("multiple WithLogging calls!")
119120
}
120-
121121
startTime := time.Now()
122122
if receivedTimestamp, ok := request.ReceivedTimestampFrom(ctx); ok {
123123
startTime = receivedTimestamp
124124
}
125125

126-
rl := newLoggedWithStartTime(req, w, startTime)
127-
rl.StacktraceWhen(stackTracePred)
126+
isTerminating := false
127+
if isTerminatingFn != nil {
128+
isTerminating = isTerminatingFn()
129+
}
130+
rl := newLoggedWithStartTime(req, w, startTime).StacktraceWhen(stackTracePred).IsTerminating(isTerminating)
128131
req = req.WithContext(context.WithValue(ctx, respLoggerContextKey, rl))
129132

130133
var logFunc func()
@@ -135,6 +138,9 @@ func withLogging(handler http.Handler, stackTracePred StacktracePred, shouldLogR
135138
}
136139
}()
137140

141+
if klog.V(3).Enabled() || (rl.isTerminating && klog.V(1).Enabled()) {
142+
defer rl.Log()
143+
}
138144
w = responsewriter.WrapForHTTP1Or2(rl)
139145
handler.ServeHTTP(w, req)
140146

@@ -205,6 +211,12 @@ func (rl *respLogger) StacktraceWhen(pred StacktracePred) *respLogger {
205211
return rl
206212
}
207213

214+
// IsTerminating informs the logger that the server is terminating.
215+
func (rl *respLogger) IsTerminating(is bool) *respLogger {
216+
rl.isTerminating = is
217+
return rl
218+
}
219+
208220
// StatusIsNot returns a StacktracePred which will cause stacktraces to be logged
209221
// for any status *not* in the given list.
210222
func StatusIsNot(statuses ...int) StacktracePred {

Diff for: staging/src/k8s.io/apiserver/pkg/server/httplog/httplog_test.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ func TestWithLogging(t *testing.T) {
6767
shouldLogRequest := func() bool { return true }
6868
var handler http.Handler
6969
handler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})
70-
handler = withLogging(withLogging(handler, DefaultStacktracePred, shouldLogRequest), DefaultStacktracePred, shouldLogRequest)
70+
handler = withLogging(withLogging(handler, DefaultStacktracePred, shouldLogRequest, nil), DefaultStacktracePred, shouldLogRequest, nil)
7171

7272
func() {
7373
defer func() {
@@ -111,7 +111,7 @@ func TestLogOf(t *testing.T) {
111111
t.Errorf("Expected %v, got %v", test.want, got)
112112
}
113113
})
114-
handler = withLogging(handler, DefaultStacktracePred, func() bool { return test.shouldLogRequest })
114+
handler = withLogging(handler, DefaultStacktracePred, func() bool { return test.shouldLogRequest }, nil)
115115
w := httptest.NewRecorder()
116116
handler.ServeHTTP(w, req)
117117
})
@@ -135,7 +135,7 @@ func TestUnlogged(t *testing.T) {
135135
}
136136
})
137137
if makeLogger {
138-
handler = WithLogging(handler, DefaultStacktracePred)
138+
handler = WithLogging(handler, DefaultStacktracePred, nil)
139139
}
140140

141141
handler.ServeHTTP(origWriter, req)
@@ -216,7 +216,7 @@ func TestRespLoggerWithDecoratedResponseWriter(t *testing.T) {
216216
}
217217
})
218218

219-
handler = withLogging(handler, DefaultStacktracePred, func() bool { return true })
219+
handler = withLogging(handler, DefaultStacktracePred, func() bool { return true }, nil)
220220
handler.ServeHTTP(test.r(), req)
221221
})
222222
}
+39
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
/*
2+
Copyright 2021 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package server
18+
19+
// newIsTerminatingFunc returns a 'func() bool' that relies on the
20+
// 'ShutdownInitiated' life cycle signal of answer if the apiserver
21+
// has started the termination process.
22+
func (c *Config) newIsTerminatingFunc() func() bool {
23+
var shutdownCh <-chan struct{}
24+
// TODO: a properly initialized Config object should always have lifecycleSignals
25+
// initialized, but some config unit tests leave lifecycleSignals as nil.
26+
// Fix the unit tests upstream and then we can remove this check.
27+
if c.lifecycleSignals.ShutdownInitiated != nil {
28+
shutdownCh = c.lifecycleSignals.ShutdownInitiated.Signaled()
29+
}
30+
31+
return func() bool {
32+
select {
33+
case <-shutdownCh:
34+
return true
35+
default:
36+
return false
37+
}
38+
}
39+
}

Diff for: staging/src/k8s.io/controller-manager/app/serve.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ func BuildHandlerChain(apiHandler http.Handler, authorizationInfo *apiserver.Aut
4848
}
4949
handler = genericapifilters.WithRequestInfo(handler, requestInfoResolver)
5050
handler = genericapifilters.WithCacheControl(handler)
51-
handler = genericfilters.WithHTTPLogging(handler)
51+
handler = genericfilters.WithHTTPLogging(handler, nil)
5252
handler = genericfilters.WithPanicRecovery(handler, requestInfoResolver)
5353

5454
return handler

0 commit comments

Comments
 (0)