Skip to content

Commit

Permalink
Merge pull request #1566 from dtantsur/context
Browse files Browse the repository at this point in the history
🌱 Use real Context and make provisioners context-aware
  • Loading branch information
metal3-io-bot authored Feb 29, 2024
2 parents 289290a + 6bcfdc4 commit e8b7579
Show file tree
Hide file tree
Showing 13 changed files with 101 additions and 86 deletions.
100 changes: 51 additions & 49 deletions controllers/metal3.io/baremetalhost_controller.go

Large diffs are not rendered by default.

16 changes: 8 additions & 8 deletions controllers/metal3.io/bmceventsubscription_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ func (r *BMCEventSubscriptionReconciler) Reconcile(ctx context.Context, request
return ctrl.Result{}, errors.Wrap(err, "failed add finalizer")
}

prov, ready, err := r.getProvisioner(request, host)
prov, ready, err := r.getProvisioner(ctx, request, host)

if err != nil {
return ctrl.Result{}, errors.Wrap(err, "failed to create provisioner")
Expand Down Expand Up @@ -172,7 +172,7 @@ func (r *BMCEventSubscriptionReconciler) createSubscription(ctx context.Context,
return nil
}

headers, err := r.getHTTPHeaders(*subscription)
headers, err := r.getHTTPHeaders(ctx, *subscription)

if err != nil {
reqLogger.Error(err, "failed to get http headers")
Expand All @@ -191,7 +191,7 @@ func (r *BMCEventSubscriptionReconciler) createSubscription(ctx context.Context,
return r.Status().Update(ctx, subscription)
}

func (r *BMCEventSubscriptionReconciler) deleteSubscription(_ context.Context, prov provisioner.Provisioner, subscription *metal3api.BMCEventSubscription) error {
func (r *BMCEventSubscriptionReconciler) deleteSubscription(ctx context.Context, prov provisioner.Provisioner, subscription *metal3api.BMCEventSubscription) error {
reqLogger := r.Log.WithName("bmceventsubscription")
reqLogger.Info("deleting subscription")

Expand All @@ -205,18 +205,18 @@ func (r *BMCEventSubscriptionReconciler) deleteSubscription(_ context.Context, p
subscription.Finalizers, metal3api.BMCEventSubscriptionFinalizer)
reqLogger.Info("cleanup is complete, removed finalizer",
"remaining", subscription.Finalizers)
if err := r.Update(context.Background(), subscription); err != nil {
if err := r.Update(ctx, subscription); err != nil {
return err
}
}

return nil
}

func (r *BMCEventSubscriptionReconciler) getProvisioner(request ctrl.Request, host *metal3api.BareMetalHost) (prov provisioner.Provisioner, ready bool, err error) {
func (r *BMCEventSubscriptionReconciler) getProvisioner(ctx context.Context, request ctrl.Request, host *metal3api.BareMetalHost) (prov provisioner.Provisioner, ready bool, err error) {
reqLogger := r.Log.WithValues("bmceventsubscription", request.NamespacedName)

prov, err = r.ProvisionerFactory.NewProvisioner(provisioner.BuildHostDataNoBMC(*host), nil)
prov, err = r.ProvisionerFactory.NewProvisioner(ctx, provisioner.BuildHostDataNoBMC(*host), nil)
if err != nil {
return prov, ready, errors.Wrap(err, "failed to create provisioner")
}
Expand All @@ -234,7 +234,7 @@ func (r *BMCEventSubscriptionReconciler) getProvisioner(request ctrl.Request, ho
return prov, ready, nil
}

func (r *BMCEventSubscriptionReconciler) getHTTPHeaders(subscription metal3api.BMCEventSubscription) ([]map[string]string, error) {
func (r *BMCEventSubscriptionReconciler) getHTTPHeaders(ctx context.Context, subscription metal3api.BMCEventSubscription) ([]map[string]string, error) {
headers := []map[string]string{}

if subscription.Spec.HTTPHeadersRef == nil {
Expand All @@ -247,7 +247,7 @@ func (r *BMCEventSubscriptionReconciler) getHTTPHeaders(subscription metal3api.B
Namespace: subscription.Spec.HTTPHeadersRef.Namespace,
}

err := r.Get(context.TODO(), secretKey, secret)
err := r.Get(ctx, secretKey, secret)

if err != nil {
return headers, err
Expand Down
4 changes: 2 additions & 2 deletions controllers/metal3.io/host_config_data_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func TestLabelSecrets(t *testing.T) {
hcd := &hostConfigData{
host: host,
log: baselog.WithName("host_config_data"),
secretManager: secretutils.NewSecretManager(baselog, c, c),
secretManager: secretutils.NewSecretManager(context.TODO(), baselog, c, c),
}

secret := newSecret(tc.name, map[string]string{"value": "somedata"})
Expand Down Expand Up @@ -343,7 +343,7 @@ func TestProvisionWithHostConfig(t *testing.T) {
hcd := &hostConfigData{
host: tc.Host,
log: baselog.WithName("host_config_data"),
secretManager: secretutils.NewSecretManager(baselog, c, c),
secretManager: secretutils.NewSecretManager(context.TODO(), baselog, c, c),
}

actualUserData, err := hcd.UserData()
Expand Down
3 changes: 2 additions & 1 deletion controllers/metal3.io/host_state_machine_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package controllers

import (
"context"
"testing"
"time"

Expand All @@ -21,7 +22,7 @@ import (

func testStateMachine(host *metal3api.BareMetalHost) *hostStateMachine {
r := newTestReconciler()
p, _ := r.ProvisionerFactory.NewProvisioner(provisioner.BuildHostData(*host, bmc.Credentials{}),
p, _ := r.ProvisionerFactory.NewProvisioner(context.TODO(), provisioner.BuildHostData(*host, bmc.Credentials{}),
func(reason, message string) {})
return newHostStateMachine(host, r, p, true)
}
Expand Down
21 changes: 11 additions & 10 deletions controllers/metal3.io/hostfirmwaresettings_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ type HostFirmwareSettingsReconciler struct {
}

type rInfo struct {
ctx context.Context
log logr.Logger
hfs *metal3api.HostFirmwareSettings
bmh *metal3api.BareMetalHost
Expand Down Expand Up @@ -117,7 +118,7 @@ func (r *HostFirmwareSettingsReconciler) Reconcile(ctx context.Context, req ctrl

// Get the corresponding baremetalhost in this namespace, if one doesn't exist don't continue processing
bmh := &metal3api.BareMetalHost{}
if err = r.Get(context.TODO(), req.NamespacedName, bmh); err != nil {
if err = r.Get(ctx, req.NamespacedName, bmh); err != nil {
reqLogger.Info("could not get baremetalhost, not running reconciler")
if k8serrors.IsNotFound(err) {
return ctrl.Result{}, nil
Expand All @@ -132,7 +133,7 @@ func (r *HostFirmwareSettingsReconciler) Reconcile(ctx context.Context, req ctrl

// Fetch the HostFirmwareSettings
hfs := &metal3api.HostFirmwareSettings{}
info := &rInfo{log: reqLogger, hfs: hfs, bmh: bmh}
info := &rInfo{ctx: ctx, log: reqLogger, hfs: hfs, bmh: bmh}
if err = r.Get(ctx, req.NamespacedName, hfs); err != nil {
// The HFS resource may have been deleted
if k8serrors.IsNotFound(err) {
Expand All @@ -144,7 +145,7 @@ func (r *HostFirmwareSettingsReconciler) Reconcile(ctx context.Context, req ctrl
}

// Create a provisioner that can access Ironic API
prov, err := r.ProvisionerFactory.NewProvisioner(provisioner.BuildHostDataNoBMC(*bmh), info.publishEvent)
prov, err := r.ProvisionerFactory.NewProvisioner(ctx, provisioner.BuildHostDataNoBMC(*bmh), info.publishEvent)
if err != nil {
return ctrl.Result{}, errors.Wrap(err, "failed to create provisioner")
}
Expand Down Expand Up @@ -175,7 +176,7 @@ func (r *HostFirmwareSettingsReconciler) Reconcile(ctx context.Context, req ctrl
}

for _, e := range info.events {
r.publishEvent(req, e)
r.publishEvent(ctx, req, e)
}

// requeue to run again after delay
Expand Down Expand Up @@ -285,7 +286,7 @@ func (r *HostFirmwareSettingsReconciler) updateStatus(info *rInfo, settings meta

t := metav1.Now()
info.hfs.Status.LastUpdated = &t
return r.Status().Update(context.TODO(), info.hfs)
return r.Status().Update(info.ctx, info.hfs)
}
return nil
}
Expand All @@ -298,15 +299,15 @@ func (r *HostFirmwareSettingsReconciler) getOrCreateFirmwareSchema(info *rInfo,
firmwareSchema := &metal3api.FirmwareSchema{}

// If a schema exists that matches, use that, otherwise create a new one
if err = r.Get(context.TODO(), client.ObjectKey{Namespace: info.hfs.ObjectMeta.Namespace, Name: schemaName},
if err = r.Get(info.ctx, client.ObjectKey{Namespace: info.hfs.ObjectMeta.Namespace, Name: schemaName},
firmwareSchema); err == nil {
info.log.Info("found existing firmwareSchema resource")

// Add hfs as owner so can be garbage collected on delete, if already an owner it will just be overwritten
if err = controllerutil.SetOwnerReference(info.hfs, firmwareSchema, r.Scheme()); err != nil {
return nil, errors.Wrap(err, "could not set owner of existing firmwareSchema")
}
if err = r.Update(context.TODO(), firmwareSchema); err != nil {
if err = r.Update(info.ctx, firmwareSchema); err != nil {
return nil, err
}

Expand Down Expand Up @@ -348,7 +349,7 @@ func (r *HostFirmwareSettingsReconciler) getOrCreateFirmwareSchema(info *rInfo,
return nil, errors.Wrap(err, "could not set owner of firmwareSchema")
}

if err = r.Create(context.TODO(), firmwareSchema); err != nil {
if err = r.Create(info.ctx, firmwareSchema); err != nil {
return nil, err
}

Expand Down Expand Up @@ -413,10 +414,10 @@ func (r *HostFirmwareSettingsReconciler) validateHostFirmwareSettings(info *rInf
return nil
}

func (r *HostFirmwareSettingsReconciler) publishEvent(request ctrl.Request, event corev1.Event) {
func (r *HostFirmwareSettingsReconciler) publishEvent(ctx context.Context, request ctrl.Request, event corev1.Event) {
reqLogger := r.Log.WithValues("hostfirmwaresettings", request.NamespacedName)
reqLogger.Info("publishing event", "reason", event.Reason, "message", event.Message)
err := r.Create(context.TODO(), &event)
err := r.Create(ctx, &event)
if err != nil {
reqLogger.Info("failed to record event, ignoring",
"reason", event.Reason, "message", event.Message, "error", err)
Expand Down
6 changes: 3 additions & 3 deletions controllers/metal3.io/preprovisioningimage_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ func (r *PreprovisioningImageReconciler) Reconcile(ctx context.Context, req ctrl
return ctrl.Result{}, nil
}

changed, err := r.update(&img, log)
changed, err := r.update(ctx, &img, log)

if k8serrors.IsNotFound(err) {
delay := getErrorRetryDelay(img.Status)
Expand Down Expand Up @@ -136,7 +136,7 @@ func configChanged(img *metal3api.PreprovisioningImage, format metal3api.ImageFo
img.Status.NetworkData == networkDataStatus)
}

func (r *PreprovisioningImageReconciler) update(img *metal3api.PreprovisioningImage, log logr.Logger) (bool, error) {
func (r *PreprovisioningImageReconciler) update(ctx context.Context, img *metal3api.PreprovisioningImage, log logr.Logger) (bool, error) {
generation := img.GetGeneration()

if !r.ImageProvider.SupportsArchitecture(img.Spec.Architecture) {
Expand All @@ -149,7 +149,7 @@ func (r *PreprovisioningImageReconciler) update(img *metal3api.PreprovisioningIm
return setError(generation, &img.Status, reasonImageConfigurationError, "No acceptable image format supported"), nil
}

secretManager := secretutils.NewSecretManager(log, r.Client, r.APIReader)
secretManager := secretutils.NewSecretManager(ctx, log, r.Client, r.APIReader)
networkData, secretStatus, err := getNetworkData(secretManager, img)
if err != nil {
if k8serrors.IsNotFound(err) {
Expand Down
3 changes: 2 additions & 1 deletion pkg/provisioner/demo/demo.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package demo

import (
"context"
"time"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -66,7 +67,7 @@ type demoProvisioner struct {
type Demo struct{}

// NewProvisioner returns a new demo Provisioner.
func (d Demo) NewProvisioner(hostData provisioner.HostData, publisher provisioner.EventPublisher) (provisioner.Provisioner, error) {
func (d Demo) NewProvisioner(_ context.Context, hostData provisioner.HostData, publisher provisioner.EventPublisher) (provisioner.Provisioner, error) {
p := &demoProvisioner{
objectMeta: hostData.ObjectMeta,
provID: hostData.ProvisionerID,
Expand Down
3 changes: 2 additions & 1 deletion pkg/provisioner/fixture/fixture.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package fixture

import (
"context"
"time"

"github.com/go-logr/logr"
Expand Down Expand Up @@ -76,7 +77,7 @@ type Fixture struct {
}

// NewProvisioner returns a new Fixture Provisioner.
func (f *Fixture) NewProvisioner(hostData provisioner.HostData, publisher provisioner.EventPublisher) (provisioner.Provisioner, error) {
func (f *Fixture) NewProvisioner(_ context.Context, hostData provisioner.HostData, publisher provisioner.EventPublisher) (provisioner.Provisioner, error) {
p := &fixtureProvisioner{
provID: hostData.ProvisionerID,
bmcCreds: hostData.BMCCredentials,
Expand Down
8 changes: 5 additions & 3 deletions pkg/provisioner/ironic/factory.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package ironic

import (
"context"
"errors"
"fmt"
"net/url"
Expand Down Expand Up @@ -78,7 +79,7 @@ func (f *ironicProvisionerFactory) init(havePreprovImgBuilder bool) error {
return nil
}

func (f ironicProvisionerFactory) ironicProvisioner(hostData provisioner.HostData, publisher provisioner.EventPublisher) (*ironicProvisioner, error) {
func (f ironicProvisionerFactory) ironicProvisioner(ctx context.Context, hostData provisioner.HostData, publisher provisioner.EventPublisher) (*ironicProvisioner, error) {
provisionerLogger := f.log.WithValues("host", ironicNodeName(hostData.ObjectMeta))

p := &ironicProvisioner{
Expand All @@ -93,15 +94,16 @@ func (f ironicProvisionerFactory) ironicProvisioner(hostData provisioner.HostDat
log: provisionerLogger,
debugLog: provisionerLogger.V(1),
publisher: publisher,
ctx: ctx,
}

return p, nil
}

// NewProvisioner returns a new Ironic Provisioner using the global
// configuration for finding the Ironic services.
func (f ironicProvisionerFactory) NewProvisioner(hostData provisioner.HostData, publisher provisioner.EventPublisher) (provisioner.Provisioner, error) {
return f.ironicProvisioner(hostData, publisher)
func (f ironicProvisionerFactory) NewProvisioner(ctx context.Context, hostData provisioner.HostData, publisher provisioner.EventPublisher) (provisioner.Provisioner, error) {
return f.ironicProvisioner(ctx, hostData, publisher)
}

func loadConfigFromEnv(havePreprovImgBuilder bool) (ironicConfig, error) {
Expand Down
3 changes: 3 additions & 0 deletions pkg/provisioner/ironic/ironic.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package ironic

import (
"context"
"fmt"
"net"
"reflect"
Expand Down Expand Up @@ -110,6 +111,8 @@ type ironicProvisioner struct {
publisher provisioner.EventPublisher
// available API features
availableFeatures clients.AvailableFeatures
// request context
ctx context.Context
}

func (p *ironicProvisioner) bmcAccess() (bmc.AccessDetails, error) {
Expand Down
5 changes: 3 additions & 2 deletions pkg/provisioner/ironic/ironic_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package ironic

import (
"context"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -48,7 +49,7 @@ func newProvisionerWithSettings(host metal3api.BareMetalHost, bmcCreds bmc.Crede

factory := newTestProvisionerFactory()
factory.clientIronic = clientIronic
return factory.ironicProvisioner(hostData, publisher)
return factory.ironicProvisioner(context.TODO(), hostData, publisher)
}

func makeHost() metal3api.BareMetalHost {
Expand Down Expand Up @@ -133,7 +134,7 @@ func TestNewNoBMCDetails(t *testing.T) {
host.Spec.BMC = metal3api.BMCDetails{}

factory := newTestProvisionerFactory()
prov, err := factory.NewProvisioner(provisioner.BuildHostData(host, bmc.Credentials{}), nullEventPublisher)
prov, err := factory.NewProvisioner(context.TODO(), provisioner.BuildHostData(host, bmc.Credentials{}), nullEventPublisher)
assert.Equal(t, nil, err)
assert.NotEqual(t, nil, prov)
}
3 changes: 2 additions & 1 deletion pkg/provisioner/provisioner.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package provisioner

import (
"context"
"errors"
"time"

Expand Down Expand Up @@ -50,7 +51,7 @@ func BuildHostDataNoBMC(host metal3api.BareMetalHost) HostData {

// Factory is the interface for creating new Provisioner objects.
type Factory interface {
NewProvisioner(hostData HostData, publish EventPublisher) (Provisioner, error)
NewProvisioner(ctx context.Context, hostData HostData, publish EventPublisher) (Provisioner, error)
}

// HostConfigData retrieves host configuration data.
Expand Down
Loading

0 comments on commit e8b7579

Please sign in to comment.