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.

Sign up for GitHub

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

Jump to bottom

Redesign plane resource types #7499

Merged
merged 1 commit into from
May 6, 2024
Merged

Redesign plane resource types #7499

merged 1 commit into from
May 6, 2024

Conversation

rynowak
Copy link
Contributor

@rynowak rynowak commented Apr 14, 2024

Description

This change splits the plane resource type into multiple types for maintainability as reviewed in radius-project/design-notes#42. See the design document for the motivation and dicussion of the specific design details. In summary, previously we had one resource type for all planes, and now we have a few separate ones.

This is a technically a breaking change but the impact on users is very low because it's mainly UCP itself that interacts with the planes.

The majority of the code changes are due to:

  • Simplifying the resource group client API
  • Updating and adding examples

Type of change

  • This pull request adds or changes features of Radius and has an approved issue (issue link required).

Fixes: #issue_number

@rynowak rynowak requested review from a team as code owners April 14, 2024 17:22
@rynowak
Copy link
Contributor Author

rynowak commented Apr 14, 2024

This is a large PR due to the number of changes required when we introduce new APIs. This change is introducing 3 new APIs! I'd recommend starting with the typespec and then looking at the routes for the the new APIs.

@codecov Codecov
Copy link

codecov bot commented Apr 14, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 69.64286% with 119 lines in your changes are missing coverage. Please review.

Project coverage is 60.31%. Comparing base (4d7ee44) to head (12b7baa).

❗ Current head 12b7baa differs from pull request most recent head 69d55ae. Consider uploading reports for the commit 69d55ae to get more accurate results

Files Patch % Lines
pkg/ucp/frontend/api/server.go 0.00% 52 Missing ⚠️
...kg/ucp/datamodel/converter/azureplane_converter.go 52.38% 7 Missing and 3 partials ⚠️
...g/ucp/datamodel/converter/radiusplane_converter.go 52.38% 7 Missing and 3 partials ⚠️
pkg/cli/clients/management.go 0.00% 8 Missing ⚠️
pkg/ucp/frontend/api/routes.go 36.36% 6 Missing and 1 partial ⚠️
.../ucp/datamodel/converter/genericplane_converter.go 45.45% 5 Missing and 1 partial ⚠️
...cp/api/v20231001preview/genericplane_conversion.go 73.33% 3 Missing and 1 partial ⚠️
pkg/ucp/datamodel/radiusplane.go 0.00% 3 Missing ⚠️
.../ucp/frontend/controller/planes/proxycontroller.go 0.00% 3 Missing ⚠️
pkg/controller/reconciler/client.go 0.00% 2 Missing ⚠️
... and 7 more
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #7499      +/-   ##
==========================================
+ Coverage   60.25%   60.31%   +0.06%     
==========================================
  Files         507      517      +10     
  Lines       26265    26432     +167     
==========================================
+ Hits        15825    15943     +118     
- Misses       9105     9140      +35     
- Partials     1335     1349      +14

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@radius-functional-tests
Copy link

radius-functional-tests bot commented Apr 14, 2024 •
edited
Loading

Radius functional test overview

🔍 Go to test action run

Name Value
Repository rynowak/radius
Commit ref 6e1d997
Unique ID func2a06e1ab8e
Image tag pr-func2a06e1ab8e
Click here to see the list of tools in the current test run
  • gotestsum 1.10.0
  • KinD: v0.20.0
  • Dapr: 1.12.0
  • Azure KeyVault CSI driver: 1.4.2
  • Azure Workload identity webhook: 1.1.0
  • Bicep recipe location ghcr.io/radius-project/dev/test/testrecipes/test-bicep-recipes/<name>:pr-func2a06e1ab8e
  • Terraform recipe location http://tf-module-server.radius-test-tf-module-server.svc.cluster.local/<name>.zip (in cluster)
  • applications-rp test image location: ghcr.io/radius-project/dev/applications-rp:pr-func2a06e1ab8e
  • controller test image location: ghcr.io/radius-project/dev/controller:pr-func2a06e1ab8e
  • ucp test image location: ghcr.io/radius-project/dev/ucpd:pr-func2a06e1ab8e
  • deployment-engine test image location: ghcr.io/radius-project/deployment-engine:latest

Test Status

⌛ Building Radius and pushing container images for functional tests...
✅ Container images build succeeded
⌛ Publishing Bicep Recipes for functional tests...
✅ Recipe publishing succeeded
⌛ Starting samples functional tests...
⌛ Starting msgrp functional tests...
⌛ Starting cli functional tests...
⌛ Starting shared functional tests...
⌛ Starting ucp functional tests...
⌛ Starting daprrp functional tests...
⌛ Starting kubernetes functional tests...
⌛ Starting datastoresrp functional tests...
✅ msgrp functional tests succeeded
✅ samples functional tests succeeded
❌ ucp functional test failed. Please check the logs for more details
✅ kubernetes functional tests succeeded
❌ datastoresrp functional test cancelled. Please check the logs for more details
❌ daprrp functional test cancelled. Please check the logs for more details
❌ shared functional test cancelled. Please check the logs for more details

@radius-functional-tests
Copy link

radius-functional-tests bot commented Apr 14, 2024 •
edited
Loading

Radius functional test overview

🔍 Go to test action run

Name Value
Repository rynowak/radius
Commit ref 727fdc7
Unique ID func2e6ea78456
Image tag pr-func2e6ea78456
Click here to see the list of tools in the current test run
  • gotestsum 1.10.0
  • KinD: v0.20.0
  • Dapr: 1.12.0
  • Azure KeyVault CSI driver: 1.4.2
  • Azure Workload identity webhook: 1.1.0
  • Bicep recipe location ghcr.io/radius-project/dev/test/testrecipes/test-bicep-recipes/<name>:pr-func2e6ea78456
  • Terraform recipe location http://tf-module-server.radius-test-tf-module-server.svc.cluster.local/<name>.zip (in cluster)
  • applications-rp test image location: ghcr.io/radius-project/dev/applications-rp:pr-func2e6ea78456
  • controller test image location: ghcr.io/radius-project/dev/controller:pr-func2e6ea78456
  • ucp test image location: ghcr.io/radius-project/dev/ucpd:pr-func2e6ea78456
  • deployment-engine test image location: ghcr.io/radius-project/deployment-engine:latest

Test Status

⌛ Building Radius and pushing container images for functional tests...
✅ Container images build succeeded
⌛ Publishing Bicep Recipes for functional tests...
✅ Recipe publishing succeeded
⌛ Starting samples functional tests...
⌛ Starting datastoresrp functional tests...
⌛ Starting kubernetes functional tests...
⌛ Starting cli functional tests...
⌛ Starting daprrp functional tests...
⌛ Starting msgrp functional tests...
⌛ Starting ucp functional tests...
⌛ Starting shared functional tests...
✅ kubernetes functional tests succeeded
✅ samples functional tests succeeded
❌ ucp functional test failed. Please check the logs for more details
❌ cli functional test cancelled. Please check the logs for more details
❌ daprrp functional test cancelled. Please check the logs for more details
❌ shared functional test cancelled. Please check the logs for more details
❌ msgrp functional test cancelled. Please check the logs for more details

@rynowak rynowak force-pushed the plane-apis branch 2 times, most recently from 6d98b60 to ce2bf68 Compare April 15, 2024 17:23
@rynowak
Copy link
Contributor Author

rynowak commented Apr 15, 2024

@youngbupark @ytimocin can you take a look when you have a chance? I need to figure out my one failing test, but that is likely a swagger update I missed.

youngbupark
youngbupark reviewed Apr 25, 2024
View reviewed changes
Copy link

@youngbupark youngbupark left a comment

Choose a reason for hiding this comment

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

Since this is a huge update, it is not easy to review the entire code thoroughly. Overall looks good to me. I will have some time to review this PR in detail tomorrow. meanwhile, please fix the test :)

pkg/cli/clients/management.go Outdated Show resolved Hide resolved
pkg/ucp/api/v20231001preview/zz_generated_response_types.go Show resolved Hide resolved
pkg/ucp/frontend/azure/routes.go Show resolved Hide resolved
@rynowak rynowak temporarily deployed to functional-tests May 6, 2024 17:43 — with GitHub Actions Inactive
@radius-functional-tests
Copy link

radius-functional-tests bot commented May 6, 2024 •
edited
Loading

Radius functional test overview

🔍 Go to test action run

Name Value
Repository rynowak/radius
Commit ref 0f1fe77
Unique ID func5639f29d9c
Image tag pr-func5639f29d9c
Click here to see the list of tools in the current test run
  • gotestsum 1.10.0
  • KinD: v0.20.0
  • Dapr: 1.12.0
  • Azure KeyVault CSI driver: 1.4.2
  • Azure Workload identity webhook: 1.1.0
  • Bicep recipe location ghcr.io/radius-project/dev/test/testrecipes/test-bicep-recipes/<name>:pr-func5639f29d9c
  • Terraform recipe location http://tf-module-server.radius-test-tf-module-server.svc.cluster.local/<name>.zip (in cluster)
  • applications-rp test image location: ghcr.io/radius-project/dev/applications-rp:pr-func5639f29d9c
  • controller test image location: ghcr.io/radius-project/dev/controller:pr-func5639f29d9c
  • ucp test image location: ghcr.io/radius-project/dev/ucpd:pr-func5639f29d9c
  • deployment-engine test image location: ghcr.io/radius-project/deployment-engine:latest

Test Status

⌛ Building Radius and pushing container images for functional tests...
✅ Container images build succeeded
⌛ Publishing Bicep Recipes for functional tests...
✅ Recipe publishing succeeded
⌛ Starting kubernetes functional tests...
⌛ Starting samples functional tests...
⌛ Starting shared functional tests...
⌛ Starting msgrp functional tests...
⌛ Starting daprrp functional tests...
⌛ Starting cli functional tests...
⌛ Starting datastoresrp functional tests...
✅ msgrp functional tests succeeded
✅ kubernetes functional tests succeeded
✅ samples functional tests succeeded
✅ ucp functional tests succeeded
✅ daprrp functional tests succeeded
✅ datastoresrp functional tests succeeded
✅ cli functional tests succeeded
✅ shared functional tests succeeded

ytimocin
ytimocin reviewed May 6, 2024
View reviewed changes
deploy/Chart/crds/radius/radapp.io_recipes.yaml Outdated Show resolved Hide resolved
pkg/ucp/api/v20231001preview/awsplane_conversion.go Outdated Show resolved Hide resolved
pkg/ucp/api/v20231001preview/common.go Show resolved Hide resolved
pkg/ucp/api/v20231001preview/radiusplane_conversion.go Show resolved Hide resolved
pkg/ucp/datamodel/converter/azureplane_converter.go Show resolved Hide resolved
pkg/ucp/frontend/api/server.go Show resolved Hide resolved
pkg/ucp/integrationtests/planes/aws_test.go Show resolved Hide resolved
...ecification/ucp/resource-manager/UCP/preview/2023-10-01-preview/examples/AwsPlanes_List.json Show resolved Hide resolved
...esource-manager/UCP/preview/2023-10-01-preview/examples/AzureCredentials_CreateOrUpdate.json Show resolved Hide resolved
...tion/ucp/resource-manager/UCP/preview/2023-10-01-preview/examples/AzureCredentials_List.json Show resolved Hide resolved
@rynowak
Copy link
Contributor Author

rynowak commented May 6, 2024

@ytimocin - updated

@rynowak rynowak temporarily deployed to functional-tests May 6, 2024 21:00 — with GitHub Actions Inactive
@radius-functional-tests
Copy link

radius-functional-tests bot commented May 6, 2024 •
edited
Loading

Radius functional test overview

🔍 Go to test action run

Name Value
Repository rynowak/radius
Commit ref 12b7baa
Unique ID func42e75a5314
Image tag pr-func42e75a5314
Click here to see the list of tools in the current test run
  • gotestsum 1.10.0
  • KinD: v0.20.0
  • Dapr: 1.12.0
  • Azure KeyVault CSI driver: 1.4.2
  • Azure Workload identity webhook: 1.1.0
  • Bicep recipe location ghcr.io/radius-project/dev/test/testrecipes/test-bicep-recipes/<name>:pr-func42e75a5314
  • Terraform recipe location http://tf-module-server.radius-test-tf-module-server.svc.cluster.local/<name>.zip (in cluster)
  • applications-rp test image location: ghcr.io/radius-project/dev/applications-rp:pr-func42e75a5314
  • controller test image location: ghcr.io/radius-project/dev/controller:pr-func42e75a5314
  • ucp test image location: ghcr.io/radius-project/dev/ucpd:pr-func42e75a5314
  • deployment-engine test image location: ghcr.io/radius-project/deployment-engine:latest

