Merge "Generate IP for vino networks using vino IPAM"

2021-06-16 22:20:57 +00:00 · 2021-06-16 22:20:57 +00:00 · 9f95cbe4cb
commit 9f95cbe4cb
parent 7737926909 0e6d38ecb8
11 changed files with 193 additions and 95 deletions
--- a/config/crd/bases/airship.airshipit.org_vinoes.yaml
+++ b/config/crd/bases/airship.airshipit.org_vinoes.yaml
@ -86,6 +86,10 @@ spec:
                      type: string
                    allocationStop:
                      type: string
+                    bridgeName:
+                      description: BridgeName is the name of the bridge to be created
+                        as libvirt network. works if AllocateNodeIP is sepcified
+                      type: string
                    dns_servers:
                      items:
                        type: string
@ -239,13 +243,8 @@ spec:
                description: PXEBootImageHostPort will be used to download the PXE
                  boot image
                type: integer
-              vmBridge:
-                description: VMBridge defines the single interface name to be used
-                  as a bridge for VMs
-                type: string
            required:
            - bmcCredentials
-            - vmBridge
            type: object
          status:
            description: VinoStatus defines the observed state of Vino
--- a/config/manager/daemonset-template.yaml
+++ b/config/manager/daemonset-template.yaml
@ -108,14 +108,14 @@ spec:
        #       host: 127.0.0.1
        #     initialDelaySeconds: 30
        #     periodSeconds: 30
-        - name: labeler
-          image: quay.io/airshipit/nodelabeler
-          imagePullPolicy: IfNotPresent
-          env:
-            - name: NODE
-              valueFrom:
-                fieldRef:
-                  fieldPath: spec.nodeName
+        # - name: labeler
+        #   image: quay.io/airshipit/nodelabeler
+        #   imagePullPolicy: IfNotPresent
+        #   env:
+        #     - name: NODE
+        #       valueFrom:
+        #         fieldRef:
+        #           fieldPath: spec.nodeName
        - name: vino-builder
          readinessProbe:
            initialDelaySeconds: 20
--- a/config/manager/network-templates.yaml
+++ b/config/manager/network-templates.yaml
@ -4,8 +4,8 @@ libvirtNetworks:
      <network>
        <name>{{ network.name }}</name>
        <forward mode='route'/>
-        <bridge name='vm-infra-bridge' stp='off' delay='0' {% if network.physicalInterface is defined %} dev='{{ network.physicalInterface }}' {% endif %}/>
-        <ip address='{{ ipam.bridge_ip | default(omit) }}' netmask='{{ ipam.bridge_subnet_netmask }}'>
+        <bridge name='{{ network.bridgeName | default('vm-infra-bridge') }}' stp='off' delay='0'/>
+        <ip address='{{ network.bridgeIP | default(omit) }}' netmask='{{ ipam.bridge_subnet_netmask }}'>
          <!-- <tftp root='/srv/tftp'/> -->
          <dhcp>
            <range start='{{ ipam.instance_ips[0] }}' end='{{ ipam.instance_ips[-1] }}'/>
--- a/config/samples/vino_cr.yaml
+++ b/config/samples/vino_cr.yaml
@ -4,7 +4,6 @@ metadata:
  name: vino-test-cr
  labels: {}
 spec:
-  vmBridge: vm-infra-bridge
  nodeLabelKeysToCopy:
    - "airshipit.org/server"
    - "airshipit.org/rack"
@ -17,13 +16,14 @@ spec:
    - name: management
      libvirtTemplate: management
      subnet: 192.168.2.0/20
-      instanceSubnet: 192.168.2.0/22
+      instanceSubnet: 192.168.4.0/22
      type: bridge
      allocationStart: 192.168.2.10
-      allocationStop: 192.168.2.14 # docs should specify that the range should = number of vms (to permit future expansion over multiple vino crs etc)
+      allocationStop: 192.168.2.24 # docs should specify that the range should = number of vms (to permit future expansion over multiple vino crs etc)
      dns_servers: ["135.188.34.124"]
      macPrefix: "52:54:00:06:00:00"
      physicalInterface: enp3s7
+      bridgeName: vm-infra
  nodes:
    - name: master
      count: 1
