Skip to content
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

Disable async deployment in function_app_deploy #322

Merged
merged 26 commits into from
Mar 12, 2025

Conversation

lucacavallaro
Copy link
Member

By default, the azure/webapps-deploy action uses the async deploy mode that ends the workflow step once the artifact is pushed onto the function app, without waiting for its restart or any successful health probe.

This behavior makes the Ping Staging Health step unreliable, as it is being executed before the actual deployment is finished, nullifying its purpose and causing unwanted swaps.

Note that, in order to work, the target function app must declare the following environment variables:

  • WEBSITE_SWAP_WARMUP_PING_PATH - a public route that the swapping process use to make sure that the source slot is healthy
  • WEBSITE_SWAP_WARMUP_PING_STATUSES - the successful status code returned by the previous mapped route (es. 200)

See the Azure documentation for more info https://learn.microsoft.com/en-us/azure/app-service/deploy-staging-slots?tabs=portal

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
By default, the `azure/webapps-deploy` action uses the async deploy mode that ends the workflow step once the artifact is pushed onto the function app, without waiting for its restart or any successful health probe.

This behavior makes the `Ping Staging Health` step unreliable, as it is being executed before the actual deployment is finished, nullifying its purpose and causing unwanted swaps.

Note that, in order to work, the target function app must declare the following environment variables:
* `WEBSITE_SWAP_WARMUP_PING_PATH` - a public route that the swapping process use to make sure that the source slot is healthy
* `WEBSITE_SWAP_WARMUP_PING_STATUSES` - the successful status code returned by the previous mapped route (es. 200)

See the Azure documentation for more info https://learn.microsoft.com/en-us/azure/app-service/deploy-staging-slots?tabs=portal
@lucacavallaro lucacavallaro requested a review from a team as a code owner February 26, 2025 16:59
Copy link

changeset-bot bot commented Feb 26, 2025

⚠️ No Changeset found

Latest commit: e139e35

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@gunzip
Copy link
Contributor

gunzip commented Feb 26, 2025

can we make the workflow fail fast if WEBSITE_SWAP_WARMUP_PING_PATH and WEBSITE_SWAP_WARMUP_PING_STATUSES are not set? maybe we can get them using cli

@gunzip gunzip self-requested a review February 26, 2025 19:41
@Krusty93
Copy link
Contributor

Krusty93 commented Feb 28, 2025

I like the changes, but how we make sure the two environment variables are set? We should add them in DX module as well

EDIT: I've seen now #323. Remember to notify developers that they need the latest TF module version if they want to use this workflow template

Copy link
Contributor

@gunzip gunzip left a comment

Choose a reason for hiding this comment

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

add a mechanism to check if variables are set in the environment and fail with a explicit message

kin0992 and others added 20 commits March 12, 2025 11:03

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Explain the issues faced when instrumenting an `ESM` application using
OpenTelemetry

Fixes #CES-783

---------

Co-authored-by: Danilo Spinelli <[email protected]>
Co-authored-by: Danilo Spinelli <[email protected]>
Enhance the documentation for AI to ensure better understanding and
uniformity across the content.

Closes CES-733

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Set `var.health_check_path` as custom warm-up path to constraint the
swap operation only when the source slots is healthy.
On March 31, 2025, support for instrumentation key ingestion will end

Resolves #CES-723
Add support for semver version ranges (e.g. ~2.4) in turbo version detection
When tier is set to `s`, but AI is enabled, the plan fails because it
assumes the APIM logger exists.
With this change, when AI is enabled, then the module creates an APIM
logger as well

Closes #273 #CES-799

---------

Co-authored-by: Mario Mupo <[email protected]>
#252)

This pull request aims to **align the sample rate settings for both the
Host and Worker** in the Azure Functions module's AI tracing. The
changes ensure consistency and potentially improve the reliability of
Application Insights sampling.

---------

Co-authored-by: Danilo Spinelli <[email protected]>
Co-authored-by: Danilo Spinelli <[email protected]>
)

Add documentation to guide developers in managing IAM setup among
resources of different Azure subscriptions

Closes #CES-825

---------

Co-authored-by: Danilo Spinelli <[email protected]>
<!--- Please always add a PR description as if nobody knows anything
about the context these changes come from. -->
<!--- Even if we are all from our internal team, we may not be on the
same page. -->
<!--- Write this PR as you were contributing to a public OSS project,
where nobody knows you and you have to earn their trust. -->
<!--- This will improve our projects in the long run! Thanks. -->

### List of changes

<!--- Describe your changes in detail -->

Add documentation to explain how and why IAM DX framework was designed

### Motivation and context

<!--- Why is this change required? What problem does it solve? -->

Making everyone understands how the framework was designed and how to
exploit it

### Type of changes

- [ ] Add new resources
- [X] Update configuration to existing resources
- [ ] Remove existing resources

### Does this introduce a change to production resources with possible
user impact?

- [ ] Yes, users may be impacted applying this change
- [X] No

### Other information

<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->

---------

Co-authored-by: Mario Mupo <[email protected]>
Co-authored-by: Danilo Spinelli <[email protected]>
…340)

Update the Azure IAM documentation to enhance clarity.
The `connection_string` for the `azurerm_api_management_logger` resource
has been introduced in version `4.1.0` of the provider

Fixes #CES-809
Corrected typographical errors and updated module versioning in the
Azure IAM documentation for clarity and accuracy.
)

<!--- Please always add a PR description as if nobody knows anything
about the context these changes come from. -->
<!--- Even if we are all from our internal team, we may not be on the
same page. -->
<!--- Write this PR as you were contributing to a public OSS project,
where nobody knows you and you have to earn their trust. -->
<!--- This will improve our projects in the long run! Thanks. -->

### List of changes

<!--- Describe your changes in detail -->
Added new modules for container app and container app environment.
Minor change in core module with new Private DNS Zone for container app.

### Motivation and context

<!--- Why is this change required? What problem does it solve? -->

### Type of changes

- [x] Add new resources
- [x] Update configuration to existing resources
- [ ] Remove existing resources

### Does this introduce a change to production resources with possible
user impact?

- [ ] Yes, users may be impacted applying this change
- [x] No

### Other information

<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->
Resolves: #CES-739

---------

Co-authored-by: Christian Calabrese <[email protected]>
Add documentation to workaround the issue of hidden AppSettings in
AppService and FunctionApp

Closes #CES-740
#342)

Update sidebar labels throughout the documentation to enhance clarity
and maintain consistency in terminology.
Fix broken link in "Ensuring Azure AppSettings Visibility in Terraform
Plans" doc.
This helps the navigation on the Azure Portal, because it links the
resource

Closes #CES-839
kin0992 and others added 3 commits March 12, 2025 11:03
Introduce guidelines for managing pull requests to promote clarity and
consistency.
Following these guidelines will help streamline the review process and
improve communication within the team

Closes #CES-758

---------

Co-authored-by: Danilo Spinelli <[email protected]>
@lucacavallaro lucacavallaro force-pushed the fixes/legacy-workflow-azcli branch from f089759 to e139e35 Compare March 12, 2025 16:37
@lucacavallaro lucacavallaro merged commit b51b474 into main Mar 12, 2025
3 checks passed
@lucacavallaro lucacavallaro deleted the fixes/legacy-workflow-azcli branch March 12, 2025 16:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants