Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🌱 Use real Context and make provisioners context-aware #1566

Merged
merged 4 commits into from
Feb 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading