Skip to content

Add new step to automatically verify if the deployed container app started successfully#909

Merged
MicroFish91 merged 50 commits intomainfrom
mwf/inevitable-salmon
Jul 25, 2025
Merged

Add new step to automatically verify if the deployed container app started successfully#909
MicroFish91 merged 50 commits intomainfrom
mwf/inevitable-salmon

Conversation

@MicroFish91
Copy link
Copy Markdown
Contributor

@MicroFish91 MicroFish91 commented Jul 17, 2025

Closes #906

image image

@MicroFish91 MicroFish91 changed the title Mwf/inevitable salmon Verify that deployment hasn't errored at container runtime and silently fallen back to a previous revision Jul 18, 2025
@MicroFish91 MicroFish91 force-pushed the mwf/inevitable-salmon branch from 6b23b6e to d5abab8 Compare July 18, 2025 17:49
@MicroFish91 MicroFish91 marked this pull request as ready for review July 18, 2025 20:49
@MicroFish91 MicroFish91 changed the base branch from main to meganmott/prodFixes July 21, 2025 18:48
Base automatically changed from meganmott/prodFixes to main July 22, 2025 20:55
Comment thread src/commands/EXECUTE_PRIORITY.md Outdated
Comment thread src/commands/createContainerApp/ContainerAppCreateStep.ts
Comment thread src/commands/deployContainerApp/deployContainerApp.ts Outdated
Comment thread src/commands/image/imageSource/ContainerAppStartVerificationStep.ts
Comment thread src/commands/image/imageSource/ContainerAppStartVerificationStep.ts Outdated
Comment thread src/commands/image/imageSource/ContainerAppStartVerificationStep.ts Outdated
Comment thread src/commands/image/imageSource/ContainerAppStartVerificationStep.ts
Comment thread src/commands/image/imageSource/ContainerAppStartVerificationStep.ts
return !!context.containerApp;
}

public addExecuteSteps(): AzureWizardExecuteStep<T>[] {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is there any reason to not just add this step upfront when we create the wizard?

Copy link
Copy Markdown
Contributor Author

@MicroFish91 MicroFish91 Jul 23, 2025

Choose a reason for hiding this comment

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

I was thinking, if this is just something that should always happen on create or update, there's no need to increase cognitive overhead where people need to remember to add this step. This just couples it directly to the steps that need it, and then no one ever needs to think about it again.

The main argument against it in my mind is that it obfuscates some of the logic where you can't just easily look at the wizard steps on the main command page to figure out this is being run. That said, we already obfuscate a lot of steps like that with subWizards, so I figured it's not all that different.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That said, if the assumption of it always needing to run after create or update is wrong, then we should definitely decouple it. It's fairly low cost either way.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yeah, I can't think of many great arguments for or against. I was mostly thinking this as well:

The main argument against it in my mind is that it obfuscates some of the logic where you can't just easily look at the wizard steps on the main command page to figure out this is being run.

While we do end up having to obfuscate some of these steps, they all tend to be conditional subwizards which unfortunately, we can't help but do.

Comment thread src/utils/azureClients.ts Outdated
@nturinski
Copy link
Copy Markdown
Member

The log message and Copilot's response is pretty smexy though 🫦

@MicroFish91
Copy link
Copy Markdown
Contributor Author

I added most of the feedback, I need to test all the scenarios again just to double check everything is still good, but probably ready to review again for the most part

@alexweininger
Copy link
Copy Markdown
Member

Yeah this is awesome

@MicroFish91 MicroFish91 force-pushed the mwf/inevitable-salmon branch from d32361c to 982ba44 Compare July 23, 2025 20:54
Comment thread package-lock.json
"@azure/storage-blob": "^12.4.1",
"@fluentui/react-components": "^9.56.2",
"@fluentui/react-icons": "^2.0.265",
"@microsoft/vscode-azext-azureutils": "^3.3.3",
"@microsoft/vscode-azext-github": "^1.0.2",
"@microsoft/vscode-azext-utils": "^3.2.1",
"@microsoft/vscode-azext-utils": "^3.3.0",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Whoa.. any idea why there are 7273 lines of code deleted? Kind of wild.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's from Azure Monitor Query, it blew my mind too, I don't really understand how it led to so many lines removed lol

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Did it change the lockfile version?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I don't think so

Copy link
Copy Markdown
Member

@nturinski nturinski 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! Really excited about this feature tbh

@MicroFish91 MicroFish91 merged commit 342c37d into main Jul 25, 2025
3 checks passed
@MicroFish91 MicroFish91 deleted the mwf/inevitable-salmon branch July 25, 2025 20:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Deployment silently fails but shows up as a success if image builds but errors at runtime

4 participants