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

ci: update CI Vagrant VM IP addresses #17900

Merged
merged 1 commit into from Nov 17, 2021

Conversation

nbusseneau
Copy link
Member

VirtualBox 6.1.28 introduced new restrictions on host-only networking: https://www.virtualbox.org/wiki/Changelog-6.1#v28
Manual: https://www.virtualbox.org/manual/ch06.html#network_hostonly

On Linux, Mac OS X and Solaris Oracle VM VirtualBox will only allow IP addresses in 192.68.56.0/21 range to be assigned to host-only adapters. For IPv6 only link-local addresses are allowed. If other ranges are desired, they can be enabled by creating /etc/vbox/networks.conf and specifying allowed ranges there. For example, to allow 10.0.0.0/8 and 192.168.0.0/16 IPv4 ranges as well as 2001::/64 range put the following lines into /etc/vbox/networks.conf:

 * 10.0.0.0/8 192.168.0.0/16
 * 2001::/64

Lines starting with the hash # are ignored. Next example allows any addresses, effectively disabling range control:

 * 0.0.0.0/0 ::/0

These new restrictions manifest in the form of the following issue:

VBoxManage: error: Code E_ACCESSDENIED (0x80070005) - Access denied (extended info not available)
VBoxManage: error: Context: "EnableStaticIPConfig(Bstr(pszIp).raw(), Bstr(pszNetmask).raw())" at line 242 of file VBoxManageHostonly.cpp

when running VBoxManage hostonlyif ipconfig command where the IP address is outside of the allowed range, as is the case in our CI:

10:32:07  ==> k8s1-1.21: Clearing any previously set network interfaces...
10:32:09  There was an error while executing `VBoxManage`, a CLI used by Vagrant
10:32:09  for controlling VirtualBox. The command and stderr is shown below.
10:32:09
10:32:09  Command: ["hostonlyif", "ipconfig", "vboxnet0", "--ip", "192.168.38.1", "--netmask", "255.255.255.0"]
10:32:09
10:32:09  Stderr: VBoxManage: error: Code E_ACCESSDENIED (0x80070005) - Access denied (extended info not available)
10:32:09  VBoxManage: error: Context: "EnableStaticIPConfig(Bstr(pszIp).raw(), Bstr(pszNetmask).raw())" at line 242 of file VBoxManageHostonly.cpp

We update the IP addresses used by Vagrant VMs to be compatible with the new restricted range so that our VMs can still be run when using VirtualBox 6.1.28 and above, without the need for the /etc/vbox/networks.conf workaround above.

VirtualBox 6.1.28 introduced new restrictions on host-only networking:
https://www.virtualbox.org/wiki/Changelog-6.1#v28

Manual: https://www.virtualbox.org/manual/ch06.html#network_hostonly

> On Linux, Mac OS X and Solaris Oracle VM VirtualBox will only allow IP
> addresses in 192.68.56.0/21 range to be assigned to host-only
> adapters. For IPv6 only link-local addresses are allowed. If other
> ranges are desired, they can be enabled by creating
> /etc/vbox/networks.conf and specifying allowed ranges there. For
> example, to allow 10.0.0.0/8 and 192.168.0.0/16 IPv4 ranges as well as
> 2001::/64 range put the following lines into /etc/vbox/networks.conf:
>
>      * 10.0.0.0/8 192.168.0.0/16
>      * 2001::/64
>
> Lines starting with the hash # are ignored. Next example allows any
> addresses, effectively disabling range control:
>
>      * 0.0.0.0/0 ::/0

These new restrictions manifest in the form of the following issue:

```
VBoxManage: error: Code E_ACCESSDENIED (0x80070005) - Access denied (extended info not available)
VBoxManage: error: Context: "EnableStaticIPConfig(Bstr(pszIp).raw(), Bstr(pszNetmask).raw())" at line 242 of file VBoxManageHostonly.cpp
```

when running `VBoxManage hostonlyif ipconfig` command where the IP
address is outside of the allowed range, as is the case in our CI:

```
10:32:07  ==> k8s1-1.21: Clearing any previously set network interfaces...
10:32:09  There was an error while executing `VBoxManage`, a CLI used by Vagrant
10:32:09  for controlling VirtualBox. The command and stderr is shown below.
10:32:09
10:32:09  Command: ["hostonlyif", "ipconfig", "vboxnet0", "--ip", "192.168.38.1", "--netmask", "255.255.255.0"]
10:32:09
10:32:09  Stderr: VBoxManage: error: Code E_ACCESSDENIED (0x80070005) - Access denied (extended info not available)
10:32:09  VBoxManage: error: Context: "EnableStaticIPConfig(Bstr(pszIp).raw(), Bstr(pszNetmask).raw())" at line 242 of file VBoxManageHostonly.cpp
```

We update the IP addresses used by Vagrant VMs to be compatible with the
new restricted range so that our VMs can still be run when using
VirtualBox 6.1.28 and above, without the need for the
`/etc/vbox/networks.conf` workaround above.

Co-authored-by: Paul Chaignon <paul@cilium.io>
Signed-off-by: Paul Chaignon <paul@cilium.io>
Signed-off-by: Nicolas Busseneau <nicolas@isovalent.com>
@nbusseneau nbusseneau added area/CI Continuous Integration testing issue or flake release-note/ci This PR makes changes to the CI. labels Nov 16, 2021
@nbusseneau nbusseneau requested review from a team November 16, 2021 13:59
@nbusseneau nbusseneau requested review from a team as code owners November 16, 2021 13:59
@nbusseneau nbusseneau requested a review from a team November 16, 2021 13:59
@nbusseneau nbusseneau requested a review from a team as a code owner November 16, 2021 13:59
@nbusseneau nbusseneau requested a review from a team November 16, 2021 13:59
@nbusseneau nbusseneau requested review from a team as code owners November 16, 2021 13:59
@nbusseneau
Copy link
Member Author

This is a re-open of #17733, which was merged accidentally and subsequently reverted in #17898.

@nbusseneau
Copy link
Member Author

nbusseneau commented Nov 16, 2021

Original PR hit issues:

@nbusseneau
Copy link
Member Author

Reposting comment from previous PR regarding the "IP replace pattern":

My pattern was:

  • Try to do 3X => 5X if possible => this perfectly matches the CI Vagrant VMs IPs.
  • Start from 60 if not possible => this perfectly matches the non-CI Vagrant VMs IPs.

But I can re-do it another way if we'd like, I mostly wanted to have a quick thing out to run tests ^^

@pchaigno
Copy link
Member

pchaigno commented Nov 16, 2021

There's a suspicion that the privileged unit test failure could actually be caused by this PR so let's retrigger to see:
/test-runtime

@pchaigno
Copy link
Member

The changes here mostly impact @cilium/ci-structure (test VMs) and @cilium/contributing (dev. VMs); other changes are trivial. So probably not worth waiting for all remaining review requests. Tests are also all passing. Marking ready to merge.

@pchaigno pchaigno added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Nov 16, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot removed the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Nov 16, 2021
@pchaigno pchaigno added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Nov 17, 2021
@ti-mo ti-mo merged commit 112396c into cilium:master Nov 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/CI Continuous Integration testing issue or flake ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/ci This PR makes changes to the CI.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants