Skip to content

Dockerfile: remove GOMETALINTER_OPTS#39900

Merged
thaJeztah merged 1 commit intomoby:masterfrom
thaJeztah:remove_GOMETALINTER_OPTS
Sep 11, 2019
Merged

Dockerfile: remove GOMETALINTER_OPTS#39900
thaJeztah merged 1 commit intomoby:masterfrom
thaJeztah:remove_GOMETALINTER_OPTS

Conversation

@thaJeztah
Copy link
Member

@thaJeztah thaJeztah commented Sep 11, 2019 •
edited
Loading

This ENV was added to the Dockerfile in b96093f (#34759),
when the repository used per-architecture Dockerfiles, and some architectures needed
a different configuration.

Now that we use a multi-arch Dockerfile, and CI uses a Jenkinsfile, we can remove
this ENV from the Dockerfile, and set it in CI instead if needed.

Also updated the wording and fixed linting issues in hack/validate/gometalinter

Note that this changes the --deadline to use the default (10m), which probably isn't a big issue (note that on the Go 1.13 update PR, we actually run into the 2m deadline #39549, which is when I found that we override the default 10m)

@thaJeztah
Dockerfile: remove GOMETALINTER_OPTS
a464a3d 
This `ENV` was added to the Dockerfile in b96093f,
when the repository used per-architecture Dockerfiles, and some architectures needed
a different configuration.

Now that we use a multi-arch Dockerfile, and CI uses a Jenkinsfile, we can remove
this `ENV` from the Dockerfile, and set it in CI instead if needed.

Also updated the wording and fixed linting issues in hack/validate/gometalinter

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah thaJeztah requested a review from tianon as a code owner September 11, 2019 06:59
@thaJeztah
Copy link
Member Author

thaJeztah commented Sep 11, 2019

ping @kolyshkin @vdemeester PTAL

@thaJeztah thaJeztah mentioned this pull request Sep 11, 2019
Merged
6 tasks
vdemeester
vdemeester approved these changes Sep 11, 2019
View reviewed changes
Copy link
Member

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

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

LGTM

@thaJeztah
Copy link
Member Author

thaJeztah commented Sep 11, 2019

Failures on RS1; this one is not (yet) marked as flaky, but was mentioned in #39747 (comment) to be timing out on RS1

Previously it was disabled on Windows (known to be failing); #34340

 --- FAIL: Test/DockerSuite/TestAPIImagesSaveAndLoad (453.65s)
     cli.go:45: assertion failed: 
         Command:  d:\CI-1\CI-a464a3d51\binary\docker.exe inspect --format {{.Id}} sha256:2b2bba38d3ebd66ee344f7fe0e024d00df1b7979b281acddb6130a5e1b86e097
         ExitCode: 1
         Error:    exit status 1
         Stdout:   
         
         Stderr:   Error: No such object: sha256:2b2bba38d3ebd66ee344f7fe0e024d00df1b7979b281acddb6130a5e1b86e097
         
         
         Failures:
         ExitCode was 1 expected 0
         Expected no error

And a known flaky; tracked through issue #38521, and for which there's a WIP PR (that stalled unfortunately); #38595

     --- FAIL: Test/DockerSuite/TestStartReturnCorrectExitCode (8.35s)
         docker_cli_start_test.go:198: assertion failed: 125 (exitCode int) != 12 (int): out: time="2019-09-11T08:32:48Z" level=error msg="error waiting for container: EOF"

Failure on RS5 was a Jenkins issue https://ci.docker.com/public/job/moby/job/PR-39900/1/execution/node/72/log/

 azwin-2-5f0fe0 was marked offline: Connection was broken: java.util.concurrent.TimeoutException: Ping started at 1568185244721 hasn't completed by 1568185484722
 	at hudson.remoting.PingThread.ping(PingThread.java:134)
 	at hudson.remoting.PingThread.run(PingThread.java:90)
``

@thaJeztah
Copy link
Member Author

thaJeztah commented Sep 11, 2019

Everything else is green; I'm merging this one, because the changes in this PR do not affect anything that's running on Windows, so those failures are definitely unrelated

@thaJeztah thaJeztah merged commit 024a28a into moby:master Sep 11, 2019
@thaJeztah thaJeztah deleted the remove_GOMETALINTER_OPTS branch September 11, 2019 16:06
@thaJeztah thaJeztah mentioned this pull request Sep 25, 2019
Merged
@thaJeztah thaJeztah added this to the 20.03.0 milestone Apr 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AkihiroSuda AkihiroSuda AkihiroSuda approved these changes

@tianon tianon Awaiting requested review from tianon tianon is a code owner

+1 more reviewer

@vdemeester vdemeester vdemeester approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area/testing process/cherry-picked status/4-merge

Projects

None yet

Milestone

20.10.0

Development

Successfully merging this pull request may close these issues.

4 participants

@thaJeztah @vdemeester @AkihiroSuda @GordonTheTurtle