Pass dynamic IPAM information to vino-buider

This patchset enables VINO controller to pass variables to vino-builder that are generated dynamically by IPAM module. Also it takes IP address of the vino bridge and injects it into node annotation togather with IPAM values. Vino-builder polls a k8s node object to get these values before proceeding. Change-Id: I5b4e23df0fa4fa980b2a6724468bc6f2d9546409
2021-04-08 00:02:34 +00:00 · 2021-04-08 00:02:34 +00:00 · 5551ec347a
commit 5551ec347a
parent e2ba26c821
12 changed files with 475 additions and 108 deletions
--- a/config/crd/bases/airship.airshipit.org_vinoes.yaml
+++ b/config/crd/bases/airship.airshipit.org_vinoes.yaml
@ -178,7 +178,7 @@ spec:
                        type: string
                    type: object
                  name:
-                    description: Parameter for Node master or worker
+                    description: Parameter for Node control-plane or worker
                    type: string
                  networkDataTemplate:
                    description: NetworkDataTemplate must have a template key
--- a/config/manager/flavor-templates.yaml
+++ b/config/manager/flavor-templates.yaml
@ -2,6 +2,8 @@ flavorTemplates:
  master:
    domainTemplate: |
      {% set nodename = 'master-' + item|string %}
+      {% if domains[nodename] is defined %}
+      {% set domain = domains[nodename] %}
      <domain type="kvm">
        <name>{{ nodename }}</name>
        <uuid>{{ nodename | hash('md5') }}</uuid>
@ -71,12 +73,14 @@ flavorTemplates:
          </controller>

          # for each interface defined in vino, e.g.
+          {% for if_name, if_values in domain.interfaces.items() %}
          <interface type='bridge'>
-            <mac address='52:54:00:83:e9:f9'/>
-            <source bridge='management'/>
+            <mac address='{{ if_values.macAddress }}'/>
+            <source bridge='{{ if_name }}'/>
            <model type='virtio'/>
-            <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
+            <address type='pci' domain='0x0000' bus='0x00' slot='0x0{{ loop.index0 }}' function='0x0'/>
          </interface>
+          {% endfor %}

          <serial type="pty">
            <source path="/dev/pts/3"/>
@ -103,6 +107,7 @@ flavorTemplates:
          <imagelabel>+42424:+104</imagelabel>
        </seclabel>
      </domain>
+      {% endif %}
    volumeTemplate: |
      {% set nodename = 'master-' + item|string %}
      <volume>
@ -116,6 +121,8 @@ flavorTemplates:
  worker:
    domainTemplate: |
      {% set nodename = 'worker-' + item|string %}
+      {% if domains[nodename] is defined %}
+      {% set domain = domains[nodename] %}
      <domain type="kvm">
        <name>{{ nodename }}</name>
        <uuid>{{ nodename | hash('md5') }}</uuid>
@ -184,13 +191,14 @@ flavorTemplates:
            <address type="pci" domain="0x0000" bus="0x00" slot="0x01" function="0x1"/>
          </controller>

-          # for each interface defined in vino, e.g.
+          {% for if_name, if_values in domain.interfaces.items() %}
          <interface type='bridge'>
-            <mac address='52:54:00:83:e9:f9'/>
-            <source bridge='management'/>
+            <mac address='{{ if_values.macAddress }}'/>
+            <source bridge='{{ if_name }}'/>
            <model type='virtio'/>
-            <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
+            <address type='pci' domain='0x0000' bus='0x00' slot='0x0{{ loop.index0 }}' function='0x0'/>
          </interface>
+          {% endfor %}

          <serial type="pty">
            <source path="/dev/pts/3"/>
@ -217,6 +225,7 @@ flavorTemplates:
          <imagelabel>+42424:+104</imagelabel>
        </seclabel>
      </domain>
+      {% endif %}
    volumeTemplate: |
      {% set nodename = 'worker-' + item|string %}
      <volume>
--- a/config/rbac/role.yaml
+++ b/config/rbac/role.yaml
@ -13,6 +13,8 @@ rules:
  verbs:
  - get
  - list
+  - patch
+  - update
  - watch
 - apiGroups:
  - ""
--- a/config/samples/vino_cr.yaml
+++ b/config/samples/vino_cr.yaml
@ -4,6 +4,7 @@ metadata:
  name: vino-test-cr
  labels: {}
 spec:
+  vmBridge: lo
  nodeLabelKeysToCopy:
    - "airshipit.org/server"
    - "airshipit.org/rack"
@ -24,7 +25,7 @@ spec:
      routes:
        - network: 10.0.0.0
          netmask: 255.255.255.0
-          gateway: 192.168.2.1 # vino will need to populate this from the nodelabel value `airshipit.org/vino.nodebridgegw`
+          gateway: $vinobridge # vino will need to populate this from the nodelabel value `airshipit.org/vino.nodebridgegw`
      dns_servers: ["135.188.34.124"]
    - name: external
      subnet: 169.0.0.0/24
@ -36,7 +37,6 @@ spec:
      allocationStart: 169.0.0.10
      allocationStop: 169.0.0.254
      macPrefix: "0A:00:00:00:00:00"
-  vmBridge: lo
  nodes:
    - name: master
      count: 1
--- a/docs/api/vino.md
+++ b/docs/api/vino.md
@ -102,6 +102,124 @@ string
 </table>
 </div>
 </div>
+<h3 id="airship.airshipit.org/v1.Builder">Builder
+</h3>
+<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>gwIPBridge</code><br>
+<em>
+string
+</em>
+</td>
+<td>
+</td>
+</tr>
+<tr>
+<td>
+<code>networks</code><br>
+<em>
+<a href="#airship.airshipit.org/v1.BuilderNetwork">
+map[string]./pkg/api/v1.BuilderNetwork
+</a>
+</em>
+</td>
+<td>
+</td>
+</tr>
+<tr>
+<td>
+<code>domains</code><br>
+<em>
+<a href="#airship.airshipit.org/v1.BuilderDomain">
+map[string]./pkg/api/v1.BuilderDomain
+</a>
+</em>
+</td>
+<td>
+</td>
+</tr>
+</tbody>
+</table>
+</div>
+</div>
+<h3 id="airship.airshipit.org/v1.BuilderDomain">BuilderDomain
+</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>interfaces</code><br>
+<em>
+<a href="#airship.airshipit.org/v1.BuilderNetworkInterface">
+map[string]./pkg/api/v1.BuilderNetworkInterface
+</a>
+</em>
+</td>
+<td>
+</td>
+</tr>
+</tbody>
+</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>
+<h3 id="airship.airshipit.org/v1.BuilderNetworkInterface">BuilderNetworkInterface
+</h3>
+<p>
+(<em>Appears on:</em>
+<a href="#airship.airshipit.org/v1.BuilderDomain">BuilderDomain</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>macAddress</code><br>
+<em>
+string
+</em>
+</td>
+<td>
+</td>
+</tr>
+</tbody>
+</table>
+</div>
+</div>
 <h3 id="airship.airshipit.org/v1.CPUConfiguration">CPUConfiguration
 </h3>
 <p>
@ -796,7 +914,7 @@ string
 </em>
 </td>
 <td>
-<p>Parameter for Node master or worker</p>
+<p>Parameter for Node control-plane or worker</p>
 </td>
 </tr>
 <tr>
--- a/pkg/api/v1/vino_builder.go
+++ b/pkg/api/v1/vino_builder.go
@ -0,0 +1,35 @@
+/*
+
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package v1
+
+type Builder struct {
+	GWIPBridge string                    `json:"gwIPBridge,omitempty"`
+	Networks   map[string]BuilderNetwork `json:"networks,omitempty"`
+	Domains    map[string]BuilderDomain  `json:"domains,omitempty"`
+}
+
+type BuilderNetworkInterface struct {
+	MACAddress string `json:"macAddress,omitempty"`
+}
+
+type BuilderNetwork struct {
+	// Placeholder for future development
+}
+
+type BuilderDomain struct {
+	Interfaces map[string]BuilderNetworkInterface `json:"interfaces,omitempty"`
+}
--- a/pkg/api/v1/vino_types.go
+++ b/pkg/api/v1/vino_types.go
@ -33,6 +33,10 @@ const (
 	VinoFinalizer = "vino.airshipit.org"
 	// EnvVarVMInterfaceName environment variable that is used to find VM interface to use for vms
 	EnvVarVMInterfaceName = "VM_BRIDGE_INTERFACE"
+	// VinoDefaultGatewayBridgeLabel is used to identify ip address of the default gateway for the VM
+	VinoDefaultGatewayBridgeLabel = "airshipit.org/vino.nodebridgegw"
+	// VinoNodeNetworkValuesAnnotation vino controller saves ip and mac address information for the node in it
+	VinoNodeNetworkValuesAnnotation = "airshipit.org/vino.network-values"
 )

 // VinoSpec defines the desired state of Vino
@ -104,7 +108,7 @@ type VMRoutes struct {

 //NodeSet node definitions
 type NodeSet struct {
-	//Parameter for Node master or worker
+	// Parameter for Node control-plane or worker
 	Name  string `json:"name,omitempty"`
 	Count int    `json:"count,omitempty"`
 	// BMHLabels labels will be copied directly to BMHs that will be created
--- a/pkg/api/v1/zz_generated.deepcopy.go
+++ b/pkg/api/v1/zz_generated.deepcopy.go
@ -55,6 +55,87 @@ func (in *BMCCredentials) DeepCopy() *BMCCredentials {
 	return out
 }

+// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
+func (in *Builder) DeepCopyInto(out *Builder) {
+	*out = *in
+	if in.Networks != nil {
+		in, out := &in.Networks, &out.Networks
+		*out = make(map[string]BuilderNetwork, len(*in))
+		for key, val := range *in {
+			(*out)[key] = val
+		}
+	}
+	if in.Domains != nil {
+		in, out := &in.Domains, &out.Domains
+		*out = make(map[string]BuilderDomain, len(*in))
+		for key, val := range *in {
+			(*out)[key] = *val.DeepCopy()
+		}
+	}
+}
+
+// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Builder.
+func (in *Builder) DeepCopy() *Builder {
+	if in == nil {
+		return nil
+	}
+	out := new(Builder)
+	in.DeepCopyInto(out)
+	return out
+}
+
+// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
+func (in *BuilderDomain) DeepCopyInto(out *BuilderDomain) {
+	*out = *in
+	if in.Interfaces != nil {
+		in, out := &in.Interfaces, &out.Interfaces
+		*out = make(map[string]BuilderNetworkInterface, len(*in))
+		for key, val := range *in {
+			(*out)[key] = val
+		}
+	}
+}
+
+// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new BuilderDomain.
+func (in *BuilderDomain) DeepCopy() *BuilderDomain {
+	if in == nil {
+		return nil
+	}
+	out := new(BuilderDomain)
+	in.DeepCopyInto(out)
+	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
+}
+
+// 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
+}
+
+// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new BuilderNetworkInterface.
+func (in *BuilderNetworkInterface) DeepCopy() *BuilderNetworkInterface {
+	if in == nil {
+		return nil
+	}
+	out := new(BuilderNetworkInterface)
+	in.DeepCopyInto(out)
+	return out
+}
+
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *CPUConfiguration) DeepCopyInto(out *CPUConfiguration) {
 	*out = *in
--- a/pkg/controllers/bmh.go
+++ b/pkg/controllers/bmh.go
@ -19,6 +19,7 @@ import (
 	"context"
 	"fmt"
 	"text/template"
+	"time"

 	"github.com/Masterminds/sprig"
 	"github.com/go-logr/logr"
@ -29,6 +30,7 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	kerror "k8s.io/apimachinery/pkg/util/errors"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/yaml"

 	vinov1 "vino/pkg/api/v1"
 	"vino/pkg/ipam"
@ -48,8 +50,8 @@ type networkTemplateValues struct {
 }

 type generatedValues struct {
-	IPAddresses  map[string]string // a map of network names to IP addresses
-	MACAddresses map[string]string // a map of network interface (link) names to MACs
+	IPAddresses  map[string]string
+	MACAddresses map[string]string
 }

 func (r *VinoReconciler) ensureBMHs(ctx context.Context, vino *vinov1.Vino) error {
@ -130,12 +132,13 @@ func (r *VinoReconciler) createIpamNetworks(ctx context.Context, vino *vinov1.Vi
 		if err != nil {
 			return err
 		}
-		if network.MACPrefix == "" {
-			logger.Info("No MACPrefix provided; using default MACPrefix %s for network %s",
-				DefaultMACPrefix, network.Name)
-			network.MACPrefix = DefaultMACPrefix
+		macPrefix := network.MACPrefix
+		if macPrefix == "" {
+			logger.Info("No MACPrefix provided; using default MACPrefix for network",
+				"default prefix", DefaultMACPrefix, "network name", network.Name)
+			macPrefix = DefaultMACPrefix
 		}
-		err = r.Ipam.AddSubnetRange(ctx, network.SubNet, subnetRange, network.MACPrefix)
+		err = r.Ipam.AddSubnetRange(ctx, network.SubNet, subnetRange, macPrefix)
 		if err != nil {
 			return err
 		}
@ -145,6 +148,19 @@ func (r *VinoReconciler) createIpamNetworks(ctx context.Context, vino *vinov1.Vi

 func (r *VinoReconciler) createBMHperPod(ctx context.Context, vino *vinov1.Vino, pod corev1.Pod) error {
 	logger := logr.FromContext(ctx)
+
+	nodeNetworkValues := map[string]generatedValues{}
+
+	k8sNode, err := r.getNode(ctx, pod)
+	if err != nil {
+		return err
+	}
+
+	ip, err := r.getBridgeIP(ctx, k8sNode)
+	if err != nil {
+		return err
+	}
+
 	for _, node := range vino.Spec.Nodes {
 		logger.Info("Creating BMHs for vino node", "node name", node.Name, "count", node.Count)
 		prefix := r.getBMHNodePrefix(vino, pod)
@ -152,58 +168,25 @@ func (r *VinoReconciler) createBMHperPod(ctx context.Context, vino *vinov1.Vino,
 			roleSuffix := fmt.Sprintf("%s-%d", node.Name, i)
 			bmhName := fmt.Sprintf("%s-%s", prefix, roleSuffix)

-			creds, err := r.reconcileBMHCredentials(ctx, vino)
-			if err != nil {
-				return err
+			creds, nodeErr := r.reconcileBMHCredentials(ctx, vino)
+			if nodeErr != nil {
+				return nodeErr
 			}

-			// Allocate an IP for each of this BMH's network interfaces
-			ipAddresses := map[string]string{}
-			macAddresses := map[string]string{}
-			for _, iface := range node.NetworkInterfaces {
-				networkName := iface.NetworkName
-				subnet := ""
-				subnetRange := vinov1.Range{}
-				for _, network := range vino.Spec.Networks {
-					if network.Name == networkName {
-						subnet = network.SubNet
-						subnetRange, err = ipam.NewRange(network.AllocationStart,
-							network.AllocationStop)
-						if err != nil {
-							return err
-						}
-						break
-					}
-				}
-				if subnet == "" {
-					return fmt.Errorf("Interface %s doesn't have a matching network defined", networkName)
-				}
-				ipAllocatedTo := fmt.Sprintf("%s/%s", bmhName, iface.NetworkName)
-				ipAddress, macAddress, er := r.Ipam.AllocateIP(ctx, subnet, subnetRange, ipAllocatedTo)
-				if er != nil {
-					return er
-				}
-				ipAddresses[networkName] = ipAddress
-				macAddresses[iface.Name] = macAddress
+			values, nodeErr := r.networkValues(ctx, bmhName, ip, node, vino)
+			if nodeErr != nil {
+				return nodeErr
+			}
+			nodeNetworkValues[roleSuffix] = values.Generated
+
+			netData, netDataNs, nodeErr := r.reconcileBMHNetworkData(ctx, node, vino, values)
+			if nodeErr != nil {
+				return nodeErr
 			}

-			values := networkTemplateValues{
-				Node:     node,
-				BMHName:  bmhName,
-				Networks: vino.Spec.Networks,
-				Generated: generatedValues{
-					IPAddresses:  ipAddresses,
-					MACAddresses: macAddresses,
-				},
-			}
-			netData, netDataNs, err := r.reconcileBMHNetworkData(ctx, node, vino, values)
-			if err != nil {
-				return err
-			}
-
-			bmcAddr, labels, err := r.getBMCAddressAndLabels(ctx, pod, vino.Spec.NodeLabelKeysToCopy, roleSuffix)
-			if err != nil {
-				return err
+			bmcAddr, labels, nodeErr := r.getBMCAddressAndLabels(ctx, k8sNode, vino.Spec.NodeLabelKeysToCopy, roleSuffix)
+			if nodeErr != nil {
+				return nodeErr
 			}

 			for label, value := range node.BMHLabels {
@ -232,15 +215,147 @@ func (r *VinoReconciler) createBMHperPod(ctx context.Context, vino *vinov1.Vino,
 			}
 			objKey := client.ObjectKeyFromObject(bmh)
 			logger.Info("Creating BMH", "name", objKey)
-			err = applyRuntimeObject(ctx, objKey, bmh, r.Client)
-			if err != nil {
-				return err
+			nodeErr = applyRuntimeObject(ctx, objKey, bmh, r.Client)
+			if nodeErr != nil {
+				return nodeErr
 			}
 		}
 	}
+	logger.Info("annotating node", "node", k8sNode.Name)
+	if err = r.annotateNode(ctx, ip, k8sNode, nodeNetworkValues); err != nil {
+		return err
+	}
 	return nil
 }

+func (r *VinoReconciler) networkValues(
+	ctx context.Context,
+	bmhName string,
+	bridgeIP string,
+	node vinov1.NodeSet,
+	vino *vinov1.Vino) (networkTemplateValues, error) {
+	// Allocate an IP for each of this BMH's network interfaces
+	ipAddresses := map[string]string{}
+	macAddresses := map[string]string{}
+	for _, iface := range node.NetworkInterfaces {
+		networkName := iface.NetworkName
+		subnet := ""
+		var err error
+		subnetRange := vinov1.Range{}
+		for netIndex, network := range vino.Spec.Networks {
+			for routeIndex, route := range network.Routes {
+				if route.Gateway == "$vinobridge" {
+					vino.Spec.Networks[netIndex].Routes[routeIndex].Gateway = bridgeIP
+				}
+			}
+			if network.Name == networkName {
+				subnet = network.SubNet
+				subnetRange, err = ipam.NewRange(network.AllocationStart,
+					network.AllocationStop)
+				if err != nil {
+					return networkTemplateValues{}, err
+				}
+				break
+			}
+		}
+		if subnet == "" {
+			return networkTemplateValues{}, fmt.Errorf("Interface %s doesn't have a matching network defined", networkName)
+		}
+		ipAllocatedTo := fmt.Sprintf("%s/%s", bmhName, iface.NetworkName)
+		ipAddress, macAddress, err := r.Ipam.AllocateIP(ctx, subnet, subnetRange, ipAllocatedTo)
+		if err != nil {
+			return networkTemplateValues{}, err
+		}
+		ipAddresses[networkName] = ipAddress
+		macAddresses[iface.Name] = macAddress
+		logr.FromContext(ctx).Info("Got MAC and IP for the network and node",
+			"MAC", macAddress, "IP", ipAddress, "bmh name", bmhName)
+	}
+	return networkTemplateValues{
+		Node:     node,
+		BMHName:  bmhName,
+		Networks: vino.Spec.Networks,
+		Generated: generatedValues{
+			IPAddresses:  ipAddresses,
+			MACAddresses: macAddresses,
+		},
+	}, nil
+}
+
+func (r *VinoReconciler) annotateNode(ctx context.Context,
+	gwIP string,
+	k8sNode *corev1.Node,
+	values map[string]generatedValues) error {
+	logr.FromContext(ctx).Info("Getting GW bridge IP from node", "node", k8sNode.Name)
+	builderValues := vinov1.Builder{
+		Domains:    make(map[string]vinov1.BuilderDomain),
+		GWIPBridge: gwIP,
+	}
+	for domainName, domain := range values {
+		builderDomain := vinov1.BuilderDomain{
+			Interfaces: make(map[string]vinov1.BuilderNetworkInterface),
+		}
+		for ifName, ifMAC := range domain.MACAddresses {
+			builderDomain.Interfaces[ifName] = vinov1.BuilderNetworkInterface{
+				MACAddress: ifMAC,
+			}
+		}
+		builderValues.Domains[domainName] = builderDomain
+	}
+
+	b, err := yaml.Marshal(builderValues)
+	if err != nil {
+		return err
+	}
+
+	annotations := k8sNode.GetAnnotations()
+	if k8sNode.GetAnnotations() == nil {
+		annotations = make(map[string]string)
+	}
+
+	annotations[vinov1.VinoNodeNetworkValuesAnnotation] = string(b)
+	k8sNode.SetAnnotations(annotations)
+
+	return r.Update(ctx, k8sNode)
+}
+
+func (r *VinoReconciler) 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 *VinoReconciler) getNode(ctx context.Context, pod corev1.Pod) (*corev1.Node, error) {
+	node := &corev1.Node{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: pod.Spec.NodeName,
+		},
+	}
+	err := r.Get(ctx, client.ObjectKeyFromObject(node), node)
+	return node, err
+}
+
 func (r *VinoReconciler) getBMHNodePrefix(vino *vinov1.Vino, pod corev1.Pod) string {
 	// TODO we need to do something about name length limitations
 	return fmt.Sprintf("%s-%s-%s", vino.Namespace, vino.Name, pod.Spec.NodeName)
@ -248,20 +363,10 @@ func (r *VinoReconciler) getBMHNodePrefix(vino *vinov1.Vino, pod corev1.Pod) str

 func (r *VinoReconciler) getBMCAddressAndLabels(
 	ctx context.Context,
-	pod corev1.Pod,
+	node *corev1.Node,
 	labelKeys []string,
 	vmName string) (string, map[string]string, error) {
-	logger := logr.FromContext(ctx).WithValues("k8s node", pod.Spec.NodeName)
-
-	node := &corev1.Node{
-		ObjectMeta: metav1.ObjectMeta{
-			Name: pod.Spec.NodeName,
-		},
-	}
-	err := r.Get(ctx, client.ObjectKeyFromObject(node), node)
-	if err != nil {
-		return "", nil, err
-	}
+	logger := logr.FromContext(ctx).WithValues("k8s node", node.Name)

 	labels := map[string]string{}

@ -336,6 +441,8 @@ func (r *VinoReconciler) reconcileBMHNetworkData(
 		return "", "", err
 	}

+	logger.Info("Genereated MAC Addresses values are", "GENERATED VALUES", values.Generated.MACAddresses)
+
 	buf := bytes.NewBuffer([]byte{})
 	err = tpl.Execute(buf, values)
 	if err != nil {
@ -343,7 +450,6 @@ func (r *VinoReconciler) reconcileBMHNetworkData(
 	}

 	name := fmt.Sprintf("%s-network-data", values.BMHName)
-
 	ns := getRuntimeNamespace()
 	netSecret := &corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
--- a/pkg/controllers/bmh_test.go
+++ b/pkg/controllers/bmh_test.go
@ -110,8 +110,9 @@ var _ = Describe("Test BMH reconciliation", func() {
 			}
 			node1 := &corev1.Node{
 				ObjectMeta: metav1.ObjectMeta{
-					Name:   "node01",
-					Labels: node1Labels,
+					Name:        "node01",
+					Labels:      node1Labels,
+					Annotations: make(map[string]string),
 				},
 				Status: corev1.NodeStatus{
 					Addresses: []corev1.NodeAddress{
@ -124,7 +125,8 @@ var _ = Describe("Test BMH reconciliation", func() {
 			}
 			node2 := &corev1.Node{
 				ObjectMeta: metav1.ObjectMeta{
-					Name: "node02",
+					Name:        "node02",
+					Annotations: make(map[string]string),
 				},
 				Status: corev1.NodeStatus{
 					Addresses: []corev1.NodeAddress{
--- a/pkg/controllers/vino_controller.go
+++ b/pkg/controllers/vino_controller.go
@ -61,7 +61,7 @@ type VinoReconciler struct {
 // +kubebuilder:rbac:groups=airship.airshipit.org,resources=vinoes/status,verbs=get;update;patch
 // +kubebuilder:rbac:groups=airship.airshipit.org,resources=ippools,verbs=get;list;watch;create;update;patch;delete
 // +kubebuilder:rbac:groups="",resources=pods,verbs=list;watch
-// +kubebuilder:rbac:groups="",resources=nodes,verbs=get;list;watch
+// +kubebuilder:rbac:groups="",resources=nodes,verbs=get;list;watch;update;patch
 // +kubebuilder:rbac:groups="",resources=secrets,verbs=get;list;watch;create;update;patch

 func (r *VinoReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
@ -109,11 +109,6 @@ func (r *VinoReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.
 		return ctrl.Result{Requeue: true}, err
 	}

-	err = r.reconcileBMHs(ctx, vino)
-	if err != nil {
-		return ctrl.Result{Requeue: true}, err
-	}
-
 	vinov1.VinoReady(vino)
 	if err := r.patchStatus(ctx, vino); err != nil {
 		err = fmt.Errorf("unable to patch status after reconciliation: %w", err)
@ -320,10 +315,24 @@ func (r *VinoReconciler) ensureDaemonSet(ctx context.Context, vino *vinov1.Vino)
 	// controller should watch for changes in daemonset to reconcile if it breaks, and change status
 	// of the vino object
 	// controlleruti.SetControllerReference(vino, ds, r.scheme)
-	ctx, cancel := context.WithTimeout(ctx, time.Second*180)
+	scheduledTimeoutCtx, cancel := context.WithTimeout(ctx, time.Second*180)
 	defer cancel()

-	return r.waitDaemonSet(ctx, ds)
+	logger := logr.FromContext(ctx)
+	logger.Info("Waiting for daemonset to become scheduled")
+	if err = r.waitDaemonSet(scheduledTimeoutCtx, dsScheduled, ds); err != nil {
+		return err
+	}
+
+	if err = r.reconcileBMHs(ctx, vino); err != nil {
+		return err
+	}
+
+	waitTimeoutCtx, cancel := context.WithTimeout(ctx, time.Second*180)
+	defer cancel()
+
+	logger.Info("Waiting for daemonset to become ready")
+	return r.waitDaemonSet(waitTimeoutCtx, dsReady, ds)
 }

 func (r *VinoReconciler) decorateDaemonSet(ctx context.Context, ds *appsv1.DaemonSet, vino *vinov1.Vino) {
@ -414,7 +423,7 @@ func setEnv(ctx context.Context, ds *appsv1.DaemonSet, vino *vinov1.Vino) {
 	}
 }

-func (r *VinoReconciler) waitDaemonSet(ctx context.Context, ds *appsv1.DaemonSet) error {
+func (r *VinoReconciler) waitDaemonSet(ctx context.Context, check dsWaitCondition, ds *appsv1.DaemonSet) error {
 	logger := logr.FromContext(ctx).WithValues(
 		"daemonset", ds.Namespace+"/"+ds.Name)
 	for {
@ -429,12 +438,11 @@ func (r *VinoReconciler) waitDaemonSet(ctx context.Context, ds *appsv1.DaemonSet
 				Namespace: ds.Namespace,
 			}, getDS)
 			if err != nil {
-				logger.Info("received error while waiting for ds to become ready, sleeping",
+				logger.Info("received error while waiting for ds to reach desired condition, sleeping",
 					"error", err.Error())
 			} else {
 				logger.Info("checking daemonset status", "status", getDS.Status)
-				if getDS.Status.DesiredNumberScheduled == getDS.Status.NumberReady &&
-					getDS.Status.DesiredNumberScheduled != 0 {
+				if check(getDS) {
 					logger.Info("DaemonSet is in ready status")
 					return nil
 				}
@ -527,3 +535,13 @@ func applyRuntimeObject(ctx context.Context, key client.ObjectKey, obj client.Ob
 func getRuntimeNamespace() string {
 	return os.Getenv("RUNTIME_NAMESPACE")
 }
+
+func dsScheduled(ds *appsv1.DaemonSet) bool {
+	return ds.Status.DesiredNumberScheduled != 0 && ds.Status.DesiredNumberScheduled == ds.Status.CurrentNumberScheduled
+}
+
+func dsReady(ds *appsv1.DaemonSet) bool {
+	return ds.Status.DesiredNumberScheduled != 0 && ds.Status.DesiredNumberScheduled == ds.Status.NumberReady
+}
+
+type dsWaitCondition func(ds *appsv1.DaemonSet) bool
--- a/roles/vino-describe-kubernetes-objects/tasks/main.yaml
+++ b/roles/vino-describe-kubernetes-objects/tasks/main.yaml
@ -28,25 +28,17 @@
    collect_namespaced_objects:
      - baremetalhosts
      - configmaps
-      - cronjobs
      - daemonsets
      - deployments
-      - endpoints
-      - ingresses
-      - jobs
-      - networkpolicies
      - pods
-      - podsecuritypolicies
-      - persistentvolumeclaims
      - replicasets
      - rolebindings
      - roles
      - secrets
      - serviceaccounts
-      - services
-      - statefulsets
      - vino
-
+      - nodes
+      - ippool

 - name: "Get context list"
  include_tasks: get-contexts.yaml