Add external deletion detection and recreation

Modify GetOrCreateOSResource to detect when a managed, non-imported
resource has been deleted externally from OpenStack. Signal the caller
via the typed ExternallyDeleted ReconcileStatus so it can clear
status.id and trigger recreation. Restore the safety guard for
unexpected nil returns from actuators.

Fix Available condition to correctly transition from Unknown to False
when a terminal error is present during resync.

Include unit tests for external deletion handling and E2E tests
covering both managed resource recreation and imported resource
terminal error behavior.

Author: eshulman2

internal/controllers/flavor/adapter_test.go

@@ -0,0 +1,66 @@
+package flavor
+
+import (
+	"testing"
+
+	"k8s.io/utils/ptr"
+
+	orcv1alpha1 "github.com/k-orc/openstack-resource-controller/v2/api/v1alpha1"
+)
+
+func TestFlavorAdapterIsImported(t *testing.T) {
+	tests := []struct {
+		name string
+		spec orcv1alpha1.FlavorSpec
+		want bool
+	}{
+		{
+			name: "no import - created by ORC",
+			spec: orcv1alpha1.FlavorSpec{
+				Resource: &orcv1alpha1.FlavorResourceSpec{},
+			},
+			want: false,
+		},
+		{
+			name: "import is nil - created by ORC",
+			spec: orcv1alpha1.FlavorSpec{
+				Import: nil,
+			},
+			want: false,
+		},
+		{
+			name: "import with non-nil ID",
+			spec: orcv1alpha1.FlavorSpec{
+				Import: &orcv1alpha1.FlavorImport{
+					ID: ptr.To("some-uuid-1234"),
+				},
+			},
+			want: true,
+		},
+		{
+			name: "import with non-nil filter",
+			spec: orcv1alpha1.FlavorSpec{
+				Import: &orcv1alpha1.FlavorImport{
+					Filter: &orcv1alpha1.FlavorFilter{
+						Name: (*orcv1alpha1.OpenStackName)(ptr.To("my-flavor")),
+					},
+				},
+			},
+			want: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			adapter := flavorAdapter{
+				Flavor: &orcv1alpha1.Flavor{
+					Spec: tt.spec,
+				},
+			}
+			got := adapter.IsImported()
+			if got != tt.want {
+				t.Errorf("IsImported() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}

internal/controllers/generic/reconciler/resource_actions.go

@@ -70,16 +70,26 @@ func GetOrCreateOSResource[
 		osResource, reconcileStatus := actuator.GetOSResourceByID(ctx, *resourceID)
 		if needsReschedule, err := reconcileStatus.NeedsReschedule(); needsReschedule {
 			if orcerrors.IsNotFound(err) {
-				// An OpenStack resource we previously referenced has been deleted unexpectedly. We can't recover from this.
+				// The OpenStack resource referenced by status.id no longer exists.
+				// For managed, non-imported resources we trigger recreation by returning
+				// nil with no error: the caller will clear status.id and re-enter the
+				// creation path on the next reconcile.
+				// For unmanaged resources or resources that were originally imported we
+				// cannot recreate them, so we return a terminal error.
+				if objAdapter.GetManagementPolicy() == orcv1alpha1.ManagementPolicyManaged && !objAdapter.IsImported() {
+					log.V(logging.Info).Info("OpenStack resource was deleted externally; will signal caller to clear status ID and trigger recreation")
+					return nil, progress.ExternallyDeleted()
+				}
 				return osResource, progress.WrapError(
 					orcerrors.Terminal(orcv1alpha1.ConditionReasonUnrecoverableError, "resource has been deleted from OpenStack"))
 			} else {
 				return osResource, reconcileStatus
 			}
 		}
-		if osResource != nil {
-			log.V(logging.Verbose).Info("Got existing OpenStack resource", "ID", actuator.GetResourceID(osResource))
+		if osResource == nil {
+			return nil, progress.WrapError(fmt.Errorf("GetOSResourceByID returned nil resource with no error for ID %q", *resourceID))
 		}
+		log.V(logging.Verbose).Info("Got existing OpenStack resource", "ID", actuator.GetResourceID(osResource))
 		return osResource, nil
 	}
 

internal/controllers/generic/reconciler/resource_actions_test.go

@@ -0,0 +1,215 @@
+/*
+Copyright 2025 The ORC Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+	http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+// Package reconciler contains unit tests for external deletion handling in
+// GetOrCreateOSResource.
+//
+// These tests cover all combinations of management policy and import status
+// when a resource's OpenStack counterpart is not found (404), verifying that:
+//
+//   - Managed, ORC-created resources trigger recreation (IsExternallyDeleted)
+//   - Managed, imported-by-ID resources return a terminal error
+//   - Managed, imported-by-filter resources return a terminal error
+//   - Unmanaged resources return a terminal error
+//   - Managed, existing resources continue through the normal update flow
+package reconciler
+
+import (
+	"context"
+	"errors"
+	"testing"
+
+	"github.com/go-logr/logr"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/utils/ptr"
+
+	orcv1alpha1 "github.com/k-orc/openstack-resource-controller/v2/api/v1alpha1"
+	orcerrors "github.com/k-orc/openstack-resource-controller/v2/internal/util/errors"
+)
+
+// --------------------------------------------------------------------------
+// Helpers for building test Flavors used in external-deletion tests.
+//
+// All helpers pre-set the controller finalizer so that GetOrCreateOSResource
+// does not attempt to call the Kubernetes client to add it.
+// --------------------------------------------------------------------------
+
+// managedFlavorImportedByFilter builds a managed Flavor that was imported via
+// a filter (has a non-nil import.Filter) and whose status.ID is already set.
+// When GetOSResourceByID returns a not-found error, IsImported() returns true
+// because GetImportFilter() is non-nil, so the controller must return a
+// terminal error rather than triggering recreation.
+func managedFlavorImportedByFilter(statusID string) fakeAdapter {
+	return fakeAdapter{
+		Flavor: &orcv1alpha1.Flavor{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:       "test-flavor",
+				Namespace:  "default",
+				Finalizers: []string{finalizerFor()},
+			},
+			Spec: orcv1alpha1.FlavorSpec{
+				ManagementPolicy: orcv1alpha1.ManagementPolicyManaged,
+				Import: &orcv1alpha1.FlavorImport{
+					Filter: &orcv1alpha1.FlavorFilter{
+						Name: ptr.To[orcv1alpha1.OpenStackName]("my-flavor"),
+					},
+				},
+			},
+			Status: orcv1alpha1.FlavorStatus{
+				ID: ptr.To(statusID),
+			},
+		},
+	}
+}
+
+// --------------------------------------------------------------------------
+// External deletion tests — all use GetOrCreateOSResource directly.
+// --------------------------------------------------------------------------
+
+// TestGetOrCreateOSResource_ExternalDeletion_ManagedOrcCreated verifies that
+// when a managed, ORC-created resource (not imported) is externally deleted,
+// GetOrCreateOSResource returns IsExternallyDeleted to signal the caller should
+// clear status.ID and trigger recreation on the next reconcile.
+func TestGetOrCreateOSResource_ExternalDeletion_ManagedOrcCreated(t *testing.T) {
+	t.Parallel()
+
+	const resourceID = "orc-created-flavor-id"
+
+	actuator := &noWriteActuator{t: t, readByIDErr: notFoundErr()}
+	adapter := managedFlavorWithStatusID(resourceID)
+
+	got, rs := GetOrCreateOSResource(context.Background(), logr.Discard(), &fakeResourceController{}, adapter, actuator)
+
+	if !rs.IsExternallyDeleted() {
+		t.Fatal("expected IsExternallyDeleted for externally-deleted managed resource")
+	}
+	if got != nil {
+		t.Errorf("expected nil osResource for recreation path, got %v", got)
+	}
+	if !actuator.getByIDCalled {
+		t.Error("GetOSResourceByID was not called: controller must attempt to fetch the resource")
+	}
+}
+
+// TestGetOrCreateOSResource_ExternalDeletion_ManagedImportedByID verifies that
+// when a managed resource originally imported by ID is externally deleted, the
+// controller returns a terminal error. Recreation is not possible for imported
+// resources because we cannot know the original creation parameters.
+func TestGetOrCreateOSResource_ExternalDeletion_ManagedImportedByID(t *testing.T) {
+	t.Parallel()
+
+	const resourceID = "imported-by-id-flavor-id"
+
+	actuator := &noWriteActuator{t: t, readByIDErr: notFoundErr()}
+	adapter := managedFlavorImportedByID(resourceID)
+
+	_, rs := GetOrCreateOSResource(context.Background(), logr.Discard(), &fakeResourceController{}, adapter, actuator)
+
+	_, err := rs.NeedsReschedule()
+	if err == nil {
+		t.Fatal("expected a terminal error for externally-deleted imported-by-ID resource, got nil")
+	}
+
+	var termErr *orcerrors.TerminalError
+	if !errors.As(err, &termErr) {
+		t.Errorf("expected a TerminalError for externally-deleted imported-by-ID resource, got %T: %v", err, err)
+	}
+	if !actuator.getByIDCalled {
+		t.Error("GetOSResourceByID was not called")
+	}
+}
+
+// TestGetOrCreateOSResource_ExternalDeletion_ManagedImportedByFilter verifies
+// that when a managed resource originally imported via a filter is externally
+// deleted, the controller returns a terminal error. As with import-by-ID,
+// recreation is not possible.
+func TestGetOrCreateOSResource_ExternalDeletion_ManagedImportedByFilter(t *testing.T) {
+	t.Parallel()
+
+	const resourceID = "imported-by-filter-flavor-id"
+
+	actuator := &noWriteActuator{t: t, readByIDErr: notFoundErr()}
+	adapter := managedFlavorImportedByFilter(resourceID)
+
+	_, rs := GetOrCreateOSResource(context.Background(), logr.Discard(), &fakeResourceController{}, adapter, actuator)
+
+	_, err := rs.NeedsReschedule()
+	if err == nil {
+		t.Fatal("expected a terminal error for externally-deleted imported-by-filter resource, got nil")
+	}
+
+	var termErr *orcerrors.TerminalError
+	if !errors.As(err, &termErr) {
+		t.Errorf("expected a TerminalError for externally-deleted imported-by-filter resource, got %T: %v", err, err)
+	}
+	if !actuator.getByIDCalled {
+		t.Error("GetOSResourceByID was not called")
+	}
+}
+
+// TestGetOrCreateOSResource_ExternalDeletion_Unmanaged verifies that when an
+// unmanaged resource is externally deleted (404), the controller returns a
+// terminal error instead of calling CreateResource.
+func TestGetOrCreateOSResource_ExternalDeletion_Unmanaged(t *testing.T) {
+	t.Parallel()
+
+	const resourceID = "unmanaged-deleted-flavor-id"
+
+	actuator := &noWriteActuator{t: t, readByIDErr: notFoundErr()}
+	adapter := unmanagedFlavorWithStatusID(resourceID)
+
+	_, rs := GetOrCreateOSResource(context.Background(), logr.Discard(), &fakeResourceController{}, adapter, actuator)
+
+	_, err := rs.NeedsReschedule()
+	if err == nil {
+		t.Fatal("expected a terminal error for externally-deleted unmanaged resource, got nil")
+	}
+
+	var termErr *orcerrors.TerminalError
+	if !errors.As(err, &termErr) {
+		t.Errorf("expected a TerminalError for externally-deleted unmanaged resource, got %T: %v", err, err)
+	}
+	if !actuator.getByIDCalled {
+		t.Error("GetOSResourceByID was not called: controller must attempt to fetch the resource")
+	}
+}
+
+// TestGetOrCreateOSResource_ExternalDeletion_ManagedResourceExists verifies
+// the normal update flow: when a managed, ORC-created resource still exists in
+// OpenStack, GetOrCreateOSResource returns the resource with a nil reconcile
+// status so the caller proceeds with reconciliation (no recreation, no error).
+func TestGetOrCreateOSResource_ExternalDeletion_ManagedResourceExists(t *testing.T) {
+	t.Parallel()
+
+	const resourceID = "existing-managed-flavor-id"
+	osResource := &fakeOSResource{ID: resourceID}
+
+	actuator := &noWriteActuator{t: t, readByIDResult: osResource}
+	adapter := managedFlavorWithStatusID(resourceID)
+
+	got, rs := GetOrCreateOSResource(context.Background(), logr.Discard(), &fakeResourceController{}, adapter, actuator)
+
+	needsReschedule, err := rs.NeedsReschedule()
+	if needsReschedule {
+		t.Fatalf("expected no rescheduling for existing resource, got needsReschedule=%v err=%v", needsReschedule, err)
+	}
+	if got == nil || got.ID != resourceID {
+		t.Errorf("expected osResource with ID=%q, got %v", resourceID, got)
+	}
+	if !actuator.getByIDCalled {
+		t.Error("GetOSResourceByID was not called")
+	}
+}

internal/controllers/generic/status/conditions.go

@@ -40,6 +40,15 @@ func SetCommonConditions[T any](
 	reconcileStatus progress.ReconcileStatus,
 	now metav1.Time,
 ) {
+	// Terminal errors make the resource definitively unavailable.
+	// Override Unknown → False so Available matches the error severity.
+	if availableStatus != metav1.ConditionTrue {
+		var terminalErr *orcerrors.TerminalError
+		if errors.As(reconcileStatus.GetError(), &terminalErr) {
+			availableStatus = metav1.ConditionFalse
+		}
+	}
+
 	availableCondition := applyconfigv1.Condition().
 		WithType(orcv1alpha1.ConditionAvailable).
 		WithStatus(availableStatus).