aspire-samples icon indicating copy to clipboard operation
aspire-samples copied to clipboard

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

Open Menghua1 opened this issue 1 year ago • 12 comments

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:

  • 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.

Menghua1 avatar May 13 '24 09:05 Menghua1

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

v-sherryfan avatar May 13 '24 11:05 v-sherryfan

@weikanglim is investigating this issue.

rajeshkamal5050 avatar May 13 '24 18:05 rajeshkamal5050

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'

weikanglim avatar May 14 '24 00:05 weikanglim

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?

rajeshkamal5050 avatar May 14 '24 06:05 rajeshkamal5050

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.

DamianEdwards avatar May 14 '24 17:05 DamianEdwards

@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)

vhvb1989 avatar May 14 '24 21:05 vhvb1989

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.

DamianEdwards avatar May 14 '24 22:05 DamianEdwards

@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.

rajeshkamal5050 avatar May 14 '24 22:05 rajeshkamal5050

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.

DamianEdwards avatar May 14 '24 22:05 DamianEdwards

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

v-sherryfan avatar May 15 '24 08:05 v-sherryfan

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

Menghua1 avatar May 16 '24 09:05 Menghua1

This was fixed in #536

DamianEdwards avatar Nov 08 '24 01:11 DamianEdwards