Carry Fix docker inspect container only reports last assigned information

[tiborvass]

Fixes #17352
Closes #17360

Ping @calavera

[calavera]

we can remove this whole file 🔥 🤘

[calavera]

Code LGTM. Waiting to see what janky thinks about it. 👏

[tiborvass]

CI has issues, we need to run the tests locally

[coolljt0725]

@tiborvass Busy this day, get no time to read the notes from the github.

return findContainerIP(d.c, id, "--host") should be return findContainerIP(d.c, id, "bridge")

Sorry for my mistake in my PR, don't know why CI got green on my PR

[thaJeztah]

Thanks so much for looking @coolljt0725, and no need to apologize (that's where reviews are for, we're all human!)

[coolljt0725]

@thaJeztah thanks
@tiborvass After investigating why return findContainerIP(d.c, id, "--host") unexpectedly make CI green and found that we can't call findContainerIP in func (d *Daemon) findContainerIP(id string) because findContainerIP connect to the daemon start by .integration-daemon-start, we should connect the daemon d here, so I think the correct fix here is

func (d *Daemon) findContainerIP(id string) string {
        out, _ := d.Cmd("inspect", fmt.Sprintf("--format='{{ .NetworkSettings.Networks.bridge.IPAddress }}'"), id)
        return strings.Trim(out, " \r\n'")
}

correct me if I'm wrong :-)

Though the original code findContainerIP(d.c, id, "--host", d.sock()) has set the "--host", d.sock() but it never been used in func findContainerIP(c *check.C, id string, vargs ...string)

[tiborvass]

diff --git a/integration-cli/docker_utils.go b/integration-cli/docker_utils.go
index 67627af..850ad2e 100644
--- a/integration-cli/docker_utils.go
+++ b/integration-cli/docker_utils.go
@@ -783,12 +783,19 @@ func dockerCmdInDirWithTimeout(timeout time.Duration, path string, args ...strin
 }

 func findContainerIP(c *check.C, id string, network string) string {
-   out, _ := dockerCmd(c, "inspect", fmt.Sprintf("--format='{{ .NetworkSettings.Networks.%s.IPAddress }}'", network), id)
+   out, err := dockerCmd(c, "inspect", fmt.Sprintf("--format='{{ .NetworkSettings.Networks.%s.IPAddress }}'", network), id)
+   if err != nil {
+       c.Warn(err)
+   }
    return strings.Trim(out, " \r\n'")
 }

 func (d *Daemon) findContainerIP(id string) string {
-   return findContainerIP(d.c, id, "--host")
+   out, err := d.Cmd("inspect", fmt.Sprintf("--format='{{ .NetworkSettings.Networks.bridge.IPAddress }}'"), id)
+   if err != nil {
+       d.c.Warn(err)
+   }
+   return strings.TrimSpace(out)
 }

 func getContainerCount() (int, error) {

@calavera @coolljt0725 does this sound ok to you guys ?

[calavera]

lgtm

[vdemeester]

@tiborvass err will be an int in findContainerIP with 0 as value (or it will fail the test if an error occured — so the Warn feels useless). You probably want to use out, _, err := dockerCmdWithError(…) instead.

[runcom]

that if err != nil (from dockerCmd) won't compile (cannot convert nil to type int) I think @vdemeester

[vdemeester]

@runcom Yeah even that 😅 😝

[tiborvass]

@vdemeester @runcom @calavera @coolljt0725 fixed thank you

[calavera]

@tiborvass it looks like c.Warn does not exist:

docker_utils.go:1010: c.Warn undefined (type *check.C has no field or method Warn)

[calavera]

@tiborvass you need this patch:

diff --git c/integration-cli/docker_utils.go i/integration-cli/docker_utils.go
index 8aea980..581766d 100644
--- c/integration-cli/docker_utils.go
+++ i/integration-cli/docker_utils.go
@@ -785,7 +785,7 @@ func dockerCmdInDirWithTimeout(timeout time.Duration, path string, args ...strin
 func findContainerIP(c *check.C, id string, network string) string {
        out, _, err := dockerCmdWithError(c, "inspect", fmt.Sprintf("--format='{{ .NetworkSettings.Networks.%s.IPAddress }}'", network), id)
        if err != nil {
-               c.Warn(err)
+               c.Log(err)
        }
        return strings.Trim(out, " \r\n'")
 }
@@ -793,7 +793,7 @@ func findContainerIP(c *check.C, id string, network string) string {
 func (d *Daemon) findContainerIP(id string) string {
        out, err := d.Cmd("inspect", fmt.Sprintf("--format='{{ .NetworkSettings.Networks.bridge.IPAddress }}'"), id)
        if err != nil {
-               d.c.Warn(err)
+               d.c.Log(err)
        }
        return strings.TrimSpace(out)
 }

[tiborvass]

@calavera yes thank you I noticed it while rebasing :)

[tiborvass]

@calavera note that i just took what was at line 787

[calavera]

LGTM

[tiborvass]

putting merge label, since it already got okays in the pr this is carrying.

[tiborvass]

@calavera @mavenugo this is the line i just changed, it doesnt error out now.

[calavera]

@tiborvass

docker_cli_links_test.go:140:
    c.Fatal(err)
... Error: failed to inspect container one: Template parsing error: template: :1:18: executing "" at <.NetworkSettings.IPA...>: IPAddress is not a field of struct type *types.NetworkSettings

[tiborvass]

@calavera fixed

[mrjana]

changes LGTM. Let's wait for the CI to become green

[tiborvass]

@mrjana CI has issues, please test locally

[calavera]

LGTM. We're going to open a PR in docker-py to fix the functional suite.

Merging.

[tiborvass]

Ping @dnephin @aanand @shin- @abronan @vieux @aluzzardi this requires an API update.

[tiborvass]

@thaJeztah removing changelog, since it's part of the new networking feature which is already in the changelog.