-
Notifications
You must be signed in to change notification settings - Fork 208
Reimplement Lambda deployment QUICK_SYNC strategy #1443
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
Changes from all commits
6a1da4a
40ee772
f3e6fe8
8bb1f52
91ade99
cdefecb
8bff2b1
9d52465
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,6 +16,8 @@ package lambda | |
|
|
||
| import ( | ||
| "context" | ||
| "errors" | ||
| "time" | ||
|
|
||
| provider "github.com/pipe-cd/pipe/pkg/app/piped/cloudprovider/lambda" | ||
| "github.com/pipe-cd/pipe/pkg/app/piped/deploysource" | ||
|
|
@@ -82,16 +84,70 @@ func decideRevisionName(in *executor.Input, fm provider.FunctionManifest, commit | |
| return | ||
| } | ||
|
|
||
| func apply(ctx context.Context, in *executor.Input, cloudProviderName string, cloudProviderCfg *config.CloudProviderLambdaConfig, fm provider.FunctionManifest) bool { | ||
| func sync(ctx context.Context, in *executor.Input, cloudProviderName string, cloudProviderCfg *config.CloudProviderLambdaConfig, fm provider.FunctionManifest) bool { | ||
| in.LogPersister.Infof("Start applying the lambda function manifest") | ||
| client, err := provider.DefaultRegistry().Client(cloudProviderName, cloudProviderCfg, in.Logger) | ||
| if err != nil { | ||
| in.LogPersister.Errorf("Unable to create Lambda client for the provider %s: %v", cloudProviderName, err) | ||
| return false | ||
| } | ||
|
|
||
| if err := client.Apply(ctx, fm); err != nil { | ||
| in.LogPersister.Errorf("Failed to apply the lambda function manifest (%v)", err) | ||
| found, err := client.IsFunctionExist(ctx, fm.Spec.Name) | ||
| if err != nil { | ||
| in.LogPersister.Errorf("Unable to validate function name %s: %v", fm.Spec.Name, err) | ||
| return false | ||
| } | ||
| if found { | ||
| if err := client.UpdateFunction(ctx, fm); err != nil { | ||
| in.LogPersister.Errorf("Failed to update lambda function %s: %v", fm.Spec.Name, err) | ||
| return false | ||
| } | ||
| } else { | ||
| if err := client.CreateFunction(ctx, fm); err != nil { | ||
| in.LogPersister.Errorf("Failed to create lambda function %s: %v", fm.Spec.Name, err) | ||
| return false | ||
| } | ||
| } | ||
|
|
||
| // TODO: Using backoff instead of time sleep waiting for a specific duration of time. | ||
| // Wait before ready to commit change. | ||
| in.LogPersister.Info("Waiting to update lambda function in progress...") | ||
| time.Sleep(3 * time.Minute) | ||
|
|
||
| // Commit version for applied Lambda function. | ||
| // Note: via the current docs of [Lambda.PublishVersion](https://docs.aws.amazon.com/sdk-for-go/api/service/lambda/#Lambda.PublishVersion) | ||
| // AWS Lambda doesn't publish a version if the function's configuration and code haven't changed since the last version. | ||
| // But currently, unchanged revision is able to make publish (versionId++) as usual. | ||
| version, err := client.PublishFunction(ctx, fm) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm concerned that when users clicked the SYNC button to sync the application without any changes.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yes, you guess right, due to the docs of PublishVersion, we have to make some changes on source/configuration of the function to be able to publish a new version of it. So the error returned here for that case is predictable. 👍
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added a note about this lack on the AWS documentation. |
||
| if err != nil { | ||
| in.LogPersister.Errorf("Failed to commit new version for Lambda function %s: %v", fm.Spec.Name, err) | ||
| return false | ||
| } | ||
|
|
||
| _, err = client.GetTrafficConfig(ctx, fm) | ||
| // Create Alias on not yet existed. | ||
| if errors.Is(err, provider.ErrNotFound) { | ||
| if err := client.CreateTrafficConfig(ctx, fm, version); err != nil { | ||
| in.LogPersister.Errorf("Failed to create traffic routing for Lambda function %s (version: %s): %v", fm.Spec.Name, version, err) | ||
| return false | ||
| } | ||
| in.LogPersister.Infof("Successfully applied the lambda function manifest") | ||
| return true | ||
| } | ||
| if err != nil { | ||
| in.LogPersister.Errorf("Failed to prepare traffic routing for Lambda function %s: %v", fm.Spec.Name, err) | ||
| return false | ||
| } | ||
|
|
||
| // Update 100% traffic to the new lambda version. | ||
| routingCfg := []provider.VersionTraffic{ | ||
| { | ||
| Version: version, | ||
| Percent: 100, | ||
| }, | ||
| } | ||
| if err = client.UpdateTrafficConfig(ctx, fm, routingCfg); err != nil { | ||
| in.LogPersister.Errorf("Failed to update traffic routing for Lambda function %s (version: %s): %v", fm.Spec.Name, version, err) | ||
| return false | ||
| } | ||
|
|
||
|
|
||
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.
fyi, the Lambda's
CreateFunction/UpdateFunctionAPI call returns right after we make a request to update the Lambda function code/config, but the update process still running internally in Lambda servers. If we makePublishFunctionrequest immediately, the errorResourceConflictException: The operation cannot be performed at this time.will be raised.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.
Really? 😢 I think fixed 3 minutes is not safe for all cases, but we don't know how much is good.
So I think in addition to waiting for a fixed amount of time (e.g. 1m) we should also do
PublishFunctionwith a constant backoff.https://github.com/pipe-cd/pipe/tree/master/pkg/backoff
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.
/ping
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'm working on it, but shall we make another PR to fix this separately from this PR 👀
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.
Yes, that is ok. Let's add a TODO and do it in another PR.