Skip to content
This repository was archived by the owner on Dec 9, 2024. It is now read-only.

Conversation

@tbarlow12
Copy link
Contributor

@tbarlow12 tbarlow12 commented Jun 28, 2019

High Level Overview

  • Uploads a timestamped archive of code to Azure Blob Storage
  • Adds same timestamp to ARM deployments to enable linking
  • Does not set WEBSITE_RUN_FROM_PACKAGE to use package directly from blob storage
    • Currently just used as an archive. Initially, thought to update the WEBSITE_RUN_FROM_PACKAGE setting with the URL for the blob (hence the implementation of the SAS URL generator), but discovered issues with cold start when using that methodology to deploy to Windows.
    • Future rollback implementation will download the appropriate archive and re-deploy it to the Function App.
    • This may (and probably will) change as the Azure Functions Premium plan is released on Linux or cold start issues are mitigated

Details

  • Adds dependency on the @azure/arm-storage package
  • Sets rollbackEnabled as default
  • Updates broken tests because of timestamped deployment name
  • Add tests for blob storage operations
  • Allows user to still specify deployment name with rollback

Resolves [AB#388], [AB#358], [AB#414] and [AB#415]

@tbarlow12 tbarlow12 closed this Jun 28, 2019
@tbarlow12 tbarlow12 reopened this Jun 28, 2019
Copy link
Contributor

@PIC123 PIC123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Great progress with the rollback, and standardizing on blobstore uploads makes things much more extensible

/**
* Initialize deployment artifact container if rollback is specified
*/
public async initialize() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this unnecessary because of the added params in the constructor?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than initializing the blob storage here, it initializes before actually creating the blob. The problem was it was trying to initialize and create a container for an account that didn't yet exist

Copy link
Contributor

@mydiemho mydiemho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in description, you said Future rollback implementation will download the appropriate archive and re-deploy it to the Function App.. So what is the current implementation?

/**
* Uploads artifact file to blob storage container
*/
private async uploadFunctionAppToBlobStorage() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would call this uploadZipArtifactToBlobStorage or uploadFunctionArtifactToBlobStorage

this.log("Deploying serverless functions...");

await this.zipDeploy(functionApp);
await this.uploadFunctionAppToBlobStorage();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

commented suggestion below but this name is kinda misleading, both are zip deploy, the only difference is zip storage location

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also why are we uploading to both place? what is the value of RUN_FROM_PACKAGE?

@tbarlow12 tbarlow12 merged commit 659282e into dev Jul 1, 2019
@tbarlow12 tbarlow12 deleted the tabarlow/code-deploy branch July 1, 2019 21:09
tbarlow12 added a commit that referenced this pull request Sep 13, 2019
## High Level Overview
- Uploads a timestamped archive of code to Azure Blob Storage
- Adds same timestamp to ARM deployments to enable linking
- **Does not set `WEBSITE_RUN_FROM_PACKAGE` to use package directly from blob storage**
  - Currently just used as an archive. Initially, thought to update the `WEBSITE_RUN_FROM_PACKAGE` setting with the URL for the blob (hence the implementation of the SAS URL generator), but discovered [issues with cold start when using that methodology to deploy to Windows](https://docs.microsoft.com/en-us/azure/azure-functions/run-functions-from-deployment-package#enabling-functions-to-run-from-a-package).
  - Future `rollback` implementation will download the appropriate archive and re-deploy it to the Function App.
  - This may (and probably will) change as the Azure Functions Premium plan is released on Linux or cold start issues are mitigated

## Details

- Adds dependency on the `@azure/arm-storage` package
- Sets `rollbackEnabled` as default
- Updates broken tests because of timestamped deployment name
- Add tests for blob storage operations
- Allows user to still specify deployment name with rollback

Resolves [AB#388], [AB#358], [AB#414] and [AB#415]
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants