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

IT-3317 #318

Merged
merged 13 commits into from
Jan 11, 2024
Merged

IT-3317 #318

merged 13 commits into from
Jan 11, 2024

Conversation

brucehoff
Copy link
Member

@brucehoff brucehoff commented Dec 15, 2023

Add template for dockerized service catalog notebook product.

This is modeled after the current RStudio notebook template, however instead of using the RStudio AMI it simply uses the base Ubuntu AMI and then runs the reverse proxy and notebook software as containers. This modularity allows the product to have the option of running either an RStudio or Jupyter notebook.

Depends on Sage-Bionetworks-IT/notebook-reverse-proxy#1

ec2/sc-ec2-linux-docker-notebook.yaml Outdated Show resolved Hide resolved
ec2/sc-ec2-linux-docker-notebook.yaml Outdated Show resolved Hide resolved
ec2/sc-ec2-linux-docker-notebook.yaml Outdated Show resolved Hide resolved
--name reverse-proxy \
--network ${NETWORK_NAME} \
--restart unless-stopped \
-e EC2_INSTANCE_ID=$(curl http://169.254.169.254/latest/meta-data/instance-id) \
Copy link
Member

Choose a reason for hiding this comment

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

i think this needs to be updated to IMDSV2? related PR: Sage-Bionetworks/service-catalog-utils#25

Copy link
Member Author

Choose a reason for hiding this comment

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

Apparently we do not have to update. I have tried this out in our dev Service Catalog and it works fine.

Copy link
Member

@zaro0508 zaro0508 Jan 10, 2024

Choose a reason for hiding this comment

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

it's because you are building on top of an older AWS AMI, the IMDSV1 will be removed in mid 2024. We should update to IMDSV2 either in this PR or in a follow on PR.

ec2/sc-ec2-linux-docker-notebook.yaml Outdated Show resolved Hide resolved
ec2/sc-ec2-linux-docker-notebook.yaml Outdated Show resolved Hide resolved
ec2/sc-ec2-linux-docker-notebook.yaml Outdated Show resolved Hide resolved
ec2/sc-ec2-linux-docker-notebook.yaml Outdated Show resolved Hide resolved
ec2/sc-ec2-linux-docker-notebook.yaml Outdated Show resolved Hide resolved
ec2/sc-ec2-linux-docker-notebook.yaml Outdated Show resolved Hide resolved
ec2/sc-ec2-linux-docker-notebook.yaml Outdated Show resolved Hide resolved
ec2/sc-ec2-linux-docker-notebook.yaml Outdated Show resolved Hide resolved
@brucehoff brucehoff requested a review from zaro0508 January 6, 2024 01:37
templates/ec2/sc-ec2-linux-docker-notebook.yaml Outdated Show resolved Hide resolved
templates/ec2/sc-ec2-linux-docker-notebook.yaml Outdated Show resolved Hide resolved
DependsOn: "InstanceProfile"
Type: Custom::SynapseTagger
Properties:
ServiceToken: !ImportValue
Copy link
Member

Choose a reason for hiding this comment

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

This lambda will take all of the ENV VARs defined in make_env_vars_file.sh file and apply them as EC2 instance tags. Some of those tags are very important to how the service catalog works. For example the value for the ACCESS_APPROVED_ROLEID tag is what is used to restrict instance access to the user who provisioned it. Since the make_env_vars_file.sh is no longer setup in this file i'm going to assume that those tags are no longer getting set. I think we'll need make_env_vars_file.sh or find an alternative way to set those tags.

Copy link
Member Author

Choose a reason for hiding this comment

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

I see. Will add to the config.

@brucehoff brucehoff requested a review from zaro0508 January 11, 2024 02:16
Copy link

dpulls bot commented Jan 11, 2024

🎉 All dependencies have been resolved !

@brucehoff brucehoff merged commit 1b53b7f into Sage-Bionetworks:master Jan 11, 2024
3 checks passed
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