Fix bugs + CI issues

Author: aldokimi

.gitignore

@@ -33,6 +33,7 @@ __pycache__/
 /openapi.json
 /.custom-deploy
 /.custom-gcl.yml
+.cursor/
 
 # kuttl generates a kubeconfig
 /kubeconfig

api/v1alpha1/volumesnapshot_types.go

@@ -68,11 +68,9 @@ type VolumeSnapshotFilter struct {
 	// +optional
 	Status *string `json:"status,omitempty"`
 
-	// volumeID is the ID of the volume the snapshot was created from
-	// +kubebuilder:validation:MinLength:=1
-	// +kubebuilder:validation:MaxLength:=255
+	// volumeRef references the ORC Volume used to filter snapshots by source volume.
 	// +optional
-	VolumeID *string `json:"volumeID,omitempty"`
+	VolumeRef *KubernetesNameRef `json:"volumeRef,omitempty"`
 }
 
 // VolumeSnapshotResourceStatus represents the observed state of the resource.

api/v1alpha1/zz_generated.deepcopy.go

@@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
 metadata:
   annotations:
-    controller-gen.kubebuilder.io/version: v0.17.1
+    controller-gen.kubebuilder.io/version: v0.20.1
   name: volumesnapshots.openstack.k-orc.cloud
 spec:
   group: openstack.k-orc.cloud
@@ -107,10 +107,10 @@ spec:
                         maxLength: 255
                         minLength: 1
                         type: string
-                      volumeID:
-                        description: volumeID is the ID of the volume the snapshot
-                          was created from
-                        maxLength: 255
+                      volumeRef:
+                        description: volumeRef references the ORC Volume used to filter
+                          snapshots by source volume.
+                        maxLength: 253
                         minLength: 1
                         type: string
                     type: object

cmd/models-schema/zz_generated.openapi.go

@@ -23,6 +23,8 @@ import (
 
 	"github.com/gophercloud/gophercloud/v2/openstack/blockstorage/v3/snapshots"
 	corev1 "k8s.io/api/core/v1"
+	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"
@@ -100,15 +102,41 @@ func (actuator volumesnapshotActuator) ListOSResourcesForImport(ctx context.Cont
 		})
 	}
 
+	volumeID, reconcileStatus := actuator.getVolumeIDForImport(ctx, obj, filter)
+	if needsReschedule, _ := reconcileStatus.NeedsReschedule(); needsReschedule {
+		return nil, reconcileStatus
+	}
+
 	listOpts := snapshots.ListOpts{
 		Name:     string(ptr.Deref(filter.Name, "")),
 		Status:   ptr.Deref(filter.Status, ""),
-		VolumeID: ptr.Deref(filter.VolumeID, ""),
+		VolumeID: volumeID,
 	}
 
 	return actuator.listOSResources(ctx, filters, listOpts), nil
 }
 
