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

Support for configurable network address for user-v2 #1626

Merged
merged 1 commit into from
Sep 3, 2023

Conversation

balajiv113
Copy link
Member

@balajiv113 balajiv113 commented Jun 13, 2023

Fixes #1551 #1333

Keep the PR as draft for the following to be resolved

What's done
The user-v2 now honours gateway and netmask properties.

networks:
  user-v2:
    mode: user-v2
    gateway: 192.168.109.1
    netmask: 255.255.255.0

With the support of configurable subnet, we also required to provided support for dynamic DNS for user-v2

Changes to DNS resolving for usernet

Before
VM <-> gvisor-tap-vsock <-> hostagent dns resolver

Now
VM <-> gvisor-tap-vsock

No UDP or TCP forwarding will happen. So DNS lookup's will be faster now

How to enable new way of DNS resolver ?
Disabling the hostResolver will enable this new DNS for vz driver and for user-v2 network.

hostResolver:
  enabled: false

Need opinion on this, as we will still use hostResolver.hosts even after enabled is false.

Behaviour change

  • With user-v2 we will not set LIMA_CIDATA_SLIRP_IP_ADDRESS. As IP address are assigned dynamically
  • DNS record for instance is changed from lima-default to lima-default.internal

TODO

  • Fix tests with hardcoded IP

@@ -6,6 +6,10 @@ images:
- location: "https://cloud-images.ubuntu.com/releases/22.04/release/ubuntu-22.04-server-cloudimg-arm64.img"
arch: "aarch64"

hostResolver:
enabled: false
Copy link
Member

Choose a reason for hiding this comment

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

Could you add a comment line to explain this?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done. But am still looking for a way to maintain the config more cleaner.

Using hostResolver.hosts without hostResolver.enabled doesn't look clean.

Copy link
Member Author

@balajiv113 balajiv113 Jun 19, 2023

Choose a reason for hiding this comment

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

@AkihiroSuda - Is the current model looking good for you ??

The other approach i had in mind was to use our custom hostResolver only for qemu slirp network and in all other cases we will use gvisor-tap-vsock. If this enabled is false, we will simply use vm dns resolver

Note: In the second approach we will loose support for IPv6 resolution

Copy link
Member

Choose a reason for hiding this comment

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

@jandubois PTAL

Copy link
Member

Choose a reason for hiding this comment

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

The other approach i had in mind was to use our custom hostResolver only for qemu slirp network and in all other cases we will use gvisor-tap-vsock.

Will this work for VPN users?
If so, we can just ignore hostResolver for user-v2 mode.

Copy link
Member Author

Choose a reason for hiding this comment

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

Will this work for VPN users?

Yes yes, user-v2 will also work for VPN users as far as i checked

If so, we can just ignore hostResolver for user-v2 mode.

So basically whether hostResolver is present or not we will not consider it right ?

Copy link
Member

@AkihiroSuda AkihiroSuda Jul 31, 2023

Choose a reason for hiding this comment

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

So basically whether hostResolver is present or not we will not consider it right ?

Right, assuming that nobody needs hostResolver.enabled = true for user-v2 mode

Copy link
Member Author

Choose a reason for hiding this comment

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

To fully understand,
hostResolver.hosts will still be used right ?? To set the custom host names.

With this option the goal is that we will remove support for enable / disable and handle internally for user-v2

Copy link
Member

Choose a reason for hiding this comment

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

To fully understand, hostResolver.hosts will still be used right ?? To set the custom host names.

Yes 👍

With this option the goal is that we will remove support for enable / disable and handle internally for user-v2

👍

Copy link
Member Author

Choose a reason for hiding this comment

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

Done the changes

@AkihiroSuda AkihiroSuda added this to the v0.17.0 milestone Jun 13, 2023
@balajiv113 balajiv113 force-pushed the subnet-user-v2 branch 2 times, most recently from b345133 to 953dfa4 Compare June 15, 2023 13:34
@AkihiroSuda
Copy link
Member

Is this still a draft?

@balajiv113
Copy link
Member Author

Is this still a draft?

Its good for review now. I was just waiting for the other issue to get closed. But we can proceed further.

@balajiv113 balajiv113 marked this pull request as ready for review June 19, 2023 07:42
@raja
Copy link

raja commented Jul 6, 2023

Is this still a draft?

Its good for review now. I was just waiting for the other issue to get closed. But we can proceed further.

It appears that the referenced gvisor pr's have been merged. Is this good to review?

@balajiv113
Copy link
Member Author

@raja Yes its up for review.

I will address any new review comments and merge conflicts over next week.

AkihiroSuda
AkihiroSuda previously approved these changes Jul 19, 2023
@AkihiroSuda
Copy link
Member

Needs rebase

@balajiv113
Copy link
Member Author

Needs rebase

Done :)

@AkihiroSuda
Copy link
Member

CI is failing

@AkihiroSuda
Copy link
Member

TEST| [INFO] Testing proxy settings are imported
TEST| [INFO] FTP_PROXY: expected=FTP_PROXY=http://192.168.5.2:2121 got=FTP_PROXY=http://192.168.104.2:2121
TEST| [ERROR] proxy environment variable not set to correct value

https://github.com/lima-vm/lima/pull/1626/checks?check_run_id=15421021585

@balajiv113
Copy link
Member Author

I have fixed this but for some reason its failing in systemd strict check. Will try to fix early next week

@AkihiroSuda AkihiroSuda modified the milestones: v0.17.0, v0.18.0 Jul 30, 2023
@balajiv113 balajiv113 force-pushed the subnet-user-v2 branch 2 times, most recently from ea4353d to 7576107 Compare July 31, 2023 05:48
@balajiv113 balajiv113 force-pushed the subnet-user-v2 branch 2 times, most recently from 23d43a1 to 6e62020 Compare August 9, 2023 12:32
Copy link
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

@@ -6,6 +6,9 @@ images:
- location: "https://cloud-images.ubuntu.com/releases/22.04/release/ubuntu-22.04-server-cloudimg-arm64.img"
arch: "aarch64"

hostResolver:
hosts:
host.docker.internal: host.lima.internal
Copy link
Member

Choose a reason for hiding this comment

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

Not sure if this is necessary here

@AkihiroSuda AkihiroSuda merged commit 336c669 into lima-vm:master Sep 3, 2023
21 checks passed
@AkihiroSuda AkihiroSuda mentioned this pull request Sep 27, 2023
@balajiv113 balajiv113 deleted the subnet-user-v2 branch November 23, 2023 05:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for configurable network address for user-v2
3 participants