Adding test workflow #52

vishwahiremat · 2023-11-22T20:02:07Z

Added a new workflow test.yaml:

Create a k3d registry and upload local-dev recipes to it.
Create a test bicep file that registers all the recipes to the environment and deploys all the recipes and test against the magpie container.

Addresses radius-project/radius#5775

Signed-off-by: Vishwanath Hiremath <vhiremath@microsoft.com>

ytimocin · 2023-11-29T22:47:51Z

.github/workflows/test.yaml

+  APP_NAME: local-dev-recipe-app
+  APP_NAMESPACE: local-dev-recipe-app
+jobs:
+  test:


If this is not supposed to run on forks please add the if check like this: https://github.com/radius-project/radius/blob/main/.github/workflows/assets.yaml#L29.

Should this check be added?

I was thinking we should run this on fork as well, wondering if there is reason why we dont want to run on forks?

ytimocin · 2023-11-29T22:48:25Z

.github/workflows/test.yaml

+          name: local_dev_recipe_test_container_logs-pod-logs
+          path: recipe-tests/pod-logs/local_dev_recipe_test_container_logs
+          retention-days: 30
+          if-no-files-found: error


nit: new line and reformat file

may need a reformat and a new line here

ytimocin · 2023-11-29T22:49:47Z

local-dev/rabbitmqqueues.bicep


 @description('Memory limit for the rabbitmq deployment')
-param memoryLimit string = '1024Mi'
+param memoryLimit string = '2048Mi'


Why did we need to increase the limits? Increasing the memory request should be fine in my opinion. Do we need to add any limits and/or requests for the CPU?

I was testing something while testing the recipe, there is no need to increase the limits. I will revert these changes back

Were they reverted?

ytimocin · 2023-11-29T22:50:34Z

local-dev/rabbitmqqueues.bicep

-    queue: queue
+    queue: 'queue'


Was this pointing to an object before and now why is it a string?

No it was pointing to string as well. I corrected it here to the right queue name

AaronCrawfis · 2023-12-01T22:40:40Z

local-dev/rabbitmqqueues.bicep

@@ -17,17 +17,14 @@ limitations under the License.
 @description('Information about what resource is calling this Recipe. Generated by Radius. For more information visit https://docs.radapp.dev/operations/custom-recipes/')
 param context object

-@description('Name of the queue. Defaults to the name of the Radius RabbitMQ resource.')


Why is this being removed? Usually we default these to be the name of the resource but allow them to be overridden

local-dev/secretstores.bicep

AaronCrawfis · 2023-12-01T22:42:31Z

local-dev/sqldatabases.bicep

