Fix stuck portable resource deletion bug

pkg/corerp/datamodel/extender.go

@@ -53,17 +53,17 @@ func (r *Extender) ResourceMetadata() *rpv1.BasicResourceProperties {
 }
 
 // ResourceTypeName returns the resource type of the extender resource.
-func (extender *Extender) ResourceTypeName() string {
+func (r *Extender) ResourceTypeName() string {
 	return ExtenderResourceType
 }
 
 // Recipe returns the ResourceRecipe associated with the Extender if the ResourceProvisioning is not set to Manual,
 // otherwise it returns nil.
-func (extender *Extender) Recipe() *portableresources.ResourceRecipe {
-	if extender.Properties.ResourceProvisioning == portableresources.ResourceProvisioningManual {
+func (r *Extender) Recipe() *portableresources.ResourceRecipe {
+	if r.Properties.ResourceProvisioning == portableresources.ResourceProvisioningManual {
 		return nil
 	}
-	return &extender.Properties.ResourceRecipe
+	return &r.Properties.ResourceRecipe
 }
 
 // ExtenderProperties represents the properties of Extender resource.

pkg/corerp/setup/setup.go

@@ -221,7 +221,7 @@ func SetupNamespace(recipeControllerConfig *controllerconfig.RecipeControllerCon
 				rp_frontend.PrepareRadiusResource[*datamodel.Extender],
 			},
 			AsyncJobController: func(options asyncctrl.Options) (asyncctrl.Controller, error) {
-				return pr_ctrl.NewCreateOrUpdateResource(options, &ext_processor.Processor{}, recipeControllerConfig.Engine, recipeControllerConfig.ResourceClient, recipeControllerConfig.ConfigLoader)
+				return pr_ctrl.NewCreateOrUpdateResource[*datamodel.Extender, datamodel.Extender](options, &ext_processor.Processor{}, recipeControllerConfig.Engine, recipeControllerConfig.ResourceClient, recipeControllerConfig.ConfigLoader)
 			},
 			AsyncOperationTimeout:    ext_ctrl.AsyncCreateOrUpdateExtenderTimeout,
 			AsyncOperationRetryAfter: AsyncOperationRetryAfter,
@@ -231,14 +231,14 @@ func SetupNamespace(recipeControllerConfig *controllerconfig.RecipeControllerCon
 				rp_frontend.PrepareRadiusResource[*datamodel.Extender],
 			},
 			AsyncJobController: func(options asyncctrl.Options) (asyncctrl.Controller, error) {
-				return pr_ctrl.NewCreateOrUpdateResource(options, &ext_processor.Processor{}, recipeControllerConfig.Engine, recipeControllerConfig.ResourceClient, recipeControllerConfig.ConfigLoader)
+				return pr_ctrl.NewCreateOrUpdateResource[*datamodel.Extender, datamodel.Extender](options, &ext_processor.Processor{}, recipeControllerConfig.Engine, recipeControllerConfig.ResourceClient, recipeControllerConfig.ConfigLoader)
 			},
 			AsyncOperationTimeout:    ext_ctrl.AsyncCreateOrUpdateExtenderTimeout,
 			AsyncOperationRetryAfter: AsyncOperationRetryAfter,
 		},
 		Delete: builder.Operation[datamodel.Extender]{
 			AsyncJobController: func(options asyncctrl.Options) (asyncctrl.Controller, error) {
-				return pr_ctrl.NewDeleteResource(options, &ext_processor.Processor{}, recipeControllerConfig.Engine, recipeControllerConfig.ConfigLoader)
+				return pr_ctrl.NewDeleteResource[*datamodel.Extender, datamodel.Extender](options, &ext_processor.Processor{}, recipeControllerConfig.Engine, recipeControllerConfig.ConfigLoader)
 			},
 			AsyncOperationTimeout:    ext_ctrl.AsyncDeleteExtenderTimeout,
 			AsyncOperationRetryAfter: AsyncOperationRetryAfter,

pkg/daprrp/datamodel/daprpubsubbroker.go

@@ -50,7 +50,7 @@ func (r *DaprPubSubBroker) ResourceMetadata() *rpv1.BasicResourceProperties {
 }
 
 // ResourceTypeName returns a string representing the resource type.
-func (daprPubSub *DaprPubSubBroker) ResourceTypeName() string {
+func (r *DaprPubSubBroker) ResourceTypeName() string {
 	return portableresources.DaprPubSubBrokersResourceType
 }
 

pkg/daprrp/datamodel/daprsecretstore.go

@@ -50,7 +50,7 @@ func (r *DaprSecretStore) ResourceMetadata() *rpv1.BasicResourceProperties {
 }
 
 // ResourceTypeName returns the resource type of the DaprSecretStore resource.
-func (daprSecretStore *DaprSecretStore) ResourceTypeName() string {
+func (r *DaprSecretStore) ResourceTypeName() string {
 	return portableresources.DaprSecretStoresResourceType
 }
 
@@ -67,9 +67,9 @@ type DaprSecretStoreProperties struct {
 
 // Recipe returns the Recipe from the DaprSecretStore Properties if ResourceProvisioning is not set to Manual,
 // otherwise it returns nil.
-func (daprSecretStore *DaprSecretStore) Recipe() *portableresources.ResourceRecipe {
-	if daprSecretStore.Properties.ResourceProvisioning == portableresources.ResourceProvisioningManual {
+func (r *DaprSecretStore) Recipe() *portableresources.ResourceRecipe {
+	if r.Properties.ResourceProvisioning == portableresources.ResourceProvisioningManual {
 		return nil
 	}
-	return &daprSecretStore.Properties.Recipe
+	return &r.Properties.Recipe
 }

pkg/daprrp/datamodel/daprstatestore.go

@@ -50,7 +50,7 @@ func (r *DaprStateStore) ResourceMetadata() *rpv1.BasicResourceProperties {
 }
 
 // ResourceTypeName returns the resource type of the DaprStateStore resource.
-func (daprStateStore *DaprStateStore) ResourceTypeName() string {
+func (r *DaprStateStore) ResourceTypeName() string {
 	return portableresources.DaprStateStoresResourceType
 }
 

pkg/datastoresrp/datamodel/mongodatabase.go

@@ -70,16 +70,16 @@ func (mongoSecrets MongoDatabaseSecrets) IsEmpty() bool {
 }
 
 // VerifyInputs checks if the manual resource provisioning fields are set and returns an error if any of them are missing.
-func (mongodb *MongoDatabase) VerifyInputs() error {
+func (r *MongoDatabase) VerifyInputs() error {
 	msgs := []string{}
-	if mongodb.Properties.ResourceProvisioning != "" && mongodb.Properties.ResourceProvisioning == portableresources.ResourceProvisioningManual {
-		if mongodb.Properties.Host == "" {
+	if r.Properties.ResourceProvisioning != "" && r.Properties.ResourceProvisioning == portableresources.ResourceProvisioningManual {
+		if r.Properties.Host == "" {
 			msgs = append(msgs, "host must be specified when resourceProvisioning is set to manual")
 		}
-		if mongodb.Properties.Port == 0 {
+		if r.Properties.Port == 0 {
 			msgs = append(msgs, "port must be specified when resourceProvisioning is set to manual")
 		}
-		if mongodb.Properties.Database == "" {
+		if r.Properties.Database == "" {
 			msgs = append(msgs, "database must be specified when resourceProvisioning is set to manual")
 		}
 	}
@@ -130,6 +130,6 @@ func (mongoSecrets *MongoDatabaseSecrets) ResourceTypeName() string {
 }
 
 // ResourceTypeName returns the resource type for Mongo database resource.
-func (mongo *MongoDatabase) ResourceTypeName() string {
+func (r *MongoDatabase) ResourceTypeName() string {
 	return portableresources.MongoDatabasesResourceType
 }

pkg/datastoresrp/datamodel/rediscache.go

@@ -54,17 +54,17 @@ func (r *RedisCache) ResourceMetadata() *rpv1.BasicResourceProperties {
 }
 
 // ResourceTypeName returns the resource type of Redis cache resource.
-func (redis *RedisCache) ResourceTypeName() string {
+func (r *RedisCache) ResourceTypeName() string {
 	return portableresources.RedisCachesResourceType
 }
 
 // Recipe returns the ResourceRecipe from the Redis cache Properties if ResourceProvisioning is not set to Manual,
 // otherwise it returns nil.
-func (redis *RedisCache) Recipe() *portableresources.ResourceRecipe {
-	if redis.Properties.ResourceProvisioning == portableresources.ResourceProvisioningManual {
+func (r *RedisCache) Recipe() *portableresources.ResourceRecipe {
+	if r.Properties.ResourceProvisioning == portableresources.ResourceProvisioningManual {
 		return nil
 	}
-	return &redis.Properties.Recipe
+	return &r.Properties.Recipe
 }
 
 // IsEmpty checks if the RedisCacheSecrets instance is empty or not.
@@ -74,13 +74,13 @@ func (redisSecrets *RedisCacheSecrets) IsEmpty() bool {
 
 // VerifyInputs checks if the required fields are set when the resourceProvisioning is set to manual
 // and returns an error if not.
-func (redisCache *RedisCache) VerifyInputs() error {
+func (r *RedisCache) VerifyInputs() error {
 	msgs := []string{}
-	if redisCache.Properties.ResourceProvisioning != "" && redisCache.Properties.ResourceProvisioning == portableresources.ResourceProvisioningManual {
-		if redisCache.Properties.Host == "" {
+	if r.Properties.ResourceProvisioning != "" && r.Properties.ResourceProvisioning == portableresources.ResourceProvisioningManual {
+		if r.Properties.Host == "" {
 			msgs = append(msgs, "host must be specified when resourceProvisioning is set to manual")
 		}
-		if redisCache.Properties.Port == 0 {
+		if r.Properties.Port == 0 {
 			msgs = append(msgs, "port must be specified when resourceProvisioning is set to manual")
 		}
 	}

pkg/datastoresrp/datamodel/sqldatabase.go

@@ -63,7 +63,7 @@ func (r *SqlDatabase) ResourceMetadata() *rpv1.BasicResourceProperties {
 }
 
 // ResourceTypeName returns the resource type of the SQL database resource.
-func (sql *SqlDatabase) ResourceTypeName() string {
+func (r *SqlDatabase) ResourceTypeName() string {
 	return portableresources.SqlDatabasesResourceType
 }
 

pkg/messagingrp/datamodel/rabbitmq.go

@@ -54,7 +54,7 @@ func (r *RabbitMQQueue) ResourceMetadata() *rpv1.BasicResourceProperties {
 }
 
 // ResourceTypeName returns the resource type name for RabbitMQ queues.
-func (rabbitmq *RabbitMQQueue) ResourceTypeName() string {
+func (r *RabbitMQQueue) ResourceTypeName() string {
 	return portableresources.RabbitMQQueuesResourceType
 }
 
@@ -94,8 +94,8 @@ func (r *RabbitMQQueue) Recipe() *portableresources.ResourceRecipe {
 }
 
 // VerifyInputs checks if the queue is provided when resourceProvisioning is set to manual and returns an error if not.
-func (rabbitmq *RabbitMQQueue) VerifyInputs() error {
-	properties := rabbitmq.Properties
+func (r *RabbitMQQueue) VerifyInputs() error {
+	properties := r.Properties
 	msgs := []string{}
 	if properties.ResourceProvisioning != "" && properties.ResourceProvisioning == portableresources.ResourceProvisioningManual {
 		if properties.Queue == "" {

pkg/portableresources/backend/controller/createorupdateresource.go

@@ -26,6 +26,7 @@ import (
 	"github.com/radius-project/radius/pkg/recipes"
 	"github.com/radius-project/radius/pkg/recipes/configloader"
 	"github.com/radius-project/radius/pkg/recipes/engine"
+	"github.com/radius-project/radius/pkg/recipes/util"
 	rpv1 "github.com/radius-project/radius/pkg/rp/v1"
 	"github.com/radius-project/radius/pkg/ucp/store"
 )
@@ -76,9 +77,23 @@ func (c *CreateOrUpdateResource[P, T]) Run(ctx context.Context, req *ctrl.Reques
 	previousOutputResources := c.copyOutputResources(data)
 
 	// Now we're ready to process recipes (if needed).
+	recipeDataModel := any(data).(datamodel.RecipeDataModel)
 	recipeOutput, err := c.executeRecipeIfNeeded(ctx, data, previousOutputResources)
 	if err != nil {
 		if recipeError, ok := err.(*recipes.RecipeError); ok {
+			// Set the deployment status to the recipe error code.
+			recipeDataModel.Recipe().DeploymentStatus = util.RecipeDeploymentStatus(recipeError.DeploymentStatus)
+			update := &store.Object{
+				Metadata: store.Metadata{
+					ID: req.ResourceID,
+				},
+				Data: recipeDataModel.(rpv1.RadiusResourceModel),
+			}
+			// Save portable resource with updated deployment status to track errors during deletion.
+			err = c.StorageClient().Save(ctx, update, store.WithETag(obj.ETag))
+			if err != nil {
+				return ctrl.Result{}, err
+			}
 			return ctrl.NewFailedResult(recipeError.ErrorDetails), nil
 		}
 		return ctrl.Result{}, err
@@ -96,11 +111,14 @@ func (c *CreateOrUpdateResource[P, T]) Run(ctx context.Context, req *ctrl.Reques
 		return ctrl.Result{}, err
 	}
 
+	if recipeDataModel.Recipe() != nil {
+		recipeDataModel.Recipe().DeploymentStatus = util.Success
+	}
 	update := &store.Object{
 		Metadata: store.Metadata{
 			ID: req.ResourceID,
 		},
-		Data: data,
+		Data: recipeDataModel.(rpv1.RadiusResourceModel),
 	}
 	err = c.StorageClient().Save(ctx, update, store.WithETag(obj.ETag))
 	if err != nil {

pkg/portableresources/backend/controller/createorupdateresource_test.go

@@ -19,6 +19,7 @@ package controller
 import (
 	"context"
 	"errors"
+	"fmt"
 	"testing"
 
 	"github.com/golang/mock/gomock"
@@ -122,9 +123,9 @@ var errorProcessorReference = processors.ResourceProcessor[*TestResource, TestRe
 var errProcessor = errors.New("processor error")
 var errConfiguration = errors.New("configuration error")
 
-var oldOutputResourceResourceID = "/subscriptions/test-sub/resourceGroups/test-rg/providers/System.Test/testResources/test1"
+var oldOutputResourceResourceID = "/subscriptions/test-sub/resourceGroups/test-rg/providers/Applications.Test/testResources/test1"
 
-var newOutputResourceResourceID = "/subscriptions/test-sub/resourceGroups/test-rg/providers/System.Test/testResources/test2"
+var newOutputResourceResourceID = "/subscriptions/test-sub/resourceGroups/test-rg/providers/Applications.Test/testResourcess/test2"
 var newOutputResource = rpv1.OutputResource{ID: resources.MustParse(newOutputResourceResourceID)}
 
 func TestCreateOrUpdateResource_Run(t *testing.T) {
@@ -199,12 +200,12 @@ func TestCreateOrUpdateResource_Run(t *testing.T) {
 			},
 			nil,
 			false,
-			&recipes.ErrRecipeNotFound{},
+			fmt.Errorf("could not find recipe %q in environment %q", "test-recipe", TestEnvironmentID),
 			nil,
 			nil,
 			nil,
 			nil,
-			&recipes.ErrRecipeNotFound{},
+			fmt.Errorf("could not find recipe %q in environment %q", "test-recipe", TestEnvironmentID),
 		},
 		{
 			"runtime-configuration-err",

pkg/portableresources/backend/controller/deleteresource.go

@@ -26,6 +26,7 @@ import (
 	"github.com/radius-project/radius/pkg/recipes"
 	"github.com/radius-project/radius/pkg/recipes/configloader"
 	"github.com/radius-project/radius/pkg/recipes/engine"
+	"github.com/radius-project/radius/pkg/recipes/util"
 	rpv1 "github.com/radius-project/radius/pkg/rp/v1"
 	"github.com/radius-project/radius/pkg/ucp/resources"
 )
@@ -74,7 +75,10 @@ func (c *DeleteResource[P, T]) Run(ctx context.Context, request *ctrl.Request) (
 	}
 
 	recipeDataModel, supportsRecipes := any(data).(datamodel.RecipeDataModel)
-	if supportsRecipes && recipeDataModel.Recipe() != nil {
+
+	// If we have a setup error (error before recipe and output resources are executed, we skip engine/driver deletion.
+	// If we have an execution error, we call engine/driver deletion.
+	if supportsRecipes && recipeDataModel.Recipe() != nil && recipeDataModel.Recipe().DeploymentStatus != util.RecipeSetupError {
 		recipeData := recipes.ResourceMetadata{
 			Name:          recipeDataModel.Recipe().Name,
 			EnvironmentID: data.ResourceMetadata().Environment,

pkg/portableresources/types.go

@@ -18,6 +18,8 @@ package portableresources
 
 import (
 	"strings"
+
+	"github.com/radius-project/radius/pkg/recipes/util"
 )
 
 const (
@@ -80,6 +82,8 @@ type ResourceRecipe struct {
 	Name string `json:"name,omitempty"`
 	// Parameters are key/value parameters to pass into the recipe at deployment
 	Parameters map[string]any `json:"parameters,omitempty"`
+	// DeploymentStatus is the deployment status of the recipe
+	DeploymentStatus util.RecipeDeploymentStatus `json:"recipeStatus,omitempty"`
 }
 
 // ResourceReference represents a reference to a resource that was deployed by the user

pkg/recipes/configloader/environment.go

@@ -25,6 +25,7 @@ import (
 	"github.com/radius-project/radius/pkg/corerp/api/v20220315privatepreview"
 	"github.com/radius-project/radius/pkg/corerp/datamodel"
 	"github.com/radius-project/radius/pkg/recipes"
+	recipes_util "github.com/radius-project/radius/pkg/recipes/util"
 	"github.com/radius-project/radius/pkg/rp/kube"
 	"github.com/radius-project/radius/pkg/rp/util"
 	"github.com/radius-project/radius/pkg/to"
@@ -124,17 +125,20 @@ func (e *environmentLoader) LoadRecipe(ctx context.Context, recipe *recipes.Reso
 
 func getRecipeDefinition(environment *v20220315privatepreview.EnvironmentResource, recipe *recipes.ResourceMetadata) (*recipes.EnvironmentDefinition, error) {
 	if environment.Properties.Recipes == nil {
-		return nil, &recipes.ErrRecipeNotFound{Name: recipe.Name, Environment: recipe.EnvironmentID}
+		err := fmt.Errorf("could not find recipe %q in environment %q", recipe.Name, recipe.EnvironmentID)
+		return nil, recipes.NewRecipeError(recipes.RecipeNotFoundFailure, err.Error(), recipes_util.RecipeSetupError, recipes.GetRecipeErrorDetails(err))
 	}
 
 	resource, err := resources.ParseResource(recipe.ResourceID)
 	if err != nil {
-		return nil, fmt.Errorf("failed to parse resourceID: %q %w", recipe.ResourceID, err)
+		err := fmt.Errorf("failed to parse resourceID: %q %w", recipe.ResourceID, err)
+		return nil, recipes.NewRecipeError(recipes.RecipeValidationFailed, err.Error(), recipes_util.RecipeSetupError, recipes.GetRecipeErrorDetails(err))
 	}
 	recipeName := recipe.Name
 	found, ok := environment.Properties.Recipes[resource.Type()][recipeName]
 	if !ok {
-		return nil, &recipes.ErrRecipeNotFound{Name: recipe.Name, Environment: recipe.EnvironmentID}
+		err := fmt.Errorf("could not find recipe %q in environment %q", recipe.Name, recipe.EnvironmentID)
+		return nil, recipes.NewRecipeError(recipes.RecipeNotFoundFailure, err.Error(), recipes_util.RecipeSetupError, recipes.GetRecipeErrorDetails(err))
 	}
 
 	definition := &recipes.EnvironmentDefinition{

pkg/recipes/configloader/environment_test.go

@@ -249,7 +249,7 @@ func TestGetRecipeDefinition(t *testing.T) {
 		metadata.ResourceID = "invalid-id"
 		_, err := getRecipeDefinition(&envResource, &metadata)
 		require.Error(t, err)
-		require.Contains(t, err.Error(), "failed to parse resourceID")
+		require.Contains(t, err.Error(), "'invalid-id' is not a valid resource id")
 	})
 
 	t.Run("recipe not found for the resource type", func(t *testing.T) {