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

[Do Not Merge] Populate Terraform recipe deployed resource IDs in recipe output #6280

Closed

Conversation

vishwahiremat
Copy link
Contributor

@vishwahiremat vishwahiremat commented Sep 18, 2023

Description

Added changed to populate Terraform recipe deployed resource IDs in recipe output.

Type of change

  • This pull request fixes a bug in Radius and has an approved issue (issue link required).
  • This pull request adds or changes features of Radius and has an approved issue (issue link required).
  • This pull request is a minor refactor, code cleanup, test improvement, or other maintenance task and doesn't change the functionality of Radius (issue link optional).

Fixes: #issue_number

Auto-generated summary

🤖 Generated by Copilot at 2984d12

Summary

No summary available (Limit exceeded: required to process 51960 tokens, but only 50000 are allowed per call)

Walkthrough

No walkthrough available (Limit exceeded: required to process 51960 tokens, but only 50000 are allowed per call)

@github-actions
Copy link

github-actions bot commented Sep 18, 2023

Radius functional test overview

🔍 Go to test action run

Name Value
Repository radius-project/radius
Commit ref refs/pull/6280/merge
Unique ID 822fe7c85c
Image tag pr-822fe7c85c
Click here to see the list of tools in the current test run
  • gotestsum 1.10.0
  • KinD: v0.20.0
  • Dapr: 1.11.0
  • Azure KeyVault CSI driver: 1.4.2
  • Azure Workload identity webhook: 1.1.0
  • Bicep recipe location radiusdev.azurecr.io/test/functional/shared/recipes/<name>:pr-822fe7c85c
  • 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-822fe7c85c
  • ucp test image location: ghcr.io/radius-project/dev/ucpd:pr-822fe7c85c
  • deployment-engine test image location: radius.azurecr.io/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 msgrp functional tests...
⌛ Starting ucp functional tests...
⌛ Starting shared functional tests...
⌛ Starting daprrp functional tests...
✅ datastoresrp functional tests succeeded
✅ msgrp functional tests succeeded
✅ samples functional tests succeeded
✅ ucp functional tests succeeded
✅ daprrp functional tests succeeded
✅ shared functional tests succeeded

@github-actions
Copy link

github-actions bot commented Sep 18, 2023

Test Results

2 970 tests   - 3   2 961 ✔️  - 3   2m 21s ⏱️ -18s
   252 suites ±0          9 💤 ±0 
       1 files   ±0          0 ±0 

Results for commit b63816e. ± Comparison against base commit 6d306a1.

This pull request removes 6 and adds 3 tests. Note that renamed tests count towards both.
github.com/radius-project/radius/pkg/armrpc/frontend/controller ‑ TestValidateEtag_IfMatch/1b0ffbf0-c51e-42a5-8e3a-2f186595214c
github.com/radius-project/radius/pkg/armrpc/frontend/controller ‑ TestValidateEtag_IfMatch/1b0ffbf0-c51e-42a5-8e3a-2f186595214c#01
github.com/radius-project/radius/pkg/portableresources/processors ‑ Test_GetOutputResourcesFromRecipe
github.com/radius-project/radius/pkg/portableresources/processors ‑ Test_GetOutputResourcesFromRecipe_Invalid
github.com/radius-project/radius/pkg/portableresources/processors ‑ Test_Validator_SetAndValidate_OutputResources/recipe_invalid_id
github.com/radius-project/radius/pkg/ucp/store ‑ TestDecodeMap_WithResourceIDs/invalid
github.com/radius-project/radius/pkg/armrpc/frontend/controller ‑ TestValidateEtag_IfMatch/bb84f64a-0a61-46f2-9a99-ed6a45751df1
github.com/radius-project/radius/pkg/armrpc/frontend/controller ‑ TestValidateEtag_IfMatch/bb84f64a-0a61-46f2-9a99-ed6a45751df1#01
github.com/radius-project/radius/pkg/ucp/store ‑ TestDecodeMap_WithResourceIDs/terraform_id

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Sep 18, 2023

Radius functional test overview

🔍 Go to test action run

Name Value
Repository radius-project/radius
Commit ref refs/pull/6280/merge
Unique ID 94cd2f6e3a
Image tag pr-94cd2f6e3a
Click here to see the list of tools in the current test run
  • gotestsum 1.10.0
  • KinD: v0.20.0
  • Dapr: 1.11.0
  • Azure KeyVault CSI driver: 1.4.2
  • Azure Workload identity webhook: 1.1.0
  • Bicep recipe location radiusdev.azurecr.io/test/functional/shared/recipes/<name>:pr-94cd2f6e3a
  • 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-94cd2f6e3a
  • ucp test image location: ghcr.io/radius-project/dev/ucpd:pr-94cd2f6e3a
  • deployment-engine test image location: radius.azurecr.io/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 datastoresrp functional tests...
⌛ Starting ucp functional tests...
⌛ Starting shared functional tests...
⌛ Starting samples functional tests...
⌛ Starting daprrp functional tests...
⌛ Starting msgrp functional tests...
✅ datastoresrp functional tests succeeded
✅ ucp functional tests succeeded
✅ samples functional tests succeeded
✅ daprrp functional tests succeeded
✅ msgrp functional tests succeeded
❌ shared functional test failed. Please check the logs for more details

// Values represents the key/value pairs of properties of the deployed resource.
Values map[string]any
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Option 2: Adding a new Property UCPResources to represent the resources deployed through UCP and Resources property represents the resources deployed otherwise(e.g terraform).
And use this information in the Processor to convert resource ids into OutputResources.

userResources, err := GetOutputResourcesFromResourcesField(*v.resourcesField)

type RecipeOutput struct {
	UCPResources []string
        Resources []string
	Secrets map[string]any
	Values map[string]any
}

// Resources represents the list of output resources deployed recipe.
//Resources []string

OutputResources []rpv1.OutputResource
Copy link
Contributor Author

@vishwahiremat vishwahiremat Sep 19, 2023

Choose a reason for hiding this comment

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

Adding OutputResources to recipeOutput response. Today we are returning the resource id strings as part of the recipe output response and as part of the processor converting into OutputResource objects.

But the resource ids returned by terraform may not in the UCP resource id format i.e it may not contain the scope segments and type segments. So we need use differnet ways of creating OutputResource for terraform resource ids.

So added the changes to create getOutputResource from resource ids in respective drivers.

@github-actions
Copy link

github-actions bot commented Sep 19, 2023

Radius functional test overview

🔍 Go to test action run

Name Value
Repository radius-project/radius
Commit ref refs/pull/6280/merge
Unique ID 7fa3223848
Image tag pr-7fa3223848
Click here to see the list of tools in the current test run
  • gotestsum 1.10.0
  • KinD: v0.20.0
  • Dapr: 1.11.0
  • Azure KeyVault CSI driver: 1.4.2
  • Azure Workload identity webhook: 1.1.0
  • Bicep recipe location radiusdev.azurecr.io/test/functional/shared/recipes/<name>:pr-7fa3223848
  • 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-7fa3223848
  • ucp test image location: ghcr.io/radius-project/dev/ucpd:pr-7fa3223848
  • deployment-engine test image location: radius.azurecr.io/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 datastoresrp functional tests...
⌛ Starting daprrp functional tests...
⌛ Starting samples functional tests...
⌛ Starting shared functional tests...
⌛ Starting ucp functional tests...
✅ datastoresrp functional tests succeeded
✅ ucp functional tests succeeded
✅ samples functional tests succeeded
✅ msgrp functional tests succeeded
✅ daprrp functional tests succeeded
❌ shared functional test failed. Please check the logs for more details

@github-actions
Copy link

66.3

For the detailed report, please go to Checks tab, click Build and Test, and then download unit_test_coverage artifact at the bottom of build page.

  • Your PR branch coverage: 66.3 %
  • main branch coverage: 66.2 %
  • diff coverage: .1 %

The coverage result does not include the functional test coverage.

@@ -46,6 +46,8 @@ type ID struct {
extensionSegments []TypeSegment
}

// Handle Terraform IDs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Option1 : Adding a new property "isUCPID" to differentiate between resources deployed thorugh UCP and non-UCP.

type ID struct {
	id                string
	isNonUCPID        bool
	scopeSegments     []ScopeSegment
	typeSegments      []TypeSegment
	extensionSegments []TypeSegment
}

And add if conditions on the struct methods to handle it for Non-UCP type resource ids.

Option 2:

type ID interface{

}
type UCPID struct{
	id                string
	scopeSegments     []ScopeSegment
	typeSegments      []TypeSegment
	extensionSegments []TypeSegment
}

type NonUCPID struct{
	id                string
}

Comment on lines +149 to +150
deployedResources := d.getDeployedOutputResources(tfState.Values.RootModule)
resources = append(resources, deployedResources...)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Getting resource ids parsing the state file.

@github-actions
Copy link

github-actions bot commented Sep 19, 2023

Radius functional test overview

🔍 Go to test action run

Name Value
Repository radius-project/radius
Commit ref refs/pull/6280/merge
Unique ID 273b8622a3
Image tag pr-273b8622a3
Click here to see the list of tools in the current test run
  • gotestsum 1.10.0
  • KinD: v0.20.0
  • Dapr: 1.11.0
  • Azure KeyVault CSI driver: 1.4.2
  • Azure Workload identity webhook: 1.1.0
  • Bicep recipe location radiusdev.azurecr.io/test/functional/shared/recipes/<name>:pr-273b8622a3
  • 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-273b8622a3
  • ucp test image location: ghcr.io/radius-project/dev/ucpd:pr-273b8622a3
  • deployment-engine test image location: radius.azurecr.io/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 msgrp functional tests...
⌛ Starting ucp functional tests...
⌛ Starting daprrp functional tests...
⌛ Starting samples functional tests...
⌛ Starting shared functional tests...
⌛ Starting datastoresrp functional tests...
✅ datastoresrp functional tests succeeded
✅ ucp functional tests succeeded
✅ msgrp functional tests succeeded
✅ samples functional tests succeeded
✅ daprrp functional tests succeeded
✅ shared functional tests succeeded

@github-actions
Copy link

github-actions bot commented Sep 19, 2023

Radius functional test overview

🔍 Go to test action run

Name Value
Repository radius-project/radius
Commit ref refs/pull/6280/merge
Unique ID 52a3a9af1d
Image tag pr-52a3a9af1d
Click here to see the list of tools in the current test run
  • gotestsum 1.10.0
  • KinD: v0.20.0
  • Dapr: 1.11.0
  • Azure KeyVault CSI driver: 1.4.2
  • Azure Workload identity webhook: 1.1.0
  • Bicep recipe location radiusdev.azurecr.io/test/functional/shared/recipes/<name>:pr-52a3a9af1d
  • 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-52a3a9af1d
  • ucp test image location: ghcr.io/radius-project/dev/ucpd:pr-52a3a9af1d
  • deployment-engine test image location: radius.azurecr.io/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 datastoresrp functional tests...
⌛ Starting daprrp functional tests...
⌛ Starting msgrp functional tests...
⌛ Starting shared functional tests...
⌛ Starting samples functional tests...
⌛ Starting ucp functional tests...
✅ datastoresrp functional tests succeeded
✅ msgrp functional tests succeeded
✅ ucp functional tests succeeded
✅ daprrp functional tests succeeded
✅ samples functional tests succeeded
✅ shared functional tests succeeded

@github-actions
Copy link

github-actions bot commented Sep 19, 2023

Radius functional test overview

🔍 Go to test action run

Name Value
Repository radius-project/radius
Commit ref refs/pull/6280/merge
Unique ID fdaa3ae994
Image tag pr-fdaa3ae994
Click here to see the list of tools in the current test run
  • gotestsum 1.10.0
  • KinD: v0.20.0
  • Dapr: 1.11.0
  • Azure KeyVault CSI driver: 1.4.2
  • Azure Workload identity webhook: 1.1.0
  • Bicep recipe location radiusdev.azurecr.io/test/functional/shared/recipes/<name>:pr-fdaa3ae994
  • 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-fdaa3ae994
  • ucp test image location: ghcr.io/radius-project/dev/ucpd:pr-fdaa3ae994
  • deployment-engine test image location: radius.azurecr.io/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 datastoresrp functional tests...
⌛ Starting msgrp functional tests...
⌛ Starting ucp functional tests...
⌛ Starting daprrp functional tests...
⌛ Starting shared functional tests...
⌛ Starting samples functional tests...
✅ datastoresrp functional tests succeeded
✅ ucp functional tests succeeded
✅ msgrp functional tests succeeded
✅ daprrp functional tests succeeded
✅ samples functional tests succeeded
✅ shared functional tests succeeded

@github-actions
Copy link

66.3

For the detailed report, please go to Checks tab, click Build and Test, and then download unit_test_coverage artifact at the bottom of build page.

  • Your PR branch coverage: 66.3 %
  • main branch coverage: 66.3 %
  • diff coverage: 0 %

The coverage result does not include the functional test coverage.

@kachawla
Copy link
Contributor

kachawla commented Oct 5, 2023

@vishwahiremat should this PR be closed since we have moved to the new approach implemented in #6430?

@vishwahiremat
Copy link
Contributor Author

@vishwahiremat should this PR be closed since we have moved to the new approach implemented in #6430?

Yes, closing this PR

@youngbupark youngbupark deleted the vishwahiremat/moving_get_output_resources_to_driver branch February 27, 2024 01:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants