dotnet / aspire-samples

MIT License
571 stars 158 forks source link

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

Open Menghua1 opened 2 months ago

Menghua1 commented 2 months ago

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: image

Repro Steps:

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

Environment:

Expected behavior: azd up command can be executed successfully.

@rajeshkamal5050, @vhvb1989 for notification.

v-sherryfan commented 2 months ago

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 commented 2 months ago

@weikanglim is investigating this issue.

rajeshkamal5050 commented 2 months ago

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

weikanglim commented 2 months ago

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 commented 2 months ago
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 commented 2 months ago

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 commented 2 months ago

@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 commented 2 months ago

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 commented 2 months ago

@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 commented 2 months ago

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 commented 2 months ago

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

Menghua1 commented 2 months ago

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