@@ -75,7 +65,7 @@ resource sql 'apps/Deployment@v1' = {
          {
            // This container is the running sql instance.
            name: 'sql'
-            image: 'mcr.microsoft.com/azure-sql-edge:${tag}'
+            image: 'mcr.microsoft.com/mssql/server:2022-latest'


Why are we changing the container image as part of this PR? Did azure sql edge not work as expected?

AaronCrawfis · 2023-12-01T22:43:23Z

tests/test-local-dev-recipes.bicep

+        mongoazure: {
+          templateKind: 'bicep'
+          plainHTTP: true
+          templatePath: 'reciperegistry:5000/recipes/local-dev/mongodatabases:latest'


Could we make reciperegistry:5000 a parameter with a default value so it could be overridden if needed?

And could we make latest a parameter as well?

AaronCrawfis · 2023-12-01T22:44:18Z

tests/test-local-dev-recipes.bicep

+    container: {
+      image: magpieimage
+      env: {
+        CONNECTION_SQL_CONNECTIONSTRING: db.connectionString()


Why is this being set manually?

tests/test-local-dev-recipes.bicep

AaronCrawfis · 2023-12-01T22:46:31Z

.github/workflows/test.yaml

+        id: wait-for-pods
+        run: |
+          label="radapp.io/application=${APP_NAME}"
+          kubectl wait --for=condition=Ready pod -l $label -n ${APP_NAMESPACE} --timeout=5m


Is this where we wait for the tests to complete? Does the magpie container not move to ready until all its tests are complete?

you are right magpie container does move to ready until all tests are complete, I just added one more step to check all pods are running fine after readiness check passed.

ytimocin · 2023-12-01T22:43:35Z

.github/scripts/install-radius.sh

+RAD_CLI_URL=https://raw.githubusercontent.com/radius-project/radius/main/deploy/install.sh
+RAD_CLI_EDGE_URL=ghcr.io/radius-project/rad/linux-amd64:latest
+
+if [[ $VERSION == "edge" ]]; then


Do we still have edge version accessible?

I copied this install script from samples https://github.com/radius-project/samples/blob/d611db9ee7687bf643b09570f979a0eeb99f4e25/.github/scripts/install-radius.sh#L23-L24

ytimocin · 2023-12-01T22:44:11Z

.github/workflows/test.yaml

+  APP_NAME: local-dev-recipe-app
+  APP_NAMESPACE: local-dev-recipe-app
+jobs:
+  test:


Should this check be added?

ytimocin · 2023-12-01T22:44:55Z

.github/workflows/test.yaml

+      - name: Install Dapr
+        run: |
+          helm repo add dapr https://dapr.github.io/helm-charts/
+          helm install dapr dapr/dapr --version=1.6 --namespace dapr-system --create-namespace --wait


We should try to use more recent Dapr if that doesn't break anything.

ytimocin · 2023-12-01T22:46:36Z

.github/workflows/test.yaml

+        id: wait-for-pods
+        run: |
+          label="radapp.io/application=${APP_NAME}"
+          kubectl wait --for=condition=Ready pod -l $label -n ${APP_NAMESPACE} --timeout=5m


@willdavsmith found another command that basically waits for the deployment to finish. You may want to check that one. It should in one of the merged PRs in the samples repo.

ytimocin · 2023-12-01T22:47:01Z

.github/workflows/test.yaml

+          name: local_dev_recipe_test_container_logs-pod-logs
+          path: recipe-tests/pod-logs/local_dev_recipe_test_container_logs
+          retention-days: 30
+          if-no-files-found: error


may need a reformat and a new line here

ytimocin · 2023-12-01T22:47:14Z

local-dev/rabbitmqqueues.bicep


 @description('Memory limit for the rabbitmq deployment')
-param memoryLimit string = '1024Mi'
+param memoryLimit string = '2048Mi'


Were they reverted?

ytimocin · 2023-12-01T22:47:39Z

local-dev/rabbitmqqueues.bicep

@@ -17,17 +17,14 @@ limitations under the License.
 @description('Information about what resource is calling this Recipe. Generated by Radius. For more information visit https://docs.radapp.dev/operations/custom-recipes/')
 param context object

-@description('Name of the queue. Defaults to the name of the Radius RabbitMQ resource.')
-param queue string = context.resource.name


Why did we remove this?

ytimocin · 2023-12-01T22:48:09Z

local-dev/sqldatabases.bicep

@@ -17,9 +17,6 @@ limitations under the License.
 @description('Information about what resource is calling this Recipe. Generated by Radius. For more information visit https://docs.radapp.dev/operations/custom-recipes/')
 param context object

-@description('Name of the SQL database. Defaults to the name of the Radius SQL resource.')
-param database string = context.resource.name


Any reason why this was also removed?

ytimocin · 2023-12-01T22:49:41Z

local-dev/sqldatabases.bicep

@@ -75,7 +65,7 @@ resource sql 'apps/Deployment@v1' = {
          {
            // This container is the running sql instance.
            name: 'sql'
-            image: 'mcr.microsoft.com/azure-sql-edge:${tag}'
+            image: 'mcr.microsoft.com/mssql/server:2022-latest'


2022-latest is not an image that will change, right? Not like latest?

Signed-off-by: Vishwanath Hiremath <vhiremath@microsoft.com>

ytimocin

LGTM but I think @AaronCrawfis should also take a look.

local-dev/pubsubbrokers.bicep

AaronCrawfis · 2023-12-07T18:44:36Z

local-dev/secretstores.bicep

@@ -47,5 +47,6 @@ output result object = {
    type: daprType
    version: daprVersion
    metadata: daprComponent.spec.metadata
+    componentName: daprComponent.metadata.name


Is this required? And if so should we add it to all the other dapr recipes?

its not required, removed it

AaronCrawfis · 2023-12-07T18:44:54Z

tests/test-local-dev-recipes.bicep

+param magpieimage string
+
+param registry string 
+
+param version string


Could we add descriptions to these?

added descriptions

AaronCrawfis

Couple small things but overall looks great!

Signed-off-by: Vishwanath Hiremath <vhiremath@microsoft.com>

SantaHub · 2024-05-23T20:39:31Z

@vishwahiremat hi i was tryna do a local deployment but having trouble with the local registry, would you help me figure what exactly its complaining about?
Few things i made sure :

The recipe is available when you do rad recipe list
Recipe is published successfully and use the --plain-http param when i published it
the deploy is tryna fetch it from the localhost, the right port and using the http.

What i need help with, its tryna fetch the manifest, how do i make sure that available ?

here is the full error if it helps

Error: {
  "code": "DeploymentFailed",
  "message": "At least one resource deployment operation failed. Please see the details for the specific operation that failed.",
  "target": "/planes/radius/local/resourceGroups/default/providers/Microsoft.Resources/deployments/rad-deploy-ee7764d4-a055-4ea7-afa5-ab81c1a5d2ff",
  "details": [
    {
      "code": "ResourceDeploymentFailure",
      "message": "Failed",
      "target": "/planes/radius/local/resourceGroups/default/providers/Applications.Datastores/redisCaches/db",
      "details": [
        {
          "code": "RecipeDownloadFailed",
          "message": "code RecipeLanguageFailure: err failed to fetch repository from the path \"localhost:51351/recipes/local-dev/rediscache:0.0.1\": Head \"http://localhost:51351/v2/recipes/local-dev/rediscache/manifests/0.0.1\": dial tcp 127.0.0.1:51351: connect: connection refused",
          "details": [
            {
              "code": "RecipeLanguageFailure",
              "message": "failed to fetch repository from the path \"localhost:51351/recipes/local-dev/rediscache:0.0.1\": Head \"http://localhost:51351/v2/recipes/local-dev/rediscache/manifests/0.0.1\": dial tcp 127.0.0.1:51351: connect: connection refused"
            }
          ]
        }
      ]
    }
  ]
}

vishwahiremat · 2024-05-24T18:28:54Z

@vishwahiremat hi i was tryna do a local deployment but having trouble with the local registry, would you help me figure what exactly its complaining about? Few things i made sure :

The recipe is available when you do rad recipe list
Recipe is published successfully and use the --plain-http param when i published it
the deploy is tryna fetch it from the localhost, the right port and using the http.

What i need help with, its tryna fetch the manifest, how do i make sure that available ?

here is the full error if it helps

Error: {
  "code": "DeploymentFailed",
  "message": "At least one resource deployment operation failed. Please see the details for the specific operation that failed.",
  "target": "/planes/radius/local/resourceGroups/default/providers/Microsoft.Resources/deployments/rad-deploy-ee7764d4-a055-4ea7-afa5-ab81c1a5d2ff",
  "details": [
    {
      "code": "ResourceDeploymentFailure",
      "message": "Failed",
      "target": "/planes/radius/local/resourceGroups/default/providers/Applications.Datastores/redisCaches/db",
      "details": [
        {
          "code": "RecipeDownloadFailed",
          "message": "code RecipeLanguageFailure: err failed to fetch repository from the path \"localhost:51351/recipes/local-dev/rediscache:0.0.1\": Head \"http://localhost:51351/v2/recipes/local-dev/rediscache/manifests/0.0.1\": dial tcp 127.0.0.1:51351: connect: connection refused",
          "details": [
            {
              "code": "RecipeLanguageFailure",
              "message": "failed to fetch repository from the path \"localhost:51351/recipes/local-dev/rediscache:0.0.1\": Head \"http://localhost:51351/v2/recipes/local-dev/rediscache/manifests/0.0.1\": dial tcp 127.0.0.1:51351: connect: connection refused"
            }
          ]
        }
      ]
    }
  ]
}

Hi @SantaHub ,
Thanks for sharing the details, From the information it looks like cluster is not able to connect to the registry. I would like to know how the cluster and local registries are crated. can you provide the command used to create the cluster and registry and the command you used to register the recipe.

rynowak · 2024-05-24T18:32:56Z

From the error message, it looks like the issue is the hostname localhost:51351. Radius is running inside your cluster, so the address of the container registry won't be the same as it was when you published the recipe (from your computer, not inside a kubernetes cluster).

vishwahiremat added 30 commits November 22, 2023 11:57

adding tests for recipes

346e308

Signed-off-by: Vishwanath Hiremath <vhiremath@microsoft.com>

testing to run test.yaml

38b22ec

Signed-off-by: Vishwanath Hiremath <vhiremath@microsoft.com>

fix syntax

9558836

Signed-off-by: Vishwanath Hiremath <vhiremath@microsoft.com>

fixing test.yaml

7760c05

Signed-off-by: Vishwanath Hiremath <vhiremath@microsoft.com>

fixing test.yaml

56056fa

Signed-off-by: Vishwanath Hiremath <vhiremath@microsoft.com>

setting output variables

836694c

Signed-off-by: Vishwanath Hiremath <vhiremath@microsoft.com>

add install radius script

4e1ed6e

Signed-off-by: Vishwanath Hiremath <vhiremath@microsoft.com>

fix test.yaml

4439cf8

Signed-off-by: Vishwanath Hiremath <vhiremath@microsoft.com>

rename file

e3e562a

Signed-off-by: Vishwanath Hiremath <vhiremath@microsoft.com>

test

f89c26f

Signed-off-by: Vishwanath Hiremath <vhiremath@microsoft.com>

test

244ddf9

Signed-off-by: Vishwanath Hiremath <vhiremath@microsoft.com>

test

5579c8e

Signed-off-by: Vishwanath Hiremath <vhiremath@microsoft.com>

test

dd31fcf

Signed-off-by: Vishwanath Hiremath <vhiremath@microsoft.com>

test

50dd80c

Signed-off-by: Vishwanath Hiremath <vhiremath@microsoft.com>

install edge version

c10a5a3

Signed-off-by: Vishwanath Hiremath <vhiremath@microsoft.com>

test

0a4b054

Signed-off-by: Vishwanath Hiremath <vhiremath@microsoft.com>

adding checkout step

ba568ed

Signed-off-by: Vishwanath Hiremath <vhiremath@microsoft.com>

adding execute permissions

20fd892

Signed-off-by: Vishwanath Hiremath <vhiremath@microsoft.com>

path fix

119de17

Signed-off-by: Vishwanath Hiremath <vhiremath@microsoft.com>

fixing k3d create command

680961c

Signed-off-by: Vishwanath Hiremath <vhiremath@microsoft.com>

adding mongo test

3961fe5

Signed-off-by: Vishwanath Hiremath <vhiremath@microsoft.com>

fix syntax error

b8d6ea4

Signed-off-by: Vishwanath Hiremath <vhiremath@microsoft.com>

fix syntax error

f42345f

Signed-off-by: Vishwanath Hiremath <vhiremath@microsoft.com>

remove provider

78ee846

Signed-off-by: Vishwanath Hiremath <vhiremath@microsoft.com>

testing

16c011b

Signed-off-by: Vishwanath Hiremath <vhiremath@microsoft.com>

adding all tests

ee4a3be

Signed-off-by: Vishwanath Hiremath <vhiremath@microsoft.com>

adding matrix

e589dc5

Signed-off-by: Vishwanath Hiremath <vhiremath@microsoft.com>

testing

ed2772f

Signed-off-by: Vishwanath Hiremath <vhiremath@microsoft.com>

test

1e6ca22

Signed-off-by: Vishwanath Hiremath <vhiremath@microsoft.com>

testing

e31b78f

Signed-off-by: Vishwanath Hiremath <vhiremath@microsoft.com>

vishwahiremat added 6 commits November 28, 2023 22:47

testing

a1f7c78

Signed-off-by: Vishwanath Hiremath <vhiremath@microsoft.com>

testing

7268235

Signed-off-by: Vishwanath Hiremath <vhiremath@microsoft.com>

remove build magpie

eef9b3b

Signed-off-by: Vishwanath Hiremath <vhiremath@microsoft.com>

have single app to deploy all recipes

2558b26

Signed-off-by: Vishwanath Hiremath <vhiremath@microsoft.com>

fix syntax error

b098527

Signed-off-by: Vishwanath Hiremath <vhiremath@microsoft.com>

remove RUN_TEST

d3ef63e

Signed-off-by: Vishwanath Hiremath <vhiremath@microsoft.com>

vishwahiremat requested review from rynowak, ytimocin and kachawla November 29, 2023 22:20

vishwahiremat changed the title ~~Adding workflow and tests~~ Adding test workflow Nov 29, 2023

ytimocin reviewed Nov 29, 2023

View reviewed changes

AaronCrawfis reviewed Dec 1, 2023

View reviewed changes

ytimocin reviewed Dec 1, 2023

View reviewed changes

vishwahiremat added 6 commits December 4, 2023 15:08

addressing comments

8c44a62

Signed-off-by: Vishwanath Hiremath <vhiremath@microsoft.com>

addressing review comments

b39260e

Signed-off-by: Vishwanath Hiremath <vhiremath@microsoft.com>

remove sql test

91d213d

Signed-off-by: Vishwanath Hiremath <vhiremath@microsoft.com>

fixing sql test

3a7f527

Signed-off-by: Vishwanath Hiremath <vhiremath@microsoft.com>

updating wait for pods

607dd57

Signed-off-by: Vishwanath Hiremath <vhiremath@microsoft.com>

upgrading dapr version

9d3158a

Signed-off-by: Vishwanath Hiremath <vhiremath@microsoft.com>

ytimocin approved these changes Dec 6, 2023

View reviewed changes

AaronCrawfis reviewed Dec 7, 2023

View reviewed changes

local-dev/pubsubbrokers.bicep Outdated Show resolved Hide resolved

AaronCrawfis reviewed Dec 7, 2023

View reviewed changes

AaronCrawfis approved these changes Dec 7, 2023

View reviewed changes

adressing comments

89e9258

Signed-off-by: Vishwanath Hiremath <vhiremath@microsoft.com>

vishwahiremat merged commit 5cd81f3 into radius-project:main Dec 7, 2023
2 checks passed

AaronCrawfis mentioned this pull request Jan 2, 2024

Add dev recipes for all Links radius-project/radius#5775

Closed

Adding test workflow #52

Adding test workflow #52

Conversation

vishwahiremat commented Nov 22, 2023 • edited by AaronCrawfis Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ytimocin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AaronCrawfis left a comment

Choose a reason for hiding this comment

SantaHub commented May 23, 2024 • edited Loading

vishwahiremat commented May 24, 2024 • edited Loading

rynowak commented May 24, 2024

vishwahiremat commented Nov 22, 2023 •

edited by AaronCrawfis

Loading

SantaHub commented May 23, 2024 •

edited

Loading

vishwahiremat commented May 24, 2024 •

edited

Loading