Skip to content

Commit

Permalink
Fix stuck portable resource deletion bug (#6247)
Browse files Browse the repository at this point in the history
# 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

<!--

Please select **one** of the following options that describes your
change and delete the others. Clearly identifying the type of change you
are making will help us review your PR faster, and is used in authoring
release notes.

If you are making a bug fix or functionality change to Radius and do not
have an associated issue link please create one now.

-->

- This pull request fixes a bug in Radius and has an approved issue
#6040

<!--

Please update the following to link the associated issue. This is
required for some kinds of changes (see above).

-->

Fixes: #6040 

## Auto-generated summary

<!--
GitHub Copilot for docs will auto-generate a summary of the PR
-->

<!--
copilot:all
-->
### <samp>🤖 Generated by Copilot at c552dea</samp>

### Summary
🚀🛠️🧪

<!--
1. 🚀 - This emoji represents the addition of new features or
functionality, such as the `SetDeploymentStatus` methods for different
resource types, the generic type parameters for the
`NewCreateOrUpdateResource` and `NewDeleteResource` functions, and the
new field and type for tracking the deployment status of resource
recipes.
2. 🛠️ - This emoji represents the improvement or refactoring of existing
code, such as the renaming of receiver names, the simplification of type
assertions, the error handling for recipe definitions and executions,
and the use of the `portableresources` package to standardize the error
statuses.
3. 🧪 - This emoji represents the update or enhancement of test cases,
such as the alignment of the test code with the actual usage of the
`Extender` type, the handling of the `ErrRecipeNotFound` error, and the
matching of the expected errors from the `driver` package.
-->
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))
  • Loading branch information
sk593 authored Sep 13, 2023
1 parent 6864ebe commit 9aa13b3
Show file tree
Hide file tree
Showing 28 changed files with 159 additions and 82 deletions.
8 changes: 4 additions & 4 deletions pkg/corerp/datamodel/extender.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion pkg/daprrp/datamodel/daprpubsubbroker.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
8 changes: 4 additions & 4 deletions pkg/daprrp/datamodel/daprsecretstore.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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
}
2 changes: 1 addition & 1 deletion pkg/daprrp/datamodel/daprstatestore.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
12 changes: 6 additions & 6 deletions pkg/datastoresrp/datamodel/mongodatabase.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
}
Expand Down Expand Up @@ -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
}
16 changes: 8 additions & 8 deletions pkg/datastoresrp/datamodel/rediscache.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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")
}
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/datastoresrp/datamodel/sqldatabase.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
6 changes: 3 additions & 3 deletions pkg/messagingrp/datamodel/rabbitmq.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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 == "" {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package controller
import (
"context"
"errors"
"fmt"
"testing"

"github.com/golang/mock/gomock"
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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",
Expand Down
6 changes: 5 additions & 1 deletion pkg/portableresources/backend/controller/deleteresource.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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,
Expand Down
4 changes: 4 additions & 0 deletions pkg/portableresources/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ package portableresources

import (
"strings"

"github.com/radius-project/radius/pkg/recipes/util"
)

const (
Expand Down Expand Up @@ -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
Expand Down
10 changes: 7 additions & 3 deletions pkg/recipes/configloader/environment.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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{
Expand Down
2 changes: 1 addition & 1 deletion pkg/recipes/configloader/environment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Loading

0 comments on commit 9aa13b3

Please sign in to comment.