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

Changes to fix dependency issues and app insights TF issues #3581

Merged
merged 11 commits into from
Jul 20, 2023

Conversation

marrobi
Copy link
Member

@marrobi marrobi commented Jun 16, 2023

Resolves #3569 #3574 #3433 #3616

What is being addressed

Add network dependencies into base workspace and switch to AzAPI for AMLPLS.

Before fixes - deployed 20 workspaces, 8 failed. After, deployed 20, one failures (clashing storage account name).

@marrobi marrobi requested a review from tamirkamara June 16, 2023 14:55
@github-actions
Copy link

github-actions bot commented Jun 16, 2023

Unit Test Results

0 tests   0 ✔️  0s ⏱️
0 suites  0 💤
0 files    0

Results for commit ac60cec.

♻️ This comment has been updated with latest results.

@marrobi
Copy link
Member Author

marrobi commented Jun 19, 2023

/test

@github-actions
Copy link

🤖 pr-bot 🤖

🏃 Running tests: https://github.com/microsoft/AzureTRE/actions/runs/5312884945 (with refid 6b5f52d7)

(in response to this comment from @marrobi)

@marrobi
Copy link
Member Author

marrobi commented Jun 19, 2023

/help

@github-actions
Copy link

🤖 pr-bot 🤖

Hello!

You can use the following commands:
    /test - build, deploy and run smoke tests on a PR
    /test-extended - build, deploy and run smoke & extended tests on a PR
    /test-extended-aad - build, deploy and run smoke & extended AAD tests on a PR
    /test-shared-services - test the deployment of shared services on a PR build
    /test-force-approve - force approval of the PR tests (i.e. skip the deployment checks)
    /test-destroy-env - delete the validation environment for a PR (e.g. to enable testing a deployment from a clean start after previous tests)
    /help - show this help

(in response to this comment from @marrobi)

@marrobi
Copy link
Member Author

marrobi commented Jun 19, 2023

/test-destroy-env

@github-actions
Copy link

Destroying PR test environment (RG: rg-tre6b5f52d7)... (run: https://github.com/microsoft/AzureTRE/actions/runs/5313475554)

1 similar comment
@github-actions
Copy link

Destroying PR test environment (RG: rg-tre6b5f52d7)... (run: https://github.com/microsoft/AzureTRE/actions/runs/5313475554)

@github-actions
Copy link

PR test environment destroy complete (RG: rg-tre6b5f52d7)

@marrobi
Copy link
Member Author

marrobi commented Jun 19, 2023

/test

@github-actions
Copy link

🤖 pr-bot 🤖

🏃 Running tests: https://github.com/microsoft/AzureTRE/actions/runs/5315115980 (with refid 6b5f52d7)

(in response to this comment from @marrobi)

@marrobi
Copy link
Member Author

marrobi commented Jun 20, 2023

/test-destroy-env

@github-actions
Copy link

Destroying PR test environment (RG: rg-tre6b5f52d7)... (run: https://github.com/microsoft/AzureTRE/actions/runs/5320887081)

3 similar comments
@github-actions
Copy link

Destroying PR test environment (RG: rg-tre6b5f52d7)... (run: https://github.com/microsoft/AzureTRE/actions/runs/5320887081)

@github-actions
Copy link

Destroying PR test environment (RG: rg-tre6b5f52d7)... (run: https://github.com/microsoft/AzureTRE/actions/runs/5320887081)

@github-actions
Copy link

Destroying PR test environment (RG: rg-tre6b5f52d7)... (run: https://github.com/microsoft/AzureTRE/actions/runs/5320887081)

@marrobi
Copy link
Member Author

marrobi commented Jun 21, 2023

/test

@github-actions
Copy link

🤖 pr-bot 🤖

🏃 Running tests: https://github.com/microsoft/AzureTRE/actions/runs/5333105180 (with refid 6b5f52d7)

(in response to this comment from @marrobi)

CHANGELOG.md Outdated Show resolved Hide resolved
@marrobi marrobi linked an issue Jun 22, 2023 that may be closed by this pull request
@marrobi marrobi added the bug Something isn't working label Jun 27, 2023
@SvenAelterman
Copy link
Collaborator

@marrobi Customer had workspace creation failure for the Airlock Import Review workspace which is remiscent of the failures when deploying some of their basic workspaces. Will updates to that workspace template need to be made too?

Any idea when this PR can be reviewed?

@marrobi
Copy link
Member Author

marrobi commented Jul 13, 2023

@tamirkamara @sachinkundu any idea when this can be reviewed? Thanks.

Copy link
Collaborator

@tamirkamara tamirkamara left a comment

Choose a reason for hiding this comment

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

I think we should focus on the Workspace and leave the core as it is for now.

@SvenAelterman
Copy link
Collaborator

I think we should focus on the Workspace and leave the core as it is for now.

I don't agree. In East US, we have seen failures during core TRE deployment with 3 of 4 recent deployments, all due to PE failures. While the solution is easy once you know (re-run, determine what TF fails against due to existing resources that weren't captured in TF state, delete those, re-run again), it's not a great experience.

@tamirkamara
Copy link
Collaborator

I think we should focus on the Workspace and leave the core as it is for now.

I don't agree. In East US, we have seen failures during core TRE deployment with 3 of 4 recent deployments, all due to PE failures. While the solution is easy once you know (re-run, determine what TF fails against due to existing resources that weren't captured in TF state, delete those, re-run again), it's not a great experience.

My comment is first about mixing things that don't have to be together. However, it's correct that I would not like to make the core fix right now - we have limited resources and core changes are much more delicate and I think we should prioritize.

…r.tf

Co-authored-by: Tamir Kamara <26870601+tamirkamara@users.noreply.github.com>
@marrobi
Copy link
Member Author

marrobi commented Jul 19, 2023

I think we should focus on the Workspace and leave the core as it is for now.

I don't agree. In East US, we have seen failures during core TRE deployment with 3 of 4 recent deployments, all due to PE failures. While the solution is easy once you know (re-run, determine what TF fails against due to existing resources that weren't captured in TF state, delete those, re-run again), it's not a great experience.

My comment is first about mixing things that don't have to be together. However, it's correct that I would not like to make the core fix right now - we have limited resources and core changes are much more delicate and I think we should prioritize.

@tamirkamara should I split this into 3 separate PRs? One for the workspaces, one for core and one for the app insights AzAPI? I am on leave next week, but can make time to at least get the private endpoint ones done providing we are ok to get them merged.

@marrobi
Copy link
Member Author

marrobi commented Jul 19, 2023

/test-extended

@github-actions
Copy link

🤖 pr-bot 🤖

🏃 Running extended tests: https://github.com/microsoft/AzureTRE/actions/runs/5600340721 (with refid 6b5f52d7)

(in response to this comment from @marrobi)

@marrobi
Copy link
Member Author

marrobi commented Jul 20, 2023

@github-actions
Copy link

🤖 pr-bot 🤖

✅ Marking tests as complete (for commit 836b7b1)

(in response to this comment from @marrobi)

@marrobi
Copy link
Member Author

marrobi commented Jul 20, 2023

/test-force-approve

@github-actions
Copy link

🤖 pr-bot 🤖

✅ Marking tests as complete (for commit 5c9be05)

(in response to this comment from @marrobi)

@marrobi marrobi enabled auto-merge (squash) July 20, 2023 13:48
@marrobi
Copy link
Member Author

marrobi commented Jul 20, 2023

/test-force-approve

@github-actions
Copy link

🤖 pr-bot 🤖

✅ Marking tests as complete (for commit ac60cec)

(in response to this comment from @marrobi)

@marrobi marrobi merged commit 6ffc341 into microsoft:main Jul 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
3 participants