OCPBUGS-67294: Require PreprovisioningImage for deprovisioning when cleaning is enabled

internal/controller/metal3.io/baremetalhost_controller.go

@@ -775,6 +775,12 @@ func (r *BareMetalHostReconciler) getPreprovImage(info *reconcileInfo, formats [
 		return nil, fmt.Errorf("failed to retrieve pre-provisioning image data: %w", err)
 	}
 
+	// If the PreprovisioningImage is being deleted, treat it as unavailable
+	if !preprovImage.DeletionTimestamp.IsZero() {
+		info.log.Info("PreprovisioningImage is being deleted, waiting for new one")
+		return nil, nil //nolint:nilnil
+	}
+
 	needsUpdate := false
 	if preprovImage.Labels == nil && len(info.host.Labels) > 0 {
 		preprovImage.Labels = make(map[string]string, len(info.host.Labels))

internal/controller/metal3.io/baremetalhost_controller_test.go

@@ -2646,6 +2646,48 @@ func TestGetPreprovImageNotCurrent(t *testing.T) {
 	assert.Nil(t, imgData)
 }
 
+func TestGetPreprovImageBeingDeleted(t *testing.T) {
+	host := newDefaultHost(t)
+	imageURL := "http://example.test/image.iso"
+	acceptFormats := []metal3api.ImageFormat{metal3api.ImageFormatISO, metal3api.ImageFormatInitRD}
+	now := metav1.Now()
+	image := &metal3api.PreprovisioningImage{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:              host.Name,
+			Namespace:         namespace,
+			DeletionTimestamp: &now, // PPI is being deleted
+			Finalizers:        []string{"test-finalizer"},
+		},
+		Spec: metal3api.PreprovisioningImageSpec{
+			Architecture:  "x86_64",
+			AcceptFormats: acceptFormats,
+		},
+		Status: metal3api.PreprovisioningImageStatus{
+			Architecture: "x86_64",
+			Format:       metal3api.ImageFormatISO,
+			ImageUrl:     imageURL,
+			Conditions: []metav1.Condition{
+				{
+					Type:   string(metal3api.ConditionImageReady),
+					Status: metav1.ConditionTrue,
+				},
+				{
+					Type:   string(metal3api.ConditionImageError),
+					Status: metav1.ConditionFalse,
+				},
+			},
+		},
+	}
+	r := newTestReconciler(t, host, image)
+	i := makeReconcileInfo(host)
+
+	// Even though the image is ready, it should be treated as unavailable
+	// because it has a DeletionTimestamp
+	imgData, err := r.getPreprovImage(i, acceptFormats)
+	require.NoError(t, err)
+	assert.Nil(t, imgData)
+}
+
 func TestPreprovImageAvailable(t *testing.T) {
 	host := newDefaultHost(t)
 	r := newTestReconciler(t, host, newSecret("network_secret_1", nil))

pkg/provisioner/ironic/ironic.go

@@ -364,7 +364,8 @@ func (p *ironicProvisioner) configureNode(data provisioner.ManagementAccessData,
 	}
 
 	switch data.State {
-	case metal3api.StateInspecting,
+	case metal3api.StateDeprovisioning,
+		metal3api.StateInspecting,
 		metal3api.StatePreparing:
 		if deployImageInfo == nil && p.config.havePreprovImgBuilder {
 			result, err = transientError(provisioner.ErrNeedsPreprovisioningImage)

pkg/provisioner/ironic/register_test.go

@@ -1392,3 +1392,77 @@ func TestRegisterDisablePowerOffNotAvail(t *testing.T) {
 	}
 	assert.Equal(t, "current ironic version does not support DisablePowerOff, refusing to manage node", result.ErrorMessage)
 }
+
+func TestRegisterDeprovisioningNeedsPreprovisioningImage(t *testing.T) {
+	// Test that when deprovisioning with cleaning enabled and no
+	// PreprovisioningImage available, ErrNeedsPreprovisioningImage is returned
+	host := makeHost()
+	host.Status.Provisioning.ID = "uuid"
+
+	ironic := testserver.NewIronic(t).
+		WithDrivers().
+		Node(nodes.Node{
+			Name:           host.Namespace + nameSeparator + host.Name,
+			UUID:           host.Status.Provisioning.ID,
+			ProvisionState: string(nodes.Manageable),
+		}).NodeUpdate(nodes.Node{
+		UUID: host.Status.Provisioning.ID,
+	})
+	ironic.Start()
+	defer ironic.Stop()
+
+	auth := clients.AuthConfig{Type: clients.NoAuth}
+	prov, err := newProvisionerWithSettings(host, bmc.Credentials{}, nil, ironic.Endpoint(), auth)
+	if err != nil {
+		t.Fatalf("could not create provisioner: %s", err)
+	}
+	// Enable preprov image builder
+	prov.config.havePreprovImgBuilder = true
+
+	// Test StateDeprovisioning with cleaning enabled (not disabled) and no PreprovisioningImage
+	_, _, err = prov.Register(provisioner.ManagementAccessData{
+		State:                 metal3api.StateDeprovisioning,
+		AutomatedCleaningMode: metal3api.CleaningModeMetadata, // Cleaning enabled
+		// No PreprovisioningImage provided
+	}, false, false)
+
+	require.Error(t, err)
+	assert.ErrorIs(t, err, provisioner.ErrNeedsPreprovisioningImage)
+}
+
+func TestRegisterDeprovisioningCleaningDisabledNoPreprovisioningImage(t *testing.T) {
+	// Test that when deprovisioning with cleaning disabled,
+	// no PreprovisioningImage is required
+	host := makeHost()
+	host.Status.Provisioning.ID = "uuid"
+
+	ironic := testserver.NewIronic(t).
+		WithDrivers().
+		Node(nodes.Node{
+			Name:           host.Namespace + nameSeparator + host.Name,
+			UUID:           host.Status.Provisioning.ID,
+			ProvisionState: string(nodes.Manageable),
+		}).NodeUpdate(nodes.Node{
+		UUID: host.Status.Provisioning.ID,
+	})
+	ironic.Start()
+	defer ironic.Stop()
+
+	auth := clients.AuthConfig{Type: clients.NoAuth}
+	prov, err := newProvisionerWithSettings(host, bmc.Credentials{}, nil, ironic.Endpoint(), auth)
+	if err != nil {
+		t.Fatalf("could not create provisioner: %s", err)
+	}
+	// Enable preprov image builder
+	prov.config.havePreprovImgBuilder = true
+
+	// Test StateDeprovisioning with cleaning disabled - should NOT require PreprovisioningImage
+	result, _, err := prov.Register(provisioner.ManagementAccessData{
+		State:                 metal3api.StateDeprovisioning,
+		AutomatedCleaningMode: metal3api.CleaningModeDisabled, // Cleaning disabled
+		// No PreprovisioningImage provided
+	}, false, false)
+
+	require.NoError(t, err)
+	assert.Empty(t, result.ErrorMessage)
+}