Allow to specify node labels for copying to BMH

With this change, vino-controller will take keys from
vino.Spec.CopyNodeLabelKeys and copy corresponding labels
from k8s node to a BMHs created.

Change-Id: Idd83766a064d406c83ee504dd89b1474aa15c49a

config/crd/bases/airship.airshipit.org_vinoes.yaml

@@ -111,6 +111,12 @@ spec:
                     type: string
                 type: object
               type: array
+            nodeLabelKeysToCopy:
+              description: NodeLabelKeysToCopy vino controller will get these labels
+                from k8s nodes and place them on BMHs that correspond to this node
+              items:
+                type: string
+              type: array
             nodeSelector:
               description: Define nodelabel parameters
               properties:
@@ -127,6 +133,13 @@ spec:
               items:
                 description: NodeSet node definitions
                 properties:
+                  bmhLabels:
+                    additionalProperties:
+                      type: string
+                    description: BMHLabels labels will be copied directly to BMHs
+                      that will be created These labels will override keys from k8s
+                      node, that are specified in vino.NodeLabelKeysToCopy
+                    type: object
                   count:
                     type: integer
                   diskDrives:
@@ -147,14 +160,6 @@ spec:
                       type:
                         type: string
                     type: object
-                  labels:
-                    description: VMNodeFlavor labels for node to be annotated
-                    properties:
-                      vmFlavor:
-                        additionalProperties:
-                          type: string
-                        type: object
-                    type: object
                   libvirtTemplate:
                     description: NamespacedName to be used to spawn VMs
                     properties:

config/samples/vino_cr.yaml

@@ -4,6 +4,9 @@ metadata:
   name: vino-test-cr
   #  labels: ...
 spec:
+  nodeLabelKeysToCopy:
+    - "airshipit.org/server"
+    - "airshipit.org/rack"
   nodeSelector:
     matchLabels:
       beta.kubernetes.io/os: linux
@@ -36,6 +39,8 @@ spec:
   nodes:
     - name: "worker"
       count: 3
+      bmhLabels:
+        airshipit.org/k8s-role: worker
       networkDataTemplate:
         name: "test-template"
         namespace: "default"

docs/api/vino.md

@@ -739,14 +739,14 @@ int
 </tr>
 <tr>
 <td>
-<code>labels</code><br>
+<code>bmhLabels</code><br>
 <em>
-<a href="#airship.airshipit.org/v1.VMNodeFlavor">
-VMNodeFlavor
-</a>
+map[string]string
 </em>
 </td>
 <td>
+<p>BMHLabels labels will be copied directly to BMHs that will be created
+These labels will override keys from k8s node, that are specified in vino.NodeLabelKeysToCopy</p>
 </td>
 </tr>
 <tr>
@@ -843,37 +843,6 @@ string
 </table>
 </div>
 </div>
-<h3 id="airship.airshipit.org/v1.VMNodeFlavor">VMNodeFlavor
-</h3>
-<p>
-(<em>Appears on:</em>
-<a href="#airship.airshipit.org/v1.NodeSet">NodeSet</a>)
-</p>
-<p>VMNodeFlavor labels for node to be annotated</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>vmFlavor</code><br>
-<em>
-map[string]string
-</em>
-</td>
-<td>
-</td>
-</tr>
-</tbody>
-</table>
-</div>
-</div>
 <h3 id="airship.airshipit.org/v1.VMRoutes">VMRoutes
 </h3>
 <p>
@@ -1055,6 +1024,18 @@ BMCCredentials
 sushy tools will use these credentials as well, to set up authentication</p>
 </td>
 </tr>
+<tr>
+<td>
+<code>nodeLabelKeysToCopy</code><br>
+<em>
+[]string
+</em>
+</td>
+<td>
+<p>NodeLabelKeysToCopy vino controller will get these labels from k8s nodes
+and place them on BMHs that correspond to this node</p>
+</td>
+</tr>
 </table>
 </td>
 </tr>
@@ -1181,6 +1162,18 @@ BMCCredentials
 sushy tools will use these credentials as well, to set up authentication</p>
 </td>
 </tr>
+<tr>
+<td>
+<code>nodeLabelKeysToCopy</code><br>
+<em>
+[]string
+</em>
+</td>
+<td>
+<p>NodeLabelKeysToCopy vino controller will get these labels from k8s nodes
+and place them on BMHs that correspond to this node</p>
+</td>
+</tr>
 </tbody>
 </table>
 </div>

pkg/api/v1/vino_types.go

@@ -52,6 +52,9 @@ type VinoSpec struct {
 	// 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"`
+	// NodeLabelKeysToCopy vino controller will get these labels from k8s nodes
+	// and place them on BMHs that correspond to this node
+	NodeLabelKeysToCopy []string `json:"nodeLabelKeysToCopy,omitempty"`
 }
 
 // BMCCredentials contain credentials that will be used to create BMH nodes
@@ -95,9 +98,11 @@ type VMRoutes struct {
 //NodeSet node definitions
 type NodeSet struct {
 	//Parameter for Node master or worker-standard
-	Name                      string              `json:"name,omitempty"`
-	Count                     int                 `json:"count,omitempty"`
-	NodeLabel                 VMNodeFlavor        `json:"labels,omitempty"`
+	Name  string `json:"name,omitempty"`
+	Count int    `json:"count,omitempty"`
+	// BMHLabels labels will be copied directly to BMHs that will be created
+	// These labels will override keys from k8s node, that are specified in vino.NodeLabelKeysToCopy
+	BMHLabels                 map[string]string   `json:"bmhLabels,omitempty"`
 	LibvirtTemplateDefinition NamespacedName      `json:"libvirtTemplate,omitempty"`
 	NetworkInterfaces         []NetworkInterface  `json:"networkInterfaces,omitempty"`
 	DiskDrives                *DiskDrivesTemplate `json:"diskDrives,omitempty"`
@@ -105,11 +110,6 @@ type NodeSet struct {
 	NetworkDataTemplate NamespacedName `json:"networkDataTemplate,omitempty"`
 }
 
-// VMNodeFlavor labels for node to be annotated
-type VMNodeFlavor struct {
-	VMFlavor map[string]string `json:"vmFlavor,omitempty"`
-}
-
 // NamespacedName to be used to spawn VMs
 type NamespacedName struct {
 	Name      string `json:"name,omitempty"`

pkg/api/v1/zz_generated.deepcopy.go

@@ -307,7 +307,13 @@ func (in *NodeSelector) DeepCopy() *NodeSelector {
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *NodeSet) DeepCopyInto(out *NodeSet) {
 	*out = *in
-	in.NodeLabel.DeepCopyInto(&out.NodeLabel)
+	if in.BMHLabels != nil {
+		in, out := &in.BMHLabels, &out.BMHLabels
+		*out = make(map[string]string, len(*in))
+		for key, val := range *in {
+			(*out)[key] = val
+		}
+	}
 	out.LibvirtTemplateDefinition = in.LibvirtTemplateDefinition
 	if in.NetworkInterfaces != nil {
 		in, out := &in.NetworkInterfaces, &out.NetworkInterfaces
@@ -349,28 +355,6 @@ func (in *Range) DeepCopy() *Range {
 	return out
 }
 
-// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
-func (in *VMNodeFlavor) DeepCopyInto(out *VMNodeFlavor) {
-	*out = *in
-	if in.VMFlavor != nil {
-		in, out := &in.VMFlavor, &out.VMFlavor
-		*out = make(map[string]string, len(*in))
-		for key, val := range *in {
-			(*out)[key] = val
-		}
-	}
-}
-
-// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new VMNodeFlavor.
-func (in *VMNodeFlavor) DeepCopy() *VMNodeFlavor {
-	if in == nil {
-		return nil
-	}
-	out := new(VMNodeFlavor)
-	in.DeepCopyInto(out)
-	return out
-}
-
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *VMRoutes) DeepCopyInto(out *VMRoutes) {
 	*out = *in
@@ -474,6 +458,11 @@ func (in *VinoSpec) DeepCopyInto(out *VinoSpec) {
 	}
 	out.DaemonSetOptions = in.DaemonSetOptions
 	out.BMCCredentials = in.BMCCredentials
+	if in.NodeLabelKeysToCopy != nil {
+		in, out := &in.NodeLabelKeysToCopy, &out.NodeLabelKeysToCopy
+		*out = make([]string, len(*in))
+		copy(*out, *in)
+	}
 }
 
 // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new VinoSpec.

pkg/controllers/bmh.go

@@ -185,19 +185,22 @@ func (r *VinoReconciler) createBMHperPod(ctx context.Context, vino *vinov1.Vino,
 				return err
 			}
 
-			// TODO extend this function to return server/rack labels as well
-			bmcAddr, err := r.getBMCAddress(ctx, pod, roleSuffix)
+			bmcAddr, labels, err := r.getBMCAddressAndLabels(ctx, pod, vino.Spec.NodeLabelKeysToCopy, roleSuffix)
 			if err != nil {
 				return err
 			}
 
+			for label, value := range node.BMHLabels {
+				labels[label] = value
+			}
+
 			bmh := &metal3.BareMetalHost{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      bmhName,
 					Namespace: getRuntimeNamespace(),
 					// TODO add rack and server labels, when we crearly define mechanism
 					// which labels we are copying
-					Labels: node.NodeLabel.VMFlavor,
+					Labels: labels,
 				},
 				Spec: metal3.BareMetalHostSpec{
 					NetworkData: &corev1.SecretReference{
@@ -227,10 +230,13 @@ func (r *VinoReconciler) getBMHNodePrefix(vino *vinov1.Vino, pod corev1.Pod) str
 	return fmt.Sprintf("%s-%s-%s", vino.Namespace, vino.Name, pod.Spec.NodeName)
 }
 
-func (r *VinoReconciler) getBMCAddress(
+func (r *VinoReconciler) getBMCAddressAndLabels(
 	ctx context.Context,
 	pod corev1.Pod,
-	vmName string) (string, error) {
+	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,
@@ -238,15 +244,25 @@ func (r *VinoReconciler) getBMCAddress(
 	}
 	err := r.Get(ctx, client.ObjectKeyFromObject(node), node)
 	if err != nil {
-		return "", err
+		return "", nil, err
+	}
+
+	labels := map[string]string{}
+
+	for _, key := range labelKeys {
+		value, ok := node.Labels[key]
+		if !ok {
+			logger.Info("Kubernetes node missing label from vino CR CopyNodeLabelKeys field", "label", key)
+		}
+		labels[key] = value
 	}
 
 	for _, addr := range node.Status.Addresses {
 		if addr.Type == corev1.NodeInternalIP {
-			return fmt.Sprintf("redfish+http://%s:%d/redfish/v1/Systems/%s", addr.Address, 8000, vmName), nil
+			return fmt.Sprintf("redfish+http://%s:%d/redfish/v1/Systems/%s", addr.Address, 8000, vmName), labels, nil
 		}
 	}
-	return "", fmt.Errorf("Node %s doesn't have internal ip address defined", node.Name)
+	return "", labels, fmt.Errorf("Node %s doesn't have internal ip address defined", node.Name)
 }
 
 // reconcileBMHCredentials returns secret name with credentials and error

pkg/controllers/bmh_test.go

@@ -39,10 +39,18 @@ var _ = Describe("Test BMH reconciliation", func() {
 		It("creates 6 BMH hosts", func() {
 			os.Setenv("RUNTIME_NAMESPACE", "vino-system")
 			defer os.Unsetenv("RUNTIME_NAMESPACE")
+			rackLabel := "airshipit.org/rack"
+			serverLabel := "airshipit.org/server"
 			vino := testVINO()
+			providedFlavorLabel := "provided-label"
+			providedFlavorValue := "provided-value"
+			vino.Spec.NodeLabelKeysToCopy = []string{rackLabel, serverLabel}
 			vino.Spec.Nodes = []vinov1.NodeSet{
 				{
-					Name:  "worker",
+					Name: "worker",
+					BMHLabels: map[string]string{
+						providedFlavorLabel: providedFlavorValue,
+					},
 					Count: 3,
 					NetworkDataTemplate: vinov1.NamespacedName{
 						Name:      "default-template",
@@ -94,9 +102,16 @@ var _ = Describe("Test BMH reconciliation", func() {
 				},
 			}
 
+			rack1 := "r1"
+			server1 := "s1"
+			node1Labels := map[string]string{
+				rackLabel:   rack1,
+				serverLabel: server1,
+			}
 			node1 := &corev1.Node{
 				ObjectMeta: metav1.ObjectMeta{
-					Name: "node01",
+					Name:   "node01",
+					Labels: node1Labels,
 				},
 				Status: corev1.NodeStatus{
 					Addresses: []corev1.NodeAddress{
@@ -148,6 +163,9 @@ var _ = Describe("Test BMH reconciliation", func() {
 
 			Expect(reconciler.Get(ctx, client.ObjectKeyFromObject(bmh), bmh)).Should(Succeed())
 			Expect(bmh.Spec.BMC.Address).To(Equal("redfish+http://10.0.0.2:8000/redfish/v1/Systems/worker-1"))
+			Expect(bmh.Labels).To(HaveKeyWithValue(rackLabel, rack1))
+			Expect(bmh.Labels).To(HaveKeyWithValue(serverLabel, server1))
+			Expect(bmh.Labels).To(HaveKeyWithValue(providedFlavorLabel, providedFlavorValue))
 			Expect(reconciler.Get(ctx, client.ObjectKeyFromObject(networkSecret), networkSecret)).Should(Succeed())
 			Expect(networkSecret.StringData["networkData"]).To(Equal("REPLACEME"))
 		})

tools/deployment/test-cr.sh

@@ -12,6 +12,14 @@ function vinoDebugInfo () {
     exit 1
 }
 
+server_label="airshipit.org/server=s1"
+rack_label="airshipit.org/rack=r1"
+copyLabel="airshipit.org/k8s-role=worker"
+
+# Label all nodes with the same rack/label. We are ok with this for this simple test.
+kubectl label node --overwrite=true --all $server_label $rack_label
+
+
 kubectl apply -f config/samples/vino_cr.yaml
 kubectl apply -f config/samples/ippool.yaml
 kubectl apply -f config/samples/network-template-secret.yaml
@@ -44,12 +52,14 @@ if ! kubectl -n vino-system rollout status ds default-vino-test-cr --timeout=10s
     vinoDebugInfo
 fi
 
-bmhCount=$(kubectl get baremetalhosts -n vino-system -o name | wc -l)
+bmhCount=$(kubectl get baremetalhosts -n vino-system -l "$server_label,$server_label,$copyLabel" -o name | wc -l)
 
 # with this setup set up, exactly 3 BMHs must have been created by VINO controller
 
 [[ "$bmhCount" -eq "3" ]]
 
+kubectl get baremetalhosts -n vino-system --show-labels=true
+
 kubectl get -o yaml -n vino-system \
     $(kubectl get secret -o name -n vino-system | grep network-data)
 kubectl get secret -o yaml -n vino-system default-vino-test-cr-credentials