-
Notifications
You must be signed in to change notification settings - Fork 3.8k
Switch to Go 1.19 #7254
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Switch to Go 1.19 #7254
Conversation
Signed-off-by: Maksym Pavlenko <[email protected]>
Signed-off-by: Maksym Pavlenko <[email protected]>
Signed-off-by: Maksym Pavlenko <[email protected]>
Signed-off-by: Maksym Pavlenko <[email protected]>
e80e4a1 to
05b0693
Compare
85f5043 to
55e402f
Compare
Signed-off-by: Maksym Pavlenko <[email protected]>
|
/test pull-containerd-sandboxed-node-e2e |
Signed-off-by: Maksym Pavlenko <[email protected]>
estesp
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
I marked this as cherry-pick for 1.6 as well since we will be supporting the 1.6 branch longer than go 1.18 will be supported. We don't necessarily need to cherry-pick right away though or use the same commits. |
fuweid
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
| Addr: s.config.Addr, | ||
| Handler: s.handler, | ||
| TLSConfig: s.config.TLSConfig, | ||
| ReadHeaderTimeout: 3 * time.Second, // Fix linter G112: Potential Slowloris Attack because ReadHeaderTimeout is not configured in the http.Server |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No sure 3 second is good enough. Just in case, the 10 Minutes maybe better than 3 second because there is no limit at the beginning.
It's not block issue. But if we change it to low tolerance value, we should mark it in change log.
Cc @mikebrow
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah this one is very likely to cause node failures..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree maybe 10 or 15min.. maybe 30?
It is follow-up of containerd#7254. This commit will increase ReadHeaderTimeout from 3s to 30m, which prevent from unexpected timeout when the node is running with high-load. 30 Minutes is longer enough to get close to before what containerd#7254 changes. And ideally, we should allow user to configure the streaming server if the users want this feature. Signed-off-by: Wei Fu <[email protected]>
It is follow-up of containerd#7254. This commit will increase ReadHeaderTimeout from 3s to 30m, which prevent from unexpected timeout when the node is running with high-load. 30 Minutes is longer enough to get close to before what containerd#7254 changes. And ideally, we should allow user to configure the streaming server if the users want this feature. Signed-off-by: Wei Fu <[email protected]>
It is follow-up of containerd#7254. This commit will increase ReadHeaderTimeout from 3s to 30m, which prevent from unexpected timeout when the node is running with high-load. 30 Minutes is longer enough to get close to before what containerd#7254 changes. And ideally, we should allow user to configure the streaming server if the users want this feature. Signed-off-by: Wei Fu <[email protected]>
Follow up #7254 (Switch to Go 1.19)
It is follow-up of containerd#7254. This commit will increase ReadHeaderTimeout from 3s to 30m, which prevent from unexpected timeout when the node is running with high-load. 30 Minutes is longer enough to get close to before what containerd#7254 changes. And ideally, we should allow user to configure the streaming server if the users want this feature. Signed-off-by: Wei Fu <[email protected]>
It is follow-up of containerd#7254. This commit will increase ReadHeaderTimeout from 3s to 30m, which prevent from unexpected timeout when the node is running with high-load. 30 Minutes is longer enough to get close to before what containerd#7254 changes. And ideally, we should allow user to configure the streaming server if the users want this feature. Signed-off-by: Wei Fu <[email protected]> (cherry picked from commit 460b053) Signed-off-by: Sebastiaan van Stijn <[email protected]>
It is follow-up of containerd#7254. This commit will increase ReadHeaderTimeout from 3s to 30m, which prevent from unexpected timeout when the node is running with high-load. 30 Minutes is longer enough to get close to before what containerd#7254 changes. And ideally, we should allow user to configure the streaming server if the users want this feature. Signed-off-by: Wei Fu <[email protected]>
It is follow-up of containerd#7254. This commit will increase ReadHeaderTimeout from 3s to 30m, which prevent from unexpected timeout when the node is running with high-load. 30 Minutes is longer enough to get close to before what containerd#7254 changes. And ideally, we should allow user to configure the streaming server if the users want this feature. Signed-off-by: Wei Fu <[email protected]>
Signed-off-by: Maksym Pavlenko [email protected]