Apply generic methods to more existing controllers

internal/controllers/flavor/actuator.go

@@ -20,18 +20,42 @@ import (
 	"context"
 
 	"github.com/gophercloud/gophercloud/v2/openstack/compute/v2/flavors"
+	"k8s.io/utils/ptr"
+	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	orcv1alpha1 "github.com/k-orc/openstack-resource-controller/api/v1alpha1"
 	"github.com/k-orc/openstack-resource-controller/internal/controllers/generic"
 	osclients "github.com/k-orc/openstack-resource-controller/internal/osclients"
+	"github.com/k-orc/openstack-resource-controller/internal/scope"
+	orcerrors "github.com/k-orc/openstack-resource-controller/internal/util/errors"
 )
 
 type flavorActuator struct {
 	*orcv1alpha1.Flavor
 	osClient osclients.ComputeClient
 }
 
-var _ generic.ResourceActuator[*flavors.Flavor] = flavorActuator{}
+func newActuator(ctx context.Context, k8sClient client.Client, scopeFactory scope.Factory, orcObject *orcv1alpha1.Flavor) (flavorActuator, error) {
+	log := ctrl.LoggerFrom(ctx)
+
+	clientScope, err := scopeFactory.NewClientScopeFromObject(ctx, k8sClient, log, orcObject)
+	if err != nil {
+		return flavorActuator{}, err
+	}
+	osClient, err := clientScope.NewComputeClient()
+	if err != nil {
+		return flavorActuator{}, err
+	}
+
+	return flavorActuator{
+		Flavor:   orcObject,
+		osClient: osClient,
+	}, nil
+}
+
+var _ generic.CreateResourceActuator[*flavors.Flavor] = flavorActuator{}
+var _ generic.DeleteResourceActuator[*flavors.Flavor] = flavorActuator{}
 
 func (obj flavorActuator) GetManagementPolicy() orcv1alpha1.ManagementPolicy {
 	return obj.Spec.ManagementPolicy
@@ -53,6 +77,13 @@ func (obj flavorActuator) GetOSResourceByStatusID(ctx context.Context) (bool, *f
 	return true, flavor, err
 }
 
+func (obj flavorActuator) GetOSResourceBySpec(ctx context.Context) (*flavors.Flavor, error) {
+	if obj.Spec.Resource == nil {
+		return nil, nil
+	}
+	return GetByFilter(ctx, obj.osClient, specToFilter(*obj.Spec.Resource))
+}
+
 func (obj flavorActuator) GetOSResourceByImportID(ctx context.Context) (bool, *flavors.Flavor, error) {
 	if obj.Spec.Import == nil {
 		return false, nil, nil
@@ -75,18 +106,45 @@ func (obj flavorActuator) GetOSResourceByImportFilter(ctx context.Context) (bool
 	return true, flavor, err
 }
 
-func (obj flavorActuator) GetOSResourceBySpec(ctx context.Context) (*flavors.Flavor, error) {
-	if obj.Spec.Resource == nil {
-		return nil, nil
+func (obj flavorActuator) CreateResource(ctx context.Context) ([]string, *flavors.Flavor, error) {
+	resource := obj.Spec.Resource
+
+	if resource == nil {
+		// Should have been caught by API validation
+		return nil, nil, orcerrors.Terminal(orcv1alpha1.OpenStackConditionReasonInvalidConfiguration, "Creation requested, but spec.resource is not set")
 	}
-	return GetByFilter(ctx, obj.osClient, specToFilter(*obj.Spec.Resource))
-}
 
-func (obj flavorActuator) CreateResource(ctx context.Context) ([]string, *flavors.Flavor, error) {
-	flavor, err := createResource(ctx, obj.Flavor, obj.osClient)
-	return nil, flavor, err
+	createOpts := flavors.CreateOpts{
+		Name:        string(getResourceName(obj.Flavor)),
+		RAM:         int(resource.RAM),
+		VCPUs:       int(resource.Vcpus),
+		Disk:        ptr.To(int(resource.Disk)),
+		Swap:        ptr.To(int(resource.Swap)),
+		IsPublic:    resource.IsPublic,
+		Ephemeral:   ptr.To(int(resource.Ephemeral)),
+		Description: string(ptr.Deref(resource.Description, "")),
+	}
+
+	osResource, err := obj.osClient.CreateFlavor(ctx, createOpts)
+	if err != nil {
+		// We should require the spec to be updated before retrying a create which returned a conflict
+		if orcerrors.IsConflict(err) {
+			err = orcerrors.Terminal(orcv1alpha1.OpenStackConditionReasonInvalidConfiguration, "invalid configuration creating resource: "+err.Error(), err)
+		}
+		return nil, nil, err
+	}
+
+	return nil, osResource, nil
 }
 
 func (obj flavorActuator) DeleteResource(ctx context.Context, flavor *flavors.Flavor) error {
 	return obj.osClient.DeleteFlavor(ctx, flavor.ID)
 }
+
+// getResourceName returns the name of the OpenStack resource we should use.
+func getResourceName(orcObject *orcv1alpha1.Flavor) orcv1alpha1.OpenStackName {
+	if orcObject.Spec.Resource.Name != nil {
+		return *orcObject.Spec.Resource.Name
+	}
+	return orcv1alpha1.OpenStackName(orcObject.Name)
+}

internal/controllers/flavor/reconcile.go

@@ -25,7 +25,6 @@ import (
 	"github.com/gophercloud/gophercloud/v2/openstack/compute/v2/flavors"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/types"
-	"k8s.io/utils/ptr"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
@@ -59,16 +58,6 @@ func (r *orcFlavorReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 	return r.reconcileNormal(ctx, orcObject)
 }
 
-func (r *orcFlavorReconciler) getOpenStackClient(ctx context.Context, orcObject *orcv1alpha1.Flavor) (osclients.ComputeClient, error) {
-	log := ctrl.LoggerFrom(ctx)
-
-	clientScope, err := r.scopeFactory.NewClientScopeFromObject(ctx, r.client, log, orcObject)
-	if err != nil {
-		return nil, err
-	}
-	return clientScope.NewComputeClient()
-}
-
 func (r *orcFlavorReconciler) reconcileNormal(ctx context.Context, orcObject *orcv1alpha1.Flavor) (_ ctrl.Result, err error) {
 	log := ctrl.LoggerFrom(ctx)
 	log.V(3).Info("Reconciling resource")
@@ -98,17 +87,12 @@ func (r *orcFlavorReconciler) reconcileNormal(ctx context.Context, orcObject *or
 		return ctrl.Result{}, r.client.Patch(ctx, orcObject, patch, client.ForceOwnership, ssaFieldOwner(SSAFinalizerTxn))
 	}
 
-	osClient, err := r.getOpenStackClient(ctx, orcObject)
+	actuator, err := newActuator(ctx, r.client, r.scopeFactory, orcObject)
 	if err != nil {
 		return ctrl.Result{}, err
 	}
 
-	adapter := flavorActuator{
-		Flavor:   orcObject,
-		osClient: osClient,
-	}
-
-	waitMsgs, osResource, err := generic.GetOrCreateOSResource(ctx, log, r.client, adapter)
+	waitMsgs, osResource, err := generic.GetOrCreateOSResource(ctx, log, r.client, actuator)
 	if err != nil {
 		return ctrl.Result{}, err
 	}
@@ -132,7 +116,7 @@ func (r *orcFlavorReconciler) reconcileNormal(ctx context.Context, orcObject *or
 	ctx = ctrl.LoggerInto(ctx, log)
 
 	if orcObject.Spec.ManagementPolicy == orcv1alpha1.ManagementPolicyManaged {
-		for _, updateFunc := range needsUpdate(osClient, orcObject, osResource) {
+		for _, updateFunc := range needsUpdate(actuator.osClient, orcObject, osResource) {
 			if err := updateFunc(ctx); err != nil {
 				addStatus(withProgressMessage("Updating the OpenStack resource"))
 				return ctrl.Result{}, fmt.Errorf("failed to update the OpenStack resource: %w", err)
@@ -163,17 +147,12 @@ func (r *orcFlavorReconciler) reconcileDelete(ctx context.Context, orcObject *or
 		}
 	}()
 
-	osClient, err := r.getOpenStackClient(ctx, orcObject)
+	actuator, err := newActuator(ctx, r.client, r.scopeFactory, orcObject)
 	if err != nil {
 		return ctrl.Result{}, err
 	}
 
-	flavorActuator := flavorActuator{
-		Flavor:   orcObject,
-		osClient: osClient,
-	}
-
-	osResource, result, err := generic.DeleteResource(ctx, log, flavorActuator, func() error {
+	osResource, result, err := generic.DeleteResource(ctx, log, actuator, func() error {
 		deleted = true
 
 		// Clear the finalizer
@@ -184,54 +163,6 @@ func (r *orcFlavorReconciler) reconcileDelete(ctx context.Context, orcObject *or
 	return result, err
 }
 
-// getResourceName returns the name of the OpenStack resource we should use.
-func getResourceName(orcObject *orcv1alpha1.Flavor) orcv1alpha1.OpenStackName {
-	if orcObject.Spec.Resource.Name != nil {
-		return *orcObject.Spec.Resource.Name
-	}
-	return orcv1alpha1.OpenStackName(orcObject.Name)
-}
-
-// createResource creates an OpenStack resource for an ORC object.
-func createResource(ctx context.Context, orcObject *orcv1alpha1.Flavor, osClient osclients.ComputeClient) (*flavors.Flavor, error) {
-	if orcObject.Spec.ManagementPolicy == orcv1alpha1.ManagementPolicyUnmanaged {
-		// Should have been caught by API validation
-		return nil, orcerrors.Terminal(orcv1alpha1.OpenStackConditionReasonInvalidConfiguration, "Not creating unmanaged resource")
-	}
-
-	log := ctrl.LoggerFrom(ctx)
-	log.V(3).Info("Creating OpenStack resource")
-
-	resource := orcObject.Spec.Resource
-
-	if resource == nil {
-		// Should have been caught by API validation
-		return nil, orcerrors.Terminal(orcv1alpha1.OpenStackConditionReasonInvalidConfiguration, "Creation requested, but spec.resource is not set")
-	}
-
-	createOpts := flavors.CreateOpts{
-		Name:        string(getResourceName(orcObject)),
-		RAM:         int(resource.RAM),
-		VCPUs:       int(resource.Vcpus),
-		Disk:        ptr.To(int(resource.Disk)),
-		Swap:        ptr.To(int(resource.Swap)),
-		IsPublic:    resource.IsPublic,
-		Ephemeral:   ptr.To(int(resource.Ephemeral)),
-		Description: string(ptr.Deref(resource.Description, "")),
-	}
-
-	osResource, err := osClient.CreateFlavor(ctx, createOpts)
-	if err != nil {
-		// We should require the spec to be updated before retrying a create which returned a conflict
-		if orcerrors.IsConflict(err) {
-			err = orcerrors.Terminal(orcv1alpha1.OpenStackConditionReasonInvalidConfiguration, "invalid configuration creating resource: "+err.Error(), err)
-		}
-		return nil, err
-	}
-
-	return osResource, nil
-}
-
 // needsUpdate returns a slice of functions that call the OpenStack API to
 // align the OpenStack resoruce to its representation in the ORC spec object.
 // Flavor does not support update yet.

internal/controllers/generic/actuator.go

@@ -33,7 +33,7 @@ const (
 	deletePollingPeriod = 1 * time.Second
 )
 
-type ResourceActuator[osResourcePT any] interface {
+type BaseResourceActuator[osResourcePT any] interface {
 	client.Object
 
 	GetManagementPolicy() orcv1alpha1.ManagementPolicy
@@ -42,15 +42,24 @@ type ResourceActuator[osResourcePT any] interface {
 	GetResourceID(osResource osResourcePT) string
 
 	GetOSResourceByStatusID(ctx context.Context) (bool, osResourcePT, error)
-	GetOSResourceByImportID(ctx context.Context) (bool, osResourcePT, error)
-	GetOSResourceByImportFilter(ctx context.Context) (bool, osResourcePT, error)
 	GetOSResourceBySpec(ctx context.Context) (osResourcePT, error)
+}
 
+type CreateResourceActuator[osResourcePT any] interface {
+	BaseResourceActuator[osResourcePT]
+
+	GetOSResourceByImportID(ctx context.Context) (bool, osResourcePT, error)
+	GetOSResourceByImportFilter(ctx context.Context) (bool, osResourcePT, error)
 	CreateResource(ctx context.Context) ([]string, osResourcePT, error)
+}
+
+type DeleteResourceActuator[osResourcePT any] interface {
+	BaseResourceActuator[osResourcePT]
+
 	DeleteResource(ctx context.Context, osResource osResourcePT) error
 }
 
-func GetOrCreateOSResource[osResourcePT *osResourceT, osResourceT any](ctx context.Context, log logr.Logger, k8sClient client.Client, actuator ResourceActuator[osResourcePT]) ([]string, osResourcePT, error) {
+func GetOrCreateOSResource[osResourcePT *osResourceT, osResourceT any](ctx context.Context, log logr.Logger, k8sClient client.Client, actuator CreateResourceActuator[osResourcePT]) ([]string, osResourcePT, error) {
 	// Get by status ID
 	if hasStatusID, osResource, err := actuator.GetOSResourceByStatusID(ctx); hasStatusID {
 		if orcerrors.IsNotFound(err) {
@@ -104,7 +113,7 @@ func GetOrCreateOSResource[osResourcePT *osResourceT, osResourceT any](ctx conte
 	return actuator.CreateResource(ctx)
 }
 
-func DeleteResource[osResourcePT *osResourceT, osResourceT any](ctx context.Context, log logr.Logger, obj ResourceActuator[osResourcePT], onComplete func() error) (osResourcePT, ctrl.Result, error) {
+func DeleteResource[osResourcePT *osResourceT, osResourceT any](ctx context.Context, log logr.Logger, obj DeleteResourceActuator[osResourcePT], onComplete func() error) (osResourcePT, ctrl.Result, error) {
 	// We always fetch the resource by ID so we can continue to report status even when waiting for a finalizer
 	hasStatusID, osResource, err := obj.GetOSResourceByStatusID(ctx)
 	if err != nil {

internal/controllers/generic/dependency.go

@@ -193,3 +193,11 @@ func (d *Dependency[objectTP, _, depTP, _, _, _]) AddDeletionGuard(mgr ctrl.Mana
 
 	return ctrlcommon.AddDeletionGuard[depTP, objectTP](mgr, finalizer, fieldOwner, getDependencyRefsForClientObject, d.GetObjects)
 }
+
+func WaitingOnCreationMsg(kind string, name string) string {
+	return fmt.Sprintf("Waiting for %s/%s to exist", kind, name)
+}
+
+func WaitingOnAvailableMsg(kind string, name string) string {
+	return fmt.Sprintf("Waiting for %s/%s to be available", kind, name)
+}