System to mount Nextflow Tower configuration #51

tthyer · 2021-08-27T16:24:19Z

Add an S3 bucket that will be populated with configuration file(s)
Add an EFS file system that will be mounted into Tower docker containers to obtain configuration
Add DataSync task that will sync the S3 bucket file(s) to the EFS file system

…ystem

…ster ASG

config/develop/nextflow-tower-config-bucket.yaml

tthyer · 2021-08-27T21:15:47Z

templates/nextflow-ecs-cluster.yaml

-          - path: /opt/nextflow/tower.yml
-            content: |
-              {{ sceptre_user_data.TowerConfigFileContents|indent(14) }}
-            permissions: '0755'


This file is removed. A real file to manage it now exists in the repo. The real file is uploaded to an S3 bucket, which is then synced to the EFS volume with a DataSync task.

tthyer · 2021-08-27T21:17:17Z

templates/nextflow-ecs-service.yaml

@@ -175,7 +175,7 @@ Resources:
  ServiceScalingTarget:
    Type: AWS::ApplicationAutoScaling::ScalableTarget
    Properties:
-      MaxCapacity: 1
+      MaxCapacity: 2


This is an attempt to get the ASG to grow when we roll out a new TaskDefinition and need to replace the old, but it doesn't resize. I've read that ASG autosizing gets turned off when a stack is deploying but can't recall where. There is a separate issue to address that problem.

tthyer · 2021-08-27T21:18:36Z

tower-config/tower.yaml

+  auth:
+    github:
+      allow-list:
+        - tess.thyer@sagebionetworks.org


follow-up PR will add valid github email addresses

BrunoGrandePhD

Looks good to me! 🚀

BrunoGrandePhD · 2021-08-27T21:36:05Z

config/develop/nextflow-tower-config-bucket.yaml

+  BucketName: {{ stack_name }}
+  AdminArns:
+    - !stack_output_external workflows-nextflow-ci-service-account::ServiceRoleArn
+    - arn:aws:iam::035458030717:role/aws-reserved/sso.amazonaws.com/AWSReservedSSO_Administrator_580e9f32ac55c4e7


Assuming that you approve and merge #50, you can use this instead.

Suggested change

- arn:aws:iam::035458030717:role/aws-reserved/sso.amazonaws.com/AWSReservedSSO_Administrator_580e9f32ac55c4e7

- {{stack_group_config.sso_admin_role.arn}}

As it happens, AdminArns were removed, but will definitely be using that stack_group_config field elsewhere

BrunoGrandePhD · 2021-08-27T21:42:03Z

templates/nextflow-tower-config-bucket.yaml

+      OwnershipControls:
+        Rules:
+          - ObjectOwnership: BucketOwnerPreferred


Nit (i.e. optional): Technically not needed here because cross-account write-access isn't enabled in the bucket policy, but it doesn't hurt to have this here.

oh good point

presumably this is now relevant

since there is cross-account access happening, that is

config/develop/nextflow-efs-file-system.yaml

zaro0508 · 2021-08-30T19:16:22Z

config/develop/nextflow-tower-config-bucket.yaml

@@ -0,0 +1,15 @@
+# {% set stack_name = "nextflow-tower-config-bucket" %}


i think you need to uncomment?

You don't! Cool huh?

i guess that's because # isn't a comment in jinja however it looks like a comment because it's a .yaml file. anyways might be nice to uncomment for readability but your call.

@zaro0508 Sadly, in order for this to stay a yaml file I would have to disable yamllint for the whole file. I thought that using the comment trick was a convenient workaround. Instead I have converted this into a j2 file. I don't really like that solution either because now I'm not getting the benefit of linting either. What do you normally do so that you get linted but can add jinja statements?

ahh, now that i know the intention behind the comment then i guess the comment is better than converting file to j2 to bypass the linter. The other option you can do is to keep as j2 but also enable a jinja linter, here's how to do it with github actions https://github.com/Sage-Bionetworks-IT/organizations-infra/blob/master/.github/workflows/aws-deploy.yaml#L19

@zaro0508 In that case I'll go back to my comment. I would actually like having a jinja linter in place but it should go into pre-commit, not in the github action.

config/develop/nextflow-tower-config-bucket.yaml

zaro0508 · 2021-08-30T21:51:05Z

templates/nextflow-efs-file-system.yaml

+      AvailabilityZoneName: us-east-1a
+      BackupPolicy:
+        Status: DISABLED
+      Encrypted: false


wondering why not enable encryption? i think we need to encrypt all storage to be PHI compliant?

