Resolve build hang when docker daemon under load #13817
Conversation
jim-minter
force-pushed
the
build_hang
branch
4 times, most recently
from
237c3d1 to
8c38b1f
Compare
|
There were other bugs, but I think flake #13694 was seen at https://ci.openshift.redhat.com/jenkins/job/test_pull_request_origin_extended/236/consoleFull#110463848456cbb9a5e4b02b88ae8c2f77 |
jim-minter
changed the title
|
@bparees ptal |
| @@ -40,7 +41,11 @@ type runtimeBuilderFactory struct{} | |||
|
|
|||
| // Builder delegates execution to S2I-specific code | |||
| func (_ runtimeBuilderFactory) Builder(config *s2iapi.Config, overrides s2ibuild.Overrides) (s2ibuild.Builder, s2iapi.BuildInfo, error) { | |||
| builder, buildInfo, err := s2i.Strategy(config, overrides) | |||
| client, err := docker.NewEngineAPIClient(config.DockerConfig) | |||
There was a problem hiding this comment.
A comment somehow connecting the dots to the docker daemon load issue might be useful months from now when someone looks at this and asks why we don't construct the client just once.
| @@ -259,7 +264,11 @@ func (s *S2IBuilder) Build() error { | |||
| return errors.New(buffer.String()) | |||
| } | |||
|
|
|||
| glog.V(4).Infof("Creating a new S2I builder with build config: %#v\n", describe.Config(config)) | |||
| client, err := docker.NewEngineAPIClient(config.DockerConfig) | |||
There was a problem hiding this comment.
A comment somehow connecting the dots to the docker daemon load issue might be useful months from now when someone looks at this and asks why we don't construct the client just once.
There was a problem hiding this comment.
Hmm, the docker daemon load issue is dealt with in the bowels of s2i; this PR is really just the bump. The API changed to require passing in a docker client, hence this diff. I'm not sure that really merits a comment (he says, having just written this explanation here).
There was a problem hiding this comment.
sorry the git diff tricked me into thinking both of these changes were in the same function and thus someone would expect to reuse the same client object.
|
agree w/ @gabemontero's suggestion, otherwise lgtm. |
openshift-bot
added
the
needs-rebase
|
Evaluated for origin test up to 815fa61 |
|
Evaluated for origin testextended up to 815fa61 |
openshift-bot
removed
the
needs-rebase
|
continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_origin/1002/) (Base Commit: b6b92db) |
|
continuous-integration/openshift-jenkins/testextended SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_origin_extended/261/) (Base Commit: b6b92db) (Extended Tests: core(builds)) |
|
[merge] |
|
Evaluated for origin merge up to 815fa61 |
|
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_request_origin/507/) (Base Commit: 03beec0) (Image: devenv-rhel7_6186) |
Resolves https://bugzilla.redhat.com/show_bug.cgi?id=1442875
[test][testextended][extended:core(builds)]