diff --git a/config/crd/bases/airship.airshipit.org_sipclusters.yaml b/config/crd/bases/airship.airshipit.org_sipclusters.yaml index 85f1d7c..3ef034d 100644 --- a/config/crd/bases/airship.airshipit.org_sipclusters.yaml +++ b/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 - storedVersions: null + conditions: [] + storedVersions: [] diff --git a/pkg/vbmh/errors.go b/pkg/vbmh/errors.go index e1654e1..9d24ba5 100644 --- a/pkg/vbmh/errors.go +++ b/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) +} diff --git a/pkg/vbmh/machines.go b/pkg/vbmh/machines.go index 580fcf0..c9a9b0b 100644 --- a/pkg/vbmh/machines.go +++ b/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 diff --git a/pkg/vbmh/vbmh_test.go b/pkg/vbmh/vbmh_test.go index 8a8ffdf..5be67ea 100644 --- a/pkg/vbmh/vbmh_test.go +++ b/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()) + }) })