Merge pull request #1845 from shiftstack/issue_1835

🐛 Fill up OpenStack cluster ReferencedResources with Image ID

Author: k8s-ci-robot

controllers/openstackcluster_controller.go

@@ -119,6 +119,16 @@ func (r *OpenStackClusterReconciler) Reconcile(ctx context.Context, req ctrl.Req
 		return reconcile.Result{}, err
 	}
 
+	if openStackCluster.Spec.Bastion != nil && openStackCluster.Spec.Bastion.Enabled {
+		if openStackCluster.Status.Bastion == nil {
+			openStackCluster.Status.Bastion = &infrav1.BastionStatus{}
+		}
+		err = compute.ResolveReferencedMachineResources(scope, &openStackCluster.Spec.Bastion.Instance, &openStackCluster.Status.Bastion.ReferencedResources)
+		if err != nil {
+			return reconcile.Result{}, err
+		}
+	}
+
 	// Handle deleted clusters
 	if !openStackCluster.DeletionTimestamp.IsZero() {
 		return r.reconcileDelete(ctx, scope, cluster, openStackCluster)
@@ -253,11 +263,7 @@ func deleteBastion(scope scope.Scope, cluster *clusterv1.Cluster, openStackClust
 			}
 		}
 
-		instanceSpec, err := bastionToInstanceSpec(openStackCluster, cluster.Name, computeService)
-		if err != nil {
-			return fmt.Errorf("failed to create bastion InstanceSpec: %w", err)
-		}
-
+		instanceSpec := bastionToInstanceSpec(openStackCluster, cluster.Name)
 		if err = computeService.DeleteInstance(openStackCluster, openStackCluster, instanceStatus, instanceSpec); err != nil {
 			handleUpdateOSCError(openStackCluster, fmt.Errorf("failed to delete bastion: %w", err))
 			return fmt.Errorf("failed to delete bastion: %w", err)
@@ -340,11 +346,7 @@ func reconcileBastion(scope scope.Scope, cluster *clusterv1.Cluster, openStackCl
 		return reconcile.Result{}, err
 	}
 
-	instanceSpec, err := bastionToInstanceSpec(openStackCluster, cluster.Name, computeService)
-	if err != nil {
-		return reconcile.Result{}, fmt.Errorf("failed to create bastion InstanceSpec: %w", err)
-	}
-
+	instanceSpec := bastionToInstanceSpec(openStackCluster, cluster.Name)
 	bastionHash, err := compute.HashInstanceSpec(instanceSpec)
 	if err != nil {
 		return reconcile.Result{}, fmt.Errorf("failed computing bastion hash from instance spec: %w", err)
@@ -435,17 +437,12 @@ func reconcileBastion(scope scope.Scope, cluster *clusterv1.Cluster, openStackCl
 	return ctrl.Result{}, nil
 }
 
-func bastionToInstanceSpec(openStackCluster *infrav1.OpenStackCluster, clusterName string, computeService *compute.Service) (*compute.InstanceSpec, error) {
-	imageID, err := computeService.GetImageID(openStackCluster.Spec.Bastion.Instance.Image)
-	if err != nil {
-		return nil, fmt.Errorf("failed to get image ID for bastion: %w", err)
-	}
-
+func bastionToInstanceSpec(openStackCluster *infrav1.OpenStackCluster, clusterName string) *compute.InstanceSpec {
 	instanceSpec := &compute.InstanceSpec{
 		Name:          bastionName(clusterName),
 		Flavor:        openStackCluster.Spec.Bastion.Instance.Flavor,
 		SSHKeyName:    openStackCluster.Spec.Bastion.Instance.SSHKeyName,
-		ImageID:       imageID,
+		ImageID:       openStackCluster.Status.Bastion.ReferencedResources.ImageID,
 		FailureDomain: openStackCluster.Spec.Bastion.AvailabilityZone,
 		RootVolume:    openStackCluster.Spec.Bastion.Instance.RootVolume,
 	}
@@ -461,7 +458,7 @@ func bastionToInstanceSpec(openStackCluster *infrav1.OpenStackCluster, clusterNa
 
 	instanceSpec.Ports = openStackCluster.Spec.Bastion.Instance.Ports
 
-	return instanceSpec, nil
+	return instanceSpec
 }
 
 func bastionName(clusterName string) string {

controllers/openstackcluster_controller_test.go

@@ -25,7 +25,6 @@ import (
 	"github.com/gophercloud/gophercloud"
 	"github.com/gophercloud/gophercloud/openstack/compute/v2/extensions/attachinterfaces"
 	"github.com/gophercloud/gophercloud/openstack/compute/v2/servers"
-	"github.com/gophercloud/gophercloud/openstack/imageservice/v2/images"
 	"github.com/gophercloud/gophercloud/openstack/networking/v2/extensions"
 	"github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/external"
 	"github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/layer3/floatingips"
@@ -234,6 +233,11 @@ var _ = Describe("OpenStackCluster controller", func() {
 		err = k8sClient.Create(ctx, capiCluster)
 		Expect(err).To(BeNil())
 		testCluster.Status = infrav1.OpenStackClusterStatus{
+			Bastion: &infrav1.BastionStatus{
+				ReferencedResources: infrav1.ReferencedMachineResources{
+					ImageID: "imageID",
+				},
+			},
 			Network: &infrav1.NetworkStatusWithSubnets{
 				NetworkStatus: infrav1.NetworkStatus{
 					Name: "network-name",
@@ -250,9 +254,6 @@ var _ = Describe("OpenStackCluster controller", func() {
 		server.ID = "adopted-bastion-uuid"
 		server.Status = "ACTIVE"
 
-		imageClientRecorder := mockScopeFactory.ImageClient.EXPECT()
-		imageClientRecorder.ListImages(gomock.Any()).Return([]images.Image{{ID: "imageID"}}, nil)
-
 		computeClientRecorder := mockScopeFactory.ComputeClient.EXPECT()
 		computeClientRecorder.ListServers(servers.ListOpts{
 			Name: "^capi-cluster-bastion$",
@@ -263,7 +264,13 @@ var _ = Describe("OpenStackCluster controller", func() {
 		networkClientRecorder.ListFloatingIP(floatingips.ListOpts{PortID: "portID"}).Return(make([]floatingips.FloatingIP, 1), nil)
 
 		res, err := reconcileBastion(scope, capiCluster, testCluster)
-		Expect(testCluster.Status.Bastion).To(Equal(&infrav1.BastionStatus{ID: "adopted-bastion-uuid", State: "ACTIVE"}))
+		Expect(testCluster.Status.Bastion).To(Equal(&infrav1.BastionStatus{
+			ID:    "adopted-bastion-uuid",
+			State: "ACTIVE",
+			ReferencedResources: infrav1.ReferencedMachineResources{
+				ImageID: "imageID",
+			},
+		}))
 		Expect(err).To(BeNil())
 		Expect(res).To(Equal(reconcile.Result{}))
 	})
@@ -286,6 +293,9 @@ var _ = Describe("OpenStackCluster controller", func() {
 			},
 			Bastion: &infrav1.BastionStatus{
 				ID: "adopted-fip-bastion-uuid",
+				ReferencedResources: infrav1.ReferencedMachineResources{
+					ImageID: "imageID",
+				},
 			},
 		}
 		err = k8sClient.Status().Update(ctx, testCluster)
@@ -298,9 +308,6 @@ var _ = Describe("OpenStackCluster controller", func() {
 		server.ID = "adopted-fip-bastion-uuid"
 		server.Status = "ACTIVE"
 
-		imageClientRecorder := mockScopeFactory.ImageClient.EXPECT()
-		imageClientRecorder.ListImages(gomock.Any()).Return([]images.Image{{ID: "imageID"}}, nil)
-
 		computeClientRecorder := mockScopeFactory.ComputeClient.EXPECT()
 		computeClientRecorder.GetServer("adopted-fip-bastion-uuid").Return(&server, nil)
 
@@ -309,7 +316,14 @@ var _ = Describe("OpenStackCluster controller", func() {
 		networkClientRecorder.ListFloatingIP(floatingips.ListOpts{PortID: "portID"}).Return([]floatingips.FloatingIP{{FloatingIP: "1.2.3.4"}}, nil)
 
 		res, err := reconcileBastion(scope, capiCluster, testCluster)
-		Expect(testCluster.Status.Bastion).To(Equal(&infrav1.BastionStatus{ID: "adopted-fip-bastion-uuid", State: "ACTIVE", FloatingIP: "1.2.3.4"}))
+		Expect(testCluster.Status.Bastion).To(Equal(&infrav1.BastionStatus{
+			ID:         "adopted-fip-bastion-uuid",
+			FloatingIP: "1.2.3.4",
+			State:      "ACTIVE",
+			ReferencedResources: infrav1.ReferencedMachineResources{
+				ImageID: "imageID",
+			},
+		}))
 		Expect(err).To(BeNil())
 		Expect(res).To(Equal(reconcile.Result{}))
 	})
@@ -332,6 +346,9 @@ var _ = Describe("OpenStackCluster controller", func() {
 			},
 			Bastion: &infrav1.BastionStatus{
 				ID: "requeue-bastion-uuid",
+				ReferencedResources: infrav1.ReferencedMachineResources{
+					ImageID: "imageID",
+				},
 			},
 		}
 		err = k8sClient.Status().Update(ctx, testCluster)
@@ -344,14 +361,17 @@ var _ = Describe("OpenStackCluster controller", func() {
 		server.ID = "requeue-bastion-uuid"
 		server.Status = "BUILD"
 
-		imageClientRecorder := mockScopeFactory.ImageClient.EXPECT()
-		imageClientRecorder.ListImages(gomock.Any()).Return([]images.Image{{ID: "imageID"}}, nil)
-
 		computeClientRecorder := mockScopeFactory.ComputeClient.EXPECT()
 		computeClientRecorder.GetServer("requeue-bastion-uuid").Return(&server, nil)
 
 		res, err := reconcileBastion(scope, capiCluster, testCluster)
-		Expect(testCluster.Status.Bastion).To(Equal(&infrav1.BastionStatus{ID: "requeue-bastion-uuid", State: "BUILD"}))
+		Expect(testCluster.Status.Bastion).To(Equal(&infrav1.BastionStatus{
+			ID:    "requeue-bastion-uuid",
+			State: "BUILD",
+			ReferencedResources: infrav1.ReferencedMachineResources{
+				ImageID: "imageID",
+			},
+		}))
 		Expect(err).To(BeNil())
 		Expect(res).To(Equal(reconcile.Result{RequeueAfter: waitForBuildingInstanceToReconcile}))
 	})
@@ -364,16 +384,22 @@ var _ = Describe("OpenStackCluster controller", func() {
 		Expect(err).To(BeNil())
 		err = k8sClient.Create(ctx, capiCluster)
 		Expect(err).To(BeNil())
+		testCluster.Status = infrav1.OpenStackClusterStatus{
+			Bastion: &infrav1.BastionStatus{
+				ReferencedResources: infrav1.ReferencedMachineResources{
+					ImageID: "imageID",
+				},
+			},
+		}
+		err = k8sClient.Status().Update(ctx, testCluster)
+		Expect(err).To(BeNil())
 
 		scope, err := mockScopeFactory.NewClientScopeFromCluster(ctx, k8sClient, testCluster, nil, logr.Discard())
 		Expect(err).To(BeNil())
 
 		server := clients.ServerExt{}
 		server.ID = "delete-bastion-uuid"
 
-		imageClientRecorder := mockScopeFactory.ImageClient.EXPECT()
-		imageClientRecorder.ListImages(gomock.Any()).Return([]images.Image{{ID: "imageID"}}, nil)
-
 		computeClientRecorder := mockScopeFactory.ComputeClient.EXPECT()
 		computeClientRecorder.ListServers(servers.ListOpts{
 			Name: "^capi-cluster-bastion$",