Skip to content

Commit

Permalink
addressing comments
Browse files Browse the repository at this point in the history
  • Loading branch information
sk593 committed Sep 12, 2023
1 parent aa1efca commit f84e218
Show file tree
Hide file tree
Showing 7 changed files with 28 additions and 30 deletions.
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 All @@ -41,10 +42,10 @@ import (
)

const (
TestResourceType = "Applications.Core/extenders"
TestResourceType = "Applications.Test/testResources"
TestEnvironmentID = "/planes/radius/local/resourceGroups/radius-test-rg/providers/Applications.Core/environments/test-env"
TestApplicationID = "/planes/radius/local/resourceGroups/radius-test-rg/providers/Applications.Core/applications/test-app"
TestResourceID = "/planes/radius/local/resourceGroups/radius-test-rg/providers/Applications.Core/extenders/tr"
TestResourceID = "/planes/radius/local/resourceGroups/radius-test-rg/providers/Applications.Test/testResources/tr"
)

type TestResource struct {
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/Applications.Core/extenders/test1"
var oldOutputResourceResourceID = "/subscriptions/test-sub/resourceGroups/test-rg/providers/Applications.Test/testResources/test1"

var newOutputResourceResourceID = "/subscriptions/test-sub/resourceGroups/test-rg/providers/Applications.Core/extenders/test2"
var newOutputResourceResourceID = "/subscriptions/test-sub/resourceGroups/test-rg/providers/Applications.Test/testResourcess/test2"
var newOutputResource = rpv1.OutputResource{ID: resources.MustParse(newOutputResourceResourceID)}

func TestCreateOrUpdateResource_Run(t *testing.T) {
Expand Down Expand Up @@ -199,12 +200,12 @@ func TestCreateOrUpdateResource_Run(t *testing.T) {
},
nil,
false,
&recipes.ErrRecipeNotFound{Name: "test-recipe", Environment: TestEnvironmentID},
fmt.Errorf("could not find recipe %q in environment %q", "test-recipe", TestEnvironmentID),
nil,
nil,
nil,
nil,
&recipes.ErrRecipeNotFound{Name: "test-recipe", Environment: TestEnvironmentID},
fmt.Errorf("could not find recipe %q in environment %q", "test-recipe", TestEnvironmentID),
},
{
"runtime-configuration-err",
Expand Down Expand Up @@ -270,15 +271,15 @@ func TestCreateOrUpdateResource_Run(t *testing.T) {

req := &ctrl.Request{
OperationID: uuid.New(),
OperationType: "APPLICATIONS.CORE/EXTENDERS|PUT", // Operation does not affect the behavior of the controller.
OperationType: "APPLICATIONS.TEST/TESTRESOURCES|PUT", // Operation does not affect the behavior of the controller.
ResourceID: TestResourceID,
CorrelationID: uuid.NewString(),
OperationTimeout: &ctrl.DefaultAsyncOperationTimeout,
}

data := map[string]any{
"name": "tr",
"type": "Applications.Core/Extenders",
"type": "Applications.Test/testResources",
"id": TestResourceID,
"location": v1.LocationGlobal,
"properties": map[string]any{
Expand Down
2 changes: 1 addition & 1 deletion pkg/portableresources/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ 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"`
// Deployment status of the recipe
// DeploymentStatus is the deployment status of the recipe
DeploymentStatus util.RecipeDeploymentStatus `json:"recipeStatus,omitempty"`
}

Expand Down
12 changes: 6 additions & 6 deletions pkg/recipes/configloader/environment.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,20 +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 {
msg := &recipes.ErrRecipeNotFound{Name: recipe.Name, Environment: recipe.EnvironmentID}
return nil, recipes.NewRecipeError(recipes.RecipeNotFoundFailure, msg.Error(), recipes_util.RecipeSetupError, recipes.GetRecipeErrorDetails(msg))
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 {
msg := fmt.Errorf("failed to parse resourceID: %q %w", recipe.ResourceID, err)
return nil, recipes.NewRecipeError(recipes.RecipeValidationFailed, err.Error(), recipes_util.RecipeSetupError, recipes.GetRecipeErrorDetails(msg))
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 {
msg := &recipes.ErrRecipeNotFound{Name: recipe.Name, Environment: recipe.EnvironmentID}
return nil, recipes.NewRecipeError(recipes.RecipeNotFoundFailure, msg.Error(), recipes_util.RecipeSetupError, recipes.GetRecipeErrorDetails(msg))
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
6 changes: 4 additions & 2 deletions pkg/recipes/engine/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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
}
4 changes: 2 additions & 2 deletions pkg/recipes/engine/engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down
6 changes: 6 additions & 0 deletions pkg/recipes/errorcodes.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,4 +40,10 @@ const (

// 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"
)
11 changes: 0 additions & 11 deletions pkg/recipes/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package recipes
import (
"bytes"
"encoding/json"
"fmt"

"github.com/radius-project/radius/pkg/corerp/datamodel"
)
Expand Down Expand Up @@ -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.
Expand Down

0 comments on commit f84e218

Please sign in to comment.