Skip to content

Commit

Permalink
Adding metrics for recipe grabage collection and refactoring engine t…
Browse files Browse the repository at this point in the history
…o take options. (#6232)

# Description

- Added RecipeEngineOperationGC metric for recipe garbage collection
operation
- Refactored code to take options for execute and delete functions in
Engine.

## 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 is a minor refactor, code cleanup, test improvement,
or other maintenance task and doesn't change the functionality of Radius
(issue link optional).

<!--

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

-->

https://github.com/radius-project/radius/issues/4446

## Auto-generated summary

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

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

### Summary
🧹🛠️📊

<!--
1. 🧹 - This emoji represents the recipe garbage collection feature,
which cleans up unused recipes from the database. It conveys the idea of
tidying up and removing clutter.
2. 🛠️ - This emoji represents the refactoring of the engine interface
and the test code, which improves the readability and simplicity of the
code. It conveys the idea of fixing and improving something.
3. 📊 - This emoji represents the observability of the recipe garbage
collection process, which records its duration as a metric. It conveys
the idea of measuring and analyzing something.
-->
This pull request refactors the engine package and its consumers to use
options structs for the Execute and Delete methods, simplifying the
interface and improving readability. It also adds metrics for the recipe
garbage collection duration in the bicep driver.

> _We're refactoring the engine code, me hearties_
> _Using options structs for Execute and Delete_
> _It makes the code more readable and tidy_
> _So heave away on the count of three_

### Walkthrough
* Add metrics for recipe garbage collection duration
([link](https://github.com/radius-project/radius/pull/6232/files?diff=unified&w=0#diff-6403665d6373b7e2a29e5a0e1d8e52b830982dfdce21983cfd83e1c434b19046R37-R39),
[link](https://github.com/radius-project/radius/pull/6232/files?diff=unified&w=0#diff-6403665d6373b7e2a29e5a0e1d8e52b830982dfdce21983cfd83e1c434b19046R54-R56),
[link](https://github.com/radius-project/radius/pull/6232/files?diff=unified&w=0#diff-6403665d6373b7e2a29e5a0e1d8e52b830982dfdce21983cfd83e1c434b19046R132-R139),
[link](https://github.com/radius-project/radius/pull/6232/files?diff=unified&w=0#diff-f12e37706fa75e9e04b8dfa01a45cf9bf2a0463e1962605b2420d0cb343922a0R146),
[link](https://github.com/radius-project/radius/pull/6232/files?diff=unified&w=0#diff-f12e37706fa75e9e04b8dfa01a45cf9bf2a0463e1962605b2420d0cb343922a0L151-R157))
* Refactor engine methods to use options structs instead of separate
parameters
([link](https://github.com/radius-project/radius/pull/6232/files?diff=unified&w=0#diff-88040b9959d0c406cb8ed95caa785345085b114078658de636164a546f910749L134-R139),
[link](https://github.com/radius-project/radius/pull/6232/files?diff=unified&w=0#diff-b5a5c7f766f17b812eb1e5bd1815303ade4f0486d243708a526832a65bbed011L336-R351),
[link](https://github.com/radius-project/radius/pull/6232/files?diff=unified&w=0#diff-c6fc204032902b38f97dc71fe64ce145d1f851419f4eef7682a2d62183172a03L89-R94),
[link](https://github.com/radius-project/radius/pull/6232/files?diff=unified&w=0#diff-f717af7a7bcb5381570a73610840036355990198ea4dc08b2c49f787c1ee09abL124-R129),
[link](https://github.com/radius-project/radius/pull/6232/files?diff=unified&w=0#diff-06702c6df95f9efe532b70614e1341244f575e5cbc030db57af268ecbfcd8c59L51-R55),
[link](https://github.com/radius-project/radius/pull/6232/files?diff=unified&w=0#diff-06702c6df95f9efe532b70614e1341244f575e5cbc030db57af268ecbfcd8c59L61-R61),
[link](https://github.com/radius-project/radius/pull/6232/files?diff=unified&w=0#diff-06702c6df95f9efe532b70614e1341244f575e5cbc030db57af268ecbfcd8c59L96-R100),
[link](https://github.com/radius-project/radius/pull/6232/files?diff=unified&w=0#diff-06702c6df95f9efe532b70614e1341244f575e5cbc030db57af268ecbfcd8c59L106-R106),
[link](https://github.com/radius-project/radius/pull/6232/files?diff=unified&w=0#diff-3fdb9931c1f2ddfc5b14d87f47b4124f2bdc1da187219fa1ad9cbe79fc326528L115-R120),
[link](https://github.com/radius-project/radius/pull/6232/files?diff=unified&w=0#diff-3fdb9931c1f2ddfc5b14d87f47b4124f2bdc1da187219fa1ad9cbe79fc326528L173-R183),
[link](https://github.com/radius-project/radius/pull/6232/files?diff=unified&w=0#diff-3fdb9931c1f2ddfc5b14d87f47b4124f2bdc1da187219fa1ad9cbe79fc326528L243-R258),
[link](https://github.com/radius-project/radius/pull/6232/files?diff=unified&w=0#diff-3fdb9931c1f2ddfc5b14d87f47b4124f2bdc1da187219fa1ad9cbe79fc326528L274-R294),
[link](https://github.com/radius-project/radius/pull/6232/files?diff=unified&w=0#diff-3fdb9931c1f2ddfc5b14d87f47b4124f2bdc1da187219fa1ad9cbe79fc326528L298-R324),
[link](https://github.com/radius-project/radius/pull/6232/files?diff=unified&w=0#diff-3fdb9931c1f2ddfc5b14d87f47b4124f2bdc1da187219fa1ad9cbe79fc326528L330-R362),
[link](https://github.com/radius-project/radius/pull/6232/files?diff=unified&w=0#diff-3fdb9931c1f2ddfc5b14d87f47b4124f2bdc1da187219fa1ad9cbe79fc326528L375-R412),
[link](https://github.com/radius-project/radius/pull/6232/files?diff=unified&w=0#diff-3fdb9931c1f2ddfc5b14d87f47b4124f2bdc1da187219fa1ad9cbe79fc326528L421-R463),
[link](https://github.com/radius-project/radius/pull/6232/files?diff=unified&w=0#diff-3fdb9931c1f2ddfc5b14d87f47b4124f2bdc1da187219fa1ad9cbe79fc326528L436-R483),
[link](https://github.com/radius-project/radius/pull/6232/files?diff=unified&w=0#diff-3fdb9931c1f2ddfc5b14d87f47b4124f2bdc1da187219fa1ad9cbe79fc326528L450-R502),
[link](https://github.com/radius-project/radius/pull/6232/files?diff=unified&w=0#diff-2dee9fa8b594b7a0e0783bd6587b452223eb5f7ba7c28a1dc0178c121a2509f0L40-R41),
[link](https://github.com/radius-project/radius/pull/6232/files?diff=unified&w=0#diff-2dee9fa8b594b7a0e0783bd6587b452223eb5f7ba7c28a1dc0178c121a2509f0L48-R55),
[link](https://github.com/radius-project/radius/pull/6232/files?diff=unified&w=0#diff-2dee9fa8b594b7a0e0783bd6587b452223eb5f7ba7c28a1dc0178c121a2509f0L63-R64),
[link](https://github.com/radius-project/radius/pull/6232/files?diff=unified&w=0#diff-11be7ca47327363d5edeaff3c6ac6389a462ff9111e1fc03ccb3a98123cd939fL31-R59))
* Define `ExecuteOptions` and `DeleteOptions` structs in
`engine/types.go`
([link](https://github.com/radius-project/radius/pull/6232/files?diff=unified&w=0#diff-11be7ca47327363d5edeaff3c6ac6389a462ff9111e1fc03ccb3a98123cd939fL31-R59))
* Update `Engine` interface to use options structs in `engine/types.go`
([link](https://github.com/radius-project/radius/pull/6232/files?diff=unified&w=0#diff-11be7ca47327363d5edeaff3c6ac6389a462ff9111e1fc03ccb3a98123cd939fL31-R59))
* Update `MockEngine` and `MockEngineMockRecorder` to use options
structs in `mock_engine.go`
([link](https://github.com/radius-project/radius/pull/6232/files?diff=unified&w=0#diff-2dee9fa8b594b7a0e0783bd6587b452223eb5f7ba7c28a1dc0178c121a2509f0L40-R41),
[link](https://github.com/radius-project/radius/pull/6232/files?diff=unified&w=0#diff-2dee9fa8b594b7a0e0783bd6587b452223eb5f7ba7c28a1dc0178c121a2509f0L48-R55),
[link](https://github.com/radius-project/radius/pull/6232/files?diff=unified&w=0#diff-2dee9fa8b594b7a0e0783bd6587b452223eb5f7ba7c28a1dc0178c121a2509f0L63-R64))
* Update `engine.Execute` and `engine.Delete` methods to use options
structs in `engine.go`
([link](https://github.com/radius-project/radius/pull/6232/files?diff=unified&w=0#diff-06702c6df95f9efe532b70614e1341244f575e5cbc030db57af268ecbfcd8c59L51-R55),
[link](https://github.com/radius-project/radius/pull/6232/files?diff=unified&w=0#diff-06702c6df95f9efe532b70614e1341244f575e5cbc030db57af268ecbfcd8c59L61-R61),
[link](https://github.com/radius-project/radius/pull/6232/files?diff=unified&w=0#diff-06702c6df95f9efe532b70614e1341244f575e5cbc030db57af268ecbfcd8c59L96-R100),
[link](https://github.com/radius-project/radius/pull/6232/files?diff=unified&w=0#diff-06702c6df95f9efe532b70614e1341244f575e5cbc030db57af268ecbfcd8c59L106-R106))
* Update `bicep.Execute` method to use options structs in
`driver/bicep.go`
([link](https://github.com/radius-project/radius/pull/6232/files?diff=unified&w=0#diff-f12e37706fa75e9e04b8dfa01a45cf9bf2a0463e1962605b2420d0cb343922a0R146),
[link](https://github.com/radius-project/radius/pull/6232/files?diff=unified&w=0#diff-f12e37706fa75e9e04b8dfa01a45cf9bf2a0463e1962605b2420d0cb343922a0L151-R157))
* Update `createorupdateresource.executeRecipeIfNeeded` and
`deleteresource.Run` methods to use options structs in
`controller/createorupdateresource.go` and
`controller/deleteresource.go`
([link](https://github.com/radius-project/radius/pull/6232/files?diff=unified&w=0#diff-88040b9959d0c406cb8ed95caa785345085b114078658de636164a546f910749L134-R139),
[link](https://github.com/radius-project/radius/pull/6232/files?diff=unified&w=0#diff-c6fc204032902b38f97dc71fe64ce145d1f851419f4eef7682a2d62183172a03L89-R94))
* Update engine tests to use options structs in `engine_test.go`
([link](https://github.com/radius-project/radius/pull/6232/files?diff=unified&w=0#diff-3fdb9931c1f2ddfc5b14d87f47b4124f2bdc1da187219fa1ad9cbe79fc326528L115-R120),
[link](https://github.com/radius-project/radius/pull/6232/files?diff=unified&w=0#diff-3fdb9931c1f2ddfc5b14d87f47b4124f2bdc1da187219fa1ad9cbe79fc326528L173-R183),
[link](https://github.com/radius-project/radius/pull/6232/files?diff=unified&w=0#diff-3fdb9931c1f2ddfc5b14d87f47b4124f2bdc1da187219fa1ad9cbe79fc326528L243-R258),
[link](https://github.com/radius-project/radius/pull/6232/files?diff=unified&w=0#diff-3fdb9931c1f2ddfc5b14d87f47b4124f2bdc1da187219fa1ad9cbe79fc326528L274-R294),
[link](https://github.com/radius-project/radius/pull/6232/files?diff=unified&w=0#diff-3fdb9931c1f2ddfc5b14d87f47b4124f2bdc1da187219fa1ad9cbe79fc326528L298-R324),
[link](https://github.com/radius-project/radius/pull/6232/files?diff=unified&w=0#diff-3fdb9931c1f2ddfc5b14d87f47b4124f2bdc1da187219fa1ad9cbe79fc326528L330-R362),
[link](https://github.com/radius-project/radius/pull/6232/files?diff=unified&w=0#diff-3fdb9931c1f2ddfc5b14d87f47b4124f2bdc1da187219fa1ad9cbe79fc326528L375-R412),
[link](https://github.com/radius-project/radius/pull/6232/files?diff=unified&w=0#diff-3fdb9931c1f2ddfc5b14d87f47b4124f2bdc1da187219fa1ad9cbe79fc326528L421-R463),
[link](https://github.com/radius-project/radius/pull/6232/files?diff=unified&w=0#diff-3fdb9931c1f2ddfc5b14d87f47b4124f2bdc1da187219fa1ad9cbe79fc326528L436-R483),
[link](https://github.com/radius-project/radius/pull/6232/files?diff=unified&w=0#diff-3fdb9931c1f2ddfc5b14d87f47b4124f2bdc1da187219fa1ad9cbe79fc326528L450-R502))
* Update controller tests to use options structs in
`createorupdateresource_test.go` and `deleteresource_test.go`
([link](https://github.com/radius-project/radius/pull/6232/files?diff=unified&w=0#diff-b5a5c7f766f17b812eb1e5bd1815303ade4f0486d243708a526832a65bbed011L336-R351),
[link](https://github.com/radius-project/radius/pull/6232/files?diff=unified&w=0#diff-f717af7a7bcb5381570a73610840036355990198ea4dc08b2c49f787c1ee09abL124-R129))
* Remove unused import from `mock_engine.go`
([link](https://github.com/radius-project/radius/pull/6232/files?diff=unified&w=0#diff-2dee9fa8b594b7a0e0783bd6587b452223eb5f7ba7c28a1dc0178c121a2509f0L13))
  • Loading branch information
vishwahiremat authored Sep 13, 2023
1 parent 9aa13b3 commit 677aa29
Show file tree
Hide file tree
Showing 11 changed files with 151 additions and 36 deletions.
6 changes: 3 additions & 3 deletions pkg/corerp/setup/setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down
14 changes: 14 additions & 0 deletions pkg/metrics/recipeenginemetrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ const (
// recipeDownloadDuration is the metric name for the recipe download duration.
recipeDownloadDuration = "recipe.download.duration"

// recipeGCDuration is the metric name for the bicep recipe garbage collection duration.
recipeGCDuration = "recipe.bicep.garbage.collection.duration"

// terraformInstallationDuration is the metric name for the Terraform installation duration.
terraformInstallationDuration = "recipe.tf.installation.duration"

Expand All @@ -48,6 +51,9 @@ const (

// RecipeEngineOperationDownloadRecipe represents the Download Recipe operation of the Recipe Engine.
RecipeEngineOperationDownloadRecipe = "download.recipe"

// RecipeEngineOperationGC represents the Garbage Collection operation of the Recipe Engine.
RecipeEngineOperationGC = "garbage.collection.recipe"
)

type recipeEngineMetrics struct {
Expand Down Expand Up @@ -123,6 +129,14 @@ func (m *recipeEngineMetrics) RecordTerraformInitializationDuration(ctx context.
}
}

// RecordRecipeGarbageCollectionDuration records the recipe garbage collection duration with the given attributes.
func (m *recipeEngineMetrics) RecordRecipeGarbageCollectionDuration(ctx context.Context, startTime time.Time, attrs []attribute.KeyValue) {
if m.valueRecorders[recipeGCDuration] != nil {
elapsedTime := float64(time.Since(startTime)) / float64(time.Millisecond)
m.valueRecorders[recipeGCDuration].Record(ctx, elapsedTime, metric.WithAttributes(attrs...))
}
}

// NewRecipeAttributes generates common attributes for recipe operations.
func NewRecipeAttributes(operationType, recipeName string, definition *recipes.EnvironmentDefinition, state string) []attribute.KeyValue {
attrs := make([]attribute.KeyValue, 0)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,12 @@ func (c *CreateOrUpdateResource[P, T]) executeRecipeIfNeeded(ctx context.Context
ResourceID: data.GetBaseResource().ID,
}

return c.engine.Execute(ctx, request, prevState)
return c.engine.Execute(ctx, engine.ExecuteOptions{
BaseOptions: engine.BaseOptions{
Recipe: request,
},
PreviousState: prevState,
})
}

func (c *CreateOrUpdateResource[P, T]) loadRuntimeConfiguration(ctx context.Context, environmentID string, applicationID string, resourceID string) (*recipes.RuntimeConfiguration, error) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -344,12 +344,22 @@ func TestCreateOrUpdateResource_Run(t *testing.T) {
if stillPassing && tt.recipeErr != nil {
stillPassing = false
eng.EXPECT().
Execute(gomock.Any(), recipeMetadata, prevState).
Execute(gomock.Any(), engine.ExecuteOptions{
BaseOptions: engine.BaseOptions{
Recipe: recipeMetadata,
},
PreviousState: prevState,
}).
Return(&recipes.RecipeOutput{}, tt.recipeErr).
Times(1)
} else if stillPassing {
eng.EXPECT().
Execute(gomock.Any(), recipeMetadata, prevState).
Execute(gomock.Any(), engine.ExecuteOptions{
BaseOptions: engine.BaseOptions{
Recipe: recipeMetadata,
},
PreviousState: prevState,
}).
Return(&recipes.RecipeOutput{}, nil).
Times(1)
}
Expand Down
7 changes: 6 additions & 1 deletion pkg/portableresources/backend/controller/deleteresource.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,12 @@ func (c *DeleteResource[P, T]) Run(ctx context.Context, request *ctrl.Request) (
ResourceID: id.String(),
}

err = c.engine.Delete(ctx, recipeData, data.OutputResources())
err = c.engine.Delete(ctx, engine.DeleteOptions{
BaseOptions: engine.BaseOptions{
Recipe: recipeData,
},
OutputResources: data.OutputResources(),
})
if err != nil {
if recipeError, ok := err.(*recipes.RecipeError); ok {
return ctrl.NewFailedResult(recipeError.ErrorDetails), nil
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,12 @@ func TestDeleteResourceRun_20220315PrivatePreview(t *testing.T) {

if tt.getErr == nil {
eng.EXPECT().
Delete(gomock.Any(), recipeData, status.OutputResources).
Delete(gomock.Any(), engine.DeleteOptions{
BaseOptions: engine.BaseOptions{
Recipe: recipeData,
},
OutputResources: status.OutputResources,
}).
Return(tt.engDelErr).
Times(1)
if tt.engDelErr == nil {
Expand Down
6 changes: 5 additions & 1 deletion pkg/recipes/driver/bicep.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,14 +144,18 @@ func (d *bicepDriver) Execute(ctx context.Context, opts ExecuteOptions) (*recipe
// in the currently deployed resources compared to the list of resources from the previous deployment needs to be deleted
// as bicep does not take care of automatically deleting the unused resources.
// Identify the output resources that are no longer relevant to the recipe.
garbageCollectionStartTime := time.Now()
diff := d.getGCOutputResources(recipeResponse.Resources, opts.PrevState)

// Deleting obsolete output resources.
err = d.deleteGCOutputResources(ctx, diff)
if err != nil {
metrics.DefaultRecipeEngineMetrics.RecordRecipeGarbageCollectionDuration(ctx, garbageCollectionStartTime,
metrics.NewRecipeAttributes(metrics.RecipeEngineOperationGC, opts.Recipe.Name, &opts.Definition, metrics.FailedOperationState))
return nil, recipes.NewRecipeError(recipes.RecipeGarbageCollectionFailed, err.Error(), recipes_util.ExecutionError, nil)
}

metrics.DefaultRecipeEngineMetrics.RecordRecipeGarbageCollectionDuration(ctx, garbageCollectionStartTime,
metrics.NewRecipeAttributes(metrics.RecipeEngineOperationGC, opts.Recipe.Name, &opts.Definition, metrics.SuccessfulOperationState))
return recipeResponse, nil
}

Expand Down
12 changes: 6 additions & 6 deletions pkg/recipes/engine/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,11 @@ type engine struct {
// Execute loads the recipe definition from the environment, finds the driver associated with the recipe, loads the
// configuration associated with the recipe, and then executes the recipe using the driver. It returns a RecipeOutput and
// an error if one occurs.
func (e *engine) Execute(ctx context.Context, recipe recipes.ResourceMetadata, prevState []string) (*recipes.RecipeOutput, error) {
func (e *engine) Execute(ctx context.Context, opts ExecuteOptions) (*recipes.RecipeOutput, error) {
executionStart := time.Now()
result := metrics.SuccessfulOperationState

recipeOutput, definition, err := e.executeCore(ctx, recipe, prevState)
recipeOutput, definition, err := e.executeCore(ctx, opts.Recipe, opts.PreviousState)
if err != nil {
result = metrics.FailedOperationState
if recipes.GetRecipeErrorDetails(err) != nil {
Expand All @@ -62,7 +62,7 @@ func (e *engine) Execute(ctx context.Context, recipe recipes.ResourceMetadata, p
}

metrics.DefaultRecipeEngineMetrics.RecordRecipeOperationDuration(ctx, executionStart,
metrics.NewRecipeAttributes(metrics.RecipeEngineOperationExecute, recipe.Name,
metrics.NewRecipeAttributes(metrics.RecipeEngineOperationExecute, opts.Recipe.Name,
definition, result))

return recipeOutput, err
Expand Down Expand Up @@ -97,11 +97,11 @@ func (e *engine) executeCore(ctx context.Context, recipe recipes.ResourceMetadat
}

// Delete calls the Delete method of the driver specified in the recipe definition to delete the output resources.
func (e *engine) Delete(ctx context.Context, recipe recipes.ResourceMetadata, outputResources []rpv1.OutputResource) error {
func (e *engine) Delete(ctx context.Context, opts DeleteOptions) error {
deletionStart := time.Now()
result := metrics.SuccessfulOperationState

definition, err := e.deleteCore(ctx, recipe, outputResources)
definition, err := e.deleteCore(ctx, opts.Recipe, opts.OutputResources)
if err != nil {
result = metrics.FailedOperationState
if recipes.GetRecipeErrorDetails(err) != nil {
Expand All @@ -110,7 +110,7 @@ func (e *engine) Delete(ctx context.Context, recipe recipes.ResourceMetadata, ou
}

metrics.DefaultRecipeEngineMetrics.RecordRecipeOperationDuration(ctx, deletionStart,
metrics.NewRecipeAttributes(metrics.RecipeEngineOperationDelete, recipe.Name,
metrics.NewRecipeAttributes(metrics.RecipeEngineOperationDelete, opts.Recipe.Name,
definition, result))

return err
Expand Down
72 changes: 62 additions & 10 deletions pkg/recipes/engine/engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,12 @@ func Test_Engine_Execute_Success(t *testing.T) {
Times(1).
Return(recipeResult, nil)

result, err := engine.Execute(ctx, recipeMetadata, prevState)
result, err := engine.Execute(ctx, ExecuteOptions{
BaseOptions: BaseOptions{
Recipe: recipeMetadata,
},
PreviousState: prevState,
})
require.NoError(t, err)
require.Equal(t, result, recipeResult)
}
Expand Down Expand Up @@ -170,7 +175,12 @@ func Test_Engine_Execute_Failure(t *testing.T) {
Times(1).
Return(nil, errors.New("failed to execute recipe"))

result, err := engine.Execute(ctx, recipeMetadata, prevState)
result, err := engine.Execute(ctx, ExecuteOptions{
BaseOptions: BaseOptions{
Recipe: recipeMetadata,
},
PreviousState: prevState,
})
require.Nil(t, result)
require.Error(t, err)
require.Equal(t, err.Error(), "failed to execute recipe")
Expand Down Expand Up @@ -240,7 +250,12 @@ func Test_Engine_Terraform_Success(t *testing.T) {
Times(1).
Return(recipeResult, nil)

result, err := engine.Execute(ctx, recipeMetadata, prevState)
result, err := engine.Execute(ctx, ExecuteOptions{
BaseOptions: BaseOptions{
Recipe: recipeMetadata,
},
PreviousState: prevState,
})
require.NoError(t, err)
require.Equal(t, result, recipeResult)
}
Expand Down Expand Up @@ -271,7 +286,12 @@ func Test_Engine_InvalidDriver(t *testing.T) {
LoadRecipe(ctx, &recipeMetadata).
Times(1).
Return(recipeDefinition, nil)
_, err := engine.Execute(ctx, recipeMetadata, prevState)
_, err := engine.Execute(ctx, ExecuteOptions{
BaseOptions: BaseOptions{
Recipe: recipeMetadata,
},
PreviousState: prevState,
})
require.Error(t, err)
require.Equal(t, "code DriverNotFoundFailure: err could not find driver `invalid`", err.Error())
}
Expand All @@ -295,7 +315,13 @@ func Test_Engine_Lookup_Error(t *testing.T) {
LoadRecipe(ctx, &recipeMetadata).
Times(1).
Return(nil, errors.New("could not find recipe mongo-azure in environment env1"))
_, err := engine.Execute(ctx, recipeMetadata, prevState)

_, err := engine.Execute(ctx, ExecuteOptions{
BaseOptions: BaseOptions{
Recipe: recipeMetadata,
},
PreviousState: prevState,
})
require.Error(t, err)
}

Expand Down Expand Up @@ -327,7 +353,13 @@ func Test_Engine_Load_Error(t *testing.T) {
LoadConfiguration(ctx, recipeMetadata).
Times(1).
Return(nil, errors.New("unable to fetch namespace information"))
_, err := engine.Execute(ctx, recipeMetadata, prevState)

_, err := engine.Execute(ctx, ExecuteOptions{
BaseOptions: BaseOptions{
Recipe: recipeMetadata,
},
PreviousState: prevState,
})
require.Error(t, err)
}

Expand Down Expand Up @@ -372,7 +404,12 @@ func Test_Engine_Delete_Success(t *testing.T) {
Times(1).
Return(nil)

err := engine.Delete(ctx, recipeMetadata, outputResources)
err := engine.Delete(ctx, DeleteOptions{
BaseOptions: BaseOptions{
Recipe: recipeMetadata,
},
OutputResources: outputResources,
})
require.NoError(t, err)
}

Expand Down Expand Up @@ -418,7 +455,12 @@ func Test_Engine_Delete_Error(t *testing.T) {
Return(fmt.Errorf("could not find API version for type %q, no supported API versions",
outputResources[0].ID))

err := engine.Delete(ctx, recipeMetadata, outputResources)
err := engine.Delete(ctx, DeleteOptions{
BaseOptions: BaseOptions{
Recipe: recipeMetadata,
},
OutputResources: outputResources,
})
require.Error(t, err)
}

Expand All @@ -433,7 +475,12 @@ func Test_Delete_InvalidDriver(t *testing.T) {
LoadRecipe(ctx, &recipeMetadata).
Times(1).
Return(&recipeDefinition, nil)
err := engine.Delete(ctx, recipeMetadata, outputResources)
err := engine.Delete(ctx, DeleteOptions{
BaseOptions: BaseOptions{
Recipe: recipeMetadata,
},
OutputResources: outputResources,
})
require.Error(t, err)
require.Equal(t, "code DriverNotFoundFailure: err could not find driver `invalid`", err.Error())
}
Expand All @@ -447,7 +494,12 @@ func Test_Delete_Lookup_Error(t *testing.T) {
LoadRecipe(ctx, &recipeMetadata).
Times(1).
Return(nil, errors.New("could not find recipe mongo-azure in environment env1"))
err := engine.Delete(ctx, recipeMetadata, outputResources)
err := engine.Delete(ctx, DeleteOptions{
BaseOptions: BaseOptions{
Recipe: recipeMetadata,
},
OutputResources: outputResources,
})
require.Error(t, err)
}

Expand Down
17 changes: 8 additions & 9 deletions pkg/recipes/engine/mock_engine.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 677aa29

Please sign in to comment.