Merge branch 'master' of github.com:gardener/gardener-extension-provi…

…der-azure into multi-zone-nat6

pkg/apis/azure/helper/scheme.go

@@ -36,13 +36,17 @@ var (
 	Scheme *runtime.Scheme
 
 	decoder runtime.Decoder
+
+	// lenientDecoder is a decoder that does not use strict mode.
+	lenientDecoder runtime.Decoder
 )
 
 func init() {
 	Scheme = runtime.NewScheme()
 	utilruntime.Must(install.AddToScheme(Scheme))
 
 	decoder = serializer.NewCodecFactory(Scheme, serializer.EnableStrict).UniversalDecoder()
+	lenientDecoder = serializer.NewCodecFactory(Scheme).UniversalDecoder()
 }
 
 // InfrastructureConfigFromInfrastructure extracts the InfrastructureConfig from the
@@ -58,12 +62,12 @@ func InfrastructureConfigFromInfrastructure(infra *extensionsv1alpha1.Infrastruc
 	return nil, fmt.Errorf("provider config is not set on the infrastructure resource")
 }
 
-// InfrastructureStatusFromInfrastructure extracts the InfrastructureStatus from the
+// InfrastructureStatusFromRaw extracts the InfrastructureStatus from the
 // ProviderStatus section of the given Infrastructure.
-func InfrastructureStatusFromInfrastructure(infra *extensionsv1alpha1.Infrastructure) (*api.InfrastructureStatus, error) {
+func InfrastructureStatusFromRaw(raw *runtime.RawExtension) (*api.InfrastructureStatus, error) {
 	config := &api.InfrastructureStatus{}
-	if infra.Status.ProviderStatus != nil && infra.Status.ProviderStatus.Raw != nil {
-		if _, _, err := decoder.Decode(infra.Status.ProviderStatus.Raw, nil, config); err != nil {
+	if raw != nil && raw.Raw != nil {
+		if _, _, err := lenientDecoder.Decode(raw.Raw, nil, config); err != nil {
 			return nil, err
 		}
 		return config, nil

pkg/controller/controlplane/actuator.go

@@ -17,37 +17,51 @@ package controlplane
 import (
 	"context"
 	"fmt"
-	"strconv"
 	"time"
 
 	"github.com/go-logr/logr"
-	"k8s.io/client-go/util/retry"
+	"k8s.io/apimachinery/pkg/api/meta"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/runtime/inject"
 
 	extensionscontroller "github.com/gardener/gardener/extensions/pkg/controller"
 	"github.com/gardener/gardener/extensions/pkg/controller/controlplane"
 	extensionsv1alpha1 "github.com/gardener/gardener/pkg/apis/extensions/v1alpha1"
-	kutil "github.com/gardener/gardener/pkg/utils/kubernetes"
+	"github.com/gardener/gardener/pkg/controllerutils"
+	reconcilerutils "github.com/gardener/gardener/pkg/controllerutils/reconciler"
 	azurev1alpha1 "github.com/gardener/remedy-controller/pkg/apis/azure/v1alpha1"
 )
 
+const (
+	// GracefulDeletionWaitInterval is the default interval for retry operations.
+	GracefulDeletionWaitInterval = 1 * time.Minute
+	// GracefulDeletionTimeout is the timeout that defines how long the actuator should wait for remedy controller resources to be deleted
+	// gracefully by the remedy controller itself
+	GracefulDeletionTimeout = 10 * time.Minute
+)
+
 // NewActuator creates a new Actuator that acts upon and updates the status of ControlPlane resources.
 func NewActuator(
 	a controlplane.Actuator,
 	logger logr.Logger,
+	gracefulDeletionTimeout time.Duration,
+	gracefulDeletionWaitInterval time.Duration,
 ) controlplane.Actuator {
 	return &actuator{
-		Actuator: a,
-		logger:   logger.WithName("azure-controlplane-actuator"),
+		Actuator:                     a,
+		logger:                       logger.WithName("azure-controlplane-actuator"),
+		gracefulDeletionTimeout:      gracefulDeletionTimeout,
+		gracefulDeletionWaitInterval: gracefulDeletionWaitInterval,
 	}
 }
 
 // actuator is an Actuator that acts upon and updates the status of ControlPlane resources.
 type actuator struct {
 	controlplane.Actuator
-	client client.Client
-	logger logr.Logger
+	client                       client.Client
+	logger                       logr.Logger
+	gracefulDeletionTimeout      time.Duration
+	gracefulDeletionWaitInterval time.Duration
 }
 
 // InjectFunc enables injecting Kubernetes dependencies into actuator's dependencies.
@@ -63,21 +77,39 @@ func (a *actuator) InjectClient(client client.Client) error {
 
 // Delete reconciles the given controlplane and cluster, deleting the additional
 // control plane components as needed.
-// Before delegating to the composed Actuator, it ensures that all remedy controller resources have been deleted.
+// Before delegating to the composed Actuator, it ensures that all remedy controller resources have been deleted gracefully.
 func (a *actuator) Delete(
 	ctx context.Context,
 	cp *extensionsv1alpha1.ControlPlane,
 	cluster *extensionscontroller.Cluster,
 ) error {
 	if cp.Spec.Purpose == nil || *cp.Spec.Purpose == extensionsv1alpha1.Normal {
-		// Delete all remaining remedy controller resources
-		if err := a.deleteRemedyControllerResources(ctx, cp); err != nil {
+		list := &azurev1alpha1.PublicIPAddressList{}
+		if err := a.client.List(ctx, list, client.InNamespace(cp.Namespace)); err != nil {
 			return err
 		}
+		if meta.LenList(list) != 0 {
+			if time.Since(cp.DeletionTimestamp.Time) <= a.gracefulDeletionTimeout {
+				a.logger.Info("Some publicipaddresses still exist. Deletion will be retried ...")
+				return &reconcilerutils.RequeueAfterError{
+					RequeueAfter: a.gracefulDeletionWaitInterval,
+				}
+			} else {
+				a.logger.Info("The timeout for waiting for publicipaddresses to be gracefully deleted has expired. They will be forcefully removed.")
+			}
+		}
 	}
 
 	// Call Delete on the composed Actuator
-	return a.Actuator.Delete(ctx, cp, cluster)
+	if err := a.Actuator.Delete(ctx, cp, cluster); err != nil {
+		return err
+	}
+
+	if cp.Spec.Purpose == nil || *cp.Spec.Purpose == extensionsv1alpha1.Normal {
+		// Delete all remaining remedy controller resources
+		return a.forceDeleteRemedyControllerResources(ctx, cp)
+	}
+	return nil
 }
 
 // Migrate reconciles the given controlplane and cluster, migrating the additional
@@ -88,61 +120,47 @@ func (a *actuator) Migrate(
 	cp *extensionsv1alpha1.ControlPlane,
 	cluster *extensionscontroller.Cluster,
 ) error {
+	// Call Migrate on the composed Actuator so that the controlplane chart is deleted and therefore
+	// the remedy controller is also removed.
+	if err := a.Actuator.Migrate(ctx, cp, cluster); err != nil {
+		return err
+	}
 	if cp.Spec.Purpose == nil || *cp.Spec.Purpose == extensionsv1alpha1.Normal {
 		// Delete all remaining remedy controller resources
-		if err := a.deleteRemedyControllerResources(ctx, cp); err != nil {
-			return err
-		}
+		return a.forceDeleteRemedyControllerResources(ctx, cp)
 	}
-
-	// Call Migrate on the composed Actuator
-	return a.Actuator.Migrate(ctx, cp, cluster)
+	return nil
 }
 
-func (a *actuator) deleteRemedyControllerResources(ctx context.Context, cp *extensionsv1alpha1.ControlPlane) error {
-	// Forcefully delete all remaining remedy controller resources
-	a.logger.Info("Deleting all remaining remedy controller resources")
+func (a *actuator) forceDeleteRemedyControllerResources(ctx context.Context, cp *extensionsv1alpha1.ControlPlane) error {
+	a.logger.Info("Removing finalizers from remedy controller resources")
 	pubipList := &azurev1alpha1.PublicIPAddressList{}
 	if err := a.client.List(ctx, pubipList, client.InNamespace(cp.Namespace)); err != nil {
 		return fmt.Errorf("could not list publicipaddresses: %w", err)
 	}
 	for _, pubip := range pubipList.Items {
-		// Add the do-not-clean annotation to the publicipaddress resource
-		// This annotation prevents attempts to clean the Azure IP address if it still exists, resulting in much faster deletion
-		if err := retry.RetryOnConflict(retry.DefaultBackoff, func() error {
-			pubip.Annotations = add(pubip.Annotations, "azure.remedy.gardener.cloud/do-not-clean", strconv.FormatBool(true))
-			return a.client.Update(ctx, &pubip)
-		}); err != nil {
-			return fmt.Errorf("could not add do-not-clean annotation on publicipaddress: %w", err)
+		if err := controllerutils.PatchRemoveFinalizers(ctx, a.client, &pubip, "azure.remedy.gardener.cloud/publicipaddress"); err != nil {
+			return fmt.Errorf("could not remove finalizers from publicipaddress: %w", err)
 		}
 	}
+
+	virtualMachineList := &azurev1alpha1.VirtualMachineList{}
+	if err := a.client.List(ctx, virtualMachineList, client.InNamespace(cp.Namespace)); err != nil {
+		return fmt.Errorf("could not list virtualmachines: %w", err)
+	}
+	for _, virtualMachine := range virtualMachineList.Items {
+		if err := controllerutils.PatchRemoveFinalizers(ctx, a.client, &virtualMachine, "azure.remedy.gardener.cloud/virtualmachine"); err != nil {
+			return fmt.Errorf("could not remove finalizers from virtualmachine: %w", err)
+		}
+	}
+
+	a.logger.Info("Deleting all remaining remedy controller resources")
 	if err := a.client.DeleteAllOf(ctx, &azurev1alpha1.PublicIPAddress{}, client.InNamespace(cp.Namespace)); err != nil {
 		return fmt.Errorf("could not delete publicipaddress resources: %w", err)
 	}
 	if err := a.client.DeleteAllOf(ctx, &azurev1alpha1.VirtualMachine{}, client.InNamespace(cp.Namespace)); err != nil {
 		return fmt.Errorf("could not delete virtualmachine resources: %w", err)
 	}
 
-	// Wait until the remaining remedy controller resources have been deleted
-	a.logger.Info("Waiting for the remaining remedy controller resources to be deleted")
-	timeoutCtx1, cancel := context.WithTimeout(ctx, 5*time.Minute)
-	defer cancel()
-	if err := kutil.WaitUntilResourcesDeleted(timeoutCtx1, a.client, &azurev1alpha1.PublicIPAddressList{}, 5*time.Second, client.InNamespace(cp.Namespace)); err != nil {
-		return fmt.Errorf("could not wait for publicipaddress resources to be deleted: %w", err)
-	}
-	timeoutCtx2, cancel := context.WithTimeout(ctx, 5*time.Minute)
-	defer cancel()
-	if err := kutil.WaitUntilResourcesDeleted(timeoutCtx2, a.client, &azurev1alpha1.VirtualMachineList{}, 5*time.Second, client.InNamespace(cp.Namespace)); err != nil {
-		return fmt.Errorf("could not wait for virtualmachine resources to be deleted: %w", err)
-	}
-
 	return nil
 }
-
-func add(m map[string]string, key, value string) map[string]string {
-	if m == nil {
-		m = make(map[string]string)
-	}
-	m[key] = value
-	return m
-}