Changes for Fargate Dockstore deploy #225

kathy-t · 2022-05-17T15:37:03Z

This PR introduces changes that are needed by the Fargate Dockstore stack (https://github.com/dockstore/dockstore-deploy/pull/478).

I added a IS_FARGATE_DEPLOY key which allows install_bootstrap to create the config files in a way that can be used by the fargate deploy. The regular docker-compose deploy should be unaffected.

Changes:

Config files for Fargate deploy have to be placed in more specific directories to avoid having all the config files in one volume
The gitHubAppPrivateKeyFile path is slightly changed in web.yml
In templates/default.nginx_http.conf.template, $webservice is set to 127.0.0.1 for Fargate deploy because containers that belong to the same task can communicate over the localhost interface.
Specified an absolute path to the webservice jar in init_migrations and init_webservice because the respective scripts are not in the same directory as the jar

denis-yuen

(this part) seems straightforward but was curious

docker-compose.yml

coverbeck

Some curiosity questions, but looks good to me, in so far as I (don't) understand Fargate.

docker-compose.yml

coverbeck · 2022-05-18T00:41:52Z

templates/init_webservice.sh.template

@@ -2,6 +2,6 @@

 cd "$(dirname "$0")"

-java -XX:MaxRAMPercentage=50.0 -XX:+ExitOnOutOfMemoryError -jar dockstore-webservice-*.jar server web.yml | tee --append /dockstore_logs/webservice.out
+java -XX:MaxRAMPercentage=50.0 -XX:+ExitOnOutOfMemoryError -jar /home/dockstore-webservice-*.jar server web.yml | tee --append /dockstore_logs/webservice.out


I think the 50% is a holdover from when we had ElasticSearch and Postgres running on the same machine. We probably should have raised this for EC2 (and could maybe have brought down the instance size in conjunction with that).

But not to digress from the Fargate PR, that could be tackled separately... Should we be setting this at all for Fargate? I don't know, just asking.

Hmmm, also Nextflow but yes. I suspect we can tune this for Fargate with some performance testing.
It probably should be a spin-off ticket to be tackled, much later.

Ah, right forgot about Nextflow... we probably don't want to go too much higher after all, at least not in the EC2 world.

install_bootstrap

…path

kathy-t · 2022-05-18T20:50:04Z

Marking this as ready for review because the corresponding dockstore-deploy PR https://github.com/dockstore/dockstore-deploy/pull/478 is ready for review

chmreid · 2022-05-19T17:49:29Z

templates/init_migration.sh.template

@@ -4,12 +4,12 @@
 cd "$(dirname "$0")"

 {{#DATABASE_GENERATED}}
-java -Ddw.database.user=postgres -Ddw.database.password="{{{ POSTGRES_DBPASSWORD }}}" -jar dockstore-webservice-*.jar db migrate web.yml --include 1.3.0.generated,1.3.1.consistency,1.4.0,1.5.0,1.6.0,1.7.0 | tee --append /dockstore_logs/webservice.out
+java -Ddw.database.user=postgres -Ddw.database.password="{{{ POSTGRES_DBPASSWORD }}}" -jar /home/dockstore-webservice-*.jar db migrate web.yml --include 1.3.0.generated,1.3.1.consistency,1.4.0,1.5.0,1.6.0,1.7.0 | tee --append /dockstore_logs/webservice.out


Related to comments in #483, the /home directory should not contain anything except home directories for users on the system. This jar should be in /home/<username>/. (probably ubuntu, but not sure what kind of container this script is being run in).

This is in the /home directory because the webservice Dockerfile put it there: https://github.com/dockstore/dockstore/blob/develop/Dockerfile#L19. The docker-compose file also puts things in the /home directory: https://github.com/dockstore/compose_setup/blob/develop/docker-compose.yml#L25. Should I be making changes to fix that?

FWIW, the home directory thing is Linux best practice. That said, since this seems to be already in use in our various environments (and for a while), I would split this out the a new ticket to use /home properly and systematically (not 100% sure, it makes sense for Docker anyway)

chmreid · 2022-05-19T17:52:01Z

templates/web.yml.template

@@ -29,7 +29,7 @@ zenodoUrl: {{ ZENODO_URL }}
 orcidClientID: {{ ORCID_CLIENT_ID }}
 orcidClientSecret: {{ ORCID_CLIENT_SECRET }}

-gitHubAppPrivateKeyFile: /home/dockstore_github_app_private_key.pem
+gitHubAppPrivateKeyFile: /home/github-key/dockstore-github-private-key.pem


The way this reads, you have a user named github-key and the only reason they exist is so their home directory can contain a .pem key. The problem is, /home/ is being used as a home directory, but the home directory should be /home/<username>.

I changed this to the /dockstore/github-key directory: 587358c

kathy-t added 8 commits May 11, 2022 16:04

Add IS_FARGATE_DEPLOY variable

274e958

Add IS_FARGATE_DEPLOY to compose.config

4bebcb9

Specify absolute path for webservice jar

ba98239

Add missing semi-colon

f34ce65

Try without quotes

a2bf281

Try turning off ipv6 for resolver

a2d145a

Try 127.0.0.1 instead of localhost

d7e9a1a

Make GITHUB_APP_PRIVATE_KEY_FILE configurable

069831e

kathy-t self-assigned this May 17, 2022

kathy-t requested review from david4096, coverbeck, denis-yuen, svonworl, chmreid and Richard-Hansen and removed request for david4096 and coverbeck May 17, 2022 19:46

denis-yuen reviewed May 17, 2022

View reviewed changes

docker-compose.yml Show resolved Hide resolved

coverbeck reviewed May 18, 2022

View reviewed changes

kathy-t added 3 commits May 18, 2022 12:57

Don't make gitHubAppPrivateKeyFile configurable. Instead, change the …

b1f3594

…path

Use dashses instead of underscores for gitHubAppPrivateKeyFile

7526400

Remove app from github private key name

3614fdc

kathy-t marked this pull request as ready for review May 18, 2022 20:50

kathy-t requested review from coverbeck and denis-yuen May 18, 2022 20:50

denis-yuen approved these changes May 18, 2022

View reviewed changes

coverbeck approved these changes May 19, 2022

View reviewed changes

chmreid reviewed May 19, 2022

View reviewed changes

Richard-Hansen approved these changes May 20, 2022

View reviewed changes

david4096 approved these changes May 23, 2022

View reviewed changes

svonworl approved these changes May 23, 2022

View reviewed changes

Use /dockstore dir instead of /home for github key

587358c

kathy-t requested a review from chmreid May 25, 2022 15:26

chmreid approved these changes May 25, 2022

View reviewed changes

kathy-t merged commit 2959c8b into develop May 26, 2022

kathy-t deleted the feature/seab-4096/cdk-fargate branch May 26, 2022 17:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Changes for Fargate Dockstore deploy #225

Changes for Fargate Dockstore deploy #225

kathy-t commented May 17, 2022 •

edited

Loading

denis-yuen left a comment

coverbeck left a comment

coverbeck May 18, 2022

denis-yuen May 18, 2022

coverbeck May 18, 2022

kathy-t commented May 18, 2022

chmreid May 19, 2022

kathy-t May 19, 2022

denis-yuen May 24, 2022

chmreid May 19, 2022 •

edited

Loading

kathy-t May 25, 2022

Changes for Fargate Dockstore deploy #225

Changes for Fargate Dockstore deploy #225

Conversation

kathy-t commented May 17, 2022 • edited Loading

denis-yuen left a comment

Choose a reason for hiding this comment

coverbeck left a comment

Choose a reason for hiding this comment

coverbeck May 18, 2022

Choose a reason for hiding this comment

denis-yuen May 18, 2022

Choose a reason for hiding this comment

coverbeck May 18, 2022

Choose a reason for hiding this comment

kathy-t commented May 18, 2022

chmreid May 19, 2022

Choose a reason for hiding this comment

kathy-t May 19, 2022

Choose a reason for hiding this comment

denis-yuen May 24, 2022

Choose a reason for hiding this comment

chmreid May 19, 2022 • edited Loading

Choose a reason for hiding this comment

kathy-t May 25, 2022

Choose a reason for hiding this comment

kathy-t commented May 17, 2022 •

edited

Loading

chmreid May 19, 2022 •

edited

Loading