Repository: knative-sandbox/kn-plugin-func
Pull request #669 (2021-11-18 17:02:17)

fix: use default socket path for TCP connections
# Changes

- :bug: Fix issues with TCP TLS connection.

resolves: #668

knative-prow-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative-sandbox/kn-plugin-func/pull/669#" title="Author self-approved">matejvasek</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=knative-sandbox%2Fkn-plugin-func).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/knative-sandbox/kn-plugin-func/blob/main/OWNERS)~~ [matejvasek]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
codecov[bot]: # [Codecov](https://codecov.io/gh/knative-sandbox/kn-plugin-func/pull/669?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox) Report
> Merging [#669](https://codecov.io/gh/knative-sandbox/kn-plugin-func/pull/669?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox) (dcbf08c) into [main](https://codecov.io/gh/knative-sandbox/kn-plugin-func/commit/836d47e54c42987eb5ea7c54f00c103f1deadbc0?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox) (836d47e) will **increase** coverage by `0.04%`.
> The diff coverage is `100.00%`.

> :exclamation: Current head dcbf08c differs from pull request most recent head 6d559b5. Consider uploading reports for the commit 6d559b5 to get more accurate results
[![Impacted file tree graph](https://codecov.io/gh/knative-sandbox/kn-plugin-func/pull/669/graphs/tree.svg?width=650&height=150&src=pr&token=da1B9bMsQ9&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox)](https://codecov.io/gh/knative-sandbox/kn-plugin-func/pull/669?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox)

```diff
@@            Coverage Diff             @@
##             main     #669      +/-   ##
==========================================
+ Coverage   37.31%   37.36%   +0.04%     
==========================================
  Files          41       41              
  Lines        3859     3862       +3     
==========================================
+ Hits         1440     1443       +3     
  Misses       2229     2229              
  Partials      190      190              
```


| [Impacted Files](https://codecov.io/gh/knative-sandbox/kn-plugin-func/pull/669?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox) | Coverage Δ | |
|---|---|---|
| [docker/docker\_client.go](https://codecov.io/gh/knative-sandbox/kn-plugin-func/pull/669/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox#diff-ZG9ja2VyL2RvY2tlcl9jbGllbnQuZ28=) | `81.94% <100.00%> (+0.78%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/knative-sandbox/kn-plugin-func/pull/669?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/knative-sandbox/kn-plugin-func/pull/669?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox). Last update [836d47e...6d559b5](https://codecov.io/gh/knative-sandbox/kn-plugin-func/pull/669?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox).

matejvasek: /hold
matejvasek: The `Func E2E Test / E2E Test (1.16.x, ubuntu-latest)` failure is probably flake. 
lance: > The `Func E2E Test / E2E Test (1.16.x, ubuntu-latest)` failure is probably flake.

I re-ran the jobs - we'll see how it goes.
matejvasek: @lance  this needs rebase on the fix for node builders
matejvasek: /unhold

Patch:
diff --git a/docker/docker_client.go b/docker/docker_client.go
index ba91917c..78fe46b5 100644
--- a/docker/docker_client.go
+++ b/docker/docker_client.go
@@ -41,6 +41,14 @@ func NewClient(defaultHost string) (dockerClient client.CommonAPIClient, dockerH
 
 	_url, err = url.Parse(dockerHost)
 	isSSH := err == nil && _url.Scheme == "ssh"
+	isTCP := err == nil && _url.Scheme == "tcp"
+
+	if isTCP {
+		// With TCP, it's difficult to determine how to expose the daemon socket to lifecycle containers,
+		// so we are defaulting to standard docker location by returning empty string.
+		// This should work well most of the time.
+		dockerHost = ""
+	}
 
 	if !isSSH {
 		dockerClient, err = client.NewClientWithOpts(client.FromEnv, client.WithAPIVersionNegotiation())


Test patch:
diff --git a/docker/docker_client_test.go b/docker/docker_client_test.go
index 2a7f9e4e..d8d8050b 100644
--- a/docker/docker_client_test.go
+++ b/docker/docker_client_test.go
@@ -18,7 +18,7 @@ import (
 
 // Test that we are creating client in accordance
 // with the DOCKER_HOST environment variable
-func TestNewDockerClient(t *testing.T) {
+func TestNewClient(t *testing.T) {
 	ctx, cancel := context.WithTimeout(context.Background(), time.Minute*1)
 	defer cancel()
 
@@ -41,6 +41,38 @@ func TestNewDockerClient(t *testing.T) {
 	}
 }
 
+func TestNewClient_DockerHost(t *testing.T) {
+	tests := []struct {
+		name                     string
+		dockerHostEnvVar         string
+		expectedRemoteDockerHost string
+	}{
+		{
+			name:                     "tcp",
+			dockerHostEnvVar:         "tcp://10.0.0.1:1234",
+			expectedRemoteDockerHost: "",
+		},
+		{
+			name:                     "unix",
+			dockerHostEnvVar:         "unix:///some/path/docker.sock",
+			expectedRemoteDockerHost: "unix:///some/path/docker.sock",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			defer WithEnvVar(t, "DOCKER_HOST", tt.dockerHostEnvVar)()
+			_, host, err := docker.NewClient(client.DefaultDockerHost)
+			if err != nil {
+				t.Fatal(err)
+			}
+			if host != tt.expectedRemoteDockerHost {
+				t.Errorf("expected docker host %q, but got %q", tt.expectedRemoteDockerHost, host)
+			}
+		})
+	}
+
+}
+
 func startMockDaemon(t *testing.T, sock string) func() {
 
 	server := http.Server{}