Skip to content
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

Update Go to 1.17.4, replace io/ioutil with io, and update golang.org/x/sys #3041

Merged
merged 3 commits into from
Dec 8, 2021

Conversation

errordeveloper
Copy link
Contributor

No description provided.

@thaJeztah
Copy link
Member

go: cannot find main module, but found vendor.conf

Looks like we need to set some GO111MODULE=off (or auto)

.circleci/config.yml Outdated Show resolved Hide resolved
@errordeveloper errordeveloper force-pushed the go117 branch 2 times, most recently from e0ae0b4 to d3078cd Compare December 3, 2021 14:36
@errordeveloper errordeveloper marked this pull request as ready for review December 3, 2021 14:44
@codecov
Copy link

codecov bot commented Dec 3, 2021

Codecov Report

Merging #3041 (a9a35e0) into master (6303310) will decrease coverage by 0.13%.
The diff coverage is 58.62%.

@@            Coverage Diff             @@
##           master    #3041      +/-   ##
==========================================
- Coverage   62.29%   62.15%   -0.14%     
==========================================
  Files         155      155              
  Lines       24533    24533              
==========================================
- Hits        15283    15249      -34     
- Misses       7656     7686      +30     
- Partials     1594     1598       +4     

@errordeveloper
Copy link
Contributor Author

@thaJeztah @crazy-max CI is green!

Dockerfile Outdated Show resolved Hide resolved
@errordeveloper
Copy link
Contributor Author

@thaJeztah I've rejigged the commits, PTAL :)

@errordeveloper errordeveloper force-pushed the go117 branch 2 times, most recently from 455868b to 4dc6dfe Compare December 3, 2021 16:01
@errordeveloper
Copy link
Contributor Author

Looks like there is flake:

time="2021-12-03T16:09:06Z" level=error msg="update failed" error="task ri6pdecw0ysjkwaqch8meac2y was already shut down when reached by updater (state: SHUTDOWN)" task.id=skjv8kyowuwfp620nr040gl0d
--- FAIL: TestUpdaterRollback (3.44s)
    --- FAIL: TestUpdaterRollback/pause/monitor_set/spec_version_unset (0.63s)
        update_test.go:272: service was updated
        update_test.go:337: 
            	Error Trace:	update_test.go:337
            	            				update_test.go:20
            	Error:      	Not equal: 
            	            	expected: "image2"
            	            	actual  : "image1"
            	            	
            	            	Diff:
            	            	--- Expected
            	            	+++ Actual
            	            	@@ -1 +1 @@
            	            	-image2
            	            	+image1
            	Test:       	TestUpdaterRollback/pause/monitor_set/spec_version_unset
FAIL
coverage: 84.9% of statements
FAIL	github.com/docker/swarmkit/manager/orchestrator/replicated	5.331s
FAIL

it's green after a re-run...

@thaJeztah
Copy link
Member

Yes, I think it's a flaky test; I restarted CI earlier, and looks green again now 👍

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM (not a maintainer though)

@thaJeztah
Copy link
Member

@dperny @tonistiigi PTAL

@crazy-max
Copy link
Member

crazy-max commented Dec 3, 2021

@thaJeztah I think we need to also upgrade golang.org/x/sys no? moby/moby#42778

@thaJeztah
Copy link
Member

Ah, yes, we should probably update that as well, although (IIRC), it only affects macOS, so not directly a huge concern in this repository (so I'd be OK with doing it separately)

@errordeveloper
Copy link
Contributor Author

I tried this:

iff --git a/vendor.conf b/vendor.conf
index 994bd666..f377c49f 100644
--- a/vendor.conf
+++ b/vendor.conf
@@ -70,7 +70,7 @@ github.com/stretchr/testify                         ffdc059bfe9ce6a4e144ba849dbe
 go.etcd.io/bbolt                                    232d8fc87f50244f9c808f4745759e08a304c029 # v1.3.5
 golang.org/x/crypto                                 c1f2f97bffc9c53fc40a1a28a5b460094c0050d9
 golang.org/x/net                                    6772e930b67bb09bf22262c7378e7d2f67cf59d1
-golang.org/x/sys                                    d19ff857e887eacb631721f188c7d365c2331456
+golang.org/x/sys                                    63515b42dcdf9544f4e6a02fd7632793fde2f72d
 golang.org/x/text                                   23ae387dee1f90d29a23c0e87ee0b46038fbed0e # v0.3.3
 golang.org/x/time                                   555d28b269f0569763d25dbe1a237ae74c6bcc82
 

but vndr seems to revamp quite a few things... I suppose I can split the commits, but separate PR is probably needed, as this changes more than was regionally foreseen.

@thaJeztah
Copy link
Member

Yes, let's do it separate to not overload this PR too much

Signed-off-by: Ilya Dmitrichenko <errordeveloper@gmail.com>
Signed-off-by: Ilya Dmitrichenko <errordeveloper@gmail.com>
@errordeveloper
Copy link
Contributor Author

@thaJeztah I forgot --signoff, fixed now 🙈

@thaJeztah
Copy link
Member

Hmm... looks like the DCO bot is not running in this repository; let me try if we can enable it

@thaJeztah
Copy link
Member

oh, hm.. don't have access to the repository settings; perhaps @justincormack is able to help 🤔

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

(DCO messages look good)

@errordeveloper
Copy link
Contributor Author

It turned out that accidental changes were somehow to do with my setup, so I'm adding another commit here to update golang.org/x/sys.

…r Go 1.17)

Signed-off-by: Ilya Dmitrichenko <errordeveloper@gmail.com>
@errordeveloper
Copy link
Contributor Author

It hits TestUpdaterRollback/pause/monitor_set/spec_version_unset flake again...

@errordeveloper
Copy link
Contributor Author

cc @tonistiigi

@thaJeztah
Copy link
Member

let me try close/reopen to kick CI, and to see if the DCO bot also gets kicked

@thaJeztah thaJeztah closed this Dec 6, 2021
@thaJeztah thaJeztah reopened this Dec 6, 2021
@thaJeztah
Copy link
Member

hm.. ok; it runs on #3042, but doesn't get triggered here 😞

@thaJeztah thaJeztah closed this Dec 6, 2021
@thaJeztah thaJeztah reopened this Dec 6, 2021
@crazy-max
Copy link
Member

@thaJeztah Looking at the events that bot listens to, pull_request.synchronize could do the trick. Implies a new commit has to be added to the HEAD ref.

@thaJeztah
Copy link
Member

@thaJeztah Looking at the events that bot listens to, pull_request.synchronize could do the trick. Implies a new commit has to be added to the HEAD ref.

Hm.. yeah, that would explain it; perhaps that's part of the bug it previously had, where it got added to all existing pull request, but never got started. Wondering how to add the bot and make it run on all PRs (including existing ones) 🤔

@thaJeztah thaJeztah changed the title Go 1.17 Update Go to 1.17.4, replace io/ioutil with io, and update golang.org/x/sys Dec 7, 2021
@errordeveloper errordeveloper mentioned this pull request Dec 7, 2021
@dperny dperny merged commit bcd628d into moby:master Dec 8, 2021
@errordeveloper errordeveloper deleted the go117 branch December 11, 2021 18:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants