Skip to content

Commit

Permalink
Fixture: store state separately to provisioner
Browse files Browse the repository at this point in the history 
The fixtureProvisioner object gets recreated for each Reconcile() call, so
it cannot be used to store state that is used to make different things
happen on each reconciliation. Some of this state was instead being stored
in the host Status, which could potentially confound some of the tests:
only the controller should modify the status, and it is the thing we are
trying to test.

Instead, create a Fixture structure that contains persistent state for a
particular Host and the provisioner Factory to create a provisioner that
has access to that state. Stop writing to the Host's Status from the
fixture provisioner.
  • Loading branch information
@zaneb
zaneb committed Jan 14, 2021
1 parent cd60f7d commit 1279a47
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 50 deletions.
31 changes: 10 additions & 21 deletions controllers/metal3.io/baremetalhost_controller_test.go
View file
Expand Up @@ -23,8 +23,6 @@ import (
"sigs.k8s.io/controller-runtime/pkg/reconcile"

metal3v1alpha1 "github.com/metal3-io/baremetal-operator/apis/metal3.io/v1alpha1"
"github.com/metal3-io/baremetal-operator/pkg/bmc"
"github.com/metal3-io/baremetal-operator/pkg/provisioner"
"github.com/metal3-io/baremetal-operator/pkg/provisioner/fixture"
"github.com/metal3-io/baremetal-operator/pkg/utils"
)
Expand Down Expand Up @@ -88,7 +86,7 @@ func newDefaultHost(t *testing.T) *metal3v1alpha1.BareMetalHost {
return newDefaultNamedHost(t.Name(), t)
}

func newTestReconcilerWithProvisionerFactory(factory provisioner.Factory, initObjs ...runtime.Object) *BareMetalHostReconciler {
func newTestReconcilerWithFixture(fix *fixture.Fixture, initObjs ...runtime.Object) *BareMetalHostReconciler {

c := fakeclient.NewFakeClient(initObjs...)

Expand All @@ -99,13 +97,14 @@ func newTestReconcilerWithProvisionerFactory(factory provisioner.Factory, initOb
return &BareMetalHostReconciler{
Client: c,
Scheme: scheme.Scheme,
ProvisionerFactory: factory,
ProvisionerFactory: fix.New,
Log: ctrl.Log.WithName("controllers").WithName("BareMetalHost"),
}
}

func newTestReconciler(initObjs ...runtime.Object) *BareMetalHostReconciler {
return newTestReconcilerWithProvisionerFactory(fixture.New, initObjs...)
fix := fixture.Fixture{}
return newTestReconcilerWithFixture(&fix, initObjs...)
}

type DoneFunc func(host *metal3v1alpha1.BareMetalHost, result reconcile.Result) bool
Expand Down Expand Up @@ -1033,12 +1032,12 @@ func TestDeleteHost(t *testing.T) {
host.DeletionTimestamp = &now
host.Status.Provisioning.ID = "made-up-id"
badSecret := newBMCCredsSecret("bmc-creds-no-user", "", "Pass")
r := newTestReconciler(host, badSecret)
fix := fixture.Fixture{}
r := newTestReconcilerWithFixture(&fix, host, badSecret)

tryReconcile(t, r, host,
func(host *metal3v1alpha1.BareMetalHost, result reconcile.Result) bool {
t.Logf("provisioning id: %q", host.Status.Provisioning.ID)
return host.Status.Provisioning.ID == ""
return fix.Deleted
},
)
})
Expand Down Expand Up @@ -1156,22 +1155,12 @@ func TestUpdateRootDeviceHints(t *testing.T) {
func TestProvisionerIsReady(t *testing.T) {
host := newDefaultHost(t)

var prov provisioner.Provisioner
r := newTestReconcilerWithProvisionerFactory(func(host *metal3v1alpha1.BareMetalHost, bmcCreds bmc.Credentials, publisher provisioner.EventPublisher) (provisioner provisioner.Provisioner, err error) {
if prov == nil {
prov, err = fixture.NewMock(host, bmcCreds, publisher, 5)
}
return prov, err
}, host)
fix := fixture.Fixture{BecomeReadyCounter: 5}
r := newTestReconcilerWithFixture(&fix, host)

tryReconcile(t, r, host,
func(host *metal3v1alpha1.BareMetalHost, result reconcile.Result) bool {
if prov == nil {
return false
}

ready, _ := prov.IsReady()
return ready
return host.Status.Provisioning.State != metal3v1alpha1.StateNone
},
)
}
Expand Down
3 changes: 2 additions & 1 deletion main.go
View file
Expand Up @@ -123,7 +123,8 @@ func main() {

if runInTestMode {
ctrl.Log.Info("using test provisioner")
return fixture.New(host, bmcCreds, publish)
fix := fixture.Fixture{}
return fix.New(host, bmcCreds, publish)
} else if runInDemoMode {
ctrl.Log.Info("using demo provisioner")
return demo.New(host, bmcCreds, publish)
Expand Down
63 changes: 35 additions & 28 deletions pkg/provisioner/fixture/fixture.go
View file
Expand Up @@ -52,25 +52,31 @@ type fixtureProvisioner struct {
log logr.Logger
// an event publisher for recording significant events
publisher provisioner.EventPublisher
// state storage for the Host
state *Fixture
}

type Fixture struct {
// counter to set the provisioner as ready
BecomeReadyCounter int
// state to manage deletion
Deleted bool
// state to manage the two-step adopt process
adopted bool
// counter to set the provisioner as ready
becomeReadyCounter int
// state to manage provisioning
image metal3v1alpha1.Image
// state to manage power
poweredOn bool
}

// New returns a new Ironic FixtureProvisioner
func New(host *metal3v1alpha1.BareMetalHost, bmcCreds bmc.Credentials, publisher provisioner.EventPublisher) (provisioner.Provisioner, error) {
return NewMock(host, bmcCreds, publisher, 0)
}

// NewMock is used in tests to build a fixture provisioner and inject additional test parameters
func NewMock(host *metal3v1alpha1.BareMetalHost, bmcCreds bmc.Credentials, publisher provisioner.EventPublisher, becomeReadyCounter int) (provisioner.Provisioner, error) {
func (f *Fixture) New(host *metal3v1alpha1.BareMetalHost, bmcCreds bmc.Credentials, publisher provisioner.EventPublisher) (provisioner.Provisioner, error) {
p := &fixtureProvisioner{
host: host,
bmcCreds: bmcCreds,
log: log.WithValues("host", host.Name),
publisher: publisher,
becomeReadyCounter: becomeReadyCounter,
host: host,
bmcCreds: bmcCreds,
log: log.WithValues("host", host.Name),
publisher: publisher,
state: f,
}
return p, nil
}
Expand Down Expand Up @@ -160,6 +166,7 @@ func (p *fixtureProvisioner) InspectHardware(force bool) (result provisioner.Res
// reading from a cache.
func (p *fixtureProvisioner) UpdateHardwareState() (hwState provisioner.HardwareState, err error) {
if !p.host.NeedsProvisioning() {
hwState.PoweredOn = &p.state.poweredOn
p.log.Info("updating hardware state")
}
return
Expand All @@ -168,8 +175,8 @@ func (p *fixtureProvisioner) UpdateHardwareState() (hwState provisioner.Hardware
// Adopt allows an externally-provisioned server to be adopted.
func (p *fixtureProvisioner) Adopt(force bool) (result provisioner.Result, err error) {
p.log.Info("adopting host")
if p.host.Spec.ExternallyProvisioned && !p.adopted {
p.adopted = true
if p.host.Spec.ExternallyProvisioned && !p.state.adopted {
p.state.adopted = true
result.Dirty = true
result.RequeueAfter = provisionRequeueDelay
}
Expand All @@ -183,10 +190,10 @@ func (p *fixtureProvisioner) Provision(hostConf provisioner.HostConfigData) (res
p.log.Info("provisioning image to host",
"state", p.host.Status.Provisioning.State)

if p.host.Status.Provisioning.Image.URL == "" {
if p.state.image.URL == "" {
p.publisher("ProvisioningComplete", "Image provisioning completed")
p.log.Info("moving to done")
p.host.Status.Provisioning.Image = *p.host.Spec.Image
p.state.image = *p.host.Spec.Image
result.Dirty = true
result.RequeueAfter = provisionRequeueDelay
}
Expand All @@ -207,10 +214,10 @@ func (p *fixtureProvisioner) Deprovision(force bool) (result provisioner.Result,
// necessary once we really have Fixture doing the deprovisioning
// and we can monitor it's status.

if p.host.Status.HardwareDetails != nil {
if p.state.image.URL != "" {
p.publisher("DeprovisionStarted", "Image deprovisioning started")
p.log.Info("clearing hardware details")
p.host.Status.HardwareDetails = nil
p.state.image = metal3v1alpha1.Image{}
result.Dirty = true
return result, nil
}
Expand All @@ -225,9 +232,9 @@ func (p *fixtureProvisioner) Deprovision(force bool) (result provisioner.Result,
func (p *fixtureProvisioner) Delete() (result provisioner.Result, err error) {
p.log.Info("deleting host")

if p.host.Status.Provisioning.ID != "" {
if !p.state.Deleted {
p.log.Info("clearing provisioning id")
p.host.Status.Provisioning.ID = ""
p.state.Deleted = true
result.Dirty = true
return result, nil
}
Expand All @@ -240,10 +247,10 @@ func (p *fixtureProvisioner) Delete() (result provisioner.Result, err error) {
func (p *fixtureProvisioner) PowerOn() (result provisioner.Result, err error) {
p.log.Info("ensuring host is powered on")

if !p.host.Status.PoweredOn {
if !p.state.poweredOn {
p.publisher("PowerOn", "Host powered on")
p.log.Info("changing status")
p.host.Status.PoweredOn = true
p.state.poweredOn = true
result.Dirty = true
return result, nil
}
Expand All @@ -256,10 +263,10 @@ func (p *fixtureProvisioner) PowerOn() (result provisioner.Result, err error) {
func (p *fixtureProvisioner) PowerOff() (result provisioner.Result, err error) {
p.log.Info("ensuring host is powered off")

if p.host.Status.PoweredOn {
if p.state.poweredOn {
p.publisher("PowerOff", "Host powered off")
p.log.Info("changing status")
p.host.Status.PoweredOn = false
p.state.poweredOn = false
result.Dirty = true
return result, nil
}
Expand All @@ -271,9 +278,9 @@ func (p *fixtureProvisioner) PowerOff() (result provisioner.Result, err error) {
func (p *fixtureProvisioner) IsReady() (result bool, err error) {
p.log.Info("checking provisioner status")

if p.becomeReadyCounter > 0 {
p.becomeReadyCounter--
if p.state.BecomeReadyCounter > 0 {
p.state.BecomeReadyCounter--
}

return p.becomeReadyCounter == 0, nil
return p.state.BecomeReadyCounter == 0, nil
}

0 comments on commit 1279a47

Please sign in to comment.