Improve resource condition/error when deletion is not possible in Azure (#4987)

* Add new DeleteAction for when Azure deletion is not possible

* Support DeletionNotSupported in condition

* Implement handling when delete is not permitted

* Add detatch annotation

* Add annotation to sample

* Add comments

* Add annotations for postgres samples

* Fix MariaDB samples

* Move addAnnotation helper onto TestContext

* Update comment

* Fix MariaDB test

* Update samples

* Fix test

* Fix samples

* Update more tests

* Update samples

* Update CRUD tests

* Fix test

* Update tests

* Update samples

* Update more tests

* Fix more tests

* Rename tests

* Fix more tests

* Update tests

* Tweak test

* Fix test

* Fix another test

* Fix test

* Update another test

* Fix test

* Check for resource deletion by other causes

* Update recording

* Change type of extension

* Update comments on annotation use

* Update comments in samples

* Clean up comment

* Expose other errors as well

* Fix typo.

* Fix lint error.

* Fix lint issues

* Tidy last comments

* Suppress test with owner update race

Author: theunrepentantgeek

v2/api/dbforpostgresql/customizations/flexible_servers_firewall_rule_extensions.go

@@ -18,16 +18,15 @@ import (
 	"github.com/Azure/azure-service-operator/v2/pkg/genruntime/extensions"
 )
 
-var _ extensions.PreReconciliationChecker = &FlexibleServersFirewallRuleExtension{}
+var _ extensions.PreReconciliationOwnerChecker = &FlexibleServersFirewallRuleExtension{}
 
-func (ext *FlexibleServersFirewallRuleExtension) PreReconcileCheck(
+func (ext *FlexibleServersFirewallRuleExtension) PreReconcileOwnerCheck(
 	ctx context.Context,
-	obj genruntime.MetaObject,
 	owner genruntime.MetaObject,
 	resourceResolver *resolver.Resolver,
 	armClient *genericarmclient.GenericClient,
 	log logr.Logger,
-	next extensions.PreReconcileCheckFunc,
+	next extensions.PreReconcileOwnerCheckFunc,
 ) (extensions.PreReconcileCheckResult, error) {
 	// Check to see if our owning server is ready for the database to be reconciled
 	// Owner nil can happen if the server/owner of the firewall rule is referenced by armID
@@ -43,5 +42,5 @@ func (ext *FlexibleServersFirewallRuleExtension) PreReconcileCheck(
 		}
 	}
 
-	return next(ctx, obj, owner, resourceResolver, armClient, log)
+	return next(ctx, owner, resourceResolver, armClient, log)
 }

v2/api/keyvault/customizations/vault_extensions.go

@@ -33,7 +33,6 @@ import (
 
 var _ extensions.ARMResourceModifier = &VaultExtension{}
 
-//nolint:staticcheck // Underscores for enum values is our convention in ASO
 const (
 	CreateMode_Default         = "default"
 	CreateMode_Recover         = "recover"
@@ -275,7 +274,7 @@ func (*VaultExtension) getOwner(
 	var id *arm.ResourceID
 
 	// No need to wait for resources that don't have an owner
-	switch owner.Result { // nolint: exhaustive
+	switch owner.Result {
 	case resolver.OwnerFoundKubernetes:
 		rg, ok := owner.Owner.(*resources.ResourceGroup)
 		if !ok {

…otection_backupinstance_20231101_test.go …ion_backupinstance_crud_20231101_test.gov2/internal/controllers/crd_dataprotection_backupinstance_20231101_test.go renamed to v2/internal/controllers/dataprotection_backupinstance_crud_20231101_test.go v2/internal/controllers/crd_dataprotection_backupinstance_20231101_test.go renamed to v2/internal/controllers/dataprotection_backupinstance_crud_20231101_test.go

@@ -71,6 +71,10 @@ func Test_DataProtection_BackupInstance_20231101_CRUD(t *testing.T) {
 		},
 	}
 
+	// Don't try to delete directly, this is not a real resource - to delete it in Azure you must delete its parent.
+	// We can delete it from the cluster by applying this annotation, but this won't change anything in Azure.
+	tc.AddAnnotation(&blobService.ObjectMeta, "serviceoperator.azure.com/reconcile-policy", "detach-on-delete")
+
 	blobContainer := &storage.StorageAccountsBlobServicesContainer{
 		ObjectMeta: tc.MakeObjectMeta("velero"),
 		Spec: storage.StorageAccountsBlobServicesContainer_Spec{

v2/internal/controllers/dbformysql_flexibleserver_20210501_test.go

@@ -244,6 +244,11 @@ func MySQLFlexibleServer_Configuration_20220101_CRUD(tc *testcommon.KubePerTestC
 			Value:     to.Ptr("20"),
 		},
 	}
+
+	// Don't try to delete directly, this is not a real resource - to delete it in Azure you must delete its parent.
+	// We can delete it from the cluster by applying this annotation, but this won't change anything in Azure.
+	tc.AddAnnotation(&configuration.ObjectMeta, "serviceoperator.azure.com/reconcile-policy", "detach-on-delete")
+
 	tc.CreateResourceAndWait(configuration)
 	tc.Expect(configuration.Status.Id).ToNot(BeNil())
 }

v2/internal/controllers/dbformysql_flexibleserver_20230630_test.go

@@ -243,6 +243,11 @@ func MySQLFlexibleServer_Configuration_20230630_CRUD(tc *testcommon.KubePerTestC
 			Value:     to.Ptr("20"),
 		},
 	}
+
+	// Don't try to delete directly, this is not a real resource - to delete it in Azure you must delete its parent.
+	// We can delete it from the cluster by applying this annotation, but this won't change anything in Azure.
+	tc.AddAnnotation(&configuration.ObjectMeta, "serviceoperator.azure.com/reconcile-policy", "detach-on-delete")
+
 	tc.CreateResourceAndWait(configuration)
 	tc.Expect(configuration.Status.Id).ToNot(BeNil())
 }

v2/internal/controllers/dbformysql_flexibleserver_20231230_test.go

@@ -238,6 +238,11 @@ func MySQLFlexibleServer_Configuration_20231230_CRUD(tc *testcommon.KubePerTestC
 			Value:     to.Ptr("20"),
 		},
 	}
+
+	// Don't try to delete directly, this is not a real resource - to delete it in Azure you must delete its parent.
+	// We can delete it from the cluster by applying this annotation, but this won't change anything in Azure.
+	tc.AddAnnotation(&configuration.ObjectMeta, "serviceoperator.azure.com/reconcile-policy", "detach-on-delete")
+
 	tc.CreateResourceAndWait(configuration)
 	tc.Expect(configuration.Status.Id).ToNot(BeNil())
 }

v2/internal/controllers/dbforpostgresql_flexibleserver_crud_v1api20221201_test.go

@@ -167,9 +167,11 @@ func FlexibleServer_Configuration_20221201_CRUD(tc *testcommon.KubePerTestContex
 		},
 	}
 
+	// Don't try to delete directly, this is not a real resource - to delete it in Azure you must delete its parent.
+	// We can delete it from the cluster by applying this annotation, but this won't change anything in Azure.
+	tc.AddAnnotation(&configuration.ObjectMeta, "serviceoperator.azure.com/reconcile-policy", "detach-on-delete")
+
 	tc.CreateResourceAndWait(configuration)
-	// This isn't a "real" resource so it cannot be deleted directly
-	// defer tc.DeleteResourceAndWait(configuration)
 
 	tc.Expect(configuration.Status.Id).ToNot(BeNil())
 	tc.Expect(configuration.Status.Value).To(Equal(to.Ptr("READ")))

v2/internal/controllers/dbforpostgresql_flexibleserver_crud_v1api20230601preview_test.go

@@ -204,9 +204,11 @@ func FlexibleServer_Configuration_20230601Preview_CRUD(tc *testcommon.KubePerTes
 		},
 	}
 
+	// Don't try to delete directly, this is not a real resource - to delete it in Azure you must delete its parent.
+	// We can delete it from the cluster by applying this annotation, but this won't change anything in Azure.
+	tc.AddAnnotation(&configuration.ObjectMeta, "serviceoperator.azure.com/reconcile-policy", "detach-on-delete")
+
 	tc.CreateResourceAndWait(configuration)
-	// This isn't a "real" resource so it cannot be deleted directly
-	// defer tc.DeleteResourceAndWait(configuration)
 
 	tc.Expect(configuration.Status.Id).ToNot(BeNil())
 	tc.Expect(configuration.Status.Value).To(Equal(to.Ptr("READ")))

v2/internal/controllers/dbforpostgresql_flexibleserver_crud_v1api20240801_test.go

@@ -226,9 +226,11 @@ func FlexibleServer_Configuration_20240801_CRUD(tc *testcommon.KubePerTestContex
 		},
 	}
 
+	// Don't try to delete directly, this is not a real resource - to delete it in Azure you must delete its parent.
+	// We can delete it from the cluster by applying this annotation, but this won't change anything in Azure.
+	tc.AddAnnotation(&configuration.ObjectMeta, "serviceoperator.azure.com/reconcile-policy", "detach-on-delete")
+
 	tc.CreateResourceAndWait(configuration)
-	// This isn't a "real" resource so it cannot be deleted directly
-	// defer tc.DeleteResourceAndWait(configuration)
 
 	tc.Expect(configuration.Status.Id).ToNot(BeNil())
 	tc.Expect(configuration.Status.Value).To(Equal(to.Ptr("READ")))
@@ -261,9 +263,11 @@ func FlexibleServer_AdvancedThreatProtection_20240801_CRUD(tc *testcommon.KubePe
 		},
 	}
 
+	// Don't try to delete directly, this is not a real resource - to delete it in Azure you must delete its parent.
+	// We can delete it from the cluster by applying this annotation, but this won't change anything in Azure.
+	tc.AddAnnotation(&threatProtection.ObjectMeta, "serviceoperator.azure.com/reconcile-policy", "detach-on-delete")
+
 	tc.CreateResourceAndWait(threatProtection)
-	// This isn't a "real" resource so it cannot be deleted directly
-	// defer tc.DeleteResourceAndWait(threatProtection)
 
 	tc.Expect(threatProtection.Status.Id).ToNot(BeNil())
 }

v2/internal/controllers/dbforpostgresql_flexibleserver_crud_v1api20250801_test.go

@@ -241,9 +241,11 @@ func FlexibleServer_Configuration_20250801_CRUD(tc *testcommon.KubePerTestContex
 		},
 	}
 
+	// Don't try to delete directly, this is not a real resource - to delete it in Azure you must delete its parent.
+	// We can delete it from the cluster by applying this annotation, but this won't change anything in Azure.
+	tc.AddAnnotation(&configuration.ObjectMeta, "serviceoperator.azure.com/reconcile-policy", "detach-on-delete")
+
 	tc.CreateResourceAndWait(configuration)
-	// This isn't a "real" resource so it cannot be deleted directly
-	// defer tc.DeleteResourceAndWait(configuration)
 
 	tc.Expect(configuration.Status.Id).ToNot(BeNil())
 	tc.Expect(configuration.Status.Value).To(Equal(to.Ptr("READ")))
@@ -276,9 +278,11 @@ func FlexibleServer_AdvancedThreatProtection_20250801_CRUD(tc *testcommon.KubePe
 		},
 	}
 
+	// Don't try to delete directly, this is not a real resource - to delete it in Azure you must delete its parent.
+	// We can delete it from the cluster by applying this annotation, but this won't change anything in Azure.
+	tc.AddAnnotation(&threatProtection.ObjectMeta, "serviceoperator.azure.com/reconcile-policy", "detach-on-delete")
+
 	tc.CreateResourceAndWait(threatProtection)
-	// This isn't a "real" resource so it cannot be deleted directly
-	// defer tc.DeleteResourceAndWait(threatProtection)
 
 	tc.Expect(threatProtection.Status.Id).ToNot(BeNil())
 }