Migrate internal storage to EFS from NFS exports #2465

dreambeyondorange · 2023-09-19T20:28:54Z

Description of changes

[Develop] Migrate internal storage to EFS from NFS exports

Add backup and restore recipes to move data in second stage images to shared filesystems
Create a new mount_internal_use_fs.rb recipe to mount the internal shared filesystems
Filter the efs filesystem arrays to mount internal shared fses in init and cx fses in config
Refactor environment recipes to be clearer in functional description and remove unnecessary recipes

Tests

Launched a cluster with 2 static nodes and verified the fses were mounted on the head node and compute nodes
Head node had all 4 fses
Compute node had all except shared_login_dir

Checklist

Make sure you are pointing to the right branch.
If you're creating a patch for a branch other than develop add the branch name as prefix in the PR title (e.g. [release-3.6]).
Check all commits' messages are clear, describing what and why vs how.
Make sure to have added unit tests or integration tests to cover the new/modified code.
Check if documentation is impacted by this change.

Please review the guidelines for contributing and Pull Request Instructions.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

codecov · 2023-09-19T20:29:58Z

Codecov Report

Patch and project coverage have no change.

Comparison is base (47c6354) 76.06% compared to head (2e361ef) 76.06%.

Additional details and impacted files

@@           Coverage Diff            @@
##           develop    #2465   +/-   ##
========================================
  Coverage    76.06%   76.06%           
========================================
  Files           13       13           
  Lines         1876     1876           
========================================
  Hits          1427     1427           
  Misses         449      449

Flag	Coverage Δ
unittests	`76.06% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

CHANGELOG.md

cookbooks/aws-parallelcluster-environment/recipes/config/mount_cx_fs.rb

cookbooks/aws-parallelcluster-environment/recipes/init/backup_internal_use_shared_data.rb

cookbooks/aws-parallelcluster-environment/recipes/init/mount_internal_use_fs.rb

cookbooks/aws-parallelcluster-environment/recipes/init/backup_internal_use_shared_data.rb

CHANGELOG.md

cookbooks/aws-parallelcluster-environment/kitchen.environment-config.yml

cookbooks/aws-parallelcluster-environment/recipes/init/backup_internal_use_shared_data.rb

CHANGELOG.md

cookbooks/aws-parallelcluster-environment/kitchen.environment-config.yml

cookbooks/aws-parallelcluster-environment/kitchen.environment-recipes.yml

cookbooks/aws-parallelcluster-environment/attributes/environment.rb

cookbooks/aws-parallelcluster-slurm/recipes/config/config_login.rb

cookbooks/aws-parallelcluster-environment/recipes/init/backup_internal_use_shared_data.rb

Add backup and restore recipes to move data in second stage images to shared filesystems Create a new mount_internal_use_fs.rb recipe to mount the internal shared filesystems Filter the efs filesystem arrays to mount internal shared fses in init and cx fses in config Refactor environment recipes to be clearer in functional description and remove unnecessary recipes

enrico-usai · 2023-09-22T15:03:45Z

cookbooks/aws-parallelcluster-environment/recipes/init/mount_internal_use_fs.rb

+  internal_efs_fs_id_array.push(efs_fs_id_array[index])
+  internal_efs_encryption_array.push(efs_encryption_in_transit_array[index])
+  internal_efs_iam_array.push(efs_iam_authorization_array[index])
+end


This block of code, from the beginning, it's "duplicated" in the efs.rb.

What about having a first recipe that parses the efs_shared_dirs creating the two internal_*_array vs cx_*_array ?

Then the mount_internal_use_fs.rb will just call the mount of the efs passing internal_*_array and the efs will mount the cx_* ones.

In the code it's enough to have it at the beginning of the init.rb before the mount, In the inspec tests we just need to add it as dependency to be executed as first.

What do you think?

I'm not sure. I prefer having code here that just filters the ones we need for the init and a separate one that filters what we need for the cx. We do this filtering process twice, so I wonder if it might be better to just put this into the attributes file and then leverage the attributes we need in each recipe. This did get auto merged with Giacomo's approval, but I will do this refactoring as a follow up

cookbooks/aws-parallelcluster-environment/recipes/init/mount_internal_use_fs.rb

enrico-usai · 2023-09-22T15:05:18Z

cookbooks/aws-parallelcluster-environment/recipes/init/restore_internal_use_shared_data.rb

+  # This is necessary to preserve any data in these directories that was
+  # generated during the build of ParallelCluster AMIs after converting to
+  # shared storage and backed up to a temporary location previously
+  # Remove the backup after the copy is done


# Remove the backup after the copy is done can be moved in the code block before the rm.

cookbooks/aws-parallelcluster-slurm/test/controls/mount_slurm_dir_spec.rb

…ws#2465)" This requires a change to the CLI that is pending review This reverts commit e4bf17b.

…2465)" (#2469) This requires a change to the CLI that is pending review This reverts commit e4bf17b.

Add backup and restore recipes to move data in second stage images to shared filesystems Create a new mount_internal_use_fs.rb recipe to mount the internal shared filesystems Filter the efs filesystem arrays to mount internal shared fses in init and cx fses in config Refactor environment recipes to be clearer in functional description and remove unnecessary recipes

…ws#2465)" (aws#2469) This requires a change to the CLI that is pending review This reverts commit e4bf17b.

dreambeyondorange force-pushed the develop branch 5 times, most recently from 0f79ce2 to b2be028 Compare September 19, 2023 21:29

gmarciani reviewed Sep 20, 2023

View reviewed changes

dreambeyondorange force-pushed the develop branch 2 times, most recently from 1364a59 to cffa14f Compare September 20, 2023 17:35

dreambeyondorange marked this pull request as ready for review September 20, 2023 17:41

dreambeyondorange requested review from a team as code owners September 20, 2023 17:41

dreambeyondorange force-pushed the develop branch 5 times, most recently from 28c41d9 to 46b03bd Compare September 21, 2023 02:01

dreambeyondorange enabled auto-merge (squash) September 21, 2023 02:13

aws deleted a comment from dreambeyondorange Sep 21, 2023

gmarciani reviewed Sep 21, 2023

View reviewed changes

enrico-usai reviewed Sep 21, 2023

View reviewed changes

dreambeyondorange force-pushed the develop branch 4 times, most recently from 29a478c to 937d422 Compare September 21, 2023 20:11

dreambeyondorange force-pushed the develop branch from 937d422 to 47140cd Compare September 21, 2023 23:22

gmarciani approved these changes Sep 22, 2023

View reviewed changes

Merge branch 'develop' into develop

2e361ef

enrico-usai reviewed Sep 22, 2023

View reviewed changes

dreambeyondorange merged commit e4bf17b into aws:develop Sep 22, 2023
27 checks passed

dreambeyondorange added a commit that referenced this pull request Sep 22, 2023

Revert "[Develop] Migrate internal storage to EFS from NFS exports (#…

02c0676

…2465)" (#2469) This requires a change to the CLI that is pending review This reverts commit e4bf17b.

dreambeyondorange mentioned this pull request Oct 5, 2023

[Develop] Add a new internal storage option, EFS, keeping EBS as the default #2480

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate internal storage to EFS from NFS exports #2465

Migrate internal storage to EFS from NFS exports #2465

dreambeyondorange commented Sep 19, 2023 •

edited

Loading

codecov bot commented Sep 19, 2023 •

edited

Loading

enrico-usai Sep 22, 2023

dreambeyondorange Sep 22, 2023

enrico-usai Sep 22, 2023

Migrate internal storage to EFS from NFS exports #2465

Migrate internal storage to EFS from NFS exports #2465

Conversation

dreambeyondorange commented Sep 19, 2023 • edited Loading

Description of changes

Tests

Checklist

codecov bot commented Sep 19, 2023 • edited Loading

Codecov Report

enrico-usai Sep 22, 2023

Choose a reason for hiding this comment

dreambeyondorange Sep 22, 2023

Choose a reason for hiding this comment

enrico-usai Sep 22, 2023

Choose a reason for hiding this comment

dreambeyondorange commented Sep 19, 2023 •

edited

Loading

codecov bot commented Sep 19, 2023 •

edited

Loading