Register Manifests during ucp startup sequence #8120

lakshmimsft · 2024-12-11T17:24:32Z

Description

Registering Manifests during ucp startup sequence.
New initializer service to help do that
using generated clients directly

Type of change

This pull request fixes a bug in Radius and has an approved issue (link).

Fixes: Part of UDT

Contributor checklist

Please verify that the PR meets the following requirements, where applicable:

An overview of proposed schema changes is included in a linked GitHub issue.
A design document PR is created in the design-notes repository, if new APIs are being introduced.
If applicable, design document has been reviewed and approved by Radius maintainers/approvers.
A PR for the samples repository is created, if existing samples are affected by the changes in this PR.
A PR for the documentation repository is created, if the changes in this PR affect the documentation or any user facing updates are made.
A PR for the recipes repository is created, if existing recipes are affected by the changes in this PR.

codecov · 2024-12-11T17:35:57Z

Codecov Report

Attention: Patch coverage is 66.35802% with 109 lines in your changes missing coverage. Please review.

Project coverage is 60.11%. Comparing base (6b8ef42) to head (1939ae8).

Files with missing lines	Patch %	Lines
pkg/cli/manifest/registermanifest.go	63.46%	25 Missing and 13 partials ⚠️
pkg/ucp/initializer/service.go	58.20%	20 Missing and 8 partials ⚠️
pkg/cli/cmd/resourceprovider/create/create.go	36.00%	12 Missing and 4 partials ⚠️
pkg/cli/manifest/testclientfactory.go	89.18%	11 Missing and 1 partial ⚠️
pkg/cli/workspaces/connection.go	0.00%	11 Missing ⚠️
pkg/cli/connections/factory.go	0.00%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8120      +/-   ##
==========================================
+ Coverage   60.04%   60.11%   +0.07%     
==========================================
  Files         581      584       +3     
  Lines       38536    38784     +248     
==========================================
+ Hits        23138    23316     +178     
- Misses      13698    13749      +51     
- Partials     1700     1719      +19

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

rynowak

Thanks, this is an improvement. Let's keep iterating on Monday.

cmd/ucpd/cmd/root.go

cmd/ucpd/ucp-dev.yaml

deploy/manifest/built-in-providers/test1.yaml

pkg/ucp/api/v20231001preview/locations_fake_test.go

pkg/ucp/hosting/hosting.go

pkg/ucp/manifestservice/service.go

pkg/ucp/ucpclient/ucpclient.go

pkg/cli/manifest/registermanifest.go

pkg/cli/manifest/testclientfactory.go

rynowak · 2024-12-20T18:37:16Z

pkg/cli/workspaces/connection.go

 // connection and an error if one occurs.
