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
feat(package/deploy): new option --image-repositories
#2465
Conversation
--image-repositories
--image-repositories
f147483
to
313181e
Compare
for resource_id, resource in template_dict.get("Resources", {}).items(): | ||
if resource.get("Properties", {}).get("PackageType", ZIP) == artifact and resource.get("Type") in [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was an existing function collector, which is been used in build command, can we use that one instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does that process the template? I will take a look at it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
its very specific to build, we would need some refactoring to make it more generic so that we can share that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would still use SamFunctionProvider
https://github.com/aws/aws-sam-cli/blob/develop/samcli/lib/providers/sam_function_provider.py#L16 which will handle parameter overrides etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are some circular dependencies that we need to take care of here, I vote we do this in a separate PR.
@mndeveci : if we dont special case how we save to the config file. It saves as below, thats why we need it.
|
076945b
to
27ce6e2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on the feedbacks, left only one comment for collecting the functions from the template. Other than that it looks good.
@@ -129,7 +131,7 @@ def run(self): | |||
print_deploy_args( | |||
self.stack_name, | |||
self.s3_bucket, | |||
self.image_repository, | |||
self.image_repositories if isinstance(self.image_repositories, dict) else self.image_repository, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nit] I think if you check that self.image_repositories is not None will be more readable
There is still no documentation on sam package (https://docs.aws.amazon.com/serverless-application-model/latest/developerguide/sam-cli-command-reference-sam-package.html) guides page. Trying to find the format to use here because I think this arg will allow me to deploy two functions with the same image. |
* feat(package/deploy): new option `--image-repositories` * tests(package/deploy): unit and integration tests * fix(lint): make lint happy * fix(comments): address PR comments * fix(image-repositories): change callback to default to {} Co-authored-by: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com>
* feat(package/deploy): new option `--image-repositories` * tests(package/deploy): unit and integration tests * fix(lint): make lint happy * fix(comments): address PR comments * fix(image-repositories): change callback to default to {} Co-authored-by: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com>
Hopefully this will help someone and save them time. There is no documentation on this as @metaskills mentioned and no examples on how to use it. So after spending more time than needed on it and looking through the source code on this, I was able to figure out what they intended. Let's say you have two functions in your template file that use the Image package type, like the following:
After you run the
Now when you run the
Where: So using the above example we would call
Where: I have this automated using a PowerShell script so you will just need to figure out how to handle it using your tooling / use case. Enjoy. |
The example from @JamesDougherty is wrong and will throw an error like this: Error: Got unexpected extra argument (Function_Logical_ID=account_id.dkr.ecr.us-east-1.amazonaws.com/Function_Image_Repository) Please, find the correct command below: Option description: --image-repositories Specify mapping of Function Logical ID to ECR Repo uri, of the form Function_Logical_ID=ECR_Repo_Uri. This option can be specified multiple times. My current version is 1.29.0. |
Hello @ygorth - Thank you for the correction. I was only testing with one repository at the moment and made the assumption that they designed it how the parameter overrides work "sam deploy ... --parameter-overrides foo=bar, hello=world". Of course it would be different. This is why it would be nice to have some official documentation so we don't have to assume anything. Anyway, thanks again. |
Hello, Any advice how to fix this issue? image name is compatible with format and regex but still errors out!: MacBook-Pro:lambda-local jamin$ sam deploy --guided Error: Invalid value for '--image-repositories': --image-repositories is not a valid format, it needs to be of the form function_logical_id=ECR_URI samconfig.toml |
Which issue(s) does this change fix?
#2447
It does not fix this issue, but allows for mapping of function to different ECR if required.
Why is this change necessary?
Allow for function to ecr mapping. each function can now be uploaded to a different ECR.
How does it address the issue?
Guided deploy prompts for an ECR per function and saves it.
What side effects does this change have?
if both
--image-repository
and--image-repositories
are specifed. One has to be removed. Guided flow looks to fill--image-repositories
but is not destructive to the configuration file and does not remove--image-repository
if its already set.Checklist
make pr
passesmake update-reproducible-reqs
if dependencies were changedNOTE: WIP and needs more tests.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.