Test Status

⌛ Building Radius and pushing container images for functional tests...
✅ Container images build succeeded
⌛ Publishing Bicep Recipes for functional tests...
✅ Recipe publishing succeeded
⌛ Starting samples functional tests...
⌛ Starting ucp functional tests...
⌛ Starting shared functional tests...
⌛ Starting daprrp functional tests...
⌛ Starting kubernetes functional tests...
⌛ Starting msgrp functional tests...
⌛ Starting cli functional tests...
⌛ Starting datastoresrp functional tests...
✅ samples functional tests succeeded
✅ msgrp functional tests succeeded
✅ ucp functional tests succeeded
✅ daprrp functional tests succeeded
✅ datastoresrp functional tests succeeded
✅ cli functional tests succeeded
✅ shared functional tests succeeded

@rynowak
Redesign plane resource types
69d55ae 
This change splits the plane resource type into multiple types for maintainability as reviewed in radius-project/design-notes#42. See the design document for the motivation and dicussion of the specific design details. In summary, previously we had one resource type for all planes, and now we have a few separate ones.

This is a technically a breaking change but the impact on users is very low because it's mainly UCP itself that interacts with the planes.

The majority of the code changes are due to:

    Simplifying the resource group client API
    Updating and adding examples

Signed-off-by: Ryan Nowak <nowakra@gmail.com>
@rynowak rynowak temporarily deployed to functional-tests May 6, 2024 21:32 — with GitHub Actions Inactive
@radius-functional-tests
Copy link

radius-functional-tests bot commented May 6, 2024 •
edited
Loading

Radius functional test overview

🔍 Go to test action run

Name Value
Repository rynowak/radius
Commit ref 69d55ae
Unique ID funcf72f55d2a0
Image tag pr-funcf72f55d2a0
Click here to see the list of tools in the current test run
  • gotestsum 1.10.0
  • KinD: v0.20.0
  • Dapr: 1.12.0
  • Azure KeyVault CSI driver: 1.4.2
  • Azure Workload identity webhook: 1.1.0
  • Bicep recipe location ghcr.io/radius-project/dev/test/testrecipes/test-bicep-recipes/<name>:pr-funcf72f55d2a0
  • Terraform recipe location http://tf-module-server.radius-test-tf-module-server.svc.cluster.local/<name>.zip (in cluster)
  • applications-rp test image location: ghcr.io/radius-project/dev/applications-rp:pr-funcf72f55d2a0
  • controller test image location: ghcr.io/radius-project/dev/controller:pr-funcf72f55d2a0
  • ucp test image location: ghcr.io/radius-project/dev/ucpd:pr-funcf72f55d2a0
  • deployment-engine test image location: ghcr.io/radius-project/deployment-engine:latest

Test Status

⌛ Building Radius and pushing container images for functional tests...
✅ Container images build succeeded
⌛ Publishing Bicep Recipes for functional tests...
✅ Recipe publishing succeeded
⌛ Starting daprrp functional tests...
⌛ Starting kubernetes functional tests...
⌛ Starting samples functional tests...
⌛ Starting msgrp functional tests...
⌛ Starting ucp functional tests...
⌛ Starting shared functional tests...
✅ kubernetes functional tests succeeded
✅ samples functional tests succeeded
✅ msgrp functional tests succeeded
✅ daprrp functional tests succeeded
✅ ucp functional tests succeeded
✅ datastoresrp functional tests succeeded
✅ cli functional tests succeeded
✅ shared functional tests succeeded

@rynowak rynowak merged commit 1f24aa9 into radius-project:main May 6, 2024
15 checks passed
@rynowak rynowak deleted the plane-apis branch May 6, 2024 21:56
@rynowak
Copy link
Contributor Author

rynowak commented May 6, 2024

Thanks @ytimocin !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@youngbupark youngbupark youngbupark left review comments

@ytimocin ytimocin ytimocin approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@rynowak @ytimocin @youngbupark