Bump go 1.13.0 #39549
Merged
Bump go 1.13.0 #39549
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
thaJeztah
commented
Jul 17, 2019
Dockerfile
Outdated
There was a problem hiding this comment.
opened checkpoint-restore/criu#743 for this as well
|
Windows build is failing, but no details what exactly is failing 😞 |
|
And new LOL, I don't see a difference; diff --git a/pkg/parsers/kernel/kernel_windows.go b/pkg/parsers/kernel/kernel_windows.go
index b7b15a1fd2..a0712ce633 100644
--- a/pkg/parsers/kernel/kernel_windows.go
+++ b/pkg/parsers/kernel/kernel_windows.go
@@ -44,7 +44,7 @@ func GetKernelVersion() (*VersionInfo, error) {
}
KVI.major = int(dwVersion & 0xFF)
- KVI.minor = int((dwVersion & 0XFF00) >> 8)
+ KVI.minor = int((dwVersion & 0xFF00) >> 8)
KVI.build = int((dwVersion & 0xFFFF0000) >> 16)
return KVI, nilOh! lowercase |
|
Failures on Experimental, Janky, PowerPC, and s390x: https://jenkins.dockerproject.org/job/Docker-PRs/54911/console Failures on Windows RS1 (fails to b 10BC0 uild) https://jenkins.dockerproject.org/job/Docker-PRs-WoW-RS1/25943/console Failures on Windows RS5 (fails to build) https://jenkins.dockerproject.org/job/Docker-PRs-WoW-RS5-Process/3036/console |
|
This one passes locally; |
|
Logs for the failing And all logs of the tests and daemon: |
thaJeztah
commented
Jul 17, 2019
There was a problem hiding this comment.
forcing IPv4 for the ping; see if that's making a difference
|
One thing I noticed on my machine (Docker for Mac) is that starting the daemon is really, really slow; looks like creating iptables rules takes a lot longer; That's almost 7 seconds! If I start the daemon with |
|
Switching to master, and using Go 1.12.7 on stretch; And without iptables: |
|
This branch again, using Go And without iptables So looks to be due to buster (perhaps the iptables version in buster switching to nftables?) |
|
Added a bit of debugging for ``, and printing Nothing that really stands out there |
|
Ok, found the problem (w.r.t. the failing tests); it's indeed because the CI machines run on Ubuntu 16.04, which uses legacy iptables, whereas the Debian buster image uses Added a hacky few lines to the https://wiki.debian.org/iptables
|
|
Looks like the performance issue is due to nftables; Switch to iptables-legacy: Switch to iptables-nft: |
|
switching to iptables-legacy fixes the tests; |
tianon
reviewed
Jul 22, 2019
hack/dind
Outdated
There was a problem hiding this comment.
Shouldn't this happen in the Dockerfile instead so we don't accidentally apply this change somewhere less harmless than a container like on a contributor's host machine?
There was a problem hiding this comment.
It's really a quick hack to see if it did the job
Surprisingly; when I did it as a RUN step, it gave a "permission denied; need to be root"
There was a problem hiding this comment.
Oh that's really odd -- AFAICT, we're not using USER or anything so it already should be root. 😓
(And doing update-alternatives shouldn't be running iptables or anything that might need additional privileges.)
There was a problem hiding this comment.
That's what I would expect as well; didn't look closely yet, but I suspect it's calling iptables (perhaps just to check if it's installed?)
|
Arg; forgot to remove |
|
Instead of removing it, I'd recommend switching it to be |
|
Yes, that's what I actually meant 😅 - updated 🤞 |
|
Will have to look into the Windows build failure; the file that it marks missing is on the repository; https://github.com/moby/moby/blob/master/vendor/cloud.google.com/go/compute/metadata/metadata.go |
73e405d to
eb4289e
Compare
|
Yes, was actually thinking of separating it from this PR (although it is "accepted" by older versions of Go) |
8f9487c to
ccdaf19
Compare
ccdaf19 to
c4e96d8
Compare
|
Interesting; this is why Looking at this, it looks like |
c4e96d8 to
810e9b2
Compare
|
ha! looks like Still wondering why it switched to use |
810e9b2 to
2e19350
Compare
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2e19350 to
38e4ae3
Compare
|
@kolyshkin @tiborvass @tianon this is all green now; ready for review! |
|
@seemethere @zelahi I'm afraid if we merge this, release scripts somewhere will not add the GO111MODULE=off. |
seemethere
approved these changes
Sep 19, 2019
tiborvass
approved these changes
Sep 19, 2019
This was referenced Feb 22, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Based on top of / depends on (when backporting, these are also needed):
I'll rebase once those are merged
testing if things work or break