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

Add Airlock Manager workspace #2505

Merged
merged 41 commits into from
Sep 15, 2022

Conversation

tanya-borisova
Copy link
Contributor

@tanya-borisova tanya-borisova commented Aug 25, 2022

Resolves #2498

What is being addressed

Add a workspace for Airlock Manager that has access to in-review storage account.
Based on work @guybartal did previously in a private repo

How is this addressed

Base workspace terraform files are downloaded onto the Docker container as a .zip archive. They are then unpacked, and a patch file is applied with the only difference required for the airlock manager workspace.

@tanya-borisova tanya-borisova linked an issue Aug 25, 2022 that may be closed by this pull request
1 task
@github-actions
Copy link

github-actions bot commented Aug 25, 2022

Unit Test Results

0 tests   - 17   0 ✔️  - 17   0s ⏱️ -12s
0 suites  -   1   0 💤 ±  0 
0 files    -   1   0 ±  0 

Results for commit 9744321. ± Comparison against base commit caf6a9c.

♻️ This comment has been updated with latest results.

@tanya-borisova tanya-borisova marked this pull request as ready for review August 25, 2022 12:46
@tanya-borisova
Copy link
Contributor Author

/test

@github-actions
Copy link

🤖 pr-bot 🤖

🏃 Running tests: https://github.com/microsoft/AzureTRE/actions/runs/2932386604 (with refid dcfc096d)

(in response to this comment from @tanya-borisova)

@marrobi
Copy link
Member

marrobi commented Aug 26, 2022

@tanya-borisova rather than duplicating so many files, can the base template assets be pulled/download from a release as part of the bundle dockerfile? Then build upon it?

I intend to do that with the task for creating a workspace with unrestricted Internet access.

@tanya-borisova
Copy link
Contributor Author

@marrobi I can give it a try, though wouldn't it introduce a dependency in the order in which base and airlock_manager workspaces will need to be built?
Assuming the airlock_manager would depend on the latest version of the base image

@marrobi
Copy link
Member

marrobi commented Aug 26, 2022

@tanya-borisova rather than depend on the image, download the terraform, and include in a seperate initial terraform step in the porter.yaml, or add additional TF files and have a single terraform step in the Porter.yaml .

@tanya-borisova
Copy link
Contributor Author

@marrobi I'm not sure I understand your suggestion. By "download the terraform", you mean terraform files? Where do you suggest to download them from?

@marrobi
Copy link
Member

marrobi commented Aug 26, 2022

Could download a published release, extract the files, or I was thinking of using sparse checkout so just get the files needed.

@martinpeck
Copy link
Member

Changes we want to see in this PR include:

  • remove the shared storage
  • ideally, we want to prevent VM to VM networking

We may need to deal with the problem that VMs attempt to mount shared storage.

@tanya-borisova tanya-borisova marked this pull request as draft September 12, 2022 10:13
@github-actions
Copy link

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

@github-actions
Copy link

Destroying branch test environment (RG: rg-tre7e5589d2)... (run: https://github.com/microsoft/AzureTRE/actions/runs/3052673745)

@github-actions
Copy link

Branch test environment destroy complete (RG: rg-tre7e5589d2)

@github-actions
Copy link

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

@tanya-borisova
Copy link
Contributor Author

/test

@github-actions
Copy link

🤖 pr-bot 🤖

🏃 Running tests: https://github.com/microsoft/AzureTRE/actions/runs/3058881081 (with refid dcfc096d)

(in response to this comment from @tanya-borisova)

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.

Approving but there're a couple of things I think need addressing.

Makefile Outdated Show resolved Hide resolved
templates/workspaces/airlock_manager/Dockerfile.tmpl Outdated Show resolved Hide resolved
templates/workspaces/airlock_manager/Dockerfile.tmpl Outdated Show resolved Hide resolved
@marrobi
Copy link
Member

marrobi commented Sep 15, 2022

@tanya-borisova can we have a docs page for this to go along site the other two workspace templates please.

@tanya-borisova
Copy link
Contributor Author

/test

@github-actions
Copy link

🤖 pr-bot 🤖

🏃 Running tests: https://github.com/microsoft/AzureTRE/actions/runs/3061126377 (with refid dcfc096d)

(in response to this comment from @tanya-borisova)

@tanya-borisova tanya-borisova merged commit a20b373 into main Sep 15, 2022
@tanya-borisova tanya-borisova deleted the tborisova/2498-template-for-review-workspace branch September 15, 2022 14:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Template for Review Workspace
4 participants