+func (actuator volumesnapshotActuator) getVolumeIDForImport(ctx context.Context, obj orcObjectPT, filter filterT) (string, progress.ReconcileStatus) {
+	if filter.VolumeRef == nil {
+		return "", nil
+	}
+
+	volumeName := string(*filter.VolumeRef)
+	volume := &orcv1alpha1.Volume{}
+	if err := actuator.k8sClient.Get(ctx, types.NamespacedName{Name: volumeName, Namespace: obj.GetNamespace()}, volume); err != nil {
+		if apierrors.IsNotFound(err) {
+			return "", progress.WaitingOnObject("Volume", volumeName, progress.WaitingOnCreation)
+		}
+		return "", progress.WrapError(err)
+	}
+
+	if !orcv1alpha1.IsAvailable(volume) || volume.Status.ID == nil || *volume.Status.ID == "" {
+		return "", progress.WaitingOnObject("Volume", volumeName, progress.WaitingOnReady)
+	}
+
+	return *volume.Status.ID, nil
+}
+
 func (actuator volumesnapshotActuator) listOSResources(ctx context.Context, filters []osclients.ResourceFilter[osResourceT], listOpts snapshots.ListOptsBuilder) iter.Seq2[*snapshots.Snapshot, error] {
 	results := actuator.osClient.ListVolumeSnapshots(ctx, listOpts)
 	return osclients.Filter(results, filters...)

config/crd/bases/openstack.k-orc.cloud_volumesnapshots.yaml

@@ -17,13 +17,60 @@ limitations under the License.
 package volumesnapshot
 
 import (
+	"context"
+	"errors"
+	"iter"
+	"strings"
 	"testing"
 
 	"github.com/gophercloud/gophercloud/v2/openstack/blockstorage/v3/snapshots"
-	orcv1alpha1 "github.com/k-orc/openstack-resource-controller/v2/api/v1alpha1"
+	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/utils/ptr"
+	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+
+	orcv1alpha1 "github.com/k-orc/openstack-resource-controller/v2/api/v1alpha1"
 )
 
+var errNotImplemented = errors.New("not implemented")
+
+type testVolumeSnapshotClient struct {
+	listOpts   snapshots.ListOptsBuilder
+	listCalled bool
+}
+
+func (c *testVolumeSnapshotClient) ListVolumeSnapshots(_ context.Context, listOpts snapshots.ListOptsBuilder) iter.Seq2[*snapshots.Snapshot, error] {
+	c.listCalled = true
+	c.listOpts = listOpts
+	return func(yield func(*snapshots.Snapshot, error) bool) {}
+}
+
+func (c *testVolumeSnapshotClient) CreateVolumeSnapshot(_ context.Context, _ snapshots.CreateOptsBuilder) (*snapshots.Snapshot, error) {
+	return nil, errNotImplemented
+}
+
+func (c *testVolumeSnapshotClient) DeleteVolumeSnapshot(_ context.Context, _ string) error {
+	return errNotImplemented
+}
+
+func (c *testVolumeSnapshotClient) GetVolumeSnapshot(_ context.Context, _ string) (*snapshots.Snapshot, error) {
+	return nil, errNotImplemented
+}
+
+func (c *testVolumeSnapshotClient) UpdateVolumeSnapshot(_ context.Context, _ string, _ snapshots.UpdateOptsBuilder) (*snapshots.Snapshot, error) {
+	return nil, errNotImplemented
+}
+
+func newTestScheme(t *testing.T) *runtime.Scheme {
+	t.Helper()
+
+	scheme := runtime.NewScheme()
+	if err := orcv1alpha1.AddToScheme(scheme); err != nil {
+		t.Fatalf("failed adding API scheme: %v", err)
+	}
+	return scheme
+}
+
 func TestNeedsUpdate(t *testing.T) {
 	testCases := []struct {
 		name         string
@@ -44,7 +91,10 @@ func TestNeedsUpdate(t *testing.T) {
 
 	for _, tt := range testCases {
 		t.Run(tt.name, func(t *testing.T) {
-			got, _ := needsUpdate(tt.updateOpts)
+			got, err := needsUpdate(tt.updateOpts)
+			if err != nil {
+				t.Fatalf("Expected no error, got: %v", err)
+			}
 			if got != tt.expectChange {
 				t.Errorf("Expected change: %v, got: %v", tt.expectChange, got)
 			}
@@ -78,7 +128,10 @@ func TestHandleNameUpdate(t *testing.T) {
 			updateOpts := snapshots.UpdateOpts{}
 			handleNameUpdate(&updateOpts, resource, osResource)
 
-			got, _ := needsUpdate(updateOpts)
+			got, err := needsUpdate(updateOpts)
+			if err != nil {
+				t.Fatalf("Expected no error, got: %v", err)
+			}
 			if got != tt.expectChange {
 				t.Errorf("Expected change: %v, got: %v", tt.expectChange, got)
 			}
@@ -109,11 +162,184 @@ func TestHandleDescriptionUpdate(t *testing.T) {
 			updateOpts := snapshots.UpdateOpts{}
 			handleDescriptionUpdate(&updateOpts, resource, osResource)
 
-			got, _ := needsUpdate(updateOpts)
+			got, err := needsUpdate(updateOpts)
+			if err != nil {
+				t.Fatalf("Expected no error, got: %v", err)
+			}
 			if got != tt.expectChange {
 				t.Errorf("Expected change: %v, got: %v", tt.expectChange, got)
 			}
 		})
 
 	}
 }
+
+func TestGetVolumeIDForImport(t *testing.T) {
+	ctx := context.Background()
+	namespace := "default"
+	volumeRef := orcv1alpha1.KubernetesNameRef("import-volume")
+	orcObject := &orcv1alpha1.VolumeSnapshot{
+		ObjectMeta: v1.ObjectMeta{
+			Name:      "import-snapshot",
+			Namespace: namespace,
+		},
+	}
+
+	t.Run("waits for missing volume", func(t *testing.T) {
+		actuator := volumesnapshotActuator{
+			k8sClient: fake.NewClientBuilder().
+				WithScheme(newTestScheme(t)).
+				Build(),
+		}
+
+		volumeID, reconcileStatus := actuator.getVolumeIDForImport(ctx, orcObject, orcv1alpha1.VolumeSnapshotFilter{
+			VolumeRef: &volumeRef,
+		})
+		if volumeID != "" {
+			t.Fatalf("expected empty volume ID, got %q", volumeID)
+		}
+		if reconcileStatus == nil {
+			t.Fatalf("expected reconcile status, got nil")
+		}
+		needsReschedule, err := reconcileStatus.NeedsReschedule()
+		if err != nil {
+			t.Fatalf("expected no error, got %v", err)
+		}
+		if !needsReschedule {
+			t.Fatalf("expected reconcile status to require reschedule")
+		}
+		if !strings.Contains(strings.Join(reconcileStatus.GetProgressMessages(), "\n"), "Waiting for Volume/import-volume to be created") {
+			t.Fatalf("expected waiting for volume creation message, got %v", reconcileStatus.GetProgressMessages())
+		}
+	})
+
+	t.Run("waits for not-ready volume", func(t *testing.T) {
+		volume := &orcv1alpha1.Volume{
+			ObjectMeta: v1.ObjectMeta{
+				Name:      "import-volume",
+				Namespace: namespace,
+			},
+		}
+
+		actuator := volumesnapshotActuator{
+			k8sClient: fake.NewClientBuilder().
+				WithScheme(newTestScheme(t)).
+				WithObjects(volume).
+				Build(),
+		}
+
+		volumeID, reconcileStatus := actuator.getVolumeIDForImport(ctx, orcObject, orcv1alpha1.VolumeSnapshotFilter{
+			VolumeRef: &volumeRef,
+		})
+		if volumeID != "" {
+			t.Fatalf("expected empty volume ID, got %q", volumeID)
+		}
+		if reconcileStatus == nil {
+			t.Fatalf("expected reconcile status, got nil")
+		}
+		needsReschedule, err := reconcileStatus.NeedsReschedule()
+		if err != nil {
+			t.Fatalf("expected no error, got %v", err)
+		}
+		if !needsReschedule {
+			t.Fatalf("expected reconcile status to require reschedule")
+		}
+		if !strings.Contains(strings.Join(reconcileStatus.GetProgressMessages(), "\n"), "Waiting for Volume/import-volume to be ready") {
+			t.Fatalf("expected waiting for volume ready message, got %v", reconcileStatus.GetProgressMessages())
+		}
+	})
+
+	t.Run("returns volume ID when volume is ready", func(t *testing.T) {
+		volume := &orcv1alpha1.Volume{
+			ObjectMeta: v1.ObjectMeta{
+				Name:      "import-volume",
+				Namespace: namespace,
+			},
+			Status: orcv1alpha1.VolumeStatus{
+				ID: ptr.To("volume-id"),
+				Conditions: []v1.Condition{
+					{
+						Type:   orcv1alpha1.ConditionAvailable,
+						Status: v1.ConditionTrue,
+					},
+				},
+			},
+		}
+
+		actuator := volumesnapshotActuator{
+			k8sClient: fake.NewClientBuilder().
+				WithScheme(newTestScheme(t)).
+				WithObjects(volume).
+				Build(),
+		}
+
+		volumeID, reconcileStatus := actuator.getVolumeIDForImport(ctx, orcObject, orcv1alpha1.VolumeSnapshotFilter{
+			VolumeRef: &volumeRef,
+		})
+		if reconcileStatus != nil {
+			t.Fatalf("expected nil reconcile status, got %v", reconcileStatus)
+		}
+		if volumeID != "volume-id" {
+			t.Fatalf("expected volume-id, got %q", volumeID)
+		}
+	})
+}
+
+func TestListOSResourcesForImport_ResolvesVolumeRefToVolumeID(t *testing.T) {
+	ctx := context.Background()
+	namespace := "default"
+	volumeRef := orcv1alpha1.KubernetesNameRef("import-volume")
+
+	volume := &orcv1alpha1.Volume{
+		ObjectMeta: v1.ObjectMeta{
+			Name:      "import-volume",
+			Namespace: namespace,
+		},
+		Status: orcv1alpha1.VolumeStatus{
+			ID: ptr.To("volume-id"),
+			Conditions: []v1.Condition{
+				{
+					Type:   orcv1alpha1.ConditionAvailable,
+					Status: v1.ConditionTrue,
+				},
+			},
+		},
+	}
+
+	osClient := &testVolumeSnapshotClient{}
+	actuator := volumesnapshotActuator{
+		osClient: osClient,
+		k8sClient: fake.NewClientBuilder().
+			WithScheme(newTestScheme(t)).
+			WithObjects(volume).
+			Build(),
+	}
+
+	orcObject := &orcv1alpha1.VolumeSnapshot{
+		ObjectMeta: v1.ObjectMeta{
+			Name:      "import-snapshot",
+			Namespace: namespace,
+		},
+	}
+
+	filter := orcv1alpha1.VolumeSnapshotFilter{
+		Name:      ptr.To(orcv1alpha1.OpenStackName("snapshot-name")),
+		VolumeRef: &volumeRef,
+	}
+
+	_, reconcileStatus := actuator.ListOSResourcesForImport(ctx, orcObject, filter)
+	if reconcileStatus != nil {
+		t.Fatalf("expected nil reconcile status, got %v", reconcileStatus)
+	}
+	if !osClient.listCalled {
+		t.Fatalf("expected ListVolumeSnapshots to be called")
+	}
+
+	listOpts, ok := osClient.listOpts.(snapshots.ListOpts)
+	if !ok {
+		t.Fatalf("expected snapshots.ListOpts, got %T", osClient.listOpts)
+	}
+	if listOpts.VolumeID != "volume-id" {
+		t.Fatalf("expected ListOpts.VolumeID to be volume-id, got %q", listOpts.VolumeID)
+	}
+}