not gonna do anything about this one?

@zaro0508 Do you think I need to encrypt a bucket that only contains config files for Tower, i.e. not PHI?

i guess not but @ahayden might think otherwise. anyways it would be pretty easy just to set encryption using the default key correct? i think we only require AES encryption on s3 buckets so i'm guessing default key here is fine.

@zaro0508 Ok, will do this in a follow-up PR later today

templates/nextflow-ecs-task-definition.yaml

templates/nextflow-tower-config-bucket.yaml

…n template

…they already have this access

…to a j2 to avoid commenting jinja statement

tthyer · 2021-09-01T14:59:52Z

@zaro0508 I have to deploy these changes in order to unblock other work, but if you have more comments I can address them in a follow-up PR

tthyer added 8 commits August 26, 2021 10:12

Add EFS file system

3623bd3

Add environment variable for tower config file path

9935ae9

Add a config bucket for nextflow tower and populate it

636aa3e

Set up DataSync task to sync data from S3 config bucket to EFS file s…

bb16499

…ystem

Make efs volume path and tower config file name configurable

cb05707

Remove unused sceptre_user_data from nextflow-ecs-cluster stack

4a63513

Fix EFS subnet -- make it dependent upon the one used for the ECS clu…

130dad0

…ster ASG

Add an hourly schedule and cloudwatch logging to DataSync task

fc5db0e

tthyer commented Aug 27, 2021

View reviewed changes

config/develop/nextflow-tower-config-bucket.yaml Outdated Show resolved Hide resolved

tthyer commented Aug 27, 2021

View reviewed changes

tthyer marked this pull request as ready for review August 27, 2021 21:18

tthyer requested a review from a team as a code owner August 27, 2021 21:18

tthyer requested a review from a team August 27, 2021 21:19

BrunoGrandePhD approved these changes Aug 27, 2021

View reviewed changes

zaro0508 requested changes Aug 30, 2021

View reviewed changes

tthyer added 2 commits August 30, 2021 16:19

Created a parameter for the efs volume name in the ecs task definitio…

c4506b4

…n template

Remove explict allowing access of config bucket to account admins -- …

eef71a1

…they already have this access

tthyer requested review from zaro0508 and BrunoGrandePhD August 30, 2021 23:25

Change nextflow-tower-config-bucket sceptre stack config from yaml in…

0fe6432

…to a j2 to avoid commenting jinja statement

tthyer force-pushed the WORKFLOWS-38/efs branch from ea2bbef to 0fe6432 Compare August 31, 2021 19:26

Add bruno.grande@sagebase.org to list of allowed github users

e9b27e9

BrunoGrandePhD approved these changes Sep 1, 2021

View reviewed changes

tthyer merged commit 985b08b into main Sep 1, 2021

tthyer deleted the WORKFLOWS-38/efs branch September 1, 2021 15:05

tthyer mentioned this pull request Sep 2, 2021

[WORKFLOWS-38] Encrypt EFS file system #55

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

System to mount Nextflow Tower configuration #51

System to mount Nextflow Tower configuration #51

tthyer commented Aug 27, 2021 •

edited

Loading

tthyer Aug 27, 2021

tthyer Aug 27, 2021

tthyer Aug 27, 2021

BrunoGrandePhD left a comment

BrunoGrandePhD Aug 27, 2021

tthyer Aug 30, 2021

BrunoGrandePhD Aug 27, 2021

tthyer Aug 27, 2021

tthyer Aug 30, 2021

tthyer Aug 31, 2021

zaro0508 Aug 30, 2021

tthyer Aug 30, 2021

zaro0508 Aug 31, 2021

tthyer Aug 31, 2021

zaro0508 Sep 1, 2021

tthyer Sep 1, 2021

zaro0508 Aug 30, 2021

zaro0508 Aug 31, 2021

tthyer Aug 31, 2021

zaro0508 Sep 1, 2021

tthyer Sep 1, 2021

tthyer commented Sep 1, 2021

	- arn:aws:iam::035458030717:role/aws-reserved/sso.amazonaws.com/AWSReservedSSO_Administrator_580e9f32ac55c4e7
	- {{stack_group_config.sso_admin_role.arn}}

		@@ -0,0 +1,15 @@
		# {% set stack_name = "nextflow-tower-config-bucket" %}

System to mount Nextflow Tower configuration #51

System to mount Nextflow Tower configuration #51

Conversation

tthyer commented Aug 27, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BrunoGrandePhD 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

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

tthyer commented Sep 1, 2021

tthyer commented Aug 27, 2021 •

edited

Loading