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.

Sign up for GitHub

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

Jump to bottom

ci: update CI Vagrant VM IP addresses #17733

Merged
merged 1 commit into from Nov 16, 2021
Merged

ci: update CI Vagrant VM IP addresses #17733

merged 1 commit into from Nov 16, 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.

@nbusseneau nbusseneau added area/CI Continuous Integration testing issue or flake release-note/ci This PR makes changes to the CI. labels Oct 28, 2021
@nbusseneau nbusseneau requested a review from a team as a code owner October 28, 2021 12:18
pchaigno
pchaigno reviewed Oct 28, 2021
View reviewed changes
Copy link
Member

@pchaigno pchaigno left a comment

Choose a reason for hiding this comment

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

Nice find!

Why are 192.168.36.0/24 and 192.168.37.0/24 not an issue? Is that because they are virtualbox__intnet?

Shouldn't we also update the IP(s) used by the development VMs (/Vagrantfile)?

@nbusseneau
Copy link
Member Author

Nice find!

Why are 192.168.36.0/24 and 192.168.37.0/24 not an issue? Is that because they are virtualbox__intnet?

Shouldn't we also update the IP(s) used by the development VMs (/Vagrantfile)?

Indeed we should, it's just me that missed them because I grepped for 192.168.38 🤦🏻 On it.

@nbusseneau nbusseneau marked this pull request as draft October 28, 2021 13:35
@nbusseneau
Copy link
Member Author

/test

pchaigno
pchaigno approved these changes Oct 28, 2021
View reviewed changes
clustermesh-apiserver/tls.rst Show resolved Hide resolved
@pchaigno
Copy link
Member

I just reviewed, twice, a draft PR 🤦

@nbusseneau
Copy link
Member Author

nbusseneau commented Nov 3, 2021 •
edited

Travis CI https://app.travis-ci.com/github/cilium/cilium/builds/240770216 hit

FAIL coverage: 2.7% of statements in ./...
FAIL	github.com/cilium/cilium/pkg/hubble/parser/threefour	0.088s
FAIL make: *** [Makefile:203: integration-tests] Error 1

Probably coming from the PR, I must have missed a destination IP or something.

@pchaigno
Copy link
Member

Probably coming from the PR, I must have missed a destination IP or something.

Fixed it, we had some hardcoded BPF events in Hubble tests, with the IP address in a binary slice.

@nbusseneau @pchaigno
ci: update CI Vagrant VM IP addresses
2a8c0c7 
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>
@pchaigno pchaigno marked this pull request as ready for review November 15, 2021 20:49
@pchaigno pchaigno requested review from a team November 15, 2021 20:49
@pchaigno pchaigno requested review from a team as code owners November 15, 2021 20:49
@pchaigno pchaigno requested a review from a team November 15, 2021 20:49
@pchaigno pchaigno requested a review from a team as a code owner November 15, 2021 20:49
@pchaigno pchaigno requested a review from a team November 15, 2021 20:49
@pchaigno pchaigno requested review from a team as code owners November 15, 2021 20:49
@pchaigno
Copy link
Member

pchaigno commented Nov 15, 2021 •
edited by maintainer-s-little-helper bot

/test

Job 'Cilium-PR-Runtime-net-next' failed and has not been observed before, so may be related to your PR:

Click to show.

Test Name

RuntimePrivilegedUnitTests Run Tests

Failure Output

FAIL: Failed to run privileged unit tests

If it is a flake, comment /mlh new-flake Cilium-PR-Runtime-net-next so I can create a new GitHub issue to track it.

@nbusseneau
Copy link
Member Author

Thanks for the fix.

@pchaigno
Copy link
Member

pchaigno commented Nov 16, 2021 •
edited by maintainer-s-little-helper bot

/mlh new-flake Cilium-PR-Runtime-net-next

👍 created #17897

@pchaigno
Copy link
Member

pchaigno commented Nov 16, 2021 •
edited

Almost all tests failed in previous run because Cilium failed to start: https://jenkins.cilium.io/job/Cilium-PR-K8s-1.16-net-next/1945/. Not exactly clear why (connectivity issue between nodes?).
/test-1.16-netnext

@ti-mo ti-mo merged commit aba7f0e into cilium:master Nov 16, 2021
@ti-mo
Copy link
Contributor

ti-mo commented Nov 16, 2021

This was accidentally merged, sorry for the trouble. Revert in #17898.

@borkmann borkmann mentioned this pull request Nov 16, 2021
Closed
@nbusseneau nbusseneau mentioned this pull request Nov 16, 2021
Merged
@nbusseneau
Copy link
Member Author

Re-opened at #17900.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pchaigno pchaigno pchaigno approved these changes

@nebril nebril Awaiting requested review from nebril nebril was automatically assigned from cilium/ci-structure

@kkourt kkourt Awaiting requested review from kkourt

@qmonnet qmonnet Awaiting requested review from qmonnet qmonnet was automatically assigned from cilium/docs-structure

@jrfastab jrfastab Awaiting requested review from jrfastab

@jrajahalme jrajahalme Awaiting requested review from jrajahalme jrajahalme was automatically assigned from cilium/proxy

@gandro gandro Awaiting requested review from gandro gandro was automatically assigned from cilium/wireguard

Assignees

@gandro gandro

@kkourt kkourt

@jrfastab jrfastab

@nebril nebril

@jrajahalme jrajahalme

@qmonnet qmonnet

Labels
area/CI Continuous Integration testing issue or flake release-note/ci This PR makes changes to the CI.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
9 participants
@nbusseneau @pchaigno @ti-mo @gandro @kkourt @jrfastab @nebril @jrajahalme @qmonnet