Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wip: Use regex anchors to be sure we get bridge from the docker network inspect command #8034

Closed

Conversation

alexjh
Copy link

@alexjh alexjh commented May 7, 2020

For whatever reason, my docker networks contain multiple bridges, two of them containing the text bridge. This change uses regex anchors to be sure we only get one.

./out/minikube-linux-amd64 version
minikube version: v1.10.0-beta.2
commit: 3a025104bda8f24025a162432afdce894d463f55
$ docker network ls
NETWORK ID          NAME                DRIVER              SCOPE
1f6a7ff7d8de        bridge              bridge              local
02b1e0157e69        docker_gwbridge     bridge              local
$ ./out/minikube-linux-amd64 mount -v 10 --alsologtostderr ./data:/data
I0507 08:54:05.927929   25256 mustload.go:64] Loading cluster: minikube
I0507 08:54:05.929063   25256 cli_runner.go:108] Run: docker inspect minikube --format={{.State.Status}}
I0507 08:54:06.017624   25256 host.go:65] Checking if "minikube" exists ...
I0507 08:54:06.018099   25256 cli_runner.go:108] Run: docker network ls --filter name=bridge --format {{.ID}}
I0507 08:54:06.103844   25256 cli_runner.go:108] Run: docker inspect --format "{{(index .IPAM.Config 0).Gateway}}" 1f6a7ff7d8de
02b1e0157e69
I0507 08:54:06.206810   25256 exit.go:58] WithError(Error getting the host IP address to use from within the VM)=inspect IP bridge network "1f6a7ff7d8de\n02b1e0157e69".: docker inspect --format "{{(index .IPAM.Config 0).Gateway}}" 1f6a7ff7d8de
02b1e0157e69: exit status 1
stdout:


stderr:
Error: No such object: 1f6a7ff7d8de
02b1e0157e69
 called from:
goroutine 1 [running]:
runtime/debug.Stack(0x0, 0x6, 0x0)
	/usr/local/go/src/runtime/debug/stack.go:24 +0x9d
k8s.io/minikube/pkg/minikube/exit.WithError(0x1b2a115, 0x3b, 0x1d99280, 0xc0000cd140)
	/home/alexh/repos/kubernetes/minikube/pkg/minikube/exit/exit.go:58 +0x34
k8s.io/minikube/cmd/minikube/cmd.glob..func12(0x2ae7120, 0xc000259fc0, 0x1, 0x4)
	/home/alexh/repos/kubernetes/minikube/cmd/minikube/cmd/mount.go:111 +0x18e8
github.com/spf13/cobra.(*Command).execute(0x2ae7120, 0xc000259f80, 0x4, 0x4, 0x2ae7120, 0xc000259f80)
	/home/alexh/go/pkg/mod/github.com/spf13/cobra@v1.0.0/command.go:846 +0x2aa
github.com/spf13/cobra.(*Command).ExecuteC(0x2ae8b60, 0x0, 0x1, 0xc0001ac640)
	/home/alexh/go/pkg/mod/github.com/spf13/cobra@v1.0.0/command.go:950 +0x349
github.com/spf13/cobra.(*Command).Execute(...)
	/home/alexh/go/pkg/mod/github.com/spf13/cobra@v1.0.0/command.go:887
k8s.io/minikube/cmd/minikube/cmd.Execute()
	/home/alexh/repos/kubernetes/minikube/cmd/minikube/cmd/root.go:108 +0x6a4
main.main()
	/home/alexh/repos/kubernetes/minikube/cmd/minikube/main.go:66 +0xea
W0507 08:54:06.207173   25256 out.go:201] Error getting the host IP address to use from within the VM: inspect IP bridge network "1f6a7ff7d8de\n02b1e0157e69".: docker inspect --format "{{(index .IPAM.Config 0).Gateway}}" 1f6a7ff7d8de
02b1e0157e69: exit status 1
stdout:


stderr:
Error: No such object: 1f6a7ff7d8de
02b1e0157e69

💣  Error getting the host IP address to use from within the VM: inspect IP bridge network "1f6a7ff7d8de\n02b1e0157e69".: docker inspect --format "{{(index .IPAM.Config 0).Gateway}}" 1f6a7ff7d8de
02b1e0157e69: exit status 1
stdout:


stderr:
Error: No such object: 1f6a7ff7d8de
02b1e0157e69


😿  minikube is exiting due to an error. If the above message is not useful, open an issue:
👉  https://github.com/kubernetes/minikube/issues/new/choose

After the change:

± ./out/minikube-linux-amd64 mount -v 10 --alsologtostderr out/:/data
I0507 08:57:33.171841   32013 mustload.go:64] Loading cluster: minikube
I0507 08:57:33.173373   32013 cli_runner.go:108] Run: docker inspect minikube --format={{.State.Status}}
I0507 08:57:33.260045   32013 host.go:65] Checking if "minikube" exists ...
I0507 08:57:33.260567   32013 cli_runner.go:108] Run: docker network ls --filter name=^bridge$ --format {{.ID}}
I0507 08:57:33.347642   32013 cli_runner.go:108] Run: docker inspect --format "{{(index .IPAM.Config 0).Gateway}}" 1f6a7ff7d8de
I0507 08:57:33.431061   32013 network.go:77] got host ip for mount in container by inspect docker network: 172.17.0.1
📁  Mounting host path out/ into VM as /data ...
    ▪ Mount type:   <no value>
    ▪ User ID:      docker
    ▪ Group ID:     docker
    ▪ Version:      9p2000.L
    ▪ Message Size: 262144
    ▪ Permissions:  755 (-rwxr-xr-x)
    ▪ Options:      map[]
    ▪ Bind Address: 172.17.0.1:40709
I0507 08:57:33.434277   32013 ssh_runner.go:148] Run: /bin/bash -c "[ "x$(findmnt -T /data | grep /data)" != "x" ] && sudo umount -f /data || echo "
🚀  Userspace file server: ufs starting
I0507 08:57:33.434729   32013 cli_runner.go:108] Run: docker inspect -f "'{{(index (index .NetworkSettings.Ports "22/tcp") 0).HostPort}}'" minikube
I0507 08:57:33.518944   32013 sshutil.go:44] new ssh client: &{IP:127.0.0.1 Port:32770 SSHKeyPath:/home/alexh/.minikube/machines/minikube/id_rsa Username:docker}
I0507 08:57:33.627018   32013 mount.go:147] unmount for /data ran successfully
I0507 08:57:33.627069   32013 ssh_runner.go:148] Run: /bin/bash -c "sudo mkdir -m 755 -p /data"
I0507 08:57:33.642723   32013 ssh_runner.go:148] Run: /bin/bash -c "sudo mount -t 9p -o dfltgid=$(grep ^docker: /etc/group | cut -d: -f3),dfltuid=$(id -u docker),msize=262144,port=40709,trans=tcp,version=9p2000.L 172.17.0.1 /data"
I0507 08:57:33.661978   32013 main.go:96] stdlog: ufs.go:141 connected
I0507 08:57:33.663962   32013 main.go:96] stdlog: srv_conn.go:133 >>> 172.17.0.3:53484 Tversion tag 65535 msize 65536 version '9P2000.L'
I0507 08:57:33.664047   32013 main.go:96] stdlog: srv_conn.go:190 <<< 172.17.0.3:53484 Rversion tag 65535 msize 65536 version '9P2000'
I0507 08:57:33.664347   32013 main.go:96] stdlog: srv_conn.go:133 >>> 172.17.0.3:53484 Tattach tag 1 fid 0 afid 4294967295 uname 'nobody' nuname 0 aname ''
I0507 08:57:33.664436   32013 main.go:96] stdlog: srv_conn.go:190 <<< 172.17.0.3:53484 Rattach tag 1 aqid (1420245 efda93af 'd')
I0507 08:57:33.666136   32013 main.go:96] stdlog: srv_conn.go:133 >>> 172.17.0.3:53484 Tstat tag 1 fid 0
I0507 08:57:33.666230   32013 main.go:96] stdlog: srv_conn.go:190 <<< 172.17.0.3:53484 Rstat tag 1 st ('out' 'alexh' 'alexh' '' q (1420245 efda93af 'd') m d775 at 0 mt 1588867011 l 4096 t 0 d 0 ext )
I0507 08:57:33.669040   32013 mount.go:72] mount successful: ""
✅  Successfully mounted out/ to /data

📌  NOTE: This process must stay alive for the mount to be accessible ...

@k8s-ci-robot
Copy link
Contributor

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.


Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label May 7, 2020
@k8s-ci-robot
Copy link
Contributor

Welcome @alexjh!

It looks like this is your first PR to kubernetes/minikube 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes/minikube has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

@k8s-ci-robot
Copy link
Contributor

Hi @alexjh. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels May 7, 2020
@minikube-bot
Copy link
Collaborator

Can one of the admins verify this patch?

