-
Notifications
You must be signed in to change notification settings - Fork 189
Installer update enhancements #288
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
Conversation
brtrvn
left a comment
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.
For the update.sh scripts you've added to the custom resources folder, I don't know that we need to do a query against Lambda (with list-functions) vs just specifying the function name like we do in the scripts in the functions folder.
Also, in the UpdateWorkflow, for any Lambda that might be completely new, can we trust (have we tested) that the shell scripts will properly no-op a call to update-function-code if it doesn't exist?
In AwsClientBuilderFactory we "cache" the SDK client instances. This implies that we may want to clear the cached instance or start over. There doesn't appear to be a way to do that without building a new Factory. Was this intentional? Are they really "cached" then or just instance variables?
getUpdateActionsFromPaths seems a bit fragile with magic numbers. Maybe just some more comments on exactly what's going on and what the expectation is?
...ain/java/com/amazon/aws/partners/saasfactory/saasboost/model/ExistingEnvironmentFactory.java
Outdated
Show resolved
Hide resolved
...ain/java/com/amazon/aws/partners/saasfactory/saasboost/model/ExistingEnvironmentFactory.java
Show resolved
Hide resolved
...ain/java/com/amazon/aws/partners/saasfactory/saasboost/model/ExistingEnvironmentFactory.java
Outdated
Show resolved
Hide resolved
installer/src/main/java/com/amazon/aws/partners/saasfactory/saasboost/Constants.java
Outdated
Show resolved
Hide resolved
...main/java/com/amazon/aws/partners/saasfactory/saasboost/clients/AwsClientBuilderFactory.java
Outdated
Show resolved
Hide resolved
|
Just checked and if no functions already exist the Which means that no This actually provides an argument against direct updating of the function code in So in either case it's no problem: when the CloudFormation stack is updated (with the new template, if it's been added) those functions will be created using the code uploaded to the proper S3 location by the |
brtrvn
left a comment
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.
The upside of running the update.sh scripts for Lambda code packages that are brand new before running the CloudFormation stack update is that it will compile and then upload the code package to S3 where CloudFormation expects it to be.
brtrvn
left a comment
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.
Changes look great
This PR consists migrates the existing Installer update logic to an enhanced version.
The original update implementation would blindly update every component of SaaS Boost: re-uploading all resources and running a complete CloudFormation update. In the case of a complicated SaaS Boost installation this could take dozens of minutes to run an update when only a small update was required (e.g. a small update to the Onboarding service would end up updating every service).
The new enhanced update implementation relies on git and the pre-existing Version parameters to determine exactly what changed, only updating what is necessary. In particular this leverages the pre-existing update scripts for each service, function, layer, and custom resource (now all aligned to be named
update.sh). In practice this means the average update should take significantly less time than before.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license