image: Replace controllerOptions with managementPolicy

Remove controllerOptions in favour of managementPolicy. Adoption is now
handled explicitly in the `import` field.

Author: mdbooth

internal/controllers/image/controller.go

@@ -35,6 +35,23 @@ import (
 	ctrlutil "sigs.k8s.io/cluster-api-provider-openstack/pkg/utils/controllers"
 )
 
+const (
+	Finalizer = "openstack.k-orc.cloud/image"
+
+	FieldOwner = "openstack.k-orc.cloud/imagecontroller"
+	// Field owner of the object finalizer.
+	SSAFinalizerTxn = "finalizer"
+	// Field owner of transient status.
+	SSAStatusTxn = "status"
+	// Field owner of persistent id field.
+	SSAIDTxn = "id"
+)
+
+// ssaFieldOwner returns the field owner for a specific named SSA transaction.
+func ssaFieldOwner(txn string) client.FieldOwner {
+	return client.FieldOwner(FieldOwner + "/" + txn)
+}
+
 const (
 	// The time to wait before reconciling again when we are expecting glance to finish some task and update status.
 	waitForGlanceImageStatusUpdate = 15 * time.Second

internal/controllers/image/reconcile.go

@@ -74,8 +74,8 @@ func (r *orcImageReconciler) reconcileNormal(ctx context.Context, orcImage *orcv
 	log := ctrl.LoggerFrom(ctx)
 	log.V(3).Info("Reconciling image")
 
-	if !controllerutil.ContainsFinalizer(orcImage, orcv1alpha1.ImageControllerFinalizer) {
-		return ctrl.Result{}, r.updateObject(ctx, orcImage)
+	if !controllerutil.ContainsFinalizer(orcImage, Finalizer) {
+		return ctrl.Result{}, r.setFinalizer(ctx, orcImage)
 	}
 
 	var statusOpts []updateStatusOpt
@@ -90,41 +90,81 @@ func (r *orcImageReconciler) reconcileNormal(ctx context.Context, orcImage *orcv
 		}
 
 		err = errors.Join(err, r.updateStatus(ctx, orcImage, statusOpts...))
+
+		var terminalError *capoerrors.TerminalError
+		if errors.As(err, &terminalError) {
+			log.Error(err, "not scheduling further reconciles for terminal error")
+			err = nil
+		}
 	}()
 
 	imageClient, err := r.getImageClient(ctx, orcImage)
 	if err != nil {
 		return ctrl.Result{}, err
 	}
 
-	var glanceImage *images.Image
-	glanceImage, err = getGlanceImage(ctx, orcImage, imageClient)
-	if err != nil {
-		if capoerrors.IsNotFound(err) {
-			// An image we previously created has been deleted unexpected. We can't recover from this.
-			err = capoerrors.Terminal(orcv1alpha1.OpenStackConditionReasonUnrecoverableError, "image has been deleted from glance")
+	glanceImage, err := func() (*images.Image, error) {
+		if orcImage.Status.ID != nil {
+			log.V(4).Info("Fetching existing glance image", "ID", *orcImage.Status.ID)
+
+			image, err := imageClient.GetImage(*orcImage.Status.ID)
+			if capoerrors.IsNotFound(err) {
+				// An image we previously referenced has been deleted unexpectedly. We can't recover from this.
+				err = capoerrors.Terminal(orcv1alpha1.OpenStackConditionReasonUnrecoverableError, "image has been deleted from glance")
+			}
+			return image, err
+		}
+
+		if orcImage.Spec.Import != nil {
+			log.V(4).Info("Importing existing glance image")
+
+			if orcImage.Spec.Import.ID != nil {
+				image, err := imageClient.GetImage(*orcImage.Spec.Import.ID)
+				if capoerrors.IsNotFound(err) {
+					// We assume that an image imported by ID must already exist. It's a terminal error if it doesn't.
+					err = capoerrors.Terminal(orcv1alpha1.OpenStackConditionReasonUnrecoverableError, "referenced image does not exist in glance")
+				}
+				return image, err
+			}
+
+			listOpts := listOptsFromImportFilter(orcImage.Spec.Import.Filter)
+			return getGlanceImageFromList(ctx, listOpts, imageClient)
+
+			// TODO: When we support 'import and manage' we need to implement
+			// setting spec.resource from the discovered glance image here.
 		}
-		return ctrl.Result{}, err
-	}
 
-	if orcImage.GetControllerOptions().GetOnCreate() == orcv1alpha1.ControllerOptionsOnCreateAdopt && glanceImage == nil {
-		log.V(3).Info("Image does not yet exist", "onCreate", orcv1alpha1.ControllerOptionsOnCreateAdopt)
-		addStatus(withProgressMessage("Waiting for glance image to be created externally"))
+		log.V(4).Info("Checking for previously created image")
 
-		return ctrl.Result{
-			RequeueAfter: waitForGlanceImageStatusUpdate,
-		}, err
+		listOpts := listOptsFromCreation(orcImage)
+		return getGlanceImageFromList(ctx, listOpts, imageClient)
+	}()
+	if err != nil {
+		return ctrl.Result{}, err
 	}
 
 	if glanceImage == nil {
+		if orcImage.Spec.Import != nil {
+			log.V(3).Info("Image does not yet exist")
+			addStatus(withProgressMessage("Waiting for glance image to be created externally"))
+
+			return ctrl.Result{RequeueAfter: waitForGlanceImageStatusUpdate}, err
+		}
+
 		glanceImage, err = createImage(ctx, orcImage, imageClient)
 		if err != nil {
 			return ctrl.Result{}, err
 		}
 	}
 	addStatus(withGlanceImage(glanceImage))
 
-	log = log.WithValues("imageID", glanceImage.ID)
+	if orcImage.Status.ID == nil {
+		if err := r.setStatusID(ctx, orcImage, glanceImage.ID); err != nil {
+			return ctrl.Result{}, err
+		}
+	}
+
+	log = log.WithValues("ID", glanceImage.ID)
 	ctx = ctrl.LoggerInto(ctx, log)
 
 	log.V(4).Info("Got glance image", "status", glanceImage.Status)
@@ -143,8 +183,8 @@ func (r *orcImageReconciler) reconcileNormal(ctx context.Context, orcImage *orcv
 
 	// Newly created image, waiting for upload, or... previous upload was interrupted and has now reset
 	case images.ImageStatusQueued:
-		// Don't attempt image creation if we're only adopting
-		if orcImage.GetControllerOptions().GetOnCreate() == orcv1alpha1.ControllerOptionsOnCreateAdopt {
+		// Don't attempt image creation if we're not managing the image
+		if orcImage.Spec.ManagementPolicy == orcv1alpha1.ManagementPolicyUnmanaged {
 			addStatus(withProgressMessage("Waiting for glance image content to be uploaded externally"))
 
 			return ctrl.Result{
@@ -213,7 +253,14 @@ func (r *orcImageReconciler) reconcileDelete(ctx context.Context, orcImage *orcv
 		}
 	}()
 
-	if orcImage.GetControllerOptions().GetOnDelete() == orcv1alpha1.ControllerOptionsOnDeleteDelete {
+	// We won't delete the resource for an unmanaged object, or if onDelete is detach
+	if orcImage.Spec.ManagementPolicy == orcv1alpha1.ManagementPolicyUnmanaged || orcImage.Spec.ManagedOptions.GetOnDelete() == orcv1alpha1.OnDeleteDetach {
+		logPolicy := []any{"managementPolicy", orcImage.Spec.ManagementPolicy}
+		if orcImage.Spec.ManagementPolicy == orcv1alpha1.ManagementPolicyManaged {
+			logPolicy = append(logPolicy, "onDelete", orcImage.Spec.ManagedOptions.GetOnDelete())
+		}
+		log.V(4).Info("Not deleting Glance image due to policy", logPolicy...)
+	} else {
 		imageClient, err := r.getImageClient(ctx, orcImage)
 		if err != nil {
 			return ctrl.Result{}, err
@@ -235,14 +282,15 @@ func (r *orcImageReconciler) reconcileDelete(ctx context.Context, orcImage *orcv
 			}
 			return ctrl.Result{RequeueAfter: 5 * time.Second}, nil
 		}
+
+		log.V(4).Info("Image is deleted")
 	}
 
 	deleted = true
-	log.V(4).Info("Image is deleted")
 
-	// Clear owned fields on the base resource, including the finalizer
+	// Clear the finalizer
 	applyConfig := orcapplyconfigv1alpha1.Image(orcImage.Name, orcImage.Namespace).WithUID(orcImage.UID)
-	return ctrl.Result{}, r.client.Patch(ctx, orcImage, ssa.ApplyConfigPatch(applyConfig), client.ForceOwnership, client.FieldOwner(orcv1alpha1.ImageControllerFieldOwner))
+	return ctrl.Result{}, r.client.Patch(ctx, orcImage, ssa.ApplyConfigPatch(applyConfig), client.ForceOwnership, ssaFieldOwner(SSAFinalizerTxn))
 }
 
 // getGlanceImage returns the glance image associated with an ORC Image, or nil if none was found.
@@ -251,12 +299,6 @@ func (r *orcImageReconciler) reconcileDelete(ctx context.Context, orcImage *orcv
 func getGlanceImage(ctx context.Context, orcImage *orcv1alpha1.Image, imageClient clients.ImageClient) (*images.Image, error) {
 	log := ctrl.LoggerFrom(ctx)
 
-	if orcImage.Status.ImageID != nil {
-		log.V(4).Info("Fetching existing glance image", "imageID", *orcImage.Status.ImageID)
-
-		return imageClient.GetImage(*orcImage.Status.ImageID)
-	}
-
 	log.V(4).Info("Looking for existing glance image to adopt")
 
 	// Check for existing image by name in case we're adopting or failed to write to status
@@ -279,10 +321,38 @@ func getGlanceImage(ctx context.Context, orcImage *orcv1alpha1.Image, imageClien
 
 // getImageName returns the name of the glance image we should use.
 func getImageName(orcImage *orcv1alpha1.Image) string {
-	if orcImage.Spec.ImageName != nil {
-		return *orcImage.Spec.ImageName
+	return ptr.Deref(orcImage.Spec.Resource.Name, orcImage.Name)
+}
+
+func listOptsFromImportFilter(filter *orcv1alpha1.ImageFilter) images.ListOptsBuilder {
+	return images.ListOpts{Name: ptr.Deref(filter.Name, "")}
+}
+
+// listOptsFromCreation returns a listOpts which will return the image which
+// would have been created from the current spec and hopefully no other image.
+// Its purpose is to automatically adopt an image that we created but failed to
+// write to status.id.
+func listOptsFromCreation(orcImage *orcv1alpha1.Image) images.ListOptsBuilder {
+	return images.ListOpts{Name: getImageName(orcImage)}
+}
+
+func getGlanceImageFromList(_ context.Context, listOpts images.ListOptsBuilder, imageClient clients.ImageClient) (*images.Image, error) {
+	glanceImages, err := imageClient.ListImages(listOpts)
+	if err != nil {
+		return nil, err
 	}
-	return orcImage.Name
+
+	if len(glanceImages) == 1 {
+		return &glanceImages[0], nil
+	}
+
+	// No image found
+	if len(glanceImages) == 0 {
+		return nil, nil
+	}
+
+	// Multiple images found
+	return nil, capoerrors.Terminal(orcv1alpha1.OpenStackConditionReasonInvalidConfiguration, fmt.Sprintf("Expected to find exactly one image to import. Found %d", len(glanceImages)))
 }
 
 // glancePropertiesFromStruct populates a properties struct using field values and glance tags defined on the given struct
@@ -306,7 +376,7 @@ func glancePropertiesFromStruct(propStruct interface{}, properties map[string]st
 		field := st.Field(i)
 		glanceTag, ok := field.Tag.Lookup(orcv1alpha1.GlanceTag)
 		if !ok {
-			return fmt.Errorf("glance tag not defined for field %s on struct %T", field.Name, st.Name)
+			panic(fmt.Errorf("glance tag not defined for field %s on struct %T", field.Name, st.Name))
 		}
 
 		value := s.Field(i)
@@ -328,23 +398,35 @@ func glancePropertiesFromStruct(propStruct interface{}, properties map[string]st
 
 // createImage creates a glance image for an ORC Image.
 func createImage(ctx context.Context, orcImage *orcv1alpha1.Image, imageClient clients.ImageClient) (*images.Image, error) {
+	if orcImage.Spec.ManagementPolicy == orcv1alpha1.ManagementPolicyUnmanaged {
+		// Should have been caught by API validation
+		return nil, capoerrors.Terminal(orcv1alpha1.OpenStackConditionReasonInvalidConfiguration, "Not creating unmanaged resource")
+	}
+
 	log := ctrl.LoggerFrom(ctx)
 	log.V(3).Info("Creating image")
 
-	if orcImage.Spec.Content == nil {
+	resource := orcImage.Spec.Resource
+
+	if resource == nil {
+		// Should have been caught by API validation
+		return nil, capoerrors.Terminal(orcv1alpha1.OpenStackConditionReasonInvalidConfiguration, "Creation requested, but spec.resource is not set")
+	}
+
+	if resource.Content == nil {
 		// Should have been caught by API validation
-		return nil, capoerrors.Terminal(orcv1alpha1.OpenStackConditionReasonInvalidConfiguration, "Creation requested, but spec.content is not set")
+		return nil, capoerrors.Terminal(orcv1alpha1.OpenStackConditionReasonInvalidConfiguration, "Creation requested, but spec.resource.content is not set")
 	}
 
-	tags := make([]string, len(orcImage.Spec.Tags))
-	for i := range orcImage.Spec.Tags {
-		tags[i] = string(orcImage.Spec.Tags[i])
+	tags := make([]string, len(resource.Tags))
+	for i := range resource.Tags {
+		tags[i] = string(resource.Tags[i])
 	}
 	// Sort tags before creation to simplify comparisons
 	slices.Sort(tags)
 
 	var minDisk, minMemory int
-	properties := orcImage.Spec.Properties
+	properties := resource.Properties
 	additionalProperties := map[string]string{}
 	if properties != nil {
 		if properties.MinDiskGB != nil {
@@ -360,19 +442,19 @@ func createImage(ctx context.Context, orcImage *orcv1alpha1.Image, imageClient c
 	}
 
 	var visibility *images.ImageVisibility
-	if orcImage.Spec.Visibility != nil {
-		visibility = ptr.To(images.ImageVisibility(*orcImage.Spec.Visibility))
+	if resource.Visibility != nil {
+		visibility = ptr.To(images.ImageVisibility(*resource.Visibility))
 	}
 
 	image, err := imageClient.CreateImage(ctx, &images.CreateOpts{
 		Name:            getImageName(orcImage),
 		Visibility:      visibility,
 		Tags:            tags,
-		ContainerFormat: string(orcImage.Spec.Content.ContainerFormat),
-		DiskFormat:      (string)(orcImage.Spec.Content.DiskFormat),
+		ContainerFormat: string(resource.Content.ContainerFormat),
+		DiskFormat:      (string)(resource.Content.DiskFormat),
 		MinDisk:         minDisk,
 		MinRAM:          minMemory,
-		Protected:       orcImage.Spec.Protected,
+		Protected:       resource.Protected,
 		Properties:      additionalProperties,
 	})