From 9aa13b3365043ebc7baf299a20840d27ab9036f2 Mon Sep 17 00:00:00 2001 From: Shruthi Kumar Date: Tue, 12 Sep 2023 19:58:13 -0700 Subject: [PATCH] Fix stuck portable resource deletion bug (#6247) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Description Adding deletion logic to address #6040. The general logic is that if we have an error before the Recipe is deployed, we don't proceed with engine/driver deletion. If there was an error during execution, we proceed with engine/driver deletion to take care of any output resources that may have been deployed. ## Type of change - This pull request fixes a bug in Radius and has an approved issue #6040 Fixes: #6040 ## Auto-generated summary ### ๐Ÿค– Generated by Copilot at c552dea ### Summary ๐Ÿš€๐Ÿ› ๏ธ๐Ÿงช Added and updated methods to set and save the deployment status of portable resources based on the recipe execution result. Improved error handling and reporting for recipe drivers and config loaders. Renamed some receiver names for consistency and readability. Updated test cases to match the expected errors and statuses. > _To manage resources with recipes_ > _We added some methods and types, please_ > _They update the status_ > _And handle the errors_ > _With `portableresources` and `drivers`_ ### Walkthrough * Add `SetDeploymentStatus` method to various resource types to update deployment status based on recipe execution result ([link](https://github.com/radius-project/radius/pull/6247/files?diff=unified&w=0#diff-b35b07eee0834a6f13240f9f22b2a09d8a933298507c72f37ff5792ba830fd02L62-R73), [link](https://github.com/radius-project/radius/pull/6247/files?diff=unified&w=0#diff-a74b5d21cf74e51c5ab93a6f4634a606a4f869aa1b9a279fb25df2fdcc386d2dR65-R69), [link](https://github.com/radius-project/radius/pull/6247/files?diff=unified&w=0#diff-e58a6737964c53219141f95631bcc9c8e7bb532d131b03919fd9593cf29b6dbaL70-R79), [link](https://github.com/radius-project/radius/pull/6247/files?diff=unified&w=0#diff-fc58b9bab039ec2431cb779e41394e5f95a51cb7f415b0da40c630f441810f3aR65-R69), [link](https://github.com/radius-project/radius/pull/6247/files?diff=unified&w=0#diff-b463cddd0e8f4aa4356725d5f2eb81f215216544149ac54b74a0a910868665a1R127-R131), [link](https://github.com/radius-project/radius/pull/6247/files?diff=unified&w=0#diff-0edf5ad38ca1c7194bb66f75828b10cd79c512f9fdabea5eb00db52e016a8f39R103-R107), [link](https://github.com/radius-project/radius/pull/6247/files?diff=unified&w=0#diff-79d6e8286f9b10662ae6ec8e91f5c32a7d53fcc762f922c4452130b1fe0005dbL66-R74), [link](https://github.com/radius-project/radius/pull/6247/files?diff=unified&w=0#diff-b5a5c7f766f17b812eb1e5bd1815303ade4f0486d243708a526832a65bbed011R83-R87)) * Change receiver names of `ResourceTypeName` and other methods for various resource types to `r` for consistency ([link](https://github.com/radius-project/radius/pull/6247/files?diff=unified&w=0#diff-b35b07eee0834a6f13240f9f22b2a09d8a933298507c72f37ff5792ba830fd02L56-R56), [link](https://github.com/radius-project/radius/pull/6247/files?diff=unified&w=0#diff-a74b5d21cf74e51c5ab93a6f4634a606a4f869aa1b9a279fb25df2fdcc386d2dL53-R53), [link](https://github.com/radius-project/radius/pull/6247/files?diff=unified&w=0#diff-e58a6737964c53219141f95631bcc9c8e7bb532d131b03919fd9593cf29b6dbaL53-R61), [link](https://github.com/radius-project/radius/pull/6247/files?diff=unified&w=0#diff-fc58b9bab039ec2431cb779e41394e5f95a51cb7f415b0da40c630f441810f3aL53-R53), [link](https://github.com/radius-project/radius/pull/6247/files?diff=unified&w=0#diff-b463cddd0e8f4aa4356725d5f2eb81f215216544149ac54b74a0a910868665a1L73-R82), [link](https://github.com/radius-project/radius/pull/6247/files?diff=unified&w=0#diff-b463cddd0e8f4aa4356725d5f2eb81f215216544149ac54b74a0a910868665a1L133-R138), [link](https://github.com/radius-project/radius/pull/6247/files?diff=unified&w=0#diff-0edf5ad38ca1c7194bb66f75828b10cd79c512f9fdabea5eb00db52e016a8f39L57-R57), [link](https://github.com/radius-project/radius/pull/6247/files?diff=unified&w=0#diff-0edf5ad38ca1c7194bb66f75828b10cd79c512f9fdabea5eb00db52e016a8f39L63-R67), [link](https://github.com/radius-project/radius/pull/6247/files?diff=unified&w=0#diff-0edf5ad38ca1c7194bb66f75828b10cd79c512f9fdabea5eb00db52e016a8f39L77-R83), [link](https://github.com/radius-project/radius/pull/6247/files?diff=unified&w=0#diff-2f8e234f7e5ff8a646351b71cfa4333f1d1f935b4116ec32bcf6e8b6d117583eL57-R65), [link](https://github.com/radius-project/radius/pull/6247/files?diff=unified&w=0#diff-2f8e234f7e5ff8a646351b71cfa4333f1d1f935b4116ec32bcf6e8b6d117583eL97-R103)) * Change `NewCreateOrUpdateResource` and `NewDeleteResource` functions to use generic type parameters instead of empty interface ([link](https://github.com/radius-project/radius/pull/6247/files?diff=unified&w=0#diff-1dfecb9519c9002f569daea7fb61bbb0b1f22e34d49ac2d03eeb3bc64e858c8fL224-R224), [link](https://github.com/radius-project/radius/pull/6247/files?diff=unified&w=0#diff-1dfecb9519c9002f569daea7fb61bbb0b1f22e34d49ac2d03eeb3bc64e858c8fL234-R234), [link](https://github.com/radius-project/radius/pull/6247/files?diff=unified&w=0#diff-1dfecb9519c9002f569daea7fb61bbb0b1f22e34d49ac2d03eeb3bc64e858c8fL241-R241)) * Change `executeRecipeIfNeeded` method to set and save deployment status of resource based on recipe error code and status ([link](https://github.com/radius-project/radius/pull/6247/files?diff=unified&w=0#diff-88040b9959d0c406cb8ed95caa785345085b114078658de636164a546f910749L79-R96), [link](https://github.com/radius-project/radius/pull/6247/files?diff=unified&w=0#diff-88040b9959d0c406cb8ed95caa785345085b114078658de636164a546f910749L99-R119), [link](https://github.com/radius-project/radius/pull/6247/files?diff=unified&w=0#diff-88040b9959d0c406cb8ed95caa785345085b114078658de636164a546f910749L123-R139)) * Change `Run` method to skip recipe deletion if deployment status is `RecipeSetupError` ([link](https://github.com/radius-project/radius/pull/6247/files?diff=unified&w=0#diff-c6fc204032902b38f97dc71fe64ce145d1f851419f4eef7682a2d62183172a03L76-R81)) * Add `SetDeploymentStatus` method to `RecipeDataModel` interface ([link](https://github.com/radius-project/radius/pull/6247/files?diff=unified&w=0#diff-fa271c3c303372742608e60b5de526ef1d2f46f492f6e33a0e7003c6b8ad8b25R31-R32)) * Add `DeploymentStatus` field to `ResourceRecipe` type ([link](https://github.com/radius-project/radius/pull/6247/files?diff=unified&w=0#diff-a84c1408454dbdd6e92b77d98a50ce8954bc2477ffbef4eabd32d9f1d587e743R83-R84)) * Add `RecipeDeploymentStatus` type and constants to `portableresources` package ([link](https://github.com/radius-project/radius/pull/6247/files?diff=unified&w=0#diff-a84c1408454dbdd6e92b77d98a50ce8954bc2477ffbef4eabd32d9f1d587e743R131-R141)) * Change `getRecipeDefinition` function to use `NewRecipeError` function with `RecipeSetupError` status ([link](https://github.com/radius-project/radius/pull/6247/files?diff=unified&w=0#diff-0b45f6a08490c2a47c6ead052f97e5b5be965617c8ab927d9b0a297088a92a6eL127-R141)) * Change `Execute` and `Delete` methods of `Bicep` and `Terraform` drivers to use `NewRecipeError` function with appropriate status ([link](https://github.com/radius-project/radius/pull/6247/files?diff=unified&w=0#diff-f12e37706fa75e9e04b8dfa01a45cf9bf2a0463e1962605b2420d0cb343922a0L82-R83), [link](https://github.com/radius-project/radius/pull/6247/files?diff=unified&w=0#diff-f12e37706fa75e9e04b8dfa01a45cf9bf2a0463e1962605b2420d0cb343922a0L90-R91), [link](https://github.com/radius-project/radius/pull/6247/files?diff=unified&w=0#diff-f12e37706fa75e9e04b8dfa01a45cf9bf2a0463e1962605b2420d0cb343922a0L101-R102), [link](https://github.com/radius-project/radius/pull/6247/files?diff=unified&w=0#diff-f12e37706fa75e9e04b8dfa01a45cf9bf2a0463e1962605b2420d0cb343922a0L129-R140), [link](https://github.com/radius-project/radius/pull/6247/files?diff=unified&w=0#diff-f12e37706fa75e9e04b8dfa01a45cf9bf2a0463e1962605b2420d0cb343922a0L151-R152), [link](https://github.com/radius-project/radius/pull/6247/files?diff=unified&w=0#diff-f12e37706fa75e9e04b8dfa01a45cf9bf2a0463e1962605b2420d0cb343922a0L164-R165), [link](https://github.com/radius-project/radius/pull/6247/files?diff=unified&w=0#diff-f12e37706fa75e9e04b8dfa01a45cf9bf2a0463e1962605b2420d0cb343922a0L179-R180), [link](https://github.com/radius-project/radius/pull/6247/files?diff=unified&w=0#diff-1b98bc9b02b96687752860c11c68d87d99ad99f24ecdfc5497fda6823abc0612L72-R73), [link](https://github.com/radius-project/radius/pull/6247/files?diff=unified&w=0#diff-1b98bc9b02b96687752860c11c68d87d99ad99f24ecdfc5497fda6823abc0612L87-R93), [link](https://github.com/radius-project/radius/pull/6247/files?diff=unified&w=0#diff-1b98bc9b02b96687752860c11c68d87d99ad99f24ecdfc5497fda6823abc0612L104-R105), [link](https://github.com/radius-project/radius/pull/6247/files?diff=unified&w=0#diff-1b98bc9b02b96687752860c11c68d87d99ad99f24ecdfc5497fda6823abc0612L119-R120), [link](https://github.com/radius-project/radius/pull/6247/files?diff=unified&w=0#diff-1b98bc9b02b96687752860c11c68d87d99ad99f24ecdfc5497fda6823abc0612L183-R184), [link](https://github.com/radius-project/radius/pull/6247/files?diff=unified&w=0#diff-1b98bc9b02b96687752860c11c68d87d99ad99f24ecdfc5497fda6823abc0612L197-R198)) * Change `GetRecipeMetadata` method of `Terraform` driver to use `NewRecipeError` function with empty status ([link](https://github.com/radius-project/radius/pull/6247/files?diff=unified&w=0#diff-1b98bc9b02b96687752860c11c68d87d99ad99f24ecdfc5497fda6823abc0612L183-R184), [link](https://github.com/radius-project/radius/pull/6247/files?diff=unified&w=0#diff-1b98bc9b02b96687752860c11c68d87d99ad99f24ecdfc5497fda6823abc0612L197-R198)) * Change `RecipeError` type to include `DeploymentStatus` field ([link](https://github.com/radius-project/radius/pull/6247/files?diff=unified&w=0#diff-f58db8b718c75e0a6b877b5750d8613d7be5aa1a4aafff11b861caeac4aff66dL20-R25)) * Change `NewRecipeError` function to accept `DeploymentStatus` parameter ([link](https://github.com/radius-project/radius/pull/6247/files?diff=unified&w=0#diff-f58db8b718c75e0a6b877b5750d8613d7be5aa1a4aafff11b861caeac4aff66dL37-R43)) * Change test cases to use `Applications.Core/extenders` resource type instead of `Applications.Test/testResources` type ([link](https://github.com/radius-project/radius/pull/6247/files?diff=unified&w=0#diff-b5a5c7f766f17b812eb1e5bd1815303ade4f0486d243708a526832a65bbed011L44-R47), [link](https://github.com/radius-project/radius/pull/6247/files?diff=unified&w=0#diff-b5a5c7f766f17b812eb1e5bd1815303ade4f0486d243708a526832a65bbed011L125-R132), [link](https://github.com/radius-project/radius/pull/6247/files?diff=unified&w=0#diff-b5a5c7f766f17b812eb1e5bd1815303ade4f0486d243708a526832a65bbed011L273-R278), [link](https://github.com/radius-project/radius/pull/6247/files?diff=unified&w=0#diff-b5a5c7f766f17b812eb1e5bd1815303ade4f0486d243708a526832a65bbed011L281-R286)) * Change test cases to expect `RecipeSetupError` or `ExecutionError` status in error details ([link](https://github.com/radius-project/radius/pull/6247/files?diff=unified&w=0#diff-b5a5c7f766f17b812eb1e5bd1815303ade4f0486d243708a526832a65bbed011L202-R212), [link](https://github.com/radius-project/radius/pull/6247/files?diff=unified&w=0#diff-a0a208a111e71713a21b404fdb3b31fd13f668e5716ab4331584027cc3d98debR477), [link](https://github.com/radius-project/radius/pull/6247/files?diff=unified&w=0#diff-ae6e6fdc3b9219f2f1b2dbfd1cfaff3ff2d941a2a4d58fd33b910bd1fbf31e92R154), [link](https://github.com/radius-project/radius/pull/6247/files?diff=unified&w=0#diff-ae6e6fdc3b9219f2f1b2dbfd1cfaff3ff2d941a2a4d58fd33b910bd1fbf31e92R209), [link](https://github.com/radius-project/radius/pull/6247/files?diff=unified&w=0#diff-ae6e6fdc3b9219f2f1b2dbfd1cfaff3ff2d941a2a4d58fd33b910bd1fbf31e92R236)) * Change test case to use `RecipeError` type with `RecipeGetMetadataFailed` code and empty status ([link](https://github.com/radius-project/radius/pull/6247/files?diff=unified&w=0#diff-ae6e6fdc3b9219f2f1b2dbfd1cfaff3ff2d941a2a4d58fd33b910bd1fbf31e92L382-R391), [link](https://github.com/radius-project/radius/pull/6247/files?diff=unified&w=0#diff-ae6e6fdc3b9219f2f1b2dbfd1cfaff3ff2d941a2a4d58fd33b910bd1fbf31e92L390-R398)) --- pkg/corerp/datamodel/extender.go | 8 +++--- pkg/daprrp/datamodel/daprpubsubbroker.go | 2 +- pkg/daprrp/datamodel/daprsecretstore.go | 8 +++--- pkg/daprrp/datamodel/daprstatestore.go | 2 +- pkg/datastoresrp/datamodel/mongodatabase.go | 12 ++++----- pkg/datastoresrp/datamodel/rediscache.go | 16 ++++++------ pkg/datastoresrp/datamodel/sqldatabase.go | 2 +- pkg/messagingrp/datamodel/rabbitmq.go | 6 ++--- .../controller/createorupdateresource.go | 20 ++++++++++++++- .../controller/createorupdateresource_test.go | 9 ++++--- .../backend/controller/deleteresource.go | 6 ++++- pkg/portableresources/types.go | 4 +++ pkg/recipes/configloader/environment.go | 10 +++++--- pkg/recipes/configloader/environment_test.go | 2 +- pkg/recipes/driver/bicep.go | 19 +++++++------- pkg/recipes/driver/bicep_test.go | 1 + pkg/recipes/driver/terraform.go | 15 +++++------ pkg/recipes/driver/terraform_test.go | 14 ++++++++--- pkg/recipes/engine/engine.go | 6 +++-- pkg/recipes/engine/engine_test.go | 4 +-- pkg/recipes/error.go | 7 ++++-- pkg/recipes/error_test.go | 6 ++++- pkg/recipes/errorcodes.go | 12 +++++++++ pkg/recipes/terraform/execute.go | 3 ++- pkg/recipes/types.go | 11 -------- pkg/recipes/util/deploymentStatus.go | 25 +++++++++++++++++++ pkg/rp/util/registry.go | 5 ++-- .../shared/resources/recipe_test.go | 6 ++--- 28 files changed, 159 insertions(+), 82 deletions(-) create mode 100644 pkg/recipes/util/deploymentStatus.go diff --git a/pkg/corerp/datamodel/extender.go b/pkg/corerp/datamodel/extender.go index a0780546a3..b8e1592015 100644 --- a/pkg/corerp/datamodel/extender.go +++ b/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. diff --git a/pkg/daprrp/datamodel/daprpubsubbroker.go b/pkg/daprrp/datamodel/daprpubsubbroker.go index 35367d6331..dfd17bba76 100644 --- a/pkg/daprrp/datamodel/daprpubsubbroker.go +++ b/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 } diff --git a/pkg/daprrp/datamodel/daprsecretstore.go b/pkg/daprrp/datamodel/daprsecretstore.go index d6bdad5e5e..b0b92777c5 100644 --- a/pkg/daprrp/datamodel/daprsecretstore.go +++ b/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 } diff --git a/pkg/daprrp/datamodel/daprstatestore.go b/pkg/daprrp/datamodel/daprstatestore.go index fde21de75c..97d4737c19 100644 --- a/pkg/daprrp/datamodel/daprstatestore.go +++ b/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 } diff --git a/pkg/datastoresrp/datamodel/mongodatabase.go b/pkg/datastoresrp/datamodel/mongodatabase.go index 322996f187..21052cfef9 100644 --- a/pkg/datastoresrp/datamodel/mongodatabase.go +++ b/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 } diff --git a/pkg/datastoresrp/datamodel/rediscache.go b/pkg/datastoresrp/datamodel/rediscache.go index 67790d7d06..660db2ce19 100644 --- a/pkg/datastoresrp/datamodel/rediscache.go +++ b/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") } } diff --git a/pkg/datastoresrp/datamodel/sqldatabase.go b/pkg/datastoresrp/datamodel/sqldatabase.go index 3cfad887af..8483d82960 100644 --- a/pkg/datastoresrp/datamodel/sqldatabase.go +++ b/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 } diff --git a/pkg/messagingrp/datamodel/rabbitmq.go b/pkg/messagingrp/datamodel/rabbitmq.go index cbc8e833e9..0cb7812dbc 100644 --- a/pkg/messagingrp/datamodel/rabbitmq.go +++ b/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 == "" { diff --git a/pkg/portableresources/backend/controller/createorupdateresource.go b/pkg/portableresources/backend/controller/createorupdateresource.go index 05aad4e8a4..ace7167b3d 100644 --- a/pkg/portableresources/backend/controller/createorupdateresource.go +++ b/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 { diff --git a/pkg/portableresources/backend/controller/createorupdateresource_test.go b/pkg/portableresources/backend/controller/createorupdateresource_test.go index ea65a55a08..abc703477d 100644 --- a/pkg/portableresources/backend/controller/createorupdateresource_test.go +++ b/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/Systems.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/Systems.Test/testResources/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", diff --git a/pkg/portableresources/backend/controller/deleteresource.go b/pkg/portableresources/backend/controller/deleteresource.go index 67d374b25a..c5990b53c5 100644 --- a/pkg/portableresources/backend/controller/deleteresource.go +++ b/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, diff --git a/pkg/portableresources/types.go b/pkg/portableresources/types.go index c77c2c268c..6deca70788 100644 --- a/pkg/portableresources/types.go +++ b/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 diff --git a/pkg/recipes/configloader/environment.go b/pkg/recipes/configloader/environment.go index f30b573ee1..1aec852ec5 100644 --- a/pkg/recipes/configloader/environment.go +++ b/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{ diff --git a/pkg/recipes/configloader/environment_test.go b/pkg/recipes/configloader/environment_test.go index 6e0623eefc..98e5934905 100644 --- a/pkg/recipes/configloader/environment_test.go +++ b/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) { diff --git a/pkg/recipes/driver/bicep.go b/pkg/recipes/driver/bicep.go index 45fde7c048..82c4c91594 100644 --- a/pkg/recipes/driver/bicep.go +++ b/pkg/recipes/driver/bicep.go @@ -32,6 +32,7 @@ import ( "github.com/radius-project/radius/pkg/portableresources/processors" "github.com/radius-project/radius/pkg/recipes" "github.com/radius-project/radius/pkg/recipes/recipecontext" + recipes_util "github.com/radius-project/radius/pkg/recipes/util" "github.com/radius-project/radius/pkg/rp/util" rpv1 "github.com/radius-project/radius/pkg/rp/v1" clients "github.com/radius-project/radius/pkg/sdk/clients" @@ -79,7 +80,7 @@ func (d *bicepDriver) Execute(ctx context.Context, opts ExecuteOptions) (*recipe if err != nil { metrics.DefaultRecipeEngineMetrics.RecordRecipeDownloadDuration(ctx, downloadStartTime, metrics.NewRecipeAttributes(metrics.RecipeEngineOperationDownloadRecipe, opts.Recipe.Name, &opts.Definition, recipes.RecipeDownloadFailed)) - return nil, recipes.NewRecipeError(recipes.RecipeDownloadFailed, err.Error(), recipes.GetRecipeErrorDetails(err)) + return nil, recipes.NewRecipeError(recipes.RecipeDownloadFailed, err.Error(), recipes_util.RecipeSetupError, recipes.GetRecipeErrorDetails(err)) } metrics.DefaultRecipeEngineMetrics.RecordRecipeDownloadDuration(ctx, downloadStartTime, metrics.NewRecipeAttributes(metrics.RecipeEngineOperationDownloadRecipe, opts.Recipe.Name, &opts.Definition, metrics.SuccessfulOperationState)) @@ -87,7 +88,7 @@ func (d *bicepDriver) Execute(ctx context.Context, opts ExecuteOptions) (*recipe // create the context object to be passed to the recipe deployment recipeContext, err := recipecontext.New(&opts.Recipe, &opts.Configuration) if err != nil { - return nil, recipes.NewRecipeError(recipes.RecipeDeploymentFailed, err.Error(), recipes.GetRecipeErrorDetails(err)) + return nil, recipes.NewRecipeError(recipes.RecipeDeploymentFailed, err.Error(), recipes_util.RecipeSetupError, recipes.GetRecipeErrorDetails(err)) } // get the parameters after resolving the conflict between developer and operator parameters @@ -98,7 +99,7 @@ func (d *bicepDriver) Execute(ctx context.Context, opts ExecuteOptions) (*recipe deploymentName := deploymentPrefix + strconv.FormatInt(time.Now().UnixNano(), 10) deploymentID, err := createDeploymentID(recipeContext.Resource.ID, deploymentName) if err != nil { - return nil, recipes.NewRecipeError(recipes.RecipeDeploymentFailed, err.Error(), recipes.GetRecipeErrorDetails(err)) + return nil, recipes.NewRecipeError(recipes.RecipeDeploymentFailed, err.Error(), recipes_util.RecipeSetupError, recipes.GetRecipeErrorDetails(err)) } // Provider config will specify the Azure and AWS scopes (if provided). @@ -126,17 +127,17 @@ func (d *bicepDriver) Execute(ctx context.Context, opts ExecuteOptions) (*recipe clients.DeploymentsClientAPIVersion, ) if err != nil { - return nil, recipes.NewRecipeError(recipes.RecipeDeploymentFailed, err.Error(), recipes.GetRecipeErrorDetails(err)) + return nil, recipes.NewRecipeError(recipes.RecipeDeploymentFailed, err.Error(), recipes_util.ExecutionError, recipes.GetRecipeErrorDetails(err)) } resp, err := poller.PollUntilDone(ctx, &runtime.PollUntilDoneOptions{Frequency: pollFrequency}) if err != nil { - return nil, recipes.NewRecipeError(recipes.RecipeDeploymentFailed, err.Error(), recipes.GetRecipeErrorDetails(err)) + return nil, recipes.NewRecipeError(recipes.RecipeDeploymentFailed, err.Error(), recipes_util.ExecutionError, recipes.GetRecipeErrorDetails(err)) } recipeResponse, err := d.prepareRecipeResponse(resp.Properties.Outputs, resp.Properties.OutputResources) if err != nil { - return nil, recipes.NewRecipeError(recipes.InvalidRecipeOutputs, fmt.Sprintf("failed to read the recipe output %q: %s", recipes.ResultPropertyName, err.Error()), recipes.GetRecipeErrorDetails(err)) + return nil, recipes.NewRecipeError(recipes.InvalidRecipeOutputs, fmt.Sprintf("failed to read the recipe output %q: %s", recipes.ResultPropertyName, err.Error()), recipes_util.ExecutionError, recipes.GetRecipeErrorDetails(err)) } // When a Radius portable resource consuming a recipe is redeployed, Garbage collection of the recipe resources that aren't included @@ -148,7 +149,7 @@ func (d *bicepDriver) Execute(ctx context.Context, opts ExecuteOptions) (*recipe // Deleting obsolete output resources. err = d.deleteGCOutputResources(ctx, diff) if err != nil { - return nil, recipes.NewRecipeError(recipes.RecipeGarbageCollectionFailed, err.Error(), nil) + return nil, recipes.NewRecipeError(recipes.RecipeGarbageCollectionFailed, err.Error(), recipes_util.ExecutionError, nil) } return recipeResponse, nil @@ -161,7 +162,7 @@ func (d *bicepDriver) Delete(ctx context.Context, opts DeleteOptions) error { orderedOutputResources, err := rpv1.OrderOutputResources(opts.OutputResources) if err != nil { - return recipes.NewRecipeError(recipes.RecipeDeletionFailed, err.Error(), recipes.GetRecipeErrorDetails(err)) + return recipes.NewRecipeError(recipes.RecipeDeletionFailed, err.Error(), "", recipes.GetRecipeErrorDetails(err)) } // Loop over each output resource and delete in reverse dependency order @@ -176,7 +177,7 @@ func (d *bicepDriver) Delete(ctx context.Context, opts DeleteOptions) error { err = d.ResourceClient.Delete(ctx, outputResource.ID.String()) if err != nil { - return recipes.NewRecipeError(recipes.RecipeDeletionFailed, err.Error(), recipes.GetRecipeErrorDetails(err)) + return recipes.NewRecipeError(recipes.RecipeDeletionFailed, err.Error(), "", recipes.GetRecipeErrorDetails(err)) } logger.Info(fmt.Sprintf("Deleted output resource: %q", outputResource.ID.String()), ucplog.LogFieldTargetResourceID, outputResource.ID.String()) diff --git a/pkg/recipes/driver/bicep_test.go b/pkg/recipes/driver/bicep_test.go index d950813d03..a1ce3f1138 100644 --- a/pkg/recipes/driver/bicep_test.go +++ b/pkg/recipes/driver/bicep_test.go @@ -474,6 +474,7 @@ func Test_Bicep_GetRecipeMetadata_Error(t *testing.T) { Code: recipes.RecipeLanguageFailure, Message: "failed to fetch repository from the path \"radiusdev.azurecr.io/test-non-existent-recipe\": radiusdev.azurecr.io/test-non-existent-recipe:latest: not found", }, + DeploymentStatus: "setupError", } require.Error(t, err) diff --git a/pkg/recipes/driver/terraform.go b/pkg/recipes/driver/terraform.go index cc0cbb6639..39e7ded72f 100644 --- a/pkg/recipes/driver/terraform.go +++ b/pkg/recipes/driver/terraform.go @@ -28,6 +28,7 @@ import ( "github.com/radius-project/radius/pkg/recipes" "github.com/radius-project/radius/pkg/recipes/terraform" + recipes_util "github.com/radius-project/radius/pkg/recipes/util" "github.com/radius-project/radius/pkg/sdk" ucp_provider "github.com/radius-project/radius/pkg/ucp/secret/provider" "github.com/radius-project/radius/pkg/ucp/ucplog" @@ -69,7 +70,7 @@ func (d *terraformDriver) Execute(ctx context.Context, opts ExecuteOptions) (*re requestDirPath, err := d.createExecutionDirectory(ctx, opts.Recipe, opts.Definition) if err != nil { - return nil, recipes.NewRecipeError(recipes.RecipeDeploymentFailed, err.Error(), recipes.GetRecipeErrorDetails(err)) + return nil, recipes.NewRecipeError(recipes.RecipeDeploymentFailed, err.Error(), recipes_util.RecipeSetupError, recipes.GetRecipeErrorDetails(err)) } defer func() { if err := os.RemoveAll(requestDirPath); err != nil { @@ -84,12 +85,12 @@ func (d *terraformDriver) Execute(ctx context.Context, opts ExecuteOptions) (*re EnvRecipe: &opts.Definition, }) if err != nil { - return nil, recipes.NewRecipeError(recipes.RecipeDeploymentFailed, err.Error(), recipes.GetRecipeErrorDetails(err)) + return nil, recipes.NewRecipeError(recipes.RecipeDeploymentFailed, err.Error(), recipes_util.ExecutionError, recipes.GetRecipeErrorDetails(err)) } recipeOutputs, err := d.prepareRecipeResponse(tfState) if err != nil { - return nil, recipes.NewRecipeError(recipes.InvalidRecipeOutputs, fmt.Sprintf("failed to read the recipe output %q: %s", recipes.ResultPropertyName, err.Error()), recipes.GetRecipeErrorDetails(err)) + return nil, recipes.NewRecipeError(recipes.InvalidRecipeOutputs, fmt.Sprintf("failed to read the recipe output %q: %s", recipes.ResultPropertyName, err.Error()), recipes_util.ExecutionError, recipes.GetRecipeErrorDetails(err)) } return recipeOutputs, nil @@ -101,7 +102,7 @@ func (d *terraformDriver) Delete(ctx context.Context, opts DeleteOptions) error requestDirPath, err := d.createExecutionDirectory(ctx, opts.Recipe, opts.Definition) if err != nil { - return recipes.NewRecipeError(recipes.RecipeDeletionFailed, err.Error(), recipes.GetRecipeErrorDetails(err)) + return recipes.NewRecipeError(recipes.RecipeDeletionFailed, err.Error(), "", recipes.GetRecipeErrorDetails(err)) } defer func() { if err := os.RemoveAll(requestDirPath); err != nil { @@ -116,7 +117,7 @@ func (d *terraformDriver) Delete(ctx context.Context, opts DeleteOptions) error EnvRecipe: &opts.Definition, }) if err != nil { - return recipes.NewRecipeError(recipes.RecipeDeletionFailed, err.Error(), recipes.GetRecipeErrorDetails(err)) + return recipes.NewRecipeError(recipes.RecipeDeletionFailed, err.Error(), "", recipes.GetRecipeErrorDetails(err)) } return nil @@ -180,7 +181,7 @@ func (d *terraformDriver) GetRecipeMetadata(ctx context.Context, opts BaseOption requestDirPath, err := d.createExecutionDirectory(ctx, opts.Recipe, opts.Definition) if err != nil { - return nil, recipes.NewRecipeError(recipes.RecipeGetMetadataFailed, err.Error(), recipes.GetRecipeErrorDetails(err)) + return nil, recipes.NewRecipeError(recipes.RecipeGetMetadataFailed, err.Error(), "", recipes.GetRecipeErrorDetails(err)) } defer func() { if err := os.RemoveAll(requestDirPath); err != nil { @@ -194,7 +195,7 @@ func (d *terraformDriver) GetRecipeMetadata(ctx context.Context, opts BaseOption EnvRecipe: &opts.Definition, }) if err != nil { - return nil, err + return nil, recipes.NewRecipeError(recipes.RecipeGetMetadataFailed, err.Error(), "", recipes.GetRecipeErrorDetails(err)) } return recipeData, nil diff --git a/pkg/recipes/driver/terraform_test.go b/pkg/recipes/driver/terraform_test.go index 7e1184ef9a..1693e47925 100644 --- a/pkg/recipes/driver/terraform_test.go +++ b/pkg/recipes/driver/terraform_test.go @@ -151,6 +151,7 @@ func Test_Terraform_Execute_DeploymentFailure(t *testing.T) { Code: recipes.RecipeDeploymentFailed, Message: "Failed to deploy terraform module", }, + DeploymentStatus: "executionError", } tfExecutor.EXPECT().Deploy(ctx, options).Times(1).Return(nil, errors.New("Failed to deploy terraform module")) @@ -205,6 +206,7 @@ func Test_Terraform_Execute_OutputsFailure(t *testing.T) { Code: recipes.InvalidRecipeOutputs, Message: "failed to read the recipe output \"result\": json: unknown field \"invalid\"", }, + DeploymentStatus: "executionError", } tfExecutor.EXPECT().Deploy(ctx, options).Times(1).Return(expectedTFState, nil) @@ -231,6 +233,7 @@ func Test_Terraform_Execute_EmptyPath(t *testing.T) { Code: recipes.RecipeDeploymentFailed, Message: "path is a required option for Terraform driver", }, + DeploymentStatus: "setupError", } _, err := driver.Execute(testcontext.New(t), ExecuteOptions{ BaseOptions: BaseOptions{ @@ -379,15 +382,20 @@ func TestTerraformDriver_GetRecipeMetadata_Failure(t *testing.T) { EnvRecipe: &envRecipe, } - expErr := errors.New("Failed to download module") - tfExecutor.EXPECT().GetRecipeMetadata(ctx, options).Times(1).Return(nil, expErr) + expErr := recipes.RecipeError{ + ErrorDetails: v1.ErrorDetails{ + Code: recipes.RecipeGetMetadataFailed, + Message: "Failed to download module", + }, + } + tfExecutor.EXPECT().GetRecipeMetadata(ctx, options).Times(1).Return(nil, errors.New("Failed to download module")) _, err := driver.GetRecipeMetadata(ctx, BaseOptions{ Recipe: recipes.ResourceMetadata{}, Definition: envRecipe, }) require.Error(t, err) - require.Equal(t, expErr, err) + require.Equal(t, &expErr, err) } func Test_Terraform_Delete_Success(t *testing.T) { diff --git a/pkg/recipes/engine/engine.go b/pkg/recipes/engine/engine.go index 100b58b7bd..581099a3ff 100644 --- a/pkg/recipes/engine/engine.go +++ b/pkg/recipes/engine/engine.go @@ -25,6 +25,7 @@ import ( "github.com/radius-project/radius/pkg/recipes" "github.com/radius-project/radius/pkg/recipes/configloader" recipedriver "github.com/radius-project/radius/pkg/recipes/driver" + "github.com/radius-project/radius/pkg/recipes/util" rpv1 "github.com/radius-project/radius/pkg/rp/v1" ) @@ -77,7 +78,7 @@ func (e *engine) executeCore(ctx context.Context, recipe recipes.ResourceMetadat configuration, err := e.options.ConfigurationLoader.LoadConfiguration(ctx, recipe) if err != nil { - return nil, definition, err + return nil, definition, recipes.NewRecipeError(recipes.RecipeConfigurationFailure, err.Error(), util.RecipeSetupError, recipes.GetRecipeErrorDetails(err)) } res, err := driver.Execute(ctx, recipedriver.ExecuteOptions{ @@ -178,7 +179,8 @@ func (e *engine) getDriver(ctx context.Context, recipeMetadata recipes.ResourceM // Determine Recipe driver type driver, ok := e.options.Drivers[definition.Driver] if !ok { - return nil, nil, fmt.Errorf("could not find driver %s", definition.Driver) + err := fmt.Errorf("could not find driver `%s`", definition.Driver) + return nil, nil, recipes.NewRecipeError(recipes.RecipeDriverNotFoundFailure, err.Error(), util.RecipeSetupError, recipes.GetRecipeErrorDetails(err)) } return definition, driver, nil } diff --git a/pkg/recipes/engine/engine_test.go b/pkg/recipes/engine/engine_test.go index ccf6805939..730ec072c1 100644 --- a/pkg/recipes/engine/engine_test.go +++ b/pkg/recipes/engine/engine_test.go @@ -273,7 +273,7 @@ func Test_Engine_InvalidDriver(t *testing.T) { Return(recipeDefinition, nil) _, err := engine.Execute(ctx, recipeMetadata, prevState) require.Error(t, err) - require.Equal(t, err.Error(), "could not find driver invalid") + require.Equal(t, "code DriverNotFoundFailure: err could not find driver `invalid`", err.Error()) } func Test_Engine_Lookup_Error(t *testing.T) { @@ -435,7 +435,7 @@ func Test_Delete_InvalidDriver(t *testing.T) { Return(&recipeDefinition, nil) err := engine.Delete(ctx, recipeMetadata, outputResources) require.Error(t, err) - require.Equal(t, err.Error(), "could not find driver invalid") + require.Equal(t, "code DriverNotFoundFailure: err could not find driver `invalid`", err.Error()) } func Test_Delete_Lookup_Error(t *testing.T) { diff --git a/pkg/recipes/error.go b/pkg/recipes/error.go index b03b7f373f..be7bb5b759 100644 --- a/pkg/recipes/error.go +++ b/pkg/recipes/error.go @@ -17,10 +17,12 @@ import ( "fmt" v1 "github.com/radius-project/radius/pkg/armrpc/api/v1" + "github.com/radius-project/radius/pkg/recipes/util" ) type RecipeError struct { - ErrorDetails v1.ErrorDetails + ErrorDetails v1.ErrorDetails + DeploymentStatus util.RecipeDeploymentStatus } // Error returns an error string describing the error code and message. @@ -34,10 +36,11 @@ func (e *RecipeError) Is(target error) bool { } // NewRecipeError creates a new RecipeError error with a given code, message and error details. -func NewRecipeError(code string, message string, details ...*v1.ErrorDetails) *RecipeError { +func NewRecipeError(code string, message string, deploymentStatus util.RecipeDeploymentStatus, details ...*v1.ErrorDetails) *RecipeError { err := new(RecipeError) err.ErrorDetails.Message = message err.ErrorDetails.Code = code + err.DeploymentStatus = deploymentStatus for _, val := range details { if val != nil { err.ErrorDetails.Details = append(err.ErrorDetails.Details, *val) diff --git a/pkg/recipes/error_test.go b/pkg/recipes/error_test.go index a9f888a61c..ff1d1ee392 100644 --- a/pkg/recipes/error_test.go +++ b/pkg/recipes/error_test.go @@ -21,6 +21,7 @@ import ( "testing" v1 "github.com/radius-project/radius/pkg/armrpc/api/v1" + "github.com/radius-project/radius/pkg/recipes/util" "github.com/stretchr/testify/require" ) @@ -51,6 +52,7 @@ func TestNewRecipeError(t *testing.T) { }, }, }, + util.RecipeSetupError, }, }, { @@ -63,11 +65,12 @@ func TestNewRecipeError(t *testing.T) { Code: RecipeDeploymentFailed, Message: "test-recipe-deployment-failed-message", }, + util.ExecutionError, }, }, } for _, tc := range errorTests { - err := NewRecipeError(tc.errorCode, tc.errorMessage, tc.errorDetails) + err := NewRecipeError(tc.errorCode, tc.errorMessage, tc.expectedErr.DeploymentStatus, tc.errorDetails) require.Equal(t, err, &tc.expectedErr) } } @@ -90,6 +93,7 @@ func TestGetRecipeErrorDetails(t *testing.T) { Code: RecipeDeploymentFailed, Message: "test-recipe-deployment-failed-message", }, + util.RecipeSetupError, }, expErrorDetails: &v1.ErrorDetails{ Code: RecipeDeploymentFailed, diff --git a/pkg/recipes/errorcodes.go b/pkg/recipes/errorcodes.go index d5c53c1d90..3b987e12e6 100644 --- a/pkg/recipes/errorcodes.go +++ b/pkg/recipes/errorcodes.go @@ -20,6 +20,9 @@ const ( // Used for recipe deployment failures. RecipeDeploymentFailed = "RecipeDeploymentFailed" + // Used for recipe validation failures. + RecipeValidationFailed = "RecipeValidationFailed" + // Used for recipe deletion failures. RecipeDeletionFailed = "RecipeDeletionFailed" @@ -34,4 +37,13 @@ const ( // Used for errors encountered when getting recipe parameters. RecipeGetMetadataFailed = "RecipeGetMetadataFailed" + + // Used for errors when checking the existence of a recipe. + RecipeNotFoundFailure = "RecipeNotFoundFailure" + + // Used for errors with recipe drivers + RecipeDriverNotFoundFailure = "DriverNotFoundFailure" + + // Used for errors with recipe configuration + RecipeConfigurationFailure = "RecipeConfigurationFailure" ) diff --git a/pkg/recipes/terraform/execute.go b/pkg/recipes/terraform/execute.go index 234df5ee71..3679da9178 100644 --- a/pkg/recipes/terraform/execute.go +++ b/pkg/recipes/terraform/execute.go @@ -33,6 +33,7 @@ import ( "github.com/radius-project/radius/pkg/recipes/terraform/config" "github.com/radius-project/radius/pkg/recipes/terraform/config/backends" "github.com/radius-project/radius/pkg/recipes/terraform/config/providers" + "github.com/radius-project/radius/pkg/recipes/util" "github.com/radius-project/radius/pkg/sdk" ucp_provider "github.com/radius-project/radius/pkg/ucp/secret/provider" "github.com/radius-project/radius/pkg/ucp/ucplog" @@ -293,7 +294,7 @@ func downloadAndInspect(ctx context.Context, workingDir string, execPath string, metrics.DefaultRecipeEngineMetrics.RecordRecipeDownloadDuration(ctx, downloadStartTime, metrics.NewRecipeAttributes(metrics.RecipeEngineOperationDownloadRecipe, options.EnvRecipe.Name, options.EnvRecipe, recipes.RecipeDownloadFailed)) - return nil, recipes.NewRecipeError(recipes.RecipeDownloadFailed, err.Error(), recipes.GetRecipeErrorDetails(err)) + return nil, recipes.NewRecipeError(recipes.RecipeDownloadFailed, err.Error(), util.RecipeSetupError, recipes.GetRecipeErrorDetails(err)) } metrics.DefaultRecipeEngineMetrics.RecordRecipeDownloadDuration(ctx, downloadStartTime, metrics.NewRecipeAttributes(metrics.RecipeEngineOperationDownloadRecipe, options.EnvRecipe.Name, diff --git a/pkg/recipes/types.go b/pkg/recipes/types.go index 2e0831ab35..b17ad9a309 100644 --- a/pkg/recipes/types.go +++ b/pkg/recipes/types.go @@ -19,7 +19,6 @@ package recipes import ( "bytes" "encoding/json" - "fmt" "github.com/radius-project/radius/pkg/corerp/datamodel" ) @@ -87,16 +86,6 @@ var ( SupportedTemplateKind = []string{TemplateKindBicep, TemplateKindTerraform} ) -type ErrRecipeNotFound struct { - Name string - Environment string -} - -// ErrRecipeNotFoundError returns an error message with the recipe name and environment when a recipe is not found. -func (e *ErrRecipeNotFound) Error() string { - return fmt.Sprintf("could not find recipe %q in environment %q", e.Name, e.Environment) -} - // RecipeOutput represents recipe deployment output. type RecipeOutput struct { // Resources represents the list of output resources deployed recipe. diff --git a/pkg/recipes/util/deploymentStatus.go b/pkg/recipes/util/deploymentStatus.go new file mode 100644 index 0000000000..57091eb92e --- /dev/null +++ b/pkg/recipes/util/deploymentStatus.go @@ -0,0 +1,25 @@ +/* +Copyright 2023 The Radius Authors. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + http://www.apache.org/licenses/LICENSE-2.0 +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package util + +type RecipeDeploymentStatus string + +const ( + // ExecutionError represents a failure status during recipe execution. + ExecutionError RecipeDeploymentStatus = "executionError" + // RecipeSetupError represents a failure that happens before a recipe or output resources are deployed. + RecipeSetupError RecipeDeploymentStatus = "setupError" + // Success represents a successful recipe execution. + Success RecipeDeploymentStatus = "success" +) diff --git a/pkg/rp/util/registry.go b/pkg/rp/util/registry.go index a7886708bd..3679c391d6 100644 --- a/pkg/rp/util/registry.go +++ b/pkg/rp/util/registry.go @@ -24,6 +24,7 @@ import ( dockerParser "github.com/novln/docker-parser" v1 "github.com/radius-project/radius/pkg/armrpc/api/v1" "github.com/radius-project/radius/pkg/recipes" + recipes_util "github.com/radius-project/radius/pkg/recipes/util" "oras.land/oras-go/v2/content" "oras.land/oras-go/v2/registry/remote" ) @@ -46,12 +47,12 @@ func ReadFromRegistry(ctx context.Context, path string, data *map[string]any) er digest, err := getDigestFromManifest(ctx, repo, tag) if err != nil { - return recipes.NewRecipeError(recipes.RecipeLanguageFailure, fmt.Sprintf("failed to fetch repository from the path %q: %s", path, err.Error()), nil) + return recipes.NewRecipeError(recipes.RecipeLanguageFailure, fmt.Sprintf("failed to fetch repository from the path %q: %s", path, err.Error()), recipes_util.RecipeSetupError, nil) } bytes, err := getBytes(ctx, repo, digest) if err != nil { - return recipes.NewRecipeError(recipes.RecipeLanguageFailure, fmt.Sprintf("failed to fetch repository from the path %q: %s", path, err.Error()), nil) + return recipes.NewRecipeError(recipes.RecipeLanguageFailure, fmt.Sprintf("failed to fetch repository from the path %q: %s", path, err.Error()), recipes_util.RecipeSetupError, nil) } err = json.Unmarshal(bytes, data) diff --git a/test/functional/shared/resources/recipe_test.go b/test/functional/shared/resources/recipe_test.go index d367fd1e1a..28ca5f5eeb 100644 --- a/test/functional/shared/resources/recipe_test.go +++ b/test/functional/shared/resources/recipe_test.go @@ -20,7 +20,7 @@ import ( "fmt" "testing" - v1 "github.com/radius-project/radius/pkg/armrpc/api/v1" + "github.com/radius-project/radius/pkg/recipes" "github.com/radius-project/radius/test/functional/shared" "github.com/radius-project/radius/test/step" "github.com/radius-project/radius/test/validation" @@ -34,8 +34,6 @@ import ( // behaviors. Some functionality needs to be tested for each driver. func Test_Recipe_NotFound(t *testing.T) { - t.Skip("Blocked by https://github.com/radius-project/radius/issues/6040") - template := "testdata/corerp-resources-recipe-notfound.bicep" name := "corerp-resources-recipe-notfound" @@ -43,7 +41,7 @@ func Test_Recipe_NotFound(t *testing.T) { Code: "ResourceDeploymentFailure", Details: []step.DeploymentErrorDetail{ { - Code: v1.CodeInternal, + Code: recipes.RecipeNotFoundFailure, MessageContains: "could not find recipe \"not found!\" in environment", }, },