Clarify contract when extension resources are migrated (gardener#8093)

* Adapt documentation to clarify extension state contract

* Adhere to clarified contract in generic `ControlPlane` actuator

docs/extensions/migration.md

@@ -13,7 +13,7 @@ The following principles should always be upheld:
 
 Two new operations have been introduced in Gardener. They can be specified as values of the `gardener.cloud/operation` annotation on an extension resource to indicate that an operation different from a normal `reconcile` should be performed by the corresponding extension controller:
 
-* The `migrate` operation is used to ask the extension controller in the source seed to stop reconciling extension resources (in case they are requeued due to errors) and perform cleanup activities, if such are required. These cleanup activities might involve removing finalizers on resources in the shoot namespace that have been previously created by the extension controller and deleting them without actually deleting any resources external to the seed cluster.
+* The `migrate` operation is used to ask the extension controller in the source seed to stop reconciling extension resources (in case they are requeued due to errors) and perform cleanup activities, if such are required. These cleanup activities might involve removing finalizers on resources in the shoot namespace that have been previously created by the extension controller and deleting them without actually deleting any resources external to the seed cluster. This is also the last opportunity for extensions to persist their state into the `.status.state` field of the reconciled extension resource before its restored in the new destination seed cluster.
 * The `restore` operation is used to ask the extension controller in the destination seed to restore any state saved in the extension resource `status`, before performing the actual reconciliation.
 
 Unlike the [reconcile operation](https://github.com/gardener/gardener/blob/master/docs/extensions/reconcile-trigger.md), extension controllers must remove the `gardener.cloud/operation` annotation at the end of a successful reconciliation when the current operation is `migrate` or `restore`, not at the beginning of a reconciliation.
@@ -22,6 +22,8 @@ Unlike the [reconcile operation](https://github.com/gardener/gardener/blob/maste
 
 All resources in the source seed that have been created by an extension controller, for example secrets, config maps, [managed resources](managedresources.md), etc., should be properly cleaned up by the extension controller when the current operation is `migrate`. As mentioned above, such resources should be deleted without actually deleting any resources external to the seed cluster.
 
+There is one exception to this: `Secret`s labeled with `persist=true` created via the [secrets manager](../development/secrets_management.md). They should be kept (i.e., the `Cleanup` function of secrets manager should not be called) and will be garbage collected automatically at the end of the `migrate` operation. This ensures that they can be properly persisted in the `ShootState` resource and get restored on the new destination seed cluster.
+
 For many custom resources, for example MCM resources, the above requirement means in practice that any finalizers should be removed before deleting the resource, in addition to ensuring that the resource deletion is not reconciled by its respective controller if there is no finalizer. For managed resources, the above requirement means in practice that the `spec.keepObjects` field should be set to `true` before deleting the extension resource.
 
 Here it is assumed that any resources that contain state needed by the extension controller can be safely deleted, since any such state has been saved as described in [Saving and Restoring Extension States](#saving-and-restoring-extension-states) at the end of the last successful reconciliation.

extensions/pkg/controller/controlplane/genericactuator/actuator.go

@@ -409,10 +409,24 @@ func (a *actuator) Delete(
 	cp *extensionsv1alpha1.ControlPlane,
 	cluster *extensionscontroller.Cluster,
 ) error {
+	sm, err := a.newSecretsManagerForControlPlane(ctx, log, cp, cluster, nil)
+	if err != nil {
+		return fmt.Errorf("failed to create secrets manager for ControlPlane: %w", err)
+	}
+
+	if err := a.delete(ctx, log, cp); err != nil {
+		return err
+	}
+
+	return sm.Cleanup(ctx)
+}
+
+func (a *actuator) delete(ctx context.Context, log logr.Logger, cp *extensionsv1alpha1.ControlPlane) error {
 	if cp.Spec.Purpose != nil && *cp.Spec.Purpose == extensionsv1alpha1.Exposure {
-		return a.deleteControlPlaneExposure(ctx, log, cp, cluster)
+		return a.deleteControlPlaneExposure(ctx, log, cp)
 	}
-	return a.deleteControlPlane(ctx, log, cp, cluster)
+
+	return a.deleteControlPlane(ctx, log, cp)
 }
 
 // deleteControlPlaneExposure reconciles the given controlplane and cluster, deleting the additional Seed
@@ -421,13 +435,7 @@ func (a *actuator) deleteControlPlaneExposure(
 	ctx context.Context,
 	log logr.Logger,
 	cp *extensionsv1alpha1.ControlPlane,
-	cluster *extensionscontroller.Cluster,
 ) error {
-	sm, err := a.newSecretsManagerForControlPlane(ctx, log, cp, cluster, nil)
-	if err != nil {
-		return fmt.Errorf("failed to create secrets manager for ControlPlane: %w", err)
-	}
-
 	// Delete control plane objects
 	if a.controlPlaneExposureChart != nil {
 		log.Info("Deleting control plane exposure with objects")
@@ -444,7 +452,7 @@ func (a *actuator) deleteControlPlaneExposure(
 		}
 	}
 
-	return sm.Cleanup(ctx)
+	return nil
 }
 
 // deleteControlPlane reconciles the given controlplane and cluster, deleting the additional Shoot
@@ -453,13 +461,7 @@ func (a *actuator) deleteControlPlane(
 	ctx context.Context,
 	log logr.Logger,
 	cp *extensionsv1alpha1.ControlPlane,
-	cluster *extensionscontroller.Cluster,
 ) error {
-	sm, err := a.newSecretsManagerForControlPlane(ctx, log, cp, cluster, nil)
-	if err != nil {
-		return fmt.Errorf("failed to create secrets manager for ControlPlane: %w", err)
-	}
-
 	// Delete the managed resources
 	if err := managedresources.Delete(ctx, a.client, cp.Namespace, StorageClassesChartResourceName, false); err != nil {
 		return fmt.Errorf("could not delete managed resource containing storage classes chart for controlplane '%s': %w", kubernetesutils.ObjectName(cp), err)
@@ -533,7 +535,7 @@ func (a *actuator) deleteControlPlane(
 		}
 	}
 
-	return sm.Cleanup(ctx)
+	return nil
 }
 
 // computeChecksums computes and returns all needed checksums. This includes the checksums for the given deployed secrets,
@@ -603,7 +605,7 @@ func (a *actuator) Migrate(
 		return fmt.Errorf("could not keep objects of managed resource containing storage classes chart for controlplane '%s': %w", kubernetesutils.ObjectName(cp), err)
 	}
 
-	return a.Delete(ctx, log, cp, cluster)
+	return a.delete(ctx, log, cp)
 }
 
 func (a *actuator) newSecretsManagerForControlPlane(ctx context.Context, log logr.Logger, cp *extensionsv1alpha1.ControlPlane, cluster *extensionscontroller.Cluster, secretConfigs []extensionssecretsmanager.SecretConfigWithOptions) (secretsmanager.Interface, error) {