Skip to content

Commit

Permalink
Use application scope namespace (#4783)
Browse files Browse the repository at this point in the history
# Description

This is to use application scope level namespace in deployment
processor.
- Fix PUT application controller bugs
- Fix Extension conversion bugs
- Update CLI to use the application scope namespace
- Update functional test to use the application-scope namespace
- Update deployment processor in CoreRP and LinkRP to use
application-scope namespace.

## Issue reference

<!--
We strive to have all PR being opened based on an issue, where the
problem or feature have been discussed prior to implementation.
-->

#3740
#4606


## Checklist

Please make sure you've completed the relevant tasks for this PR, out of
the following list:

* [x] Code compiles correctly
* [x] Adds necessary unit tests for change
* [x] Adds necessary E2E tests for change
* [x] Unit tests passing
* [x] Extended the documentation / Created issue for it
  • Loading branch information
youngbupark authored Dec 8, 2022
1 parent bf211ce commit c76ce22
Show file tree
Hide file tree
Showing 52 changed files with 480 additions and 307 deletions.
17 changes: 11 additions & 6 deletions pkg/cli/cmd/run/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"fmt"
"os"

"github.com/Azure/go-autorest/autorest/to"
"github.com/fatih/color"
"github.com/project-radius/radius/pkg/cli"
"github.com/project-radius/radius/pkg/cli/cmd/commonflags"
Expand Down Expand Up @@ -114,17 +115,21 @@ func (r *Runner) Run(ctx context.Context) error {
return nil
}

// We don't expect an error here because we already deployed to the environment
environment, err := client.GetEnvDetails(ctx, r.EnvironmentName)
app, err := client.ShowApplication(ctx, r.ApplicationName)
if err != nil {
return err
}

namespace := ""
switch compute := environment.Properties.Compute.(type) {
case *v20220315privatepreview.KubernetesCompute:
namespace = *compute.Namespace
default:
appStatus := app.Properties.Status
if appStatus != nil && appStatus.Compute != nil {
kube, ok := appStatus.Compute.(*v20220315privatepreview.KubernetesCompute)
if ok && kube.Namespace != nil {
namespace = to.String(kube.Namespace)
}
}

if namespace == "" {
return &cli.FriendlyError{Message: "Only kubernetes runtimes are supported."}
}

Expand Down
24 changes: 13 additions & 11 deletions pkg/cli/cmd/run/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,24 +175,26 @@ func Test_Run(t *testing.T) {
}).
Times(1)

environment := v20220315privatepreview.EnvironmentResource{
Properties: &v20220315privatepreview.EnvironmentProperties{
Compute: &v20220315privatepreview.KubernetesCompute{
Kind: to.Ptr(v20220315privatepreview.EnvironmentComputeKindKubernetes),
Namespace: to.Ptr("test-namespace"),
app := v20220315privatepreview.ApplicationResource{
Properties: &v20220315privatepreview.ApplicationProperties{
Status: &v20220315privatepreview.ResourceStatus{
Compute: &v20220315privatepreview.KubernetesCompute{
Kind: to.Ptr("kubernetes"),
Namespace: to.Ptr("test-namespace-app"),
},
},
},
}

clientMock := clients.NewMockApplicationsManagementClient(ctrl)
clientMock.EXPECT().
GetEnvDetails(gomock.Any(), radcli.TestEnvironmentName).
Return(environment, nil).
Times(1)
clientMock.EXPECT().
CreateApplicationIfNotFound(gomock.Any(), "test-application", gomock.Any()).
Return(nil).
Times(1)
clientMock.EXPECT().
ShowApplication(gomock.Any(), "test-application").
Return(app, nil).
Times(1)

workspace := &workspaces.Workspace{
Connection: map[string]interface{}{
Expand Down Expand Up @@ -242,13 +244,13 @@ func Test_Run(t *testing.T) {
// Logstream is scoped to application and namespace
require.Equal(t, runner.ApplicationName, logStreamOptions.ApplicationName)
require.Equal(t, "kind-kind", logStreamOptions.KubeContext)
require.Equal(t, "test-namespace", logStreamOptions.Namespace)
require.Equal(t, "test-namespace-app", logStreamOptions.Namespace)

portforwardOptions := <-portforwardOptionsChan
// Port-forward is scoped to application and namespace
require.Equal(t, runner.ApplicationName, portforwardOptions.ApplicationName)
require.Equal(t, "kind-kind", portforwardOptions.KubeContext)
require.Equal(t, "test-namespace", portforwardOptions.Namespace)
require.Equal(t, "test-namespace-app", portforwardOptions.Namespace)

// Shut down the log stream and verify result
cancel()
Expand Down
2 changes: 1 addition & 1 deletion pkg/cli/cmd/workspace/create/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ Workspaces allow you to manage multiple Radius platforms and environments using
You can easily define and switch between workspaces to deploy and manage applications across local, test, and production environments.`,
Args: ValidateArgs(),
Example: `
# Create a workspace with name 'myworkspace' and kuberentes context 'aks'
# Create a workspace with name 'myworkspace' and kubernetes context 'aks'
rad workspace create kubernetes myworkspace --context aks
# Create a workspace with name of current kubernetes context in current kubernetes context
rad workspace create kubernetes`,
Expand Down
30 changes: 10 additions & 20 deletions pkg/cli/deployment/diagnostics.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,47 +158,37 @@ func (dc *ARMDiagnosticsClient) findNamespaceOfContainer(ctx context.Context, re
return "", fmt.Errorf("could not namespace for container %q:%w", resourceName, err)
}

obj, ok = applicationResponse.Properties["environment"]
obj, ok = applicationResponse.Properties["status"]
if !ok {
return "", fmt.Errorf("could not find namespace for container %q", resourceName)
}

environment, ok := obj.(string)
status, ok := obj.(map[string]any)
if !ok {
return "", fmt.Errorf("could not find namespace for container %q", resourceName)
}

id, err = resources.ParseResource(environment)
if err != nil {
return "", fmt.Errorf("could not namespace for container %q:%w", resourceName, err)
}

environmentResponse, err := dc.EnvironmentClient.Get(ctx, id.Name(), nil)
if err != nil {
return "", fmt.Errorf("could not namespace for container %q:%w", resourceName, err)
}

obj, ok = environmentResponse.Properties["compute"]
obj, ok = status["compute"]
if !ok {
return "", fmt.Errorf("could not find namespace for container %q", resourceName)
}

compute, ok := obj.(map[string]interface{})
compute, ok := obj.(map[string]any)
if !ok {
return "", fmt.Errorf("could not find namespace for container %q", resourceName)
}

obj, ok = compute["namespace"]
if !ok {
kind, ok := compute["kind"].(string)
if !ok || !strings.EqualFold(kind, "kubernetes") {
return "", fmt.Errorf("could not find namespace for container %q", resourceName)
}

namespace, ok := obj.(string)
if !ok {
return "", fmt.Errorf("could not find namespace for container %q", resourceName)
namespace, ok := compute["namespace"].(string)
if ok {
return namespace, nil
}

return namespace, nil
return "", fmt.Errorf("could not find namespace for container %q", resourceName)
}

// Note: If an error is returned, any streams that were created before the error will also be returned.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ func (dst *ApplicationResource) ConvertFrom(src conv.DataModelInterface) error {
func fromAppExtensionClassificationDataModel(e datamodel.Extension) ApplicationExtensionClassification {
switch e.Kind {
case datamodel.KubernetesMetadata:
var ann, lbl = getFromExtensionClassificationFields(e)
var ann, lbl = fromExtensionClassificationFields(e)
return &ApplicationKubernetesMetadataExtension{
Kind: to.StringPtr(string(e.Kind)),
Annotations: *to.StringMapPtr(ann),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -506,7 +506,7 @@ func fromExtensionClassificationDataModel(e datamodel.Extension) ContainerExtens
Provides: to.StringPtr(e.DaprSidecar.Provides),
}
case datamodel.KubernetesMetadata:
var ann, lbl = getFromExtensionClassificationFields(e)
var ann, lbl = fromExtensionClassificationFields(e)
return &ContainerKubernetesMetadataExtension{
Kind: to.StringPtr(string(e.Kind)),
Annotations: *to.StringMapPtr(ann),
Expand All @@ -532,7 +532,7 @@ func toVolumeBaseDataModel(v Volume) datamodel.VolumeBase {
}
}

func getFromExtensionClassificationFields(e datamodel.Extension) (map[string]string, map[string]string) {
func fromExtensionClassificationFields(e datamodel.Extension) (map[string]string, map[string]string) {
var ann map[string]string
var lbl map[string]string

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -223,10 +223,9 @@ func fromEnvironmentComputeKind(kind rp.EnvironmentComputeKind) *string {

// fromExtensionClassificationEnvDataModel: Converts from base datamodel to versioned datamodel
func fromEnvExtensionClassificationDataModel(e datamodel.Extension) EnvironmentExtensionClassification {

switch e.Kind {
case datamodel.KubernetesMetadata:
var ann, lbl = getFromExtensionClassificationFields(e)
var ann, lbl = fromExtensionClassificationFields(e)
return &EnvironmentKubernetesMetadataExtension{
Kind: to.StringPtr(string(e.Kind)),
Annotations: *to.StringMapPtr(ann),
Expand Down
2 changes: 1 addition & 1 deletion pkg/corerp/backend/deployment/deployment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -717,7 +717,7 @@ func Test_Render(t *testing.T) {
_, err := dp.Render(ctx, resourceID, &testResource)
require.Error(t, err)
require.Equal(t, v1.CodeInvalid, err.(*conv.ErrClientRP).Code)
require.Equal(t, "linked application ID \"/subscriptions/test-subscription/resourceGroups/test-resource-group/providers/Applications.Core/app/test-application\" for resource \"/subscriptions/test-subscription/resourceGroups/test-resource-group/providers/Applications.Core/containers/test-resource\" has invalid application resource type.", err.(*conv.ErrClientRP).Message)
require.Equal(t, "linked \"/subscriptions/test-subscription/resourceGroups/test-resource-group/providers/Applications.Core/app/test-application\" has invalid Applications.Core/applications resource type.", err.(*conv.ErrClientRP).Message)
})

t.Run("Missing output resource provider", func(t *testing.T) {
Expand Down
89 changes: 16 additions & 73 deletions pkg/corerp/backend/deployment/deploymentprocessor.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import (
"github.com/project-radius/radius/pkg/resourcemodel"
"github.com/project-radius/radius/pkg/rp"
"github.com/project-radius/radius/pkg/rp/outputresource"
rp_util "github.com/project-radius/radius/pkg/rp/util"
"github.com/project-radius/radius/pkg/ucp/dataprovider"
"github.com/project-radius/radius/pkg/ucp/resources"
"github.com/project-radius/radius/pkg/ucp/store"
Expand Down Expand Up @@ -79,7 +80,7 @@ type ResourceData struct {
OutputResources []outputresource.OutputResource
ComputedValues map[string]interface{}
SecretValues map[string]rp.SecretValueReference
AppID resources.ID // Application ID for which the resource is created
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
}

Expand All @@ -99,12 +100,14 @@ func (dp *deploymentProcessor) Render(ctx context.Context, resourceID resources.
return renderers.RendererOutput{}, err
}
// 2. fetch the application properties from the DB
appProperties, err := dp.getApplicationProperties(ctx, res.AppID, resourceID.String())
app := &corerp_dm.Application{}
err = rp_util.FetchScopeResource(ctx, dp.sp, res.AppID.String(), app)
if err != nil {
return renderers.RendererOutput{}, err
}
// 3. fetch the environment resource from the db to get the Namespace
env, err := dp.fetchEnvironment(ctx, appProperties.BasicResourceProperties.Environment, resourceID)
env := &corerp_dm.Environment{}
err = rp_util.FetchScopeResource(ctx, dp.sp, app.Properties.Environment, env)
if err != nil {
return renderers.RendererOutput{}, err
}
Expand All @@ -125,7 +128,13 @@ func (dp *deploymentProcessor) Render(ctx context.Context, resourceID resources.
return renderers.RendererOutput{}, err
}

appOptions, err := dp.getAppOptions(ctx, appProperties)
c := app.Properties.Status.Compute
// Override environment-scope namespace with application-scope kubernetes namespace.
if c != nil && c.Kind == rp.KubernetesComputeKind {
envOptions.Namespace = c.KubernetesCompute.Namespace
}

appOptions, err := dp.getAppOptions(ctx, &app.Properties)
if err != nil {
return renderers.RendererOutput{}, err
}
Expand Down Expand Up @@ -573,16 +582,16 @@ func (dp *deploymentProcessor) getResourceDataByID(ctx context.Context, resource
}

func (dp *deploymentProcessor) buildResourceDependency(resourceID resources.ID, applicationID string, resource conv.DataModelInterface, outputResources []outputresource.OutputResource, computedValues map[string]interface{}, secretValues map[string]rp.SecretValueReference, recipeData link_dm.RecipeData) (ResourceData, error) {
var appID resources.ID
var appID *resources.ID
if applicationID != "" {
parsedID, err := resources.ParseResource(applicationID)
if err != nil {
return ResourceData{}, conv.NewClientErrInvalidRequest(fmt.Sprintf("application ID %q for the resource %q is not a valid id. Error: %s", applicationID, resourceID.String(), err.Error()))
}
appID = parsedID
appID = &parsedID
} else if strings.EqualFold(resourceID.ProviderNamespace(), resources.LinkRPNamespace) {
// Application id is optional for link resource types
appID = resources.ID{}
appID = nil
} else {
return ResourceData{}, fmt.Errorf("missing required application id for the resource %q", resourceID.String())
}
Expand Down Expand Up @@ -632,69 +641,3 @@ func (dp *deploymentProcessor) getRendererDependency(ctx context.Context, depend

return rendererDependency, nil
}

// getApplicationProperties returns application properties linked to the application fetched from the db
func (dp *deploymentProcessor) getApplicationProperties(ctx context.Context, appID resources.ID, resourceID string) (*corerp_dm.ApplicationProperties, error) {
errMsg := "failed to fetch the application %q for the resource %q. Err: %w"

appIDType := appID.Type()
app := &corerp_dm.Application{}
if !strings.EqualFold(appIDType, app.ResourceTypeName()) {
return nil, conv.NewClientErrInvalidRequest(fmt.Sprintf("linked application ID %q for resource %q has invalid application resource type.", appID.String(), resourceID))
}

sc, err := dp.sp.GetStorageClient(ctx, appIDType)
if err != nil {
return nil, fmt.Errorf(errMsg, appID.String(), resourceID, err)
}

res, err := sc.Get(ctx, appID.String())
if err != nil {
if errors.Is(&store.ErrNotFound{}, err) {
return nil, conv.NewClientErrInvalidRequest(fmt.Sprintf("linked application %q for resource %q does not exist", appID.String(), resourceID))
}
return nil, fmt.Errorf(errMsg, appID.String(), resourceID, err)
}
err = res.As(app)
if err != nil {
return nil, fmt.Errorf(errMsg, appID.String(), resourceID, err)
}

return &app.Properties, nil
}

// fetchEnvironment fetches the environment resource from the db for getting the namespace to deploy the resources
func (dp *deploymentProcessor) fetchEnvironment(ctx context.Context, environmentID string, resourceID resources.ID) (*corerp_dm.Environment, error) {
envId, err := resources.ParseResource(environmentID)
if err != nil {
return nil, err
}

env := &corerp_dm.Environment{}

if !strings.EqualFold(envId.Type(), env.ResourceTypeName()) {
return nil, conv.NewClientErrInvalidRequest(fmt.Sprintf("environment id %q linked to the application for resource %s is not a valid environment type. Error: %s", envId.Type(), resourceID, err.Error()))
}

sc, err := dp.sp.GetStorageClient(ctx, envId.Type())
if err != nil {
return nil, err
}

const errMsg = "failed to fetch the environment %q for the resource %q. Error: %w"

res, err := sc.Get(ctx, envId.String())
if err != nil {
if errors.Is(&store.ErrNotFound{}, err) {
return nil, conv.NewClientErrInvalidRequest(fmt.Sprintf("linked environment %q for resource %s does not exist", environmentID, resourceID))
}
return nil, fmt.Errorf(errMsg, environmentID, resourceID, err)
}

err = res.As(env)
if err != nil {
return nil, fmt.Errorf(errMsg, environmentID, resourceID, err)
}

return env, nil
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@ import (
rp_frontend "github.com/project-radius/radius/pkg/rp/frontend"
rp_kube "github.com/project-radius/radius/pkg/rp/kube"
"github.com/project-radius/radius/pkg/ucp/resources"

"github.com/go-logr/logr"
corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

var _ ctrl.Controller = (*CreateOrUpdateApplication)(nil)
Expand Down Expand Up @@ -57,6 +62,8 @@ func NewCreateOrUpdateApplication(opts ctrl.Options) (ctrl.Controller, error) {
// +-----------------+--------------------+-------------------------------+-------------------------------+

func (a *CreateOrUpdateApplication) populateKubernetesNamespace(ctx context.Context, old, newResource *datamodel.Application) (rest.Response, error) {
logger := logr.FromContextOrDiscard(ctx)

serviceCtx := v1.ARMRequestContextFromContext(ctx)

kubeNamespace := ""
Expand Down Expand Up @@ -105,17 +112,26 @@ func (a *CreateOrUpdateApplication) populateKubernetesNamespace(ctx context.Cont
}
}

// TODO: Enable this in https://github.com/project-radius/radius/pull/4783
// if !kubernetes.IsValidObjectName(kubeNamespace) {
// return rest.NewBadRequestResponse(fmt.Sprintf("'%s' is the invalid namespace. This must be at most 63 alphanumeric characters or '-'. Please specify the valid namespace in properties.extensions[*].kubernetesNamespaceOverride.", kubeNamespace)), nil
// }
if !kubernetes.IsValidObjectName(kubeNamespace) {
return rest.NewBadRequestResponse(fmt.Sprintf("'%s' is the invalid namespace. This must be at most 63 alphanumeric characters or '-'. Please specify a valid namespace using 'kubernetesNamespace' extension in '$.properties.extensions[*]'.", kubeNamespace)), nil
}

// Populate kubernetes namespace to internal metadata property for query indexing.
newResource.Properties.Status.Compute = &rp.EnvironmentCompute{
Kind: rp.KubernetesComputeKind,
KubernetesCompute: rp.KubernetesComputeProperties{Namespace: kubeNamespace},
}

// TODO: Move it to backend controller - https://github.com/project-radius/radius/issues/4742
err = a.KubeClient().Create(ctx, &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: kubeNamespace}})
if apierrors.IsAlreadyExists(err) {
logger.Info("Using existing namespace", "namespace", kubeNamespace)
} else if err != nil {
return nil, err
} else {
logger.Info("Created the namespace", "namespace", kubeNamespace)
}

return nil, nil
}

Expand Down
Loading

0 comments on commit c76ce22

Please sign in to comment.