-func (ws Workspace) Connect() (sdk.Connection, error) {
+func (ws Workspace) Connect(ctx context.Context) (sdk.Connection, error) {


Did you find any places in code that call connect but don't call test-connection?

I think this is a good refactor, but I'm trying to figure out if it's safe to change.

yes, all current calls were followed by another call to test connection

rynowak · 2024-12-20T18:41:15Z

pkg/ucp/integrationtests/resourceproviders/resourceproviders_test.go

+func Test_ResourceProvider_RegisterManifests(t *testing.T) {
+	server := testhost.Start(t, testhost.TestHostOptionFunc(func(options *ucp.Options) {
+		options.Config.Initialization.ManifestDirectory = "testdata/manifests"
+	}))


Awesome 👍

rynowak · 2024-12-20T18:42:48Z

pkg/ucp/integrationtests/resourceproviders/resourceproviders_test.go

+	createRadiusPlane(server)
+
+	// Wait for the manifest to be registered
+	time.Sleep(5 * time.Second)


Can we do this with require.Eventually instead?

Picking a hardcoded value has a lot of downsides:

If it's too short, the test will be unreliable

If it's too long, we end up waiting a lot

You can use require.Eventually with a long-ish timeout and it will be mitigate both of these issues. eg: retry every 1 second with a 30 second deadline.

rynowak · 2024-12-20T18:43:46Z

pkg/cli/cmd/resourceprovider/create/create.go

-	client, err := r.ConnectionFactory.CreateApplicationsManagementClient(ctx, *r.Workspace)
-	if err != nil {
-		return err
+	// Initialize the client factory


Can you update this comment to explain why this is being done inside Run?

rynowak

A few more small things to look at. This is almost there!

Signed-off-by: lakshmimsft <[email protected]>

radius-functional-tests · 2024-12-21T00:45:13Z

Radius functional test overview

🔍 Go to test action run

Name	Value
Repository	lakshmimsft/radius-mainfork
Commit ref	`fb26995`
Unique ID	func7d93474e9f
Image tag	pr-func7d93474e9f

Click here to see the list of tools in the current test run

gotestsum 1.12.0
KinD: v0.20.0
Dapr:
Azure KeyVault CSI driver: 1.4.2
Azure Workload identity webhook: 1.3.0
Bicep recipe location ghcr.io/radius-project/dev/test/testrecipes/test-bicep-recipes/<name>:pr-func7d93474e9f
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-func7d93474e9f
dynamic-rp test image location: ghcr.io/radius-project/dev/dynamic-rp:pr-func7d93474e9f
controller test image location: ghcr.io/radius-project/dev/controller:pr-func7d93474e9f
ucp test image location: ghcr.io/radius-project/dev/ucpd:pr-func7d93474e9f
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 corerp-cloud functional tests...
⌛ Starting ucp-cloud functional tests...
✅ ucp-cloud functional tests succeeded
✅ corerp-cloud functional tests succeeded

Signed-off-by: lakshmimsft <[email protected]>

radius-functional-tests · 2024-12-21T03:00:35Z

Radius functional test overview

🔍 Go to test action run

Name	Value
Repository	lakshmimsft/radius-mainfork
Commit ref	`1939ae8`
Unique ID	func9ce86ddc8e
Image tag	pr-func9ce86ddc8e

Click here to see the list of tools in the current test run

gotestsum 1.12.0
KinD: v0.20.0
Dapr:
Azure KeyVault CSI driver: 1.4.2
Azure Workload identity webhook: 1.3.0
Bicep recipe location ghcr.io/radius-project/dev/test/testrecipes/test-bicep-recipes/<name>:pr-func9ce86ddc8e
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-func9ce86ddc8e
dynamic-rp test image location: ghcr.io/radius-project/dev/dynamic-rp:pr-func9ce86ddc8e
controller test image location: ghcr.io/radius-project/dev/controller:pr-func9ce86ddc8e
ucp test image location: ghcr.io/radius-project/dev/ucpd:pr-func9ce86ddc8e
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 corerp-cloud functional tests...
⌛ Starting ucp-cloud functional tests...
✅ ucp-cloud functional tests succeeded
✅ corerp-cloud functional tests succeeded

lakshmimsft temporarily deployed to publish-bicep December 11, 2024 17:24 — with GitHub Actions Inactive

lakshmimsft force-pushed the lakshmimsft/upducpstartup branch from 4c242d7 to 4cef599 Compare December 11, 2024 23:48

lakshmimsft temporarily deployed to publish-bicep December 11, 2024 23:48 — with GitHub Actions Inactive

lakshmimsft force-pushed the lakshmimsft/upducpstartup branch from 4cef599 to 1e20af2 Compare December 11, 2024 23:52

lakshmimsft temporarily deployed to publish-bicep December 11, 2024 23:52 — with GitHub Actions Inactive

rynowak reviewed Dec 13, 2024

View reviewed changes

lakshmimsft temporarily deployed to publish-bicep December 16, 2024 17:17 — with GitHub Actions Inactive

lakshmimsft force-pushed the lakshmimsft/upducpstartup branch from f432f1e to 15bf5c0 Compare December 16, 2024 17:26

lakshmimsft temporarily deployed to publish-bicep December 16, 2024 17:27 — with GitHub Actions Inactive

lakshmimsft force-pushed the lakshmimsft/upducpstartup branch from 15bf5c0 to 31c016b Compare December 18, 2024 16:32

lakshmimsft changed the title ~~[WIP] UDT Draft PR - Register Manifests during ucp startup sequence~~ Register Manifests during ucp startup sequence Dec 18, 2024

lakshmimsft force-pushed the lakshmimsft/upducpstartup branch from 31c016b to ddba2d1 Compare December 18, 2024 17:24

lakshmimsft temporarily deployed to publish-bicep December 18, 2024 17:25 — with GitHub Actions Inactive

lakshmimsft force-pushed the lakshmimsft/upducpstartup branch from ddba2d1 to 2b6ddf2 Compare December 18, 2024 22:33

lakshmimsft temporarily deployed to publish-bicep December 18, 2024 22:33 — with GitHub Actions Inactive

lakshmimsft marked this pull request as ready for review December 18, 2024 22:33

lakshmimsft requested review from a team as code owners December 18, 2024 22:33

lakshmimsft force-pushed the lakshmimsft/upducpstartup branch from 2b6ddf2 to 2027d07 Compare December 18, 2024 22:55

lakshmimsft temporarily deployed to publish-bicep December 18, 2024 22:55 — with GitHub Actions Inactive

lakshmimsft marked this pull request as draft December 19, 2024 00:21

lakshmimsft force-pushed the lakshmimsft/upducpstartup branch from 2027d07 to 626eaa3 Compare December 19, 2024 17:44

lakshmimsft temporarily deployed to publish-bicep December 19, 2024 17:45 — with GitHub Actions Inactive

lakshmimsft force-pushed the lakshmimsft/upducpstartup branch from 626eaa3 to fd4ef05 Compare December 19, 2024 19:15

lakshmimsft temporarily deployed to publish-bicep December 19, 2024 19:15 — with GitHub Actions Inactive

lakshmimsft marked this pull request as ready for review December 19, 2024 19:22

lakshmimsft force-pushed the lakshmimsft/upducpstartup branch from fd4ef05 to 4f9399d Compare December 19, 2024 19:32

lakshmimsft temporarily deployed to publish-bicep December 19, 2024 19:32 — with GitHub Actions Inactive

lakshmimsft force-pushed the lakshmimsft/upducpstartup branch from 4f9399d to bda5c6f Compare December 19, 2024 19:37

lakshmimsft force-pushed the lakshmimsft/upducpstartup branch from 820ce9a to 0295fe2 Compare December 20, 2024 17:49

lakshmimsft temporarily deployed to publish-bicep December 20, 2024 17:50 — with GitHub Actions Inactive

rynowak reviewed Dec 20, 2024

View reviewed changes

pkg/cli/manifest/registermanifest.go Show resolved Hide resolved

rynowak reviewed Dec 20, 2024

View reviewed changes

pkg/cli/manifest/testclientfactory.go Show resolved Hide resolved

rynowak reviewed Dec 20, 2024

View reviewed changes

rynowak requested changes Dec 20, 2024

View reviewed changes

lakshmimsft force-pushed the lakshmimsft/upducpstartup branch from 0295fe2 to ff99f6b Compare December 21, 2024 00:25

lakshmimsft had a problem deploying to publish-bicep December 21, 2024 00:25 — with GitHub Actions Error

lakshmimsft added 8 commits December 20, 2024 16:27

draft

a3346c3

Signed-off-by: lakshmimsft <[email protected]>

draft

1819e4f

Signed-off-by: lakshmimsft <[email protected]>

draft 3

f2c16c4

Signed-off-by: lakshmimsft <[email protected]>

add service

9e49119

Signed-off-by: lakshmimsft <[email protected]>

still draft

fbdbf3d

Signed-off-by: lakshmimsft <[email protected]>

upd per discussion

0290830

Signed-off-by: lakshmimsft <[email protected]>

rebase issue

0d1696c

Signed-off-by: lakshmimsft <[email protected]>

update per comments

f325b57

Signed-off-by: lakshmimsft <[email protected]>

lakshmimsft force-pushed the lakshmimsft/upducpstartup branch from ff99f6b to 2975a86 Compare December 21, 2024 00:27

lakshmimsft temporarily deployed to publish-bicep December 21, 2024 00:27 — with GitHub Actions Inactive

lakshmimsft force-pushed the lakshmimsft/upducpstartup branch from 2975a86 to fb26995 Compare December 21, 2024 00:36

lakshmimsft temporarily deployed to publish-bicep December 21, 2024 00:36 — with GitHub Actions Inactive

lakshmimsft temporarily deployed to functional-tests December 21, 2024 00:44 — with GitHub Actions Inactive

adding integration test

1939ae8

Signed-off-by: lakshmimsft <[email protected]>

lakshmimsft force-pushed the lakshmimsft/upducpstartup branch from fb26995 to 1939ae8 Compare December 21, 2024 00:46

lakshmimsft deployed to publish-bicep December 21, 2024 00:46 — with GitHub Actions Active

lakshmimsft deployed to functional-tests December 21, 2024 02:59 — with GitHub Actions Active

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Register Manifests during ucp startup sequence #8120

Register Manifests during ucp startup sequence #8120

lakshmimsft commented Dec 11, 2024 •

edited

Loading

codecov bot commented Dec 11, 2024 •

edited

Loading

rynowak left a comment

rynowak Dec 20, 2024

rynowak Dec 20, 2024

lakshmimsft Dec 20, 2024

rynowak Dec 20, 2024

rynowak Dec 20, 2024

rynowak Dec 20, 2024

rynowak left a comment

radius-functional-tests bot commented Dec 21, 2024 •

edited

Loading

radius-functional-tests bot commented Dec 21, 2024 •

edited

Loading

Register Manifests during ucp startup sequence #8120

Are you sure you want to change the base?

Register Manifests during ucp startup sequence #8120

Conversation

lakshmimsft commented Dec 11, 2024 • edited Loading

Description

Type of change

Contributor checklist

codecov bot commented Dec 11, 2024 • edited Loading

Codecov Report

rynowak left a comment

Choose a reason for hiding this comment

rynowak Dec 20, 2024

Choose a reason for hiding this comment

rynowak Dec 20, 2024

Choose a reason for hiding this comment

lakshmimsft Dec 20, 2024

Choose a reason for hiding this comment

rynowak Dec 20, 2024

Choose a reason for hiding this comment

rynowak Dec 20, 2024

Choose a reason for hiding this comment

rynowak Dec 20, 2024

Choose a reason for hiding this comment

rynowak left a comment

Choose a reason for hiding this comment

radius-functional-tests bot commented Dec 21, 2024 • edited Loading

Radius functional test overview

Test Status

radius-functional-tests bot commented Dec 21, 2024 • edited Loading

Radius functional test overview

Test Status

lakshmimsft commented Dec 11, 2024 •

edited

Loading

codecov bot commented Dec 11, 2024 •

edited

Loading

radius-functional-tests bot commented Dec 21, 2024 •

edited

Loading

radius-functional-tests bot commented Dec 21, 2024 •

edited

Loading