Skip to content

Update start workflows container script for unified storage feature to create workflows sub dir #674

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

Merged
merged 4 commits into from
Jun 16, 2025

Conversation

ruijiang-rjian
Copy link
Contributor

@ruijiang-rjian ruijiang-rjian commented Jun 5, 2025

Description

For projects with unified storage(s3), there will be a project shared dir /home/sagemaker-user/shared-files/ to replace /home/sagemaker-user/src/, this change is mainly to adapt to this new use case: if it's using git, everything is kept the same; if it's using unified storage, then we're creating /home/sagemaker-user/shared-files/workflows/ and all the sub-directories under /workflows

Type of Change

  • Image update - Bug fix
  • Image update - New feature
  • Image update - Breaking change
  • SMD image build tool update
  • Documentation update

Release Information

Does this change need to be included in patch version releases? By default, any pull requests will only be added to the next SMD image minor version release once they are merged in template folder. Only critical bug fix or security update should be applied to new patch versions of existed image minor versions.

  • Yes (Critical bug fix or security update)
  • No (New feature or non-critical change)
  • N/A (Not an image update)

If yes, please explain why:
[Explain the criticality of this change and why it should be included in patch releases]

How Has This Been Tested?

[Describe the tests you ran]

Checklist:

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works

Test Screenshots (if applicable):

tested in gamma project with the new start container script, local runner was able to start and verified workflows subfolder was created under the new "shared-files" project directory:
Tested the following scenarios:

  1. bash /etc/sagemaker-ui/workflows/start-workflows-container.sh (backwards compatibility)
  2. bash /etc/sagemaker-ui/workflows/start-workflows-container.sh "$is_s3_flag” (with is_s3_flag=1, Git project)
  3. bash /etc/sagemaker-ui/workflows/start-workflows-container.sh "$is_s3_flag” (is_s3_flag=0, shared storage project: Project is using S3 storage, project directory set to: /home/sagemaker-user/shared-files)
Screenshot 2025-06-05 at 16 05 06
sagemaker-user@default:~$ bash /etc/sagemaker-ui/workflows/start-workflows-container.sh
Hit:1 https://download.docker.com/linux/ubuntu jammy InRelease
Hit:2 https://apt.corretto.aws stable InRelease                                                                                                   
Hit:3 http://security.ubuntu.com/ubuntu jammy-security InRelease                                                                                  
Hit:4 http://archive.ubuntu.com/ubuntu jammy InRelease
Hit:5 http://archive.ubuntu.com/ubuntu jammy-updates InRelease
Hit:6 http://archive.ubuntu.com/ubuntu jammy-backports InRelease
Reading package lists... Done
Hit:1 https://download.docker.com/linux/ubuntu jammy InRelease
Hit:2 https://apt.corretto.aws stable InRelease                                                                                
Hit:3 http://archive.ubuntu.com/ubuntu jammy InRelease                                                                         
Hit:4 http://archive.ubuntu.com/ubuntu jammy-updates InRelease
Hit:5 http://security.ubuntu.com/ubuntu jammy-security InRelease
Hit:6 http://archive.ubuntu.com/ubuntu jammy-backports InRelease
Reading package lists... Done
Reading package lists... Done
Building dependency tree... Done
Reading state information... Done
docker-ce-cli is already the newest version (5:28.2.2-1~ubuntu.22.04~jammy).
docker-compose-plugin is already the newest version (2.29.2-1~ubuntu.22.04~jammy).
0 upgraded, 0 newly installed, 0 to remove and 0 not upgraded.
WARNING! Your password will be stored unencrypted in /home/sagemaker-user/.docker/config.json.
Configure a credential helper to remove this warning. See
https://docs.docker.com/engine/reference/commandline/login/#credential-stores

Login Succeeded
[+] Running 3/3
 ✔ Container mwaa-292-db         Healthy                                                                                                     10.7s 
 ✔ Container mwaa-292-webserver  Started                                                                                                     10.8s 
 ✔ Container mwaa-292-scheduler  Started                                                                                                     10.8s 
/opt/conda/lib/python3.12/site-packages/supervisor/options.py:13: UserWarning: pkg_resources is deprecated as an API. See https://setuptools.pypa.io/en/latest/pkg_resources.html. The pkg_resources package is slated for removal as early as 2025-11-30. Refrain from using this package or pin to Setuptools<81.
  import pkg_resources
workflows_healthcheck: started

Related Issues

[Link any related issues here]

Additional Notes

[Any additional information that might be helpful for reviewers]

@ruijiang-rjian ruijiang-rjian requested a review from a team as a code owner June 5, 2025 20:09
Copy link

@agupta01 agupta01 left a comment

Choose a reason for hiding this comment

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

If these changes get built into a patch SMD and deployed to existing spaces (without storage), then will this break existing functionality?

@ruijiang-rjian
Copy link
Contributor Author

ruijiang-rjian commented Jun 6, 2025

If these changes get built into a patch SMD and deployed to existing spaces (without storage), then will this break existing functionality?

no it shouldn't break, I've tested it in an existing Gamma space without storage too to make sure it's backwards compatible

DZ_PROJECT_S3PATH=$(jq -r '.AdditionalMetadata.ProjectS3Path' < $RESOURCE_METADATA_FILE)

# Helper to check if the project is using Git storage
is_s3_storage() {
Copy link

Choose a reason for hiding this comment

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

Can we reuse this definition from here - #668 as a param?

Copy link

@vasu2856 vasu2856 left a comment

Choose a reason for hiding this comment

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

Can you refactor to have these common functions in the post launch script and just use these as params within the workflows container?

@claytonparnell claytonparnell merged commit 92b3e6b into aws:main Jun 16, 2025
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.

6 participants