Bump to Go 1.11.0 by thaJeztah · Pull Request #2435 · containerd/containerd

thaJeztah · 2018-07-05T08:10:27Z

Already saw some breakage in moby/moby#37358, so lets run it here as well

Changelog: https://tip.golang.org/doc/go1.11

thaJeztah · 2018-07-05T10:44:09Z

content/testsuite/testsuite.go

Opened a ticket upstream for the gofmt change; golang/go#26228

thaJeztah · 2018-07-05T11:01:21Z

temporarily cherry-picked #2420 to fix compilation on 1.11

thaJeztah · 2018-07-05T11:30:17Z

Okay, we're getting closer; 3 out of four pass (on Linux);

There's a panic on Go 1.11beta1 with CGO_ENABLED=1; https://api.travis-ci.org/v3/job/400360537/log.txt

?   	github.com/containerd/containerd/cmd/containerd-shim	1.013s	coverage: 9.3% of statements [no test files]
panic: invalid argument

goroutine 1 [running]:
github.com/containerd/containerd/cmd/containerd-stress.init.0()
	/home/travis/gopath/src/github.com/containerd/containerd/cmd/containerd-stress/main.go:61 +0x73c
FAIL	github.com/containerd/containerd/cmd/containerd-stress	0.024s
make: *** [coverage] Error 1

Panic is done here;

containerd/cmd/containerd-stress/main.go

Lines 59 to 62 in c3971b1

    
           // set higher ulimits 
        
           if err := setRlimit(); err != nil { 
        
           	panic(err) 
        
           }

Originating from

containerd/cmd/containerd-stress/rlimit_unix.go

Lines 25 to 40 in c3971b1

    
           func setRlimit() error { 
        
           	rlimit := uint64(100000) 
        
           	if rlimit > 0 { 
        
           		var limit syscall.Rlimit 
        
           		if err := syscall.Getrlimit(syscall.RLIMIT_NOFILE, &limit); err != nil { 
        
           			return err 
        
           		} 
        
           		if limit.Cur < rlimit { 
        
           			limit.Cur = rlimit 
        
           			if err := syscall.Setrlimit(syscall.RLIMIT_NOFILE, &limit); err != nil { 
        
           				return err 
        
           			} 
        
           		} 
        
           	} 
        
           	return nil 
        
           }

mlaventure · 2018-07-05T14:58:28Z

syscall.Setrlimit(...) is the most likely culprit, they may have been a breaking change in the go code.

mlaventure · 2018-07-05T15:03:29Z

Just checked, no changes to those syscalls. May be a regression in CGO, may want to print the value returned by Getrlimit() to ensure it's not corrupted somehow

thaJeztah · 2018-07-23T18:02:55Z

Failure on Windows; looks like there were some more format errors;

?   	github.com/containerd/containerd/version	[no test files]
# github.com/containerd/containerd/windows
windows\process.go:124: Wrapf format %s has arg s of wrong type github.com/containerd/containerd/runtime.Status
windows\task.go:287: Wrapf format %d has arg id of wrong type string
windows\task.go:300: Wrapf format %d has arg t.id of wrong type string
Makefile:223: recipe for target 'coverage' failed
mingw32-make: *** [coverage] Error 2

thaJeztah · 2018-07-23T18:03:47Z

Pushed a commit here, and as a separate PR; #2487

codecov-io · 2018-07-23T18:29:34Z

Codecov Report

Merging #2435 into master will increase coverage by 1.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #2435      +/-   ##
==========================================
+ Coverage   43.74%   44.76%   +1.01%     
==========================================
  Files         100       93       -7     
  Lines       10728     9555    -1173     
==========================================
- Hits         4693     4277     -416     
+ Misses       5305     4585     -720     
+ Partials      730      693      -37

Flag	Coverage Δ
#linux	`48.98% <ø> (+1.56%)`	⬆️
#windows	`41.04% <ø> (+0.13%)`	⬆️

Impacted Files	Coverage Δ
mount/lookup_unix.go	`0% <0%> (-75%)`	⬇️
oci/spec.go	`40% <0%> (-53.72%)`	⬇️
mount/mount_linux.go	`0% <0%> (-31.02%)`	⬇️
archive/compression/compression.go	`43.93% <0%> (-14.76%)`	⬇️
mount/mountinfo_linux.go	`53.06% <0%> (-8.17%)`	⬇️
content/local/writer.go	`52.63% <0%> (-5.22%)`	⬇️
metadata/content.go	`39.32% <0%> (-4.49%)`	⬇️
metadata/leases.go	`40.6% <0%> (-3.68%)`	⬇️
metadata/migrations.go	`42.3% <0%> (-1.45%)`	⬇️
archive/tar.go	`43.11% <0%> (-0.03%)`	⬇️
... and 37 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c444666...07237e3. Read the comment docs.

thaJeztah · 2018-07-23T19:07:57Z

Yay! All green now!

Let me rebase to get rid of the extra commit

crosbymichael · 2018-07-25T15:14:06Z

Why would we bump to a beta version?

thaJeztah · 2018-07-25T15:19:22Z

@crosbymichael sorry, probably should've kept the [do not merge]; mainly opening this to catch possible regressions before it leaves "beta"

thaJeztah · 2018-08-28T09:16:58Z

.travis.yml

This needs to be changed to 1.11.x once the first patch release for Go 1.11 is released

Can you update this now? Now that 1.2 is out I think if you change this to 1.11.x we can merge this.

thaJeztah · 2018-08-28T09:17:49Z

Go 1.11 was released, so this is now no longer "WIP"

crosbymichael · 2018-08-28T15:33:11Z

LGTM

dmcgowan · 2018-08-28T21:50:54Z

Any reason or risk to changing this before 1.2? My preference would be to leave this open to test but merge after 1.2.0-rc.0.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

thaJeztah · 2018-10-25T20:14:16Z

@estesp rebased and updated; thanks for the ping 👍

estesp

LGTM

thaJeztah mentioned this pull request Jul 5, 2018

Workaround for gofmt change in Go 1.11 #2436

Merged

thaJeztah force-pushed the bump_go_1.11 branch 2 times, most recently from 70f6a53 to eefabd4 Compare July 5, 2018 09:12

thaJeztah commented Jul 5, 2018

View reviewed changes

content/testsuite/testsuite.go Outdated

Copy link
Copy Markdown

Member Author

thaJeztah Jul 5, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opened a ticket upstream for the gofmt change; golang/go#26228

thaJeztah force-pushed the bump_go_1.11 branch from 3a21169 to e082ad1 Compare July 23, 2018 15:04

thaJeztah force-pushed the bump_go_1.11 branch from 8745d02 to 712dffc Compare July 23, 2018 19:08

thaJeztah changed the title ~~[do not merge] Bump to Go 1.11~~ Bump to Go 1.11beta2 Jul 23, 2018

thaJeztah changed the title ~~Bump to Go 1.11beta2~~ [do not merge] Bump to Go 1.11beta2 Jul 25, 2018

thaJeztah force-pushed the bump_go_1.11 branch from 712dffc to e576186 Compare August 6, 2018 13:55

thaJeztah changed the title ~~[do not merge] Bump to Go 1.11beta2~~ [do not merge] Bump to Go 1.11beta3 Aug 6, 2018

thaJeztah force-pushed the bump_go_1.11 branch from e576186 to 5e906a2 Compare August 15, 2018 10:45

thaJeztah changed the title ~~[do not merge] Bump to Go 1.11beta3~~ [do not merge] Bump to Go 1.11rc1 Aug 15, 2018

thaJeztah force-pushed the bump_go_1.11 branch from 5e906a2 to eaea620 Compare August 28, 2018 09:16

thaJeztah commented Aug 28, 2018

View reviewed changes

thaJeztah changed the title ~~[do not merge] Bump to Go 1.11rc1~~ Bump to Go 1.11.0 Aug 28, 2018

AkihiroSuda mentioned this pull request Oct 21, 2018

Bump Go versions #2738

Closed

Bump to Go 1.11.x

07237e3

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

thaJeztah force-pushed the bump_go_1.11 branch from eaea620 to 07237e3 Compare October 25, 2018 20:13

estesp approved these changes Oct 25, 2018

View reviewed changes

mlaventure approved these changes Oct 26, 2018

View reviewed changes

crosbymichael merged commit 86b8c48 into containerd:master Oct 26, 2018

thaJeztah deleted the bump_go_1.11 branch October 26, 2018 19:20

thaJeztah mentioned this pull request Nov 6, 2018

bump containerd to v1.2.0-13-g8afcade1 moby/moby#38062

Closed

This was referenced Mar 7, 2019

[release/1.2 backport] Bump to Go 1.11.x #3077

Merged

[release/1.1 backport] Bump to Go 1.11.x #3078

Merged

Conversation

thaJeztah commented Jul 5, 2018

Uh oh!

thaJeztah Jul 5, 2018

Choose a reason for hiding this comment

Uh oh!

thaJeztah commented Jul 5, 2018

Uh oh!

thaJeztah commented Jul 5, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mlaventure commented Jul 5, 2018

Uh oh!

mlaventure commented Jul 5, 2018

Uh oh!

thaJeztah commented Jul 23, 2018

Uh oh!

thaJeztah commented Jul 23, 2018

Uh oh!

codecov-io commented Jul 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

thaJeztah commented Jul 23, 2018

Uh oh!

crosbymichael commented Jul 25, 2018

Uh oh!

thaJeztah commented Jul 25, 2018

Uh oh!

thaJeztah Aug 28, 2018

Choose a reason for hiding this comment

Uh oh!

estesp Oct 25, 2018

Choose a reason for hiding this comment

Uh oh!

thaJeztah commented Aug 28, 2018

Uh oh!

crosbymichael commented Aug 28, 2018

Uh oh!

dmcgowan commented Aug 28, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thaJeztah commented Oct 25, 2018

Uh oh!

estesp left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

thaJeztah commented Jul 5, 2018 •

edited

Loading

codecov-io commented Jul 23, 2018 •

edited

Loading

dmcgowan commented Aug 28, 2018 •

edited

Loading