-
Notifications
You must be signed in to change notification settings - Fork 160
feat: Rollback Plugin #191
Conversation
src/services/armService.ts
Outdated
| const parameterValue = deployment.parameters[key]; | ||
| if (parameterValue) { | ||
| deploymentParameters[key] = { value: deployment.parameters[key] }; | ||
| deploymentParameters[key] = (typeof parameterValue === "string") ? { value: parameterValue } : { value: parameterValue.value } |
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.
@wbreza When I get the ARM template from the previous deployment, the parameter values look like:
{
type: "String",
value: "value1"
}
When I just used that object rather than extracting the value, it fails. I guess it doesn't like the type attribute for some reason. Do you have any cleaner ways for how to do this?
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 deployTemplate(...) is called it is called with deployment params of simple key/value pairs and then converted into the final format ARM templates expect by default. What I imagine is happening is you are passing the params as already final constructed ARM params from the pre-existing deployment.
wbreza
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.
Looking good! Added a few questions and suggestions.
| expect(sls.cli.log).lastCalledWith(expectedLogStatement); | ||
| }); | ||
|
|
||
| it("lists deployments without timestamps", async () => { |
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.
Why would a deployment NOT have a timestamp?
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 think i see now - You are interpolating it from the file name - not the timestamp from the actual deployment?
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 "stamp" it when the package is being created. I wanted to use the timestamp from the deployment, but since someone "could" have the same deployment name, I'd have to scan through all deployments to check which one was most recent. It seemed more safe to just include the timestamp in the name of the deployment. I checked the response from the deployment of the resource group, and the only property included is the provisioning state
src/services/armService.ts
Outdated
| const parameterValue = deployment.parameters[key]; | ||
| if (parameterValue) { | ||
| deploymentParameters[key] = { value: deployment.parameters[key] }; | ||
| deploymentParameters[key] = (typeof parameterValue === "string") ? { value: parameterValue } : { value: parameterValue.value } |
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 deployTemplate(...) is called it is called with deployment params of simple key/value pairs and then converted into the final format ARM templates expect by default. What I imagine is happening is you are passing the params as already final constructed ARM params from the pre-existing deployment.
src/services/functionAppService.ts
Outdated
| await this.uploadZippedArfifactToFunctionApp(functionApp); | ||
| await this.uploadZippedArtifactToBlobStorage(); | ||
| const functionZipFile = this.getFunctionZipFile(); | ||
| await this.uploadZippedArfifactToFunctionApp(functionApp, functionZipFile); |
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.
Can these be done in parallel via Promise.all() ??
| } | ||
|
|
||
| private async uploadZippedArfifactToFunctionApp(functionApp) { | ||
| public async uploadZippedArfifactToFunctionApp(functionApp: Site, functionZipFile: string) { |
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.
why switching to public? Needed?
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.
It's used inside the rollback service now. to re-deploy function zip to the function app
src/services/rollbackService.ts
Outdated
| const { template } = await resourceService.getDeploymentTemplate(deployment.name); | ||
| return { | ||
| template, | ||
| parameters: deployment.properties.parameters, |
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.
If you convert the params back to the expected format here you can remove the special case in the ArmService that you had asked me about.
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.
will do. thanks
7d84091 to
ce90f3f
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.
Looks good to me! Still testing it out fully locally, and you have some conflicts that need to be resolved first, but everything seems good! Very excited to have this feature out
ce90f3f to
057c446
Compare
## Updates - Adds new `rollback` plugin - Fixes `deploy list` to show actual timestamp from name, which is used for rollback - Adds required functionality to `AzureBlobStorageService` and `ArmService` - Adds previously missing tests of required functionality ## How to Use 1. Get a timestamp. Use ``` sls deploy list ``` to retrieve one. 2. Roll back function app to timestamp ``` sls rollback -t <timestamp> ``` ## Options In `deploy` section of `serverless.yml`, you can specify whether to run from a blob URL or deploy the package directly to the function app by setting: ```yaml deploy: # Default is false runFromBlobUrl: true|false ``` Resolves [AB#317]
Updates
rollbackplugindeploy listto show actual timestamp from name, which is used for rollbackAzureBlobStorageServiceandArmServiceHow to Use
to retrieve one.
2. Roll back function app to timestamp
Options
In
deploysection ofserverless.yml, you can specify whether to run from a blob URL or deploy the package directly to the function app by setting:Resolves [AB#317]