Avoid returning a list of any networks that contain `bridge`.
@alexjh alexjh force-pushed the filter-specific-docker-bridge branch from 4c77cf7 to 3741e4c Compare May 7, 2020 16:05
@alexjh
Copy link
Author

alexjh commented May 7, 2020

/check-cla

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels May 7, 2020
@alexjh
Copy link
Author

alexjh commented May 7, 2020

/assign @afbjorklund

Copy link
Member

@medyagh medyagh left a comment

Choose a reason for hiding this comment

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

the right way to fix this get list of all networks of type bridge, and then inspect them and see which one has the minikube container in it

$ docker network inspect bridge
[
    {
        "Name": "bridge",
        "Id": "c2c5df7bc09a28dceec8df47e9f24053da7a310b0fc967b8f6140f5bcd2d71af",
        "Created": "2020-05-02T00:23:57.187808648Z",
        "Scope": "local",
        "Driver": "bridge",
        "EnableIPv6": false,
        "IPAM": {
            "Driver": "default",
            "Options": null,
            "Config": [
                {
                    "Subnet": "172.17.0.0/16",
                    "Gateway": "172.17.0.1"
                }
            ]
        },
        "Internal": false,
        "Attachable": false,
        "Ingress": false,
        "ConfigFrom": {
            "Network": ""
        },
        "ConfigOnly": false,
        "Containers": {
            "00530626b5b0af4b03a72b9b6243c069b39747de92d9dcfe1dc4ecf5ff35e61d": {
                "Name": "minikube",
                "EndpointID": "dfc84498b1f7a690f246afa322b620bf5ca051107fd252bae2b8494e5e841ee5",
                "MacAddress": "02:42:ac:11:00:02",
                "IPv4Address": "172.17.0.2/16",
                "IPv6Address": ""
            }
        },
        "Options": {
            "com.docker.network.bridge.default_bridge": "true",
            "com.docker.network.bridge.enable_icc": "true",
            "com.docker.network.bridge.enable_ip_masquerade": "true",
            "com.docker.network.bridge.host_binding_ipv4": "0.0.0.0",
            "com.docker.network.bridge.name": "docker0",
            "com.docker.network.driver.mtu": "1500"
        },
        "Labels": {}
    }
]

(in that case we would need to pass the profile name to the dockerGatewayIP()

Copy link
Collaborator

@afbjorklund afbjorklund left a comment

Choose a reason for hiding this comment

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

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 7, 2020
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: afbjorklund, alexjh

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 7, 2020
@medyagh medyagh removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 7, 2020
@afbjorklund
Copy link
Collaborator

Okay, let's do something else

@afbjorklund afbjorklund removed their assignment May 7, 2020
@medyagh medyagh requested a review from afbjorklund May 7, 2020 20:41
@medyagh medyagh changed the title Use regex anchors to be sure we get bridge from the docker network inspect command wip: Use regex anchors to be sure we get bridge from the docker network inspect command May 8, 2020
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 8, 2020
@afbjorklund
Copy link
Collaborator

The comment in #8131 is that this second bridge network gets created by Docker Swarm

@alexjh
Copy link
Author

alexjh commented May 13, 2020

@medyagh

Could we instead get it from the container itself?

docker inspect minikube -f '{{ range $index, $element := .NetworkSettings.Networks }}{{$element.Gateway}}{{end}}'

@medyagh
Copy link
Member

medyagh commented May 13, 2020

@medyagh

Could we instead get it from the container itself?

docker inspect minikube -f '{{ range $index, $element := .NetworkSettings.Networks }}{{$element.Gateway}}{{end}}'

yeah that works too, if you can prove it fixes the issue I am okay with that too

@afbjorklund
Copy link
Collaborator

afbjorklund commented May 14, 2020

yeah that works too, if you can prove it fixes the issue I am okay with that too

I did a containerGatewayIP function for this, that we are using for podman

But I have only tested on Linux, so I don't know if it works with the VMs

@medyagh
Copy link
Member

medyagh commented May 28, 2020

@alexjh are you still interested to resolve the review comments?

@medyagh
Copy link
Member

medyagh commented Jun 3, 2020

@alexjh I have not heared from you , I will have to close this PR if I dont hear from you

@priyawadhwa
Copy link

Hey @alexjh as some time has passed without activity I'm going to close this PR -- feel free to open it whenever you have a chance!

@afbjorklund
Copy link
Collaborator

I still think this solution would have been the easiest fix to the issue, but I guess we can do something better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants