Fail to run "azd up" for sample "AspireShop"

[Menghua1]

Describe the issue:
Run the command azd up and get stuck at the azd deploy step. There is no response for a long time. The page is as follows:

Repro Steps:

1. Clone code.
2. Run command cd samples/AspireShop.
3. Run command azd init.
4. Run command azd up.

Environment:

• Azd version: azd version 1.10.0-beta.1-daily.3780018 (commit bb526cbd20cfbd9d2ebadc2e34f1058e8595ff4f)
• Aspire version: 8.0.0-preview.7.24251.11
• Sample: AspireShop
• Branch: main

Expected behavior:
azd up command can be executed successfully.

@rajeshkamal5050, @vhvb1989 for notification.

[v-sherryfan]

not repro: azd version 1.10.0-beta.1-daily.3768065 + Aspire 8.0.0
repro: azd version 1.10.0-beta.1-daily.3780018 + Aspire 8.0.0

[rajeshkamal5050]

@weikanglim is investigating this issue.

[rajeshkamal5050]

Fix out Revert "containerapp: Wait for revision to be ready" #3901

[weikanglim]

We might see a different failure when manual testing the app even after the revert. It looks like the revision did indeed fail:

Container 'basketcache' was terminated with exit code '' and reason 'VolumeMountFailure'

[rajeshkamal5050]

Container 'basketcache' was terminated with exit code '' and reason 'VolumeMountFailure'

@vhvb1989 @weikanglim are suspecting that with the fix for mounting volumes the limitations around container volumes is impacting the AspireShop sample when using WithDataVolume() i.e., azure files with SMB

@davidfowl @DamianEdwards @mitchdenny @ellismg any thoughts?

[DamianEdwards]

Yes I expect that AspireShop won't work due to it using WithDataVolume() on a PostgreSQL resource, which we know doesn't work on ACA. We could update the sample to conditionally call it only when running and not when publishing so that it can still be published, but it will of course behave differently and complicates the code.

[vhvb1989]

@DamianEdwards , I am thinking on having a switch on azd (like an ENV VAR) to override settings for the WithDataVolume() default behavior on azd (create storage account + fileShare SBM + envContainer File mount + aca volume + container mount).

But, ideally, can we have the api WithDataVolume() to take more config parameters? Or a decorator like .PublishAsNFS().
Having those settings in the manifest would be ideal to keep the source of truth on AppHost program (w/o conditioning the volume, but adding the exact config required for adding a volume for each case)

[DamianEdwards]

Eventually I think we'd add support for customizing volume aspects that are specific to ACA and the target environment via the CDK work. These kinds of aspects don't seem generic enough in my mind to promote them to the general volume modeling.

[rajeshkamal5050]

@DamianEdwards Assuming we are publishing the AspireShop sample. Can we for build/GA add the dev/run mode in apphost program with comments? So users are aware and it doesn't fail during deployment.

[DamianEdwards]

The issue with that is that it assumes the app is only deployed to ACA via AZD. I think adding a comment that describes the limitations in that case is OK, but complicating the sample based on specifics regarding volumes in ACA and certain containers seems like a bad precedent.

[v-sherryfan]

Not repro on VS 17.10 GA FB + latest daily azd [1.10.0-beta.1-daily.3787247]

[Menghua1]

In the latest round of testing, the issue no longer exists.