aws-samples / sagemaker-custom-project-templates

MIT No Attribution
214 stars 155 forks source link

mlops-multi-account-cdk template: downgrade ruby to 2.7; add CodeBuildAdminAccess to launch role #78

Closed hrvg closed 1 year ago

hrvg commented 1 year ago

Thanks for this great template! While experimenting with this solution, I found two issues. Happy to discuss.

Description of changes:

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

fotinosk commented 1 year ago

Hello Hervé, thank you for the PR, I believe since the time you raised the PR, ruby 3.2 came into support, so I have upgraded to that version

hrvg commented 1 year ago

Hello Hervé, thank you for the PR, I believe since the time you raised the PR, ruby 3.2 came into support, so I have upgraded to that version

I don't think that is the case. I just checked the available runtimes which shows:

Version Images
2.6 Amazon Linux 2 AArch64 standard:2.0 Ubuntu standard:5.0
2.7 Amazon Linux 2 AArch64 standard:2.0 Ubuntu standard:5.0
3.1 Amazon Linux 2 x86_64 standard:4.0 Ubuntu standard:6.0
3.2 Amazon Linux 2 x86_64 standard:5.0 Amazon Linux 2 AArch64 standard:3.0 Ubuntu standard:7.0

I think the confusion arises from the fact that STANDARD_5_0 actually refers to the Ubuntu standard:5.0 and not the Amazon Linux 2 x86_64 standard:5.0 image (AMAZON_LINUX_2_5) (docs).

fotinosk commented 1 year ago

Can you please confirm with Linux image is being used (you should be able to see it from CodeBuild), there is a common bug where the environment gets overwritten

hrvg commented 1 year ago

I just reproduced the error with ruby 3.1 for the service pipeline. As you mentioned, that is expected because that pipeline does not specify its build image and thus defaults to LinuxBuildImage.STANDARD_7_0. That is the issue I realized in https://github.com/aws-samples/sagemaker-custom-project-templates/pull/78/commits/f34d4af46bd22ab6c1fff2b07ce422e210e2d7f1 when I upgraded from 2.7 back to 3.1.

The issue is that the template pipeline constructs do specify the build image to LinuxBuildImage.STANDARD_5_0 which is not compatible with ruby 3.1. So either the ruby version is fixed or the build image has to change.

fotinosk commented 1 year ago

Yes, you are right. I just noticed that too. I was testing on that one when I upgraded to 3.2. PR has been now approved - thank you for raising this!