--- a/config/samples/vino_cr_4_workers_1_cp.yaml
+++ b/config/samples/vino_cr_4_workers_1_cp.yaml
@ -4,7 +4,6 @@ metadata:
  name: vino-test-cr
  labels: {}
 spec:
-  vmBridge: vm-infra-bridge
  nodeLabelKeysToCopy:
    - "airshipit.org/server"
    - "airshipit.org/rack"
@ -17,10 +16,10 @@ spec:
    - name: management
      libvirtTemplate: management
      subnet: 192.168.2.0/20
-      instanceSubnet: 192.168.2.0/22
+      instanceSubnet: 192.168.4.0/22
      type: ipv4
      allocationStart: 192.168.2.10
-      allocationStop: 192.168.2.14 # docs should specify that the range should = number of vms (to permit future expansion over multiple vino crs etc)
+      allocationStop: 192.168.2.25 # docs should specify that the range should = number of vms (to permit future expansion over multiple vino crs etc)
      routes:
        - network: 10.0.0.0
          netmask: 255.255.255.0
@ -28,6 +27,7 @@ spec:
      dns_servers: ["135.188.34.124"]
      macPrefix: "52:54:00:06:00:00"
      physicalInterface: enp3s7
+      bridgeName: vm-infra
  nodes:
    - name: master
      count: 1
--- a/docs/api/vino.md
+++ b/docs/api/vino.md
@ -149,8 +149,8 @@ int
 <td>
 <code>networks</code><br>
 <em>
-<a href="#airship.airshipit.org/v1.Network">
-[]Network
+<a href="#airship.airshipit.org/v1.BuilderNetwork">
+[]BuilderNetwork
 </a>
 </em>
 </td>
@ -245,6 +245,26 @@ string
 </tr>
 <tr>
 <td>
+<code>enableVNC</code><br>
+<em>
+bool
+</em>
+</td>
+<td>
+</td>
+</tr>
+<tr>
+<td>
+<code>vncPassword</code><br>
+<em>
+string
+</em>
+</td>
+<td>
+</td>
+</tr>
+<tr>
+<td>
 <code>interfaces</code><br>
 <em>
 <a href="#airship.airshipit.org/v1.BuilderNetworkInterface">
@ -259,6 +279,73 @@ string
 </table>
 </div>
 </div>
+<h3 id="airship.airshipit.org/v1.BuilderNetwork">BuilderNetwork
+</h3>
+<p>
+(<em>Appears on:</em>
+<a href="#airship.airshipit.org/v1.Builder">Builder</a>)
+</p>
+<div class="md-typeset__scrollwrap">
+<div class="md-typeset__table">
+<table>
+<thead>
+<tr>
+<th>Field</th>
+<th>Description</th>
+</tr>
+</thead>
+<tbody>
+<tr>
+<td>
+<code>bridgeIP</code><br>
+<em>
+string
+</em>
+</td>
+<td>
+</td>
+</tr>
+<tr>
+<td>
+<code>bridgeMAC</code><br>
+<em>
+string
+</em>
+</td>
+<td>
+</td>
+</tr>
+<tr>
+<td>
+<code>range</code><br>
+<em>
+<a href="#airship.airshipit.org/v1.Range">
+Range
+</a>
+</em>
+</td>
+<td>
+</td>
+</tr>
+<tr>
+<td>
+<code>Network</code><br>
+<em>
+<a href="#airship.airshipit.org/v1.Network">
+Network
+</a>
+</em>
+</td>
+<td>
+<p>
+(Members of <code>Network</code> are embedded into this type.)
+</p>
+</td>
+</tr>
+</tbody>
+</table>
+</div>
+</div>
 <h3 id="airship.airshipit.org/v1.BuilderNetworkInterface">BuilderNetworkInterface
 </h3>
 <p>
@ -784,7 +871,7 @@ string
 </h3>
 <p>
 (<em>Appears on:</em>
-<a href="#airship.airshipit.org/v1.Builder">Builder</a>, 
+<a href="#airship.airshipit.org/v1.BuilderNetwork">BuilderNetwork</a>, 
 <a href="#airship.airshipit.org/v1.VinoSpec">VinoSpec</a>)
 </p>
 <p>Network defines libvirt networks</p>
@ -919,6 +1006,18 @@ string
 <p>LibvirtTemplate identifies which libvirt template to be used to create a network</p>
 </td>
 </tr>
+<tr>
+<td>
+<code>bridgeName</code><br>
+<em>
+string
+</em>
+</td>
+<td>
+<p>BridgeName is the name of the bridge to be created as libvirt network.
+works if AllocateNodeIP is sepcified</p>
+</td>
+</tr>
 </tbody>
 </table>
 </div>
@ -1150,6 +1249,17 @@ string
 <p>BootInterfaceName interface name to use to boot virtual machines</p>
 </td>
 </tr>
+<tr>
+<td>
+<code>enableVNC</code><br>
+<em>
+bool
+</em>
+</td>
+<td>
+<p>EnableVNC create VNC for graphical interaction with the VM that will be created.</p>
+</td>
+</tr>
 </tbody>
 </table>
 </div>
@ -1158,6 +1268,7 @@ string
 </h3>
 <p>
 (<em>Appears on:</em>
+<a href="#airship.airshipit.org/v1.BuilderNetwork">BuilderNetwork</a>, 
 <a href="#airship.airshipit.org/v1.IPPoolSpec">IPPoolSpec</a>)
 </p>
 <p>Range has (inclusive) bounds within a subnet from which IPs can be allocated</p>
@ -1353,17 +1464,6 @@ DaemonSetOptions
 </tr>
 <tr>
 <td>
-<code>vmBridge</code><br>
-<em>
-string
-</em>
-</td>
-<td>
-<p>VMBridge defines the single interface name to be used as a bridge for VMs</p>
-</td>
-</tr>
-<tr>
-<td>
 <code>bmcCredentials</code><br>
 <em>
 <a href="#airship.airshipit.org/v1.BMCCredentials">
@ -1513,17 +1613,6 @@ DaemonSetOptions
 </tr>
 <tr>
 <td>
-<code>vmBridge</code><br>
-<em>
-string
-</em>
-</td>
-<td>
-<p>VMBridge defines the single interface name to be used as a bridge for VMs</p>
-</td>
-</tr>
-<tr>
-<td>
 <code>bmcCredentials</code><br>
 <em>
 <a href="#airship.airshipit.org/v1.BMCCredentials">
--- a/pkg/api/v1/vino_builder.go
+++ b/pkg/api/v1/vino_builder.go
@ -22,7 +22,7 @@ type Builder struct {
 	PXEBootImageHost     string `json:"pxeBootImageHost,omitempty"`
 	PXEBootImageHostPort int    `json:"pxeBootImageHostPort,omitempty"`

-	Networks []Network `json:"networks,omitempty"`
+	Networks []BuilderNetwork `json:"networks,omitempty"`
 	// (TODO) change json tag to cpuConfiguration when vino-builder has these chanages as well
 	CPUConfiguration CPUConfiguration `json:"configuration,omitempty"`
 	Domains          []BuilderDomain  `json:"domains,omitempty"`
@ -36,6 +36,13 @@ type BuilderNetworkInterface struct {
 	NetworkInterface `json:",inline"`
 }

+type BuilderNetwork struct {
+	BridgeIP  string `json:"bridgeIP,omitempty"`
+	BridgeMAC string `json:"bridgeMAC,omitempty"`
+	Range     Range  `json:"range,omitempty"`
+	Network   `json:",inline"`
+}
+
 // BuilderDomain represents a VINO libvirt domain
 type BuilderDomain struct {
 	Name           string `json:"name,omitempty"`
--- a/pkg/api/v1/vino_types.go
+++ b/pkg/api/v1/vino_types.go
@ -63,8 +63,6 @@ type VinoSpec struct {
 	Nodes []NodeSet `json:"nodes,omitempty"`
 	// DaemonSetOptions defines how vino will spawn daemonset on nodes
 	DaemonSetOptions DaemonSetOptions `json:"daemonSetOptions,omitempty"`
-	// VMBridge defines the single interface name to be used as a bridge for VMs
-	VMBridge string `json:"vmBridge"`
 	// BMCCredentials contain credentials that will be used to create BMH nodes
 	// sushy tools will use these credentials as well, to set up authentication
 	BMCCredentials BMCCredentials `json:"bmcCredentials"`
@ -118,6 +116,9 @@ type Network struct {
 	PhysicalInterface string `json:"physicalInterface,omitempty"`
 	// LibvirtTemplate identifies which libvirt template to be used to create a network
 	LibvirtTemplate string `json:"libvirtTemplate,omitempty"`
+	// BridgeName is the name of the bridge to be created as libvirt network.
+	// works if AllocateNodeIP is sepcified
+	BridgeName string `json:"bridgeName,omitempty"`
 }

 // VMRoutes defined
--- a/pkg/api/v1/zz_generated.deepcopy.go
+++ b/pkg/api/v1/zz_generated.deepcopy.go
@ -60,7 +60,7 @@ func (in *Builder) DeepCopyInto(out *Builder) {
 	*out = *in
 	if in.Networks != nil {
 		in, out := &in.Networks, &out.Networks
-		*out = make([]Network, len(*in))
+		*out = make([]BuilderNetwork, len(*in))
 		for i := range *in {
 			(*in)[i].DeepCopyInto(&(*out)[i])
 		}
@ -107,6 +107,23 @@ func (in *BuilderDomain) DeepCopy() *BuilderDomain {
 	return out
 }

+// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
+func (in *BuilderNetwork) DeepCopyInto(out *BuilderNetwork) {
+	*out = *in
+	out.Range = in.Range
+	in.Network.DeepCopyInto(&out.Network)
+}
+
+// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new BuilderNetwork.
+func (in *BuilderNetwork) DeepCopy() *BuilderNetwork {
+	if in == nil {
+		return nil
+	}
+	out := new(BuilderNetwork)
+	in.DeepCopyInto(out)
+	return out
+}
+
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *BuilderNetworkInterface) DeepCopyInto(out *BuilderNetworkInterface) {
 	*out = *in
--- a/pkg/controllers/vino_controller.go
+++ b/pkg/controllers/vino_controller.go
@ -229,11 +229,6 @@ func (r *VinoReconciler) decorateDaemonSet(ctx context.Context, ds *appsv1.Daemo
 	ds.Namespace = getRuntimeNamespace()
 	ds.Name = r.getDaemonSetName(vino)

-	// TODO develop logic to derive all required ENV variables from VINO CR, and pass them
-	// to setENV function instead
-	if vino.Spec.VMBridge != "" {
-		setEnv(ctx, ds, vinov1.EnvVarVMInterfaceName, vino.Spec.VMBridge)
-	}
 	setEnv(ctx, ds, vinov1.EnvVarBasicAuthUsername, vino.Spec.BMCCredentials.Username)
 	setEnv(ctx, ds, vinov1.EnvVarBasicAuthPassword, vino.Spec.BMCCredentials.Password)

--- a/pkg/managers/bmh.go
+++ b/pkg/managers/bmh.go
@ -20,7 +20,6 @@ import (
 	"fmt"
 	"net"
 	"text/template"
-	"time"

 	"github.com/Masterminds/sprig"
 	"github.com/go-logr/logr"
@ -46,7 +45,7 @@ type networkTemplateValues struct {
 	BMHName string

 	Node     vinov1.NodeSet // the specific node type to be templated
-	Networks []vinov1.Network
+	Networks []vinov1.BuilderNetwork
 	vinov1.BuilderDomain
 }

@ -265,7 +264,7 @@ func (r *BMHManager) setBMHs(ctx context.Context, pod corev1.Pod, nodeCount int)
 	vinoBuilder := vinov1.Builder{
 		PXEBootImageHost:     r.ViNO.Spec.PXEBootImageHost,
 		PXEBootImageHostPort: r.ViNO.Spec.PXEBootImageHostPort,
-		Networks:             r.ViNO.Spec.Networks,
+		Networks:             nodeNetworks,
 		CPUConfiguration:     r.ViNO.Spec.CPUConfiguration,
 		Domains:              domains,
 		NodeCount:            nodeCount,
@ -276,27 +275,36 @@ func (r *BMHManager) setBMHs(ctx context.Context, pod corev1.Pod, nodeCount int)
 // nodeNetworks returns a copy of node network with a unique per node values
 func (r *BMHManager) nodeNetworks(ctx context.Context,
 	globalNetworks []vinov1.Network,
-	k8sNode *corev1.Node) ([]vinov1.Network, error) {
-	for netIndex, network := range globalNetworks {
-		for routeIndex, route := range network.Routes {
+	k8sNode *corev1.Node) ([]vinov1.BuilderNetwork, error) {
+	builderNetworks := []vinov1.BuilderNetwork{}
+	for _, network := range globalNetworks {
+		builderNetwork := vinov1.BuilderNetwork{
+			Network: network,
+		}
+
+		r.Logger.Info("Getting GW bridge IP from node", "node", k8sNode.Name)
+		bridgeIP, mac, err := r.getBridgeIPandMAC(ctx, network, k8sNode)
+		if err != nil {
+			return []vinov1.BuilderNetwork{}, err
+		}
+		builderNetwork.BridgeIP = bridgeIP
+		builderNetwork.BridgeMAC = mac
+
+		for routeIndex, route := range builderNetwork.Network.Routes {
 			if route.Gateway == "$vinobridge" {
-				r.Logger.Info("Getting GW bridge IP from node", "node", k8sNode.Name)
-				bridgeIP, err := r.getBridgeIP(ctx, k8sNode)
-				if err != nil {
-					return []vinov1.Network{}, err
-				}
-				globalNetworks[netIndex].Routes[routeIndex].Gateway = bridgeIP
+				builderNetwork.Network.Routes[routeIndex].Gateway = bridgeIP
 			}
 		}
+		builderNetworks = append(builderNetworks, builderNetwork)
 	}
-	return globalNetworks, nil
+	return builderNetworks, nil
 }

 func (r *BMHManager) domainSpecificNetValues(
 	ctx context.Context,
 	bmhName string,
 	node vinov1.NodeSet,
-	networks []vinov1.Network) (networkTemplateValues, error) {
+	networks []vinov1.BuilderNetwork) (networkTemplateValues, error) {
 	// Allocate an IP for each of this BMH's network interfaces
 	bootMAC := ""
 	domainInterfaces := []vinov1.BuilderNetworkInterface{}
@ -370,31 +378,14 @@ func (r *BMHManager) annotateNode(ctx context.Context, k8sNode *corev1.Node, vin
 	return r.Update(ctx, k8sNode)
 }

-func (r *BMHManager) getBridgeIP(ctx context.Context, k8sNode *corev1.Node) (string, error) {
-	ctxTimeout, cancel := context.WithTimeout(ctx, 30*time.Second)
-	defer cancel()
-
-	for {
-		select {
-		case <-ctxTimeout.Done():
-			return "", ctx.Err()
-		default:
-			node := &corev1.Node{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: k8sNode.Name,
-				},
-			}
-			if err := r.Get(ctx, client.ObjectKeyFromObject(node), node); err != nil {
-				return "", err
-			}
-
-			ip, exist := k8sNode.Labels[vinov1.VinoDefaultGatewayBridgeLabel]
-			if exist {
-				return ip, nil
-			}
-			time.Sleep(10 * time.Second)
-		}
+func (r *BMHManager) getBridgeIPandMAC(ctx context.Context,
+	network vinov1.Network,
+	k8sNode *corev1.Node) (string, string, error) {
+	subnetRange, err := ipam.NewRange(network.AllocationStart, network.AllocationStop)
+	if err != nil {
+		return "", "", err
 	}
+	return r.Ipam.AllocateIP(ctx, network.SubNet, subnetRange, k8sNode.Name)
 }

 func (r *BMHManager) getNode(ctx context.Context, pod corev1.Pod) (*corev1.Node, error) {
@ -403,8 +394,7 @@ func (r *BMHManager) getNode(ctx context.Context, pod corev1.Pod) (*corev1.Node,
 			Name: pod.Spec.NodeName,
 		},
 	}
-	err := r.Get(ctx, client.ObjectKeyFromObject(node), node)
-	return node, err
+	return node, r.Get(ctx, client.ObjectKeyFromObject(node), node)
 }

 func (r *BMHManager) getBMHNodePrefix(pod corev1.Pod) string {