Add NetworkData validation to BMH

This change introduces validation to ensure that NetworkData is present
when creating a machine. If not, the machine is skipped and placed in
NotScheduled state.

This change also includes a test case to validate this behavior occurs.

Signed-off-by: Alexander Hughes <Alexander.Hughes@pm.me>

restoring kustomization

Signed-off-by: Alexander Hughes <Alexander.Hughes@pm.me>

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

@@ -4,7 +4,7 @@ apiVersion: apiextensions.k8s.io/v1beta1
 kind: CustomResourceDefinition
 metadata:
   annotations:
-    controller-gen.kubebuilder.io/version: (devel)
+    controller-gen.kubebuilder.io/version: v0.2.5
   creationTimestamp: null
   name: sipclusters.airship.airshipit.org
 spec:
@@ -115,5 +115,5 @@ status:
   acceptedNames:
     kind: ""
     plural: ""
-  conditions: null
+  conditions: []
-  storedVersions: null
+  storedVersions: []

pkg/vbmh/errors.go

@@ -2,6 +2,9 @@ package vbmh
 
 import (
 	"fmt"
+
+	metal3 "github.com/metal3-io/baremetal-operator/apis/metal3.io/v1alpha1"
+
 	airshipv1 "sipcluster/pkg/api/v1"
 )
 
@@ -43,3 +46,12 @@ type ErrorUknownSpreadTopology struct {
 func (e ErrorUknownSpreadTopology) Error() string {
 	return fmt.Sprintf("Uknown spread topology '%s'", e.Topology)
 }
+
+// ErrorNetworkDataNotFound is returned when NetworkData metadata is missing from BMH
+type ErrorNetworkDataNotFound struct {
+	BMH metal3.BareMetalHost
+}
+
+func (e ErrorNetworkDataNotFound) Error() string {
+	return fmt.Sprintf("vBMH Host %v does not define NetworkData, but is required for scheduling.", e.BMH)
+}

pkg/vbmh/machines.go

@@ -99,7 +99,11 @@ func (m *Machine) String() string {
 		m.BMH.ObjectMeta.Name, m.ScheduleStatus, m.VMRole)
 }
 
-func NewMachine(bmh metal3.BareMetalHost, nodeRole airshipv1.VMRoles, schedState ScheduledState) (m *Machine) {
+func NewMachine(bmh metal3.BareMetalHost, nodeRole airshipv1.VMRoles, schedState ScheduledState) (m *Machine, e error) {
+	// Add logic to check if required fields exist.
+	if bmh.Spec.NetworkData == nil {
+		return nil, &ErrorNetworkDataNotFound{BMH: bmh}
+	}
 	return &Machine{
 		BMH:            bmh,
 		ScheduleStatus: schedState,
@@ -107,7 +111,7 @@ func NewMachine(bmh metal3.BareMetalHost, nodeRole airshipv1.VMRoles, schedState
 		Data: &MachineData{
 			IPOnInterface: make(map[string]string),
 		},
-	}
+	}, nil
 }
 
 type MachineData struct {
@@ -289,7 +293,12 @@ func (ml *MachineList) countScheduledAndTobeScheduled(nodeRole airshipv1.VMRoles
 		if readyScheduled {
 			logger.Info("BMH host is not yet marked as ready to be scheduled, marking it as ready to be scheduled")
 			// Add it to the list.
-			ml.Machines[bmh.ObjectMeta.Name] = NewMachine(bmh, nodeRole, Scheduled)
+			m, err := NewMachine(bmh, nodeRole, Scheduled)
+			if err != nil {
+				logger.Info("BMH did not meet scheduling requirements", "error", err.Error())
+				continue
+			}
+			ml.Machines[bmh.ObjectMeta.Name] = m
 			ml.ReadyForScheduleCount[nodeRole]++
 		}
 	}
@@ -348,7 +357,12 @@ func (ml *MachineList) scheduleIt(nodeRole airshipv1.VMRoles, nodeCfg airshipv1.
 			// Only if its not in the list already
 			if validBmh {
 				// Lets add it to the list as a schedulable thing
-				ml.Machines[bmh.ObjectMeta.Name] = NewMachine(bmh, nodeRole, ToBeScheduled)
+				m, err := NewMachine(bmh, nodeRole, ToBeScheduled)
+				if err != nil {
+					logger.Info("Skipping BMH host as it did not meet creation requirements", "error", err.Error())
+					continue
+				}
+				ml.Machines[bmh.ObjectMeta.Name] = m
 				ml.ReadyForScheduleCount[nodeRole]++
 				// TODO Probable should remove the bmh from the
 				// list so if there are other node targets they

pkg/vbmh/vbmh_test.go

@@ -21,11 +21,13 @@ const (
 
 var _ = Describe("MachineList", func() {
 	var machineList *MachineList
+	var err error
 	BeforeEach(func() {
 		nodes := map[string]*Machine{}
 		for n := 0; n < numNodes; n++ {
 			bmh, _ := testutil.CreateBMH(n, "default", "master", 6)
-			nodes[bmh.Name] = NewMachine(*bmh, airshipv1.VMMaster, NotScheduled)
+			nodes[bmh.Name], err = NewMachine(*bmh, airshipv1.VMMaster, NotScheduled)
+			Expect(err).To(BeNil())
 		}
 
 		machineList = &MachineList{
@@ -94,7 +96,8 @@ var _ = Describe("MachineList", func() {
 	It("Should retrieve the BMH IP from the BMH's NetworkData secret when infra services are defined", func() {
 		// Create a BMH with a NetworkData secret
 		bmh, secret := testutil.CreateBMH(1, "default", "master", 6)
-
+		m, err := NewMachine(*bmh, airshipv1.VMMaster, NotScheduled)
+		Expect(err).To(BeNil())
 		var objs []runtime.Object
 		objs = append(objs, bmh)
 		objs = append(objs, secret)
@@ -105,7 +108,7 @@ var _ = Describe("MachineList", func() {
 				Namespace: "default",
 			},
 			Machines: map[string]*Machine{
-				bmh.Name: NewMachine(*bmh, airshipv1.VMMaster, NotScheduled),
+				bmh.Name: m,
 			},
 			Log: ctrl.Log.WithName("controllers").WithName("SIPCluster"),
 		}
@@ -131,7 +134,8 @@ var _ = Describe("MachineList", func() {
 	It("Should not retrieve the BMH IP from the BMH's NetworkData secret if no infraServices are defined", func() {
 		// Create a BMH with a NetworkData secret
 		bmh, secret := testutil.CreateBMH(1, "default", "master", 6)
-
+		m, err := NewMachine(*bmh, airshipv1.VMMaster, NotScheduled)
+		Expect(err).To(BeNil())
 		var objs []runtime.Object
 		objs = append(objs, bmh)
 		objs = append(objs, secret)
@@ -142,7 +146,7 @@ var _ = Describe("MachineList", func() {
 				Namespace: "default",
 			},
 			Machines: map[string]*Machine{
-				bmh.Name: NewMachine(*bmh, airshipv1.VMMaster, NotScheduled),
+				bmh.Name: m,
 			},
 			Log: ctrl.Log.WithName("controllers").WithName("SIPCluster"),
 		}
@@ -167,4 +171,12 @@ var _ = Describe("MachineList", func() {
 		sipCluster := testutil.CreateSIPCluster("subcluster-1", "default", 1, 3)
 		Expect(machineList.Extrapolate(*sipCluster, k8sClient)).To(BeTrue())
 	})
+
+	It("Should not schedule BMH if it is missing networkdata", func() {
+		// Create a BMH without NetworkData
+		bmh, _ := testutil.CreateBMH(1, "default", "master", 6)
+		bmh.Spec.NetworkData = nil
+		_, err := NewMachine(*bmh, airshipv1.VMMaster, NotScheduled)
+		Expect(err).ToNot(BeNil())
+	})
 })