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

Use unique resource names in test Terraform Recipe #7108

Merged
merged 2 commits into from
Feb 5, 2024

Conversation

kachawla
Copy link
Contributor

@kachawla kachawla commented Feb 1, 2024

Description

Trying a fix for #7060. Resource name uniqueness is tied to resource group right now, potentially causing concurrency issues.

Type of change

  • This pull request fixes a bug in 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).

Attempting fix for #7060

@radius-functional-tests
Copy link

radius-functional-tests bot commented Feb 1, 2024

Radius functional test overview

🔍 Go to test action run

Name Value
Repository kachawla/radius
Commit ref 0a5c1e0
Unique ID a81acb7acd
Image tag pr-a81acb7acd
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/functional/shared/recipes/<name>:pr-a81acb7acd
  • 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-a81acb7acd
  • controller test image location: ghcr.io/radius-project/dev/controller:pr-a81acb7acd
  • ucp test image location: ghcr.io/radius-project/dev/ucpd:pr-a81acb7acd
  • 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 samples functional tests...
⌛ Starting shared functional tests...
⌛ Starting msgrp functional tests...
⌛ Starting kubernetes functional tests...
⌛ Starting datastoresrp functional tests...
✅ samples functional tests succeeded
✅ msgrp functional tests succeeded
✅ kubernetes functional tests succeeded
✅ daprrp functional tests succeeded
✅ ucp functional tests succeeded
✅ datastoresrp functional tests succeeded
❌ shared functional test failed. Please check the logs for more details

ytimocin
ytimocin previously approved these changes Feb 2, 2024
Copy link
Contributor

@ytimocin ytimocin left a comment

Choose a reason for hiding this comment

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

Are you going to add anything else to this PR @kachawla? So far LGTM. We can test it out.

storage_account_name = azurerm_storage_account.test_storage_account.name
}

resource "azurerm_storage_blob" "test_blob" {
name = "test-blob"
Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch. I think it might be related to this or test-container above. I don't think it is due to another long-running test deleting it but a functional test that happens to be running at the same time might be deleting one of these.

Copy link
Contributor Author

@kachawla kachawla Feb 4, 2024

Choose a reason for hiding this comment

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

I don't think it is due to another long-running test deleting it but a functional test that happens to be running at the same time might be deleting one of these.

Interesting. Are we sharing resource groups between functional tests and long running tests?

I think it might be related to this or test-container above.

The failure is due to test-container but I just decided to add uniqueness to all resource names.

Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting. Are we sharing resource groups between functional tests and long running tests?

No, we are not actually. They all belong to a resource group, right? Can't seem to find out why this is caused for now.

@ytimocin
Copy link
Contributor

ytimocin commented Feb 2, 2024

Just saw this error: "terraform destroy failure: exit status 1\n\nError: name (\"account-a_dH0Fo4N8E\") can only consist of lowercase letters and numbers, and must be between 3 and 24 characters long\n\n with module.default.azurerm_storage_account.test_storage_account,\n on .terraform/modules/default/main.tf line 15, in resource \"azurerm_storage_account\" \"test_storage_account\":\n 15: name = \"account-${random_id.unique_name.id}\"\n\n"

@kachawla
Copy link
Contributor Author

kachawla commented Feb 4, 2024

Are you going to add anything else to this PR @kachawla? So far LGTM. We can test it out.

No, just wanted to check that it runs successfully with the updated names, but looks like we need to make it compliant with naming restrictions, will look into it and update.

@radius-functional-tests
Copy link

radius-functional-tests bot commented Feb 4, 2024

Radius functional test overview

🔍 Go to test action run

Name Value
Repository kachawla/radius
Commit ref 508e442
Unique ID 690335d0d4
Image tag pr-690335d0d4
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/functional/shared/recipes/<name>:pr-690335d0d4
  • 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-690335d0d4
  • controller test image location: ghcr.io/radius-project/dev/controller:pr-690335d0d4
  • ucp test image location: ghcr.io/radius-project/dev/ucpd:pr-690335d0d4
  • 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 datastoresrp functional tests...
⌛ Starting ucp functional tests...
⌛ Starting daprrp functional tests...
⌛ Starting shared functional tests...
⌛ Starting msgrp functional tests...
⌛ Starting kubernetes functional tests...
⌛ Starting samples functional tests...
✅ msgrp functional tests succeeded
✅ kubernetes functional tests succeeded
✅ samples functional tests succeeded
✅ ucp functional tests succeeded
✅ datastoresrp functional tests succeeded
✅ daprrp functional tests succeeded
❌ shared functional test failed. Please check the logs for more details

Signed-off-by: karishma-chawla <74574173+karishma-chawla@users.noreply.github.com>
@radius-functional-tests
Copy link

radius-functional-tests bot commented Feb 4, 2024

Radius functional test overview

🔍 Go to test action run

Name Value
Repository kachawla/radius
Commit ref f818784
Unique ID f7a74579bd
Image tag pr-f7a74579bd
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/functional/shared/recipes/<name>:pr-f7a74579bd
  • 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-f7a74579bd
  • controller test image location: ghcr.io/radius-project/dev/controller:pr-f7a74579bd
  • ucp test image location: ghcr.io/radius-project/dev/ucpd:pr-f7a74579bd
  • 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 msgrp functional tests...
⌛ Starting kubernetes functional tests...
⌛ Starting daprrp functional tests...
⌛ Starting ucp functional tests...
⌛ Starting samples functional tests...
⌛ Starting shared 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
✅ shared functional tests succeeded

@kachawla kachawla marked this pull request as ready for review February 4, 2024 06:44
@kachawla kachawla requested review from a team as code owners February 4, 2024 06:44
@radius-functional-tests
Copy link

radius-functional-tests bot commented Feb 4, 2024

Radius functional test overview

🔍 Go to test action run

Name Value
Repository kachawla/radius
Commit ref 2fd8558
Unique ID bfd14554e1
Image tag pr-bfd14554e1
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/functional/shared/recipes/<name>:pr-bfd14554e1
  • 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-bfd14554e1
  • controller test image location: ghcr.io/radius-project/dev/controller:pr-bfd14554e1
  • ucp test image location: ghcr.io/radius-project/dev/ucpd:pr-bfd14554e1
  • 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 msgrp functional tests...
⌛ Starting daprrp functional tests...
⌛ Starting samples functional tests...
⌛ Starting datastoresrp functional tests...
⌛ Starting kubernetes functional tests...
⌛ Starting shared functional tests...
✅ msgrp functional tests succeeded
✅ samples functional tests succeeded
✅ kubernetes functional tests succeeded
✅ ucp functional tests succeeded
✅ daprrp functional tests succeeded
✅ datastoresrp functional tests succeeded
✅ shared functional tests succeeded

@kachawla
Copy link
Contributor Author

kachawla commented Feb 5, 2024

Are you going to add anything else to this PR @kachawla? So far LGTM. We can test it out.

No, just wanted to check that it runs successfully with the updated names, but looks like we need to make it compliant with naming restrictions, will look into it and update.

Updated

@kachawla kachawla merged commit 76f5785 into radius-project:main Feb 5, 2024
16 checks passed
@kachawla kachawla deleted the kachawla/tfrecipe-test branch February 8, 2024 01:33
willdavsmith pushed a commit to willdavsmith/radius that referenced this pull request Mar 4, 2024
# Description

Trying a fix for radius-project#7060.
Resource name uniqueness is tied to resource group right now,
potentially causing concurrency issues.

## Type of change

<!--

Please select **one** of the following options that describes your
change and delete the others. Clearly identifying the type of change you
are making will help us review your PR faster, and is used in authoring
release notes.

If you are making a bug fix or functionality change to Radius and do not
have an associated issue link please create one now.

-->

- This pull request fixes a bug in 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).

<!--

Please update the following to link the associated issue. This is
required for some kinds of changes (see above).

-->

Fixes: radius-project#7060

Signed-off-by: karishma-chawla <74574173+karishma-chawla@users.noreply.github.com>
Co-authored-by: karishma-chawla <74574173+karishma-chawla@users.noreply.github.com>
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.

3 participants