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

remove static ip in docker and podman #13766

Closed

Conversation

zhan9san
Copy link
Contributor

fix #13729

Would yo kindly review this PR?

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Mar 10, 2022
@k8s-ci-robot
Copy link
Contributor

Hi @zhan9san. 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 the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Mar 10, 2022
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: zhan9san
To complete the pull request process, please assign prezha after the PR has been reviewed.
You can assign the PR to them by writing /assign @prezha in a comment when ready.

The full list of commands accepted by this bot can be found 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

@minikube-bot
Copy link
Collaborator

Can one of the admins verify this patch?

@medyagh
Copy link
Member

medyagh commented Mar 17, 2022

@zhan9san would u plz state the reason u want to remove this ? this will make the driver less stable

@zhan9san
Copy link
Contributor Author

zhan9san commented Mar 18, 2022

Hi @medyagh

If we remove the static IP, docker daemon will assign an available IP to minikube container automatically.

Currently, if the driver is docker or podman, it's some sort of hard-code in minikube container IP,

index := driver.IndexFromMachineName(d.NodeConfig.MachineName)
if int(ip[3])+index > 255 {
return fmt.Errorf("too many machines to calculate an IP")
}
ip[3] += byte(index)
klog.Infof("calculated static IP %q for the %q container", ip.String(), d.NodeConfig.MachineName)
params.IP = ip.String()

In other words, it always passes the first IP in the subnet to minikube container, no matter it is available or not.

This issue exists in both two scenarios. Both of them would fail.

For minikube bridge network

  1. minikube start --driver=docker
  2. minikube stop
  3. docker run --rm -d --network minikube nginx
  4. minikube start --driver=docker

For non-minikube bridge network

  1. docker network create non-minikube
  2. docker run --rm -d --network non-minikube nginx
  3. minikube start --driver=docker --network non-minikube
❯ docker network ls
NETWORK ID     NAME                        DRIVER    SCOPE
5850e3b88677   bridge                      bridge    local
9901496684c6   host                        host      local
e1a5f17161a7   jenkins-in-docker_default   bridge    local
6c0240901aa0   knboard_default             bridge    local
5a9da0551e18   minikube                    bridge    local
a93741be50ae   none                        null      local
~                                                                                                                     09:14:28
❯ docker run -d --network minikube nginx
563f26651be44c5a10901671e71607abf9ab2308e4132098c3863cf20b0b803a
~                                                                                                                     09:15:07
❯ minikube start --driver=docker
😄  minikube v1.25.2 on Darwin 11.4
✨  Using the docker driver based on existing profile
👍  Starting control plane node minikube in cluster minikube
🚜  Pulling base image ...
🔄  Restarting existing docker container for "minikube" ...
🤦  StartHost failed, but will try again: provision: get ssh host-port: unable to inspect a not running container to get SSH port
🔄  Restarting existing docker container for "minikube" ...
😿  Failed to start docker container. Running "minikube delete" may fix it: provision: get ssh host-port: unable to inspect a not running container to get SSH port

❌  Exiting due to GUEST_PROVISION_CONTAINER_EXITED: Docker container exited prematurely after it was created, consider investigating Docker's performance/health.

~ 14s                                                                                                                 09:15:32
❯ docker stop 563f26651be44c5a10901671e71607abf9ab2308e4132098c3863cf20b0b803a
563f26651be44c5a10901671e71607abf9ab2308e4132098c3863cf20b0b803a
~                                                                                                                     09:16:08
❯ minikube start --driver=docker
😄  minikube v1.25.2 on Darwin 11.4
✨  Using the docker driver based on existing profile
👍  Starting control plane node minikube in cluster minikube
🚜  Pulling base image ...
🔄  Restarting existing docker container for "minikube" ...
🐳  Preparing Kubernetes v1.23.3 on Docker 20.10.12 ...
    ▪ kubelet.housekeeping-interval=5m
🔎  Verifying Kubernetes components...
💡  After the addon is enabled, please run "minikube tunnel" and your ingress resources would be available at "127.0.0.1"
    ▪ Using image registry.cn-hangzhou.aliyuncs.com/google_containers/storage-provisioner:v5
    ▪ Using image registry.cn-hangzhou.aliyuncs.com/google_containers/nginx-ingress-controller:v1.1.1
    ▪ Using image registry.cn-hangzhou.aliyuncs.com/google_containers/kube-webhook-certgen:v1.1.1
    ▪ Using image registry.cn-hangzhou.aliyuncs.com/google_containers/kube-webhook-certgen:v1.1.1
🔎  Verifying ingress addon...
🌟  Enabled addons: storage-provisioner, default-storageclass, ingress
🏄  Done! kubectl is now configured to use "minikube" cluster and "default" namespace by default

@zhan9san
Copy link
Contributor Author

Besides, if someone expects to use specific static IP address, it's better to implement it like below.

minikube start --driver=docker --extra-opt=ip=192.168.50.2.

This follows how docker does, dhcp by default whereas --ip string IPv4 address (e.g., 172.30.100.104) for specific scenario.

@k8s-ci-robot
Copy link
Contributor

@zhan9san: PR needs rebase.

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 the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 31, 2022
@medyagh medyagh closed this Apr 13, 2022
@medyagh
Copy link
Member

medyagh commented Apr 13, 2022

I would rather this PR be implemented as a feature #13050

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can't start minikube inside docker network
4 participants