Onboarding fixes

internal/controller/onboarding_controller.go

@@ -22,6 +22,7 @@ import (
 	"errors"
 	"fmt"
 	"net/http"
+	"slices"
 	"strings"
 	"time"
 
@@ -30,7 +31,6 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
-	"k8s.io/client-go/util/retry"
 	ctrl "sigs.k8s.io/controller-runtime"
 	k8sclient "sigs.k8s.io/controller-runtime/pkg/client"
 	logger "sigs.k8s.io/controller-runtime/pkg/log"
@@ -62,7 +62,6 @@ const (
 	testAggregateName         = "tenant_filter_tests"
 	testProjectName           = "test"
 	testDomainName            = "cc3test"
-	testFlavorName            = "c_k_c2_m2_v2"
 	testImageName             = "cirros-d240801-kvm"
 	testPrefixName            = "ohooc-"
 	testVolumeType            = "kvm-pilot"
@@ -79,6 +78,7 @@ type OnboardingController struct {
 }
 
 // +kubebuilder:rbac:groups=kvm.cloud.sap,resources=hypervisors,verbs=get;list;watch;patch
+// +kubebuilder:rbac:groups=kvm.cloud.sap,resources=hypervisors/status,verbs=get;list;watch;patch
 func (r *OnboardingController) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
 	log := logger.FromContext(ctx).WithName(req.Name)
 	ctx = logger.IntoContext(ctx, log)
@@ -103,9 +103,7 @@ func (r *OnboardingController) Reconcile(ctx context.Context, req ctrl.Request)
 
 	// We bail here out, because the openstack api is not the best to poll
 	if hv.Status.HypervisorID == "" || hv.Status.ServiceID == "" {
-		if err := retry.RetryOnConflict(retry.DefaultRetry, func() error {
-			return r.ensureNovaProperties(ctx, hv)
-		}); err != nil {
+		if err := r.ensureNovaProperties(ctx, hv); err != nil {
 			if errors.Is(err, errRequeue) {
 				return ctrl.Result{RequeueAfter: defaultWaitTime}, nil
 			}
@@ -207,6 +205,22 @@ func (r *OnboardingController) initialOnboarding(ctx context.Context, hv *kvmv1.
 		return fmt.Errorf("failed to agg to test aggregate %w", err)
 	}
 
+	var errs []error
+	for aggregateName, aggregate := range aggs {
+		if aggregateName == testAggregateName || aggregateName == zone {
+			continue
+		}
+		if slices.Contains(aggregate.Hosts, host) {
+			if err := removeFromAggregate(ctx, r.computeClient, aggs, host, aggregateName); err != nil {
+				errs = append(errs, err)
+			}
+		}
+	}
+
+	if len(errs) > 0 {
+		return fmt.Errorf("failed to remove host %v from aggregates due to %w", host, errors.Join(errs...))
+	}
+
 	// The service may be forced down previously due to an HA event,
 	// so we need to ensure it not only enabled, but also not forced to be down.
 	falseVal := false
@@ -469,72 +483,22 @@ func (r *OnboardingController) createOrGetTestServer(ctx context.Context, zone,
 		return foundServer, nil
 	}
 
-	flavorPages, err := flavors.ListDetail(r.testComputeClient, nil).AllPages(ctx)
-	if err != nil {
-		return nil, err
-	}
-	extractedFlavors, err := flavors.ExtractFlavors(flavorPages)
+	flavorRef, err := r.findTestFlavor(ctx)
 	if err != nil {
 		return nil, err
 	}
 
-	var flavorRef string
-	for _, flavor := range extractedFlavors {
-		if flavor.Name == testFlavorName {
-			flavorRef = flavor.ID
-			break
-		}
-	}
-
-	if flavorRef == "" {
-		return nil, errors.New("couldn't find flavor")
-	}
-
-	var imageRef string
-
-	imagePages, err := images.List(r.testImageClient, images.ListOpts{Name: testImageName}).AllPages(ctx)
+	imageRef, err := r.findTestImage(ctx)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("could not list networks due to %w", err)
 	}
 
-	imagesList, err := images.ExtractImages(imagePages)
+	networkRef, err := r.findTestNetwork(ctx)
 	if err != nil {
-		return nil, err
-	}
-
-	for _, image := range imagesList {
-		if image.Name == testImageName {
-			imageRef = image.ID
-			break
-		}
-	}
-
-	if imageRef == "" {
-		return nil, errors.New("couldn't find image")
-	}
-
-	falseVal := false
-	networkPages, err := networks.List(r.testNetworkClient, networks.ListOpts{Shared: &falseVal}).AllPages(ctx)
-	if err != nil {
-		return nil, err
-	}
-
-	extractedNetworks, err := networks.ExtractNetworks(networkPages)
-	if err != nil {
-		return nil, err
-	}
-
-	var networkRef string
-	for _, network := range extractedNetworks {
-		networkRef = network.ID
-		break
-	}
-
-	if networkRef == "" {
-		return nil, errors.New("couldn't find network")
+		return nil, fmt.Errorf("could not extract network due to %w", err)
 	}
 
-	log.Info("creating server", "name", serverName)
+	log.Info("creating server", "name", serverName, "flavor", flavorRef)
 	server, err := servers.Create(ctx, r.testComputeClient, servers.CreateOpts{
 		Name:             serverName,
 		AvailabilityZone: fmt.Sprintf("%v:%v", zone, computeHost),
@@ -566,6 +530,67 @@ func (r *OnboardingController) createOrGetTestServer(ctx context.Context, zone,
 	return server, nil
 }
 
+func (r *OnboardingController) findTestNetwork(ctx context.Context) (string, error) {
+	falseVal := false
+	networkPages, err := networks.List(r.testNetworkClient, networks.ListOpts{Shared: &falseVal}).AllPages(ctx)
+	if err != nil {
+		return "", err
+	}
+
+	extractedNetworks, err := networks.ExtractNetworks(networkPages)
+	if err != nil {
+		return "", err
+	}
+
+	for _, network := range extractedNetworks {
+		return network.ID, nil
+	}
+
+	return "", errors.New("couldn't find network")
+}
+
+func (r *OnboardingController) findTestImage(ctx context.Context) (string, error) {
+	imagePages, err := images.List(r.testImageClient, images.ListOpts{Name: testImageName}).AllPages(ctx)
+	if err != nil {
+		return "", err
+	}
+
+	imagesList, err := images.ExtractImages(imagePages)
+	if err != nil {
+		return "", err
+	}
+
+	for _, image := range imagesList {
+		if image.Name == testImageName {
+			return image.ID, nil
+		}
+	}
+
+	return "", errors.New("couldn't find image")
+}
+
+func (r *OnboardingController) findTestFlavor(ctx context.Context) (string, error) {
+	flavorPages, err := flavors.ListDetail(r.testComputeClient, flavors.ListOpts{SortDir: "asc", SortKey: "memory_mb"}).AllPages(ctx)
+	if err != nil {
+		return "", err
+	}
+
+	extractedFlavors, err := flavors.ExtractFlavors(flavorPages)
+	if err != nil {
+		return "", err
+	}
+
+	for _, flavor := range extractedFlavors {
+		_, found := flavor.ExtraSpecs["capabilities:hypervisor_type"]
+		if !found {
+			// Flavor does not restrict the hypervisor-type
+			return flavor.ID, nil
+		}
+	}
+
+	return "", errors.New("couldn't find flavor")
+}
+
 // SetupWithManager sets up the controller with the Manager.
 func (r *OnboardingController) SetupWithManager(mgr ctrl.Manager) error {
 	ctx := context.Background()