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

[develop] Support spaces on lxd #12721

Merged

Conversation

achilleasa
Copy link
Contributor

This PR adds the last remaining bits to ensure that spaces are properly supported when using lxd as the substrate.

First, the PR corrects an issue with the lxd-specific subnet discovery logic that got introduced via a set of prior PRs (#11788, #11831, #11831). Note that this work is a 2.9 feature and has not been yet released. More specifically:

When we use lxd as a substrate (vs deploying an lxd workload on a machine managed by another substrate), Subnets() output should only include networks that are able to be bridged.

Given that an lxd server may be either local or remote, we cannot guarantee that we will be able to create bridges for regular NICs (as we do in the containerizer's bridge policy logic). Therefore, by ensuring that all reported subnets can be bridged, we allow users to define spaces and guarantee that the lxd provider will always be able to use the bridge NICs as parents of devices that get injected into the container specs constructed by StartInstance.

The second half of this PR completes the work for supporting spaces on lxd by:

  1. ensuring that we populate the AvailabilityZones field with the lxd server's name when discovering subnets. This is a prerequisite for the provisioner to be able to resolve space constraints into a set of (provider) subnet IDs prior to invoking the provider's StartInstance method.
  2. ensuring that we add any extra required NICs to container to satisfy the subnet requirements passed to StartInstance. The implementation includes de-dupping logic to avoid creating NICs that are bridged to the same host bridge.

QA steps

NOTE: Subnet discovery is disabled on lxd < 3.0 (xenial) so none of this code is invoked. However, the subnet discovery logic is slightly different on bionic and focal so they must both be tested. While testing I used a bionic instance on MAAS as a remote lxd cloud.

Example for focal:

$ juju bootstrap lxd test --no-gui

# YMMV based on what bridges are visible to lxd. You will need to adjust the following steps accordingly.
$ juju subnets
 juju subnets
subnets:
  10.0.0.0/24:
    type: ipv4
    provider-id: subnet-virmaasbr0-10.0.0.0/24
    provider-network-id: net-virmaasbr0
    status: in-use
    space: alpha
    zones:
    - mars
  10.42.0.0/24:
    type: ipv4
    provider-id: subnet-virmaasbr1-10.42.0.0/24
    provider-network-id: net-virmaasbr1
    status: in-use
    space: alpha
    zones:
    - mars
  10.232.217.0/24:
    type: ipv4
    provider-id: subnet-lxdbr0-10.232.217.0/24
    provider-network-id: net-lxdbr0
    status: in-use
    space: alpha
    zones:
    - mars
  172.17.0.0/16:
    type: ipv4
    provider-id: subnet-docker0-172.17.0.0/16
    provider-network-id: net-docker0
    status: in-use
    space: alpha
    zones:
    - mars
  192.168.122.0/24:
    type: ipv4
    provider-id: subnet-virbr0-192.168.122.0/24
    provider-network-id: net-virbr0
    status: in-use
    space: alpha
    zones:
    - mars
 
$ juju add-space space1  10.0.0.0/24 10.42.0.0/24

$ juju spaces
Name    Space ID  Subnets
alpha   0         10.232.217.0/24
                  172.17.0.0/16
                  192.168.122.0/24
space1  2         10.0.0.0/24
                  10.42.0.0/24


# Add a machine to that space
$ juju add-machine --constraints spaces=space1
$  juju show-machine 0
model: default
machines:
  "0":
    ....
     network-interfaces:
      eth0:
        ip-addresses:
        - 10.232.217.253   <---- this is bridged to lxdbr0 on my host
        mac-address: 00:16:3e:b0:4a:f4
        gateway: 10.232.217.1
        space: alpha
        is-up: true
      eth1:
        ip-addresses:
        - 10.42.0.194
        mac-address: 00:16:3e:61:48:df
        space: space1
        is-up: true
      eth2:
        ip-addresses:
        - 10.0.0.199
        mac-address: 00:16:3e:9e:a3:49
        space: space1
        is-up: true
    ...
    constraints: spaces=space1
    
# Try to deploy with --bind
$ juju deploy cs:wordpress --bind space1

# Wait for machine to start...
$ juju ssh wordpress/0 'ip a | grep eth.*@'
51: eth0@if52: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP group default qlen 1000
53: eth1@if54: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP group default qlen 1000
55: eth2@if56: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP group default qlen 1000

Documentation changes

We should tick the supports spaces box (with a on 2.9+ )

The presense of the AZ field is a prerequisite for the provisioner to
resolve required spaces to a set of subnet IDs so they can be passed to
StartInstance.
When we use lxd as a substrate (vs deploying an lxd workload on a
machine managed by another substrate), Subnets() output should only
include networks that are able to be bridged.

Given that an lxd server may be either local or remote, we cannot
guarantee that we will be able to create bridges for regular NICs (as we
do in the containerizer's bridge policy logic). Therefore, by ensuring
that all reported subnets can be bridged, we allow users to define
spaces and guarantee that the lxd provider will always be able to use
the bridge NICs as parents of devices that get injected into the
container specs constructed by StartInstance.
This change concludes the work to support spaces on lxd.
Copy link
Member

@manadart manadart left a comment

Choose a reason for hiding this comment

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

Nice job.

@@ -436,6 +436,10 @@ type StubClient struct {
NetworkState map[string]api.NetworkState
}

func (conn *StubClient) GetNetwork(string) (*api.Network, string, error) {
Copy link
Member

Choose a reason for hiding this comment

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

Really should use mocks here one day...

@achilleasa
Copy link
Contributor Author

$$merge$$

@jujubot jujubot merged commit f63ce41 into juju:develop Mar 5, 2021
@achilleasa achilleasa deleted the 2.9-support-space-constraints-for-lxd branch March 5, 2021 17:34
@achilleasa achilleasa changed the title [2.9] Support spaces on lxd [develop] Support spaces on lxd Mar 5, 2021
jujubot added a commit that referenced this pull request Mar 8, 2021
#12723

This is a back-port of #12721 to 2.9. The original was meant to land on 2.9 first but I fat fingered the base when I created the PR :-(
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants