Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make CreateOrUpdate controller async for link resources #5006

Merged
merged 16 commits into from
Jan 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions pkg/armrpc/frontend/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ package controller
import (
"context"
"net/http"
"time"

v1 "github.com/project-radius/radius/pkg/armrpc/api/v1"
sm "github.com/project-radius/radius/pkg/armrpc/asyncoperation/statusmanager"
Expand Down Expand Up @@ -54,6 +55,9 @@ type ResourceOptions[T any] struct {

// UpdateFilters is a slice of filters that execute prior to updating a resource.
UpdateFilters []UpdateFilter[T]

// AsyncOperationTimeout is the default timeout duration of async put operation.
AsyncOperationTimeout time.Duration
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this the best place to put this property?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so, as this can accessed for all the operations for the resource.

}

// TODO: Remove Controller when all controller uses Operation
Expand Down
13 changes: 13 additions & 0 deletions pkg/armrpc/frontend/controller/operation.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@ import (
"github.com/project-radius/radius/pkg/ucp/store"
)

const (
// defaultAsyncPutTimeout is the default timeout duration of async put operation.
defaultAsyncPutTimeout = time.Duration(2) * time.Minute
)

// Operation is the base operation controller.
type Operation[P interface {
*T
Expand Down Expand Up @@ -218,3 +223,11 @@ func (b *Operation[P, T]) DeleteFilters() []DeleteFilter[T] {
func (b *Operation[P, T]) UpdateFilters() []UpdateFilter[T] {
return b.resourceOptions.UpdateFilters
}

// AsyncOperationTimeout returns the timeput for the operation.
func (b *Operation[P, T]) AsyncOperationTimeout() time.Duration {
if b.resourceOptions.AsyncOperationTimeout == 0 {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please add the simple unittest for this ?

Suggested change
if b.resourceOptions.AsyncOperationTimeout == 0 {
if b.resourceOptions.AsyncOperationTimeout == time.Time{} {

return defaultAsyncPutTimeout
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but this function is not only used for put calls, right? delete timeout should be different?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

currently we use the same default timeout duration for put and delete, i.e 2 mins and we can override it by providing timeout in routes

}
return b.resourceOptions.AsyncOperationTimeout
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,12 @@ package defaultoperation
import (
"context"
"net/http"
"time"

v1 "github.com/project-radius/radius/pkg/armrpc/api/v1"
ctrl "github.com/project-radius/radius/pkg/armrpc/frontend/controller"
"github.com/project-radius/radius/pkg/armrpc/rest"
)

var (
// defaultAsyncDeleteTimeout is the default timeout duration of async delete operation.
defaultAsyncDeleteTimeout = time.Duration(120) * time.Second
)

// DefaultAsyncDelete is the controller implementation to delete async resource.
type DefaultAsyncDelete[P interface {
*T
Expand Down Expand Up @@ -58,7 +52,7 @@ func (e *DefaultAsyncDelete[P, T]) Run(ctx context.Context, w http.ResponseWrite
}
}

if r, err := e.PrepareAsyncOperation(ctx, old, v1.ProvisioningStateAccepted, defaultAsyncDeleteTimeout, &etag); r != nil || err != nil {
if r, err := e.PrepareAsyncOperation(ctx, old, v1.ProvisioningStateAccepted, e.AsyncOperationTimeout(), &etag); r != nil || err != nil {
return r, err
}

Expand Down
8 changes: 1 addition & 7 deletions pkg/armrpc/frontend/defaultoperation/defaultasyncput.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,12 @@ package defaultoperation
import (
"context"
"net/http"
"time"

v1 "github.com/project-radius/radius/pkg/armrpc/api/v1"
ctrl "github.com/project-radius/radius/pkg/armrpc/frontend/controller"
"github.com/project-radius/radius/pkg/armrpc/rest"
)

var (
// defaultAsyncPutTimeout is the default timeout duration of async put operation.
defaultAsyncPutTimeout = time.Duration(120) * time.Second
)

// DefaultAsyncPut is the controller implementation to create or update async resource.
type DefaultAsyncPut[P interface {
*T
Expand Down Expand Up @@ -59,7 +53,7 @@ func (e *DefaultAsyncPut[P, T]) Run(ctx context.Context, w http.ResponseWriter,
}
}

if r, err := e.PrepareAsyncOperation(ctx, newResource, v1.ProvisioningStateAccepted, defaultAsyncPutTimeout, &etag); r != nil || err != nil {
if r, err := e.PrepareAsyncOperation(ctx, newResource, v1.ProvisioningStateAccepted, e.AsyncOperationTimeout(), &etag); r != nil || err != nil {
return r, err
}

Expand Down
6 changes: 4 additions & 2 deletions pkg/corerp/backend/controller/createorupdateresource.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,10 @@ func (c *CreateOrUpdateResource) Run(ctx context.Context, request *ctrl.Request)

oldOutputResources := deploymentDataModel.OutputResources()

deploymentDataModel.ApplyDeploymentOutput(deploymentOutput)

err = deploymentDataModel.ApplyDeploymentOutput(deploymentOutput)
if err != nil {
return ctrl.Result{}, err
}
if !isNewResource {
diff := rpv1.GetGCOutputResources(deploymentDataModel.OutputResources(), oldOutputResources)
err = c.DeploymentProcessor().Delete(ctx, id, diff)
Expand Down
16 changes: 8 additions & 8 deletions pkg/corerp/backend/deployment/deploymentprocessor.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,8 @@ type ResourceData struct {
OutputResources []rpv1.OutputResource
ComputedValues map[string]any
SecretValues map[string]rpv1.SecretValueReference
AppID *resources.ID // Application ID for which the resource is created
RecipeData link_dm.RecipeData // Relevant only for links created with recipes to find relevant connections created by that recipe
AppID *resources.ID // Application ID for which the resource is created
RecipeData linkrp.RecipeData // Relevant only for links created with recipes to find relevant connections created by that recipe
}

func (dp *deploymentProcessor) Render(ctx context.Context, resourceID resources.ID, resource v1.DataModelInterface) (renderers.RendererOutput, error) {
Expand Down Expand Up @@ -496,25 +496,25 @@ func (dp *deploymentProcessor) getResourceDataByID(ctx context.Context, resource
if err = resource.As(obj); err != nil {
return ResourceData{}, fmt.Errorf(errMsg, resourceID.String(), err)
}
return dp.buildResourceDependency(resourceID, obj.Properties.Application, obj, obj.Properties.Status.OutputResources, obj.ComputedValues, obj.SecretValues, link_dm.RecipeData{})
return dp.buildResourceDependency(resourceID, obj.Properties.Application, obj, obj.Properties.Status.OutputResources, obj.ComputedValues, obj.SecretValues, linkrp.RecipeData{})
case strings.ToLower(gateway.ResourceType):
obj := &corerp_dm.Gateway{}
if err = resource.As(obj); err != nil {
return ResourceData{}, fmt.Errorf(errMsg, resourceID.String(), err)
}
return dp.buildResourceDependency(resourceID, obj.Properties.Application, obj, obj.Properties.Status.OutputResources, obj.ComputedValues, obj.SecretValues, link_dm.RecipeData{})
return dp.buildResourceDependency(resourceID, obj.Properties.Application, obj, obj.Properties.Status.OutputResources, obj.ComputedValues, obj.SecretValues, linkrp.RecipeData{})
case strings.ToLower(volume.ResourceType):
obj := &corerp_dm.VolumeResource{}
if err = resource.As(obj); err != nil {
return ResourceData{}, fmt.Errorf(errMsg, resourceID.String(), err)
}
return dp.buildResourceDependency(resourceID, obj.Properties.Application, obj, obj.Properties.Status.OutputResources, obj.ComputedValues, obj.SecretValues, link_dm.RecipeData{})
return dp.buildResourceDependency(resourceID, obj.Properties.Application, obj, obj.Properties.Status.OutputResources, obj.ComputedValues, obj.SecretValues, linkrp.RecipeData{})
case strings.ToLower(httproute.ResourceType):
obj := &corerp_dm.HTTPRoute{}
if err = resource.As(obj); err != nil {
return ResourceData{}, fmt.Errorf(errMsg, resourceID.String(), err)
}
return dp.buildResourceDependency(resourceID, obj.Properties.Application, obj, obj.Properties.Status.OutputResources, obj.ComputedValues, obj.SecretValues, link_dm.RecipeData{})
return dp.buildResourceDependency(resourceID, obj.Properties.Application, obj, obj.Properties.Status.OutputResources, obj.ComputedValues, obj.SecretValues, linkrp.RecipeData{})
case strings.ToLower(linkrp.MongoDatabasesResourceType):
obj := &link_dm.MongoDatabase{}
if err = resource.As(obj); err != nil {
Expand Down Expand Up @@ -544,7 +544,7 @@ func (dp *deploymentProcessor) getResourceDataByID(ctx context.Context, resource
if err = resource.As(obj); err != nil {
return ResourceData{}, fmt.Errorf(errMsg, resourceID.String(), err)
}
return dp.buildResourceDependency(resourceID, obj.Properties.Application, obj, obj.Properties.Status.OutputResources, obj.ComputedValues, obj.SecretValues, link_dm.RecipeData{})
return dp.buildResourceDependency(resourceID, obj.Properties.Application, obj, obj.Properties.Status.OutputResources, obj.ComputedValues, obj.SecretValues, linkrp.RecipeData{})
case strings.ToLower(linkrp.DaprStateStoresResourceType):
obj := &link_dm.DaprStateStore{}
if err = resource.As(obj); err != nil {
Expand Down Expand Up @@ -574,7 +574,7 @@ func (dp *deploymentProcessor) getResourceDataByID(ctx context.Context, resource
}
}

func (dp *deploymentProcessor) buildResourceDependency(resourceID resources.ID, applicationID string, resource v1.DataModelInterface, outputResources []rpv1.OutputResource, computedValues map[string]any, secretValues map[string]rpv1.SecretValueReference, recipeData link_dm.RecipeData) (ResourceData, error) {
func (dp *deploymentProcessor) buildResourceDependency(resourceID resources.ID, applicationID string, resource v1.DataModelInterface, outputResources []rpv1.OutputResource, computedValues map[string]any, secretValues map[string]rpv1.SecretValueReference, recipeData linkrp.RecipeData) (ResourceData, error) {
var appID *resources.ID
if applicationID != "" {
parsedID, err := resources.ParseResource(applicationID)
Expand Down
7 changes: 4 additions & 3 deletions pkg/corerp/backend/deployment/deploymentprocessor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"github.com/project-radius/radius/pkg/corerp/model"
"github.com/project-radius/radius/pkg/corerp/renderers"
"github.com/project-radius/radius/pkg/corerp/renderers/container"
"github.com/project-radius/radius/pkg/linkrp"
linkrp_dm "github.com/project-radius/radius/pkg/linkrp/datamodel"
linkrp_renderers "github.com/project-radius/radius/pkg/linkrp/renderers"
"github.com/project-radius/radius/pkg/linkrp/renderers/mongodatabases"
Expand Down Expand Up @@ -182,9 +183,9 @@ func buildMongoDBLinkWithRecipe() linkrp_dm.MongoDatabase {
Mode: linkrp_dm.LinkModeRecipe,
},
LinkMetadata: linkrp_dm.LinkMetadata{
RecipeData: linkrp_dm.RecipeData{
RecipeProperties: linkrp_dm.RecipeProperties{
LinkRecipe: linkrp_dm.LinkRecipe{
RecipeData: linkrp.RecipeData{
RecipeProperties: linkrp.RecipeProperties{
LinkRecipe: linkrp.LinkRecipe{
Name: "mongoDB",
Parameters: map[string]any{
"ResourceGroup": "testRG",
Expand Down
3 changes: 2 additions & 1 deletion pkg/corerp/datamodel/application.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,9 @@ func (e *Application) ResourceTypeName() string {
}

// ApplyDeploymentOutput applies the properties changes based on the deployment output.
func (c *Application) ApplyDeploymentOutput(do rpv1.DeploymentOutput) {
func (c *Application) ApplyDeploymentOutput(do rpv1.DeploymentOutput) error {
c.Properties.Status.OutputResources = do.DeployedOutputResources
mishrapratikshya marked this conversation as resolved.
Show resolved Hide resolved
return nil
}

// OutputResources returns the output resources array.
Expand Down
3 changes: 2 additions & 1 deletion pkg/corerp/datamodel/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,11 @@ func (c ContainerResource) ResourceTypeName() string {
}

// ApplyDeploymentOutput applies the properties changes based on the deployment output.
func (c *ContainerResource) ApplyDeploymentOutput(do rpv1.DeploymentOutput) {
func (c *ContainerResource) ApplyDeploymentOutput(do rpv1.DeploymentOutput) error {
c.Properties.Status.OutputResources = do.DeployedOutputResources
c.ComputedValues = do.ComputedValues
c.SecretValues = do.SecretValues
return nil
}

// OutputResources returns the output resources array.
Expand Down
3 changes: 2 additions & 1 deletion pkg/corerp/datamodel/gateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,14 @@ func (g *Gateway) ResourceTypeName() string {
}

// ApplyDeploymentOutput applies the properties changes based on the deployment output.
func (g *Gateway) ApplyDeploymentOutput(do rpv1.DeploymentOutput) {
func (g *Gateway) ApplyDeploymentOutput(do rpv1.DeploymentOutput) error {
g.Properties.Status.OutputResources = do.DeployedOutputResources
g.ComputedValues = do.ComputedValues
g.SecretValues = do.SecretValues
if url, ok := do.ComputedValues["url"].(string); ok {
g.Properties.URL = url
}
return nil
}

// OutputResources returns the output resources array.
Expand Down
3 changes: 2 additions & 1 deletion pkg/corerp/datamodel/httproute.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ func (h *HTTPRoute) ResourceTypeName() string {
}

// ApplyDeploymentOutput applies the properties changes based on the deployment output.
func (h *HTTPRoute) ApplyDeploymentOutput(do rpv1.DeploymentOutput) {
func (h *HTTPRoute) ApplyDeploymentOutput(do rpv1.DeploymentOutput) error {
if h.Properties != nil {
h.Properties.Status.OutputResources = do.DeployedOutputResources
}
Expand All @@ -46,6 +46,7 @@ func (h *HTTPRoute) ApplyDeploymentOutput(do rpv1.DeploymentOutput) {
if url, ok := do.ComputedValues["url"].(string); ok {
h.Properties.URL = url
}
return nil
}

// OutputResources returns the output resources array.
Expand Down
3 changes: 2 additions & 1 deletion pkg/corerp/datamodel/volume.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,11 @@ func (h *VolumeResource) ResourceTypeName() string {
}

// ApplyDeploymentOutput applies the properties changes based on the deployment output.
func (h *VolumeResource) ApplyDeploymentOutput(do rpv1.DeploymentOutput) {
func (h *VolumeResource) ApplyDeploymentOutput(do rpv1.DeploymentOutput) error {
h.Properties.Status.OutputResources = do.DeployedOutputResources
h.ComputedValues = do.ComputedValues
h.SecretValues = do.SecretValues
return nil
}

// OutputResources returns the output resources array.
Expand Down
8 changes: 4 additions & 4 deletions pkg/linkrp/api/v20220315privatepreview/datamodel_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
"github.com/Azure/azure-sdk-for-go/sdk/azcore/to"
autorestTo "github.com/Azure/go-autorest/autorest/to"
v1 "github.com/project-radius/radius/pkg/armrpc/api/v1"
"github.com/project-radius/radius/pkg/linkrp/datamodel"
"github.com/project-radius/radius/pkg/linkrp"
)

func toProvisioningStateDataModel(state *ProvisioningState) v1.ProvisioningState {
Expand Down Expand Up @@ -78,8 +78,8 @@ func fromSystemDataModel(s v1.SystemData) *SystemData {
}
}

func toRecipeDataModel(r *Recipe) datamodel.LinkRecipe {
recipe := datamodel.LinkRecipe{
func toRecipeDataModel(r *Recipe) linkrp.LinkRecipe {
recipe := linkrp.LinkRecipe{
Name: autorestTo.String(r.Name),
}

Expand All @@ -89,7 +89,7 @@ func toRecipeDataModel(r *Recipe) datamodel.LinkRecipe {
return recipe
}

func fromRecipeDataModel(r datamodel.LinkRecipe) *Recipe {
func fromRecipeDataModel(r linkrp.LinkRecipe) *Recipe {
return &Recipe{
Name: autorestTo.StringPtr(r.Name),
Parameters: r.Parameters,
Expand Down
103 changes: 103 additions & 0 deletions pkg/linkrp/backend/controller/createorupdateresource.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
// ------------------------------------------------------------
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.
// ------------------------------------------------------------

package controller

import (
"context"
"errors"
"net/http"

v1 "github.com/project-radius/radius/pkg/armrpc/api/v1"
ctrl "github.com/project-radius/radius/pkg/armrpc/asyncoperation/controller"
rpv1 "github.com/project-radius/radius/pkg/rp/v1"
"github.com/project-radius/radius/pkg/ucp/resources"
"github.com/project-radius/radius/pkg/ucp/store"
)

var _ ctrl.Controller = (*CreateOrUpdateResource)(nil)

// CreateOrUpdateResource is the async operation controller to create or update Applications.Link resources.
type CreateOrUpdateResource struct {
ctrl.BaseController
}

// NewCreateOrUpdateResource creates the CreateOrUpdateResource controller instance.
func NewCreateOrUpdateResource(opts ctrl.Options) (ctrl.Controller, error) {
return &CreateOrUpdateResource{ctrl.NewBaseAsyncController(opts)}, nil
}

func (c *CreateOrUpdateResource) Run(ctx context.Context, req *ctrl.Request) (ctrl.Result, error) {
obj, err := c.StorageClient().Get(ctx, req.ResourceID)
if err != nil && !errors.Is(&store.ErrNotFound{}, err) {
return ctrl.Result{}, err
}

isNewResource := false
ytimocin marked this conversation as resolved.
Show resolved Hide resolved
if errors.Is(&store.ErrNotFound{}, err) {
isNewResource = true
}

opType, _ := v1.ParseOperationType(req.OperationType)
if opType.Method == http.MethodPatch && errors.Is(&store.ErrNotFound{}, err) {
kachawla marked this conversation as resolved.
Show resolved Hide resolved
return ctrl.Result{}, err
}

// This code is general and we might be processing an async job for a resource or a scope, so using the general Parse function.
id, err := resources.Parse(req.ResourceID)
if err != nil {
return ctrl.Result{}, err
}

dataModel, err := getDataModel(id)
if err != nil {
return ctrl.Result{}, err
}

if err = obj.As(dataModel); err != nil {
return ctrl.Result{}, err
}

rendererOutput, err := c.LinkDeploymentProcessor().Render(ctx, id, dataModel)
if err != nil {
return ctrl.Result{}, err
}

deploymentOutput, err := c.LinkDeploymentProcessor().Deploy(ctx, id, rendererOutput)
if err != nil {
return ctrl.Result{}, err
}

deploymentDataModel, ok := dataModel.(rpv1.DeploymentDataModel)
if !ok {
return ctrl.NewFailedResult(v1.ErrorDetails{Message: "deployment data model conversion error"}), err
}
Comment on lines +73 to +76
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can move this check after L61 before rendering and deploying.


oldOutputResources := deploymentDataModel.OutputResources()
err = deploymentDataModel.ApplyDeploymentOutput(deploymentOutput)
if err != nil {
return ctrl.Result{}, err
}

if !isNewResource {
diff := rpv1.GetGCOutputResources(deploymentDataModel.OutputResources(), oldOutputResources)
err = c.LinkDeploymentProcessor().Delete(ctx, id, diff)
if err != nil {
return ctrl.Result{}, err
}
}
nr := &store.Object{
Metadata: store.Metadata{
ID: req.ResourceID,
},
Data: deploymentDataModel,
}
err = c.StorageClient().Save(ctx, nr, store.WithETag(obj.ETag))
if err != nil {
return ctrl.Result{}, err
}

return ctrl.Result{}, err
}
Loading