Skip to content

fix: When executing cli.setuphijackconn, call cli.dialer () after exe…#43801

Closed
yunnet wants to merge 6 commits intomoby:masterfrom
yunnet:master
Closed

fix: When executing cli.setuphijackconn, call cli.dialer () after exe…#43801
yunnet wants to merge 6 commits intomoby:masterfrom
yunnet:master

Conversation

@yunnet
Copy link
Copy Markdown

@yunnet yunnet commented Jul 13, 2022

…cuting "dialer := cli.dialer ()",

TLSClientConfig is not null when Scheme =" HTTP ", causing an error.
The error message is: OCI runtime exec failed: exec failed: unable to start container process: cannot connect to the Docker daemon. Is 'docker daemon' running on this host?

- What I did

- How I did it

- How to verify it

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

…cuting "dialer := cli.dialer ()",

TLSClientConfig is not null when Scheme =" HTTP ", causing an error.
The error message is: OCI runtime exec failed: exec failed: unable to start container process: cannot connect to the Docker daemon. Is 'docker daemon' running on this host?
Copy link
Copy Markdown
Member

@AkihiroSuda AkihiroSuda left a comment

Choose a reason for hiding this comment

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

Thanks

@AkihiroSuda
Copy link
Copy Markdown
Member

Thanks, but please sign the commit for DCO
https://github.com/apps/dco

(run git commit -a -s --amend, and make sure that the Signed-off-by: NAME <EMAIL> line with your real name is included in the commit message)

@thaJeztah thaJeztah added status/2-code-review dco/no Automatically set by a bot when one of the commits lacks proper signature process/cherry-pick kind/bugfix PR's that fix bugs labels Jul 13, 2022
@thaJeztah thaJeztah added this to the v-next milestone Jul 13, 2022
Comment thread client/client.go
Comment on lines +314 to +316
if cli.scheme == "http" {
transport.TLSClientConfig = nil
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@AkihiroSuda I'm wondering what the root cause is of this; I had a quick look at some of the code, and looking at

moby/client/client.go

Lines 146 to 158 in 0910306

if c.scheme == "" {
c.scheme = "http"
tlsConfig := resolveTLSConfig(c.client.Transport)
if tlsConfig != nil {
// TODO(stevvooe): This isn't really the right way to write clients in Go.
// `NewClient` should probably only take an `*http.Client` and work from there.
// Unfortunately, the model of having a host-ish/url-thingy as the connection
// string has us confusing protocol and transport layers. We continue doing
// this to avoid breaking existing clients but this should be addressed.
c.scheme = "https"
}
}

That appears to change scheme to https if TLS Config was found; is something wrong with the initialisation code?

yunnet added 3 commits July 13, 2022 16:39
…cuting "dialer := cli.dialer ()",

TLSClientConfig is not null when Scheme =" HTTP ", causing an error.
The error message is: OCI runtime exec failed: exec failed: unable to start container process: cannot connect to the Docker daemon. Is 'docker daemon' running on this host?

Signed-off-by: yunnet <[email protected]>
@dmcgowan
Copy link
Copy Markdown
Member

I suggest closing this one, the code has changed and this might not be relevant anymore

@thaJeztah
Copy link
Copy Markdown
Member

Thanks for looking; let me close this one 👍

@thaJeztah thaJeztah closed this Dec 18, 2023
@thaJeztah thaJeztah removed this from the v-future milestone Dec 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dco/no Automatically set by a bot when one of the commits lacks proper signature kind/bugfix PR's that fix bugs status/2-code-review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants