Shutdown VMs before Deletion

Tries to shutdown the OpenStack VM before deleting it. This way even
Pods form Daemonsets are shut down more gracefully and services like
license daemons on the VMs can be properly shutdown.

Related to #1973

Author: shaardie

api/v1beta1/conditions_consts.go

@@ -38,6 +38,8 @@ const (
 	InstanceDeletedReason = "InstanceDeleted"
 	// InstanceNotReadyReason used when the instance is in a pending state.
 	InstanceNotReadyReason = "InstanceNotReady"
+	// InstanceStopFailedReason used when stopping the instance failed.
+	InstanceStopFailedReason = "InstanceStopFailed"
 	// InstanceDeleteFailedReason used when deleting the instance failed.
 	InstanceDeleteFailedReason = "InstanceDeleteFailed"
 	// OpenstackErrorReason used when there is an error communicating with OpenStack.

controllers/openstackserver_controller.go

@@ -257,6 +257,14 @@ func (r *OpenStackServerReconciler) reconcileDelete(scope *scope.WithLogger, ope
 			return fmt.Errorf("delete volumes: %w", err)
 		}
 	} else {
+		// Try to stop the VM, if active before deletion
+		if instanceStatus.State() == infrav1.InstanceStateActive {
+			if err := computeService.StopInstance(openStackServer, instanceStatus); err != nil {
+				v1beta1conditions.MarkFalse(openStackServer, infrav1.InstanceReadyCondition, infrav1.InstanceStopFailedReason, clusterv1beta1.ConditionSeverityError, "Stopping instance failed: %v", err)
+				return fmt.Errorf("stop instance: %w", err)
+			}
+		}
+
 		if err := computeService.DeleteInstance(openStackServer, instanceStatus); err != nil {
 			v1beta1conditions.MarkFalse(openStackServer, infrav1.InstanceReadyCondition, infrav1.InstanceDeleteFailedReason, clusterv1beta1.ConditionSeverityError, "Deleting instance failed: %v", err)
 			return fmt.Errorf("delete instance: %w", err)

pkg/clients/compute.go

@@ -61,6 +61,7 @@ type ComputeClient interface {
 	ListFlavors() ([]flavors.Flavor, error)
 	CreateServer(createOpts servers.CreateOptsBuilder, schedulerHints servers.SchedulerHintOptsBuilder) (*servers.Server, error)
 	DeleteServer(serverID string) error
+	StopServer(serverID string) error
 	GetServer(serverID string) (*servers.Server, error)
 	ListServers(listOpts servers.ListOptsBuilder) ([]servers.Server, error)
 
@@ -141,6 +142,12 @@ func (c computeClient) DeleteServer(serverID string) error {
 	return mc.ObserveRequestIgnoreNotFound(err)
 }
 
+func (c computeClient) StopServer(serverID string) error {
+	mc := metrics.NewMetricPrometheusContext("server", "stop")
+	err := servers.Stop(context.TODO(), c.client, serverID).ExtractErr()
+	return mc.ObserveRequestIgnoreNotFound(err)
+}
+
 func (c computeClient) GetServer(serverID string) (*servers.Server, error) {
 	var server servers.Server
 	mc := metrics.NewMetricPrometheusContext("server", "get")
@@ -230,6 +237,10 @@ func (e computeErrorClient) DeleteServer(_ string) error {
 	return e.error
 }
 
+func (e computeErrorClient) StopServer(_ string) error {
+	return e.error
+}
+
 func (e computeErrorClient) GetServer(_ string) (*servers.Server, error) {
 	return nil, e.error
 }

pkg/clients/mock/compute.go

@@ -45,6 +45,7 @@ import (
 const (
 	retryIntervalInstanceStatus = 10 * time.Second
 	timeoutInstanceCreate       = 5
+	timeoutInstanceStop         = 5 * time.Minute
 	timeoutInstanceDelete       = 5 * time.Minute
 )
 
@@ -560,6 +561,37 @@ func (s *Service) deleteVolume(instanceName string, nameSuffix string) error {
 	return s.getVolumeClient().DeleteVolume(volume.ID, volumes.DeleteOpts{})
 }
 
+func (s *Service) StopInstance(eventObject runtime.Object, instanceStatus *InstanceStatus) error {
+	instance := instanceStatus.InstanceIdentifier()
+	err := s.getComputeClient().StopServer(instance.ID)
+	if err != nil {
+		if capoerrors.IsNotFound(err) {
+			record.Eventf(eventObject, "SuccessfulStopServer", "Server %s with id %s did not exist", instance.Name, instance.ID)
+			return nil
+		}
+		record.Warnf(eventObject, "FailedStopServer", "Failed to stop server %s with id %s: %v", instance.Name, instance.ID, err)
+		return err
+	}
+	err = wait.PollUntilContextTimeout(context.TODO(), retryIntervalInstanceStatus, timeoutInstanceStop, true, func(_ context.Context) (bool, error) {
+		i, err := s.GetInstanceStatus(instance.ID)
+		if err != nil {
+			return false, err
+		}
+		// a little bit noisy, but since the code is new, I guess this is a little bit easier for debugging purposes
+		record.Eventf(eventObject, "Server State", "server %s with id %s has state %v", instance.Name, instance.ID, i.State())
+		if i.State() == infrav1.InstanceStateShutoff {
+			return true, nil
+		}
+		return false, nil
+	})
+	if err != nil {
+		record.Warnf(eventObject, "FailedStopServer", "Failed to stop server %s with id %s: %v", instance.Name, instance.ID, err)
+		return err
+	}
+	record.Eventf(eventObject, "SuccessfulStopServer", "Stopped server %s with id %s", instance.Name, instance.ID)
+	return nil
+}
+
 func (s *Service) GetInstanceStatus(resourceID string) (instance *InstanceStatus, err error) {
 	if resourceID == "" {
 		return nil, fmt.Errorf("resourceId should be specified to get detail")