Manual fixups to auto-generated code

Signed-off-by: Stephen Finucane <stephenfin@redhat.com>

Author: stephenfin

README.md

@@ -69,6 +69,7 @@ kubectl delete -f $ORC_RELEASE
 |:---------------------------:|:-------:|:-------:|:--------:|
 | flavor                      |         |    ✔    |     ✔    |
 | floating ip                 |         |    ◐    |     ◐    |
+| host aggregate              |         |    ◐    |     ◐    |
 | image                       |    ✔    |    ✔    |     ✔    |
 | network                     |         |    ◐    |     ◐    |
 | port                        |         |    ◐    |     ◐    |

api/v1alpha1/hostaggregate_types.go

@@ -18,24 +18,18 @@ package v1alpha1
 
 // HostAggregateResourceSpec contains the desired state of the resource.
 type HostAggregateResourceSpec struct {
+	// TODO(stephenfin): Enforce that the name should not contain a colon.
 	// name will be the name of the created resource. If not specified, the
 	// name of the ORC object will be used.
 	// +optional
-	Name *OpenStackName `json:"name,omitempty"`
+	Name *OpenStackName `json:"name"`
 
-	// description is a human-readable description for the resource.
+	// availabilityZone is the availability zone of the host aggregate.
 	// +kubebuilder:validation:MinLength:=1
 	// +kubebuilder:validation:MaxLength:=255
+	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="availabilityZone is immutable"
 	// +optional
-	Description *string `json:"description,omitempty"`
-
-	// TODO(scaffolding): Add more types.
-	// To see what is supported, you can take inspiration from the CreateOpts stucture from
-	// github.com/gophercloud/gophercloud/v2/openstack/compute/v2/aggregates
-	//
-	// Until you have implemented mutability for the field, you must add a CEL validation
-	// preventing the field being modified:
-	// `// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="<fieldname> is immutable"`
+	AvailabilityZone *string `json:"availabilityZone,omitempty"`
 }
 
 // HostAggregateFilter defines an existing resource by its properties
@@ -44,31 +38,38 @@ type HostAggregateFilter struct {
 	// name of the existing resource
 	// +optional
 	Name *OpenStackName `json:"name,omitempty"`
-
-	// description of the existing resource
-	// +kubebuilder:validation:MinLength:=1
-	// +kubebuilder:validation:MaxLength:=255
-	// +optional
-	Description *string `json:"description,omitempty"`
-
-	// TODO(scaffolding): Add more types.
-	// To see what is supported, you can take inspiration from the ListOpts stucture from
-	// github.com/gophercloud/gophercloud/v2/openstack/compute/v2/aggregates
 }
 
 // HostAggregateResourceStatus represents the observed state of the resource.
 type HostAggregateResourceStatus struct {
+	// The availability zone of the host aggregate.
+	AvailabilityZone string `json:"availabilityZone"`
+
+	// A list of host ids in this aggregate.
+	//Hosts []string `json:"hosts"`
+
+	// The ID of the host aggregate.
+	ID int `json:"id"`
+
+	// Metadata key and value pairs associate with the aggregate.
+	// Metadata map[string]string `json:"metadata"`
+
 	// name is a Human-readable name for the resource. Might not be unique.
 	// +kubebuilder:validation:MaxLength=1024
-	// +optional
-	Name string `json:"name,omitempty"`
+	Name string `json:"name"`
 
-	// description is a human-readable description for the resource.
-	// +kubebuilder:validation:MaxLength=1024
-	// +optional
-	Description string `json:"description,omitempty"`
+	// The date and time when the resource was created.
+	// CreatedAt time.Time `json:"-"`
+
+	// The date and time when the resource was updated,
+	// if the resource has not been updated, this field will show as null.
+	// UpdatedAt time.Time `json:"-"`
+
+	// The date and time when the resource was deleted,
+	// if the resource has not been deleted yet, this field will be null.
+	// DeletedAt time.Time `json:"-"`
 
-	// TODO(scaffolding): Add more types.
-	// To see what is supported, you can take inspiration from the HostAggregate stucture from
-	// github.com/gophercloud/gophercloud/v2/openstack/compute/v2/aggregates
+	// A boolean indicates whether this aggregate is deleted or not,
+	// if it has not been deleted, false will appear.
+	// Deleted bool `json:"deleted"`
 }

cmd/manager/main.go

@@ -30,6 +30,7 @@ import (
 	"github.com/k-orc/openstack-resource-controller/v2/internal/controllers/flavor"
 	"github.com/k-orc/openstack-resource-controller/v2/internal/controllers/floatingip"
 	"github.com/k-orc/openstack-resource-controller/v2/internal/controllers/generic/interfaces"
+	"github.com/k-orc/openstack-resource-controller/v2/internal/controllers/hostaggregate"
 	"github.com/k-orc/openstack-resource-controller/v2/internal/controllers/image"
 	"github.com/k-orc/openstack-resource-controller/v2/internal/controllers/network"
 	"github.com/k-orc/openstack-resource-controller/v2/internal/controllers/port"
@@ -112,6 +113,7 @@ func main() {
 		server.New(scopeFactory),
 		servergroup.New(scopeFactory),
 		project.New(scopeFactory),
+		hostaggregate.New(scopeFactory),
 	}
 
 	restConfig := ctrl.GetConfigOrDie()

cmd/resource-generator/main.go

@@ -85,6 +85,9 @@ var resources []templateFields = []templateFields{
 		IsNotNamed:       true, // FloatingIP is not named in OpenStack
 		ExistingOSClient: true,
 	},
+	{
+		Name: "HostAggregate",
+	},
 	{
 		Name: "Image",
 		SpecExtraValidations: []specExtraValidation{

config/samples/openstack_v1alpha1_hostaggregate.yaml

@@ -5,10 +5,8 @@ metadata:
   name: hostaggregate-sample
 spec:
   cloudCredentialsRef:
-    # TODO(scaffolding): Use openstack-admin if the resouce needs admin credentials to be created
-    cloudName: openstack
+    cloudName: openstack-admin
     secretName: openstack-clouds
   managementPolicy: managed
   resource:
-    description: Sample HostAggregate
-    # TODO(scaffolding): Add all fields the resource supports
+    availabilityZone: sample-az1

internal/controllers/hostaggregate/actuator.go

@@ -19,10 +19,10 @@ package hostaggregate
 import (
 	"context"
 	"iter"
+	"strconv"
 
 	"github.com/gophercloud/gophercloud/v2/openstack/compute/v2/aggregates"
 	corev1 "k8s.io/api/core/v1"
-	"k8s.io/utils/ptr"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
@@ -36,7 +36,7 @@ import (
 
 // OpenStack resource types
 type (
-	osResourceT = aggregates.HostAggregate
+	osResourceT = aggregates.Aggregate
 
 	createResourceActuator = interfaces.CreateResourceActuator[orcObjectPT, orcObjectT, filterT, osResourceT]
 	deleteResourceActuator = interfaces.DeleteResourceActuator[orcObjectPT, orcObjectT, osResourceT]
@@ -52,12 +52,14 @@ type hostaggregateActuator struct {
 var _ createResourceActuator = hostaggregateActuator{}
 var _ deleteResourceActuator = hostaggregateActuator{}
 
+// TODO(stephenfin): I suspect we need to change the interface since Nova expects integer IDs
 func (hostaggregateActuator) GetResourceID(osResource *osResourceT) string {
-	return osResource.ID
+	return strconv.Itoa(osResource.ID)
 }
 
 func (actuator hostaggregateActuator) GetOSResourceByID(ctx context.Context, id string) (*osResourceT, progress.ReconcileStatus) {
-	resource, err := actuator.osClient.GetHostAggregate(ctx, id)
+	iid, err := strconv.Atoi(id)
+	resource, err := actuator.osClient.GetHostAggregate(ctx, iid)
 	if err != nil {
 		return nil, progress.WrapError(err)
 	}
@@ -70,30 +72,36 @@ func (actuator hostaggregateActuator) ListOSResourcesForAdoption(ctx context.Con
 		return nil, false
 	}
 
-	// TODO(scaffolding) If you need to filter resources on fields that the List() function
-	// of gophercloud does not support, it's possible to perform client-side filtering.
-	// Check osclients.ResourceFilter
+	var filters []osclients.ResourceFilter[osResourceT]
 
-	listOpts := aggregates.ListOpts{
-		Name:        getResourceName(orcObject),
-		Description: ptr.Deref(resourceSpec.Description, ""),
-	}
+	// NOTE: The API doesn't allow filtering by name or description, we'll have to do it client-side.
+	filters = append(filters,
+		func(f *aggregates.Aggregate) bool {
+			name := getResourceName(orcObject)
+			// Compare non-pointer values
+			return f.Name == name
+		},
+	)
 
-	return actuator.osClient.ListHostAggregates(ctx, listOpts), true
+	return actuator.listOSResources(ctx, filters), true
 }
 
 func (actuator hostaggregateActuator) ListOSResourcesForImport(ctx context.Context, obj orcObjectPT, filter filterT) (iter.Seq2[*osResourceT, error], progress.ReconcileStatus) {
-	// TODO(scaffolding) If you need to filter resources on fields that the List() function
-	// of gophercloud does not support, it's possible to perform client-side filtering.
-	// Check osclients.ResourceFilter
+	var filters []osclients.ResourceFilter[osResourceT]
 
-	listOpts := aggregates.ListOpts{
-		Name:        string(ptr.Deref(filter.Name, "")),
-		Description: string(ptr.Deref(filter.Description, "")),
-		// TODO(scaffolding): Add more import filters
+	// NOTE: The API doesn't allow filtering by name or description, we'll have to do it client-side.
+	if filter.Name != nil {
+		filters = append(filters, func(f *aggregates.Aggregate) bool {
+			return f.Name == string(*filter.Name)
+		})
 	}
 
-	return actuator.osClient.ListHostAggregates(ctx, listOpts), nil
+	return actuator.listOSResources(ctx, filters), nil
+}
+
+func (actuator hostaggregateActuator) listOSResources(ctx context.Context, filters []osclients.ResourceFilter[osResourceT]) iter.Seq2[*aggregates.Aggregate, error] {
+	volumetypes := actuator.osClient.ListHostAggregates(ctx)
+	return osclients.Filter(volumetypes, filters...)
 }
 
 func (actuator hostaggregateActuator) CreateResource(ctx context.Context, obj orcObjectPT) (*osResourceT, progress.ReconcileStatus) {
@@ -106,8 +114,6 @@ func (actuator hostaggregateActuator) CreateResource(ctx context.Context, obj or
 	}
 	createOpts := aggregates.CreateOpts{
 		Name:        getResourceName(obj),
-		Description: ptr.Deref(resource.Description, ""),
-		// TODO(scaffolding): Add more fields
 	}
 
 	osResource, err := actuator.osClient.CreateHostAggregate(ctx, createOpts)
@@ -138,7 +144,6 @@ func (actuator hostaggregateActuator) updateResource(ctx context.Context, obj or
 	updateOpts := aggregates.UpdateOpts{}
 
 	handleNameUpdate(&updateOpts, obj, osResource)
-	handleDescriptionUpdate(&updateOpts, resource, osResource)
 
 	// TODO(scaffolding): add handler for all fields supporting mutability
 
@@ -167,7 +172,7 @@ func (actuator hostaggregateActuator) updateResource(ctx context.Context, obj or
 }
 
 func needsUpdate(updateOpts aggregates.UpdateOpts) (bool, error) {
-	updateOptsMap, err := updateOpts.ToHostAggregateUpdateMap()
+	updateOptsMap, err := updateOpts.ToAggregatesUpdateMap()
 	if err != nil {
 		return false, err
 	}
@@ -183,14 +188,7 @@ func needsUpdate(updateOpts aggregates.UpdateOpts) (bool, error) {
 func handleNameUpdate(updateOpts *aggregates.UpdateOpts, obj orcObjectPT, osResource *osResourceT) {
 	name := getResourceName(obj)
 	if osResource.Name != name {
-		updateOpts.Name = &name
-	}
-}
-
-func handleDescriptionUpdate(updateOpts *aggregates.UpdateOpts, resource *resourceSpecT, osResource *osResourceT) {
-	description := ptr.Deref(resource.Description, "")
-	if osResource.Description != description {
-		updateOpts.Description = &description
+		updateOpts.Name = name
 	}
 }
 

internal/controllers/hostaggregate/actuator_test.go

@@ -37,7 +37,7 @@ func TestNeedsUpdate(t *testing.T) {
 		},
 		{
 			name:         "Updated opts",
-			updateOpts:   aggregates.UpdateOpts{Name: ptr.To("updated")},
+			updateOpts:   aggregates.UpdateOpts{Name: "updated"},
 			expectChange: true,
 		},
 	}
@@ -86,34 +86,3 @@ func TestHandleNameUpdate(t *testing.T) {
 
 	}
 }
-
-func TestHandleDescriptionUpdate(t *testing.T) {
-	ptrToDescription := ptr.To[string]
-	testCases := []struct {
-		name          string
-		newValue      *string
-		existingValue string
-		expectChange  bool
-	}{
-		{name: "Identical", newValue: ptrToDescription("desc"), existingValue: "desc", expectChange: false},
-		{name: "Different", newValue: ptrToDescription("new-desc"), existingValue: "desc", expectChange: true},
-		{name: "No value provided, existing is set", newValue: nil, existingValue: "desc", expectChange: true},
-		{name: "No value provided, existing is empty", newValue: nil, existingValue: "", expectChange: false},
-	}
-
-	for _, tt := range testCases {
-		t.Run(tt.name, func(t *testing.T) {
-			resource := &orcv1alpha1.HostAggregateResourceSpec{Description: tt.newValue}
-			osResource := &osResourceT{Description: tt.existingValue}
-
-			updateOpts := aggregates.UpdateOpts{}
-			handleDescriptionUpdate(&updateOpts, resource, osResource)
-
-			got, _ := needsUpdate(updateOpts)
-			if got != tt.expectChange {
-				t.Errorf("Expected change: %v, got: %v", tt.expectChange, got)
-			}
-		})
-
-	}
-}

internal/controllers/hostaggregate/status.go

@@ -52,12 +52,5 @@ func (hostaggregateStatusWriter) ApplyResourceStatus(log logr.Logger, osResource
 	resourceStatus := orcapplyconfigv1alpha1.HostAggregateResourceStatus().
 		WithName(osResource.Name)
 
-	// TODO(scaffolding): add all of the fields supported in the HostAggregateResourceStatus struct
-	// If a zero-value isn't expected in the response, place it behind a conditional
-
-	if osResource.Description != "" {
-		resourceStatus.WithDescription(osResource.Description)
-	}
-
 	statusApply.WithResource(resourceStatus)
 }

internal/controllers/hostaggregate/tests/hostaggregate-create-full/00-assert.yaml

@@ -6,8 +6,6 @@ metadata:
 status:
   resource:
     name: hostaggregate-create-full-override
-    description: HostAggregate from "create full" test
-    # TODO(scaffolding): Add all fields the resource supports
   conditions:
   - type: Available
     status: "True"
@@ -25,4 +23,3 @@ resourceRefs:
       ref: hostaggregate
 assertAll:
     - celExpr: "hostaggregate.status.id != ''"
-    # TODO(scaffolding): Add more checks

internal/controllers/hostaggregate/tests/hostaggregate-create-full/00-create-resource.yaml

@@ -5,11 +5,8 @@ metadata:
   name: hostaggregate-create-full
 spec:
   cloudCredentialsRef:
-    # TODO(scaffolding): Use openstack-admin if the resouce needs admin credentials to be created
-    cloudName: openstack
+    cloudName: openstack-admin
     secretName: openstack-clouds
   managementPolicy: managed
   resource:
     name: hostaggregate-create-full-override
-    description: HostAggregate from "create full" test
-    # TODO(scaffolding): Add all fields the resource supports