-
Notifications
You must be signed in to change notification settings - Fork 686
Generation of Dapr-specific manifest resources #862
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
Generation of Dapr-specific manifest resources #862
Conversation
Signed-off-by: Phillip Hoff <[email protected]>
Signed-off-by: Phillip Hoff <[email protected]>
samples/dapr/AppHost/Program.cs
Outdated
var builder = DistributedApplication.CreateBuilder(args); | ||
|
||
builder.AddDapr(); | ||
|
||
string resourcesRelativePath = @"../Resources"; | ||
string stateStoreRelativePath = Path.Combine(resourcesRelativePath, "statestore.yaml"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will be problematic depending on where you run from. I wonder if there's anything we can do here to make this stable. Maybe an env variable in launch settings?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The intent was that component file paths be relative to the referenced project file (directory), which would be similar to how paths are expressed when using Dapr run files. That still could problematic if the same component is referenced by multiple projects but where those projects are not at the same relative location in the solution. A environment/configuration value might be ok...I'll think about it a bit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We made a fix in my latest PR. You can get the apphost project directory from the builder so it's stable no matter where you run from. This can be a follow up PR.
src/Aspire.Hosting.Dapr/IDistributedApplicationBuilderExtensions.cs
Outdated
Show resolved
Hide resolved
Signed-off-by: Phillip Hoff <[email protected]>
Signed-off-by: Phillip Hoff <[email protected]>
Signed-off-by: Phillip Hoff <[email protected]>
Signed-off-by: Phillip Hoff <[email protected]>
Signed-off-by: Phillip Hoff <[email protected]>
Signed-off-by: Phillip Hoff <[email protected]>
Signed-off-by: Phillip Hoff <[email protected]>
Signed-off-by: Phillip Hoff <[email protected]>
src/Aspire.Hosting.Dapr/DaprDistributedApplicationLifecycleHook.cs
Outdated
Show resolved
Hide resolved
src/Aspire.Hosting.Dapr/DaprDistributedApplicationLifecycleHook.cs
Outdated
Show resolved
Hide resolved
Signed-off-by: Phillip Hoff <[email protected]>
Signed-off-by: Phillip Hoff <[email protected]>
src/Aspire.Hosting/Lifecycle/IDistributedApplicationLifecycleHook.cs
Outdated
Show resolved
Hide resolved
Signed-off-by: Phillip Hoff <[email protected]>
Signed-off-by: Phillip Hoff <[email protected]>
Signed-off-by: Phillip Hoff <[email protected]>
Signed-off-by: Phillip Hoff <[email protected]>
Signed-off-by: Phillip Hoff <[email protected]>
Signed-off-by: Phillip Hoff <[email protected]>
Signed-off-by: Phillip Hoff <[email protected]>
Signed-off-by: Phillip Hoff <[email protected]>
src/Aspire.Hosting.Dapr/IDistributedApplicationBuilderExtensions.cs
Outdated
Show resolved
Hide resolved
This PR is looking pretty complete. Let's try to get this in today or tomorrow @philliphoff. |
Signed-off-by: Phillip Hoff <[email protected]>
Signed-off-by: Phillip Hoff <[email protected]>
Signed-off-by: Phillip Hoff <[email protected]>
@philliphoff Any paths specified need to be resolved as relative to the manifest. There's a new helper for that btw. |
I'll take a look at that. |
Also see 098c33b |
Signed-off-by: Phillip Hoff <[email protected]>
builder.AddProject<Projects.DaprServiceA>("servicea") | ||
.WithDaprSidecar("service-a"); | ||
.WithDaprSidecar("service-a") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like you've got an overloaded version of the WithReference
extension method. Given you are doing that I'm wondering whether we need the WithDaprSidecar
call? Couldn't your custom implementation of `WithReference check to see that the annotation is present (and could the name be derived?).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean that, when referencing components, that the need for the sidecar can be inferred?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah that is what I was thinking. Just removing a bit of (potentially) unnecessary ceremony?
@@ -8,6 +8,7 @@ | |||
</PropertyGroup> | |||
|
|||
<ItemGroup> | |||
<PackageReference Include="Dapr.AspNetCore" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might not be necessary in the case of Dapr, but I am wondering whether we should be creating a Aspire.Dapr
package for the service side like we do with other Aspire components. For example should we be automatically wiring up some telemetry and healthchecks for dapr interactions within the service code for consistency.
Adding @eerhardt for his thoughts on this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there are definitely opportunities to create Aspire components specific to Dapr, that wrap the Dapr .NET SDK is ways that make sense to tie into Aspire features (service discovery, perhaps?). I haven't had an opportunity to look closely at Aspire components and what it really means to create one. For example, Dapr already ties into Open Telemetry at the sidecar level, which is ultimately doing everything on behalf of the application, so I don't know if there would be anything specific to wrap in the SDK.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK ... we don't need to solve it in this PR I don't think given Dapr already integrates very well with DI.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@davidfowl I'm inclined to approve this PR now and get it in so we can work with the deployment tool folks to see if we can get this working end to end. |
@davidfowl @mitchdenny Is there any assumption that the manifest is always generated in the app host directory (as seems to be the case by default, anyway). If not, then that would mean relative paths specified in the I guess the best practice would be for users to always pass full paths (based on |
Paths in the manifest are relative to the manifest file. We just recently updated the API surface for manifest generation to have a context which exposes a |
Right, but say a developer specifies a relative path in an options object. When using Aspire to run the app, it seems reasonable for that path to be relative to the app host directory. However, when using Aspire to generate the manifest, and that manifest is configured to be placed in a different directory than the app host, that relative path no longer works; it would need to be converted. While that's certainly possible, the places where manifest writing happens are part of annotations and not necessarily in a place to easily acquire the app host directory from the DI container. I wonder if the Or, just tell developers to not use relative paths in Aspire APIs which avoids the issue. |
This should still work. With your current code it'll work since you are first resolving paths relative to the apphost, then using GetMainfestRelativePath.
It can, and will. We can merge this PR 😄 |
Customizes the generation of manifest resources for Dapr (sidecar) resources, exposing configured options, which is a prerequisite for tools to deploy Dapr-enabled Aspire applications (such as Azure Dev CLI to deploy to Azure Container Apps).
Customizes the generation of manifest resources for Dapr (sidecar) resources, exposing configured options, which is a prerequisite for tools to deploy Dapr-enabled Aspire applications (such as Azure Dev CLI to deploy to Azure Container Apps).
Dapr Component Resources
This change includes the addition of a new "Dapr component" resource that represents an individual Dapr component, such as a state store or pub-sub.
Components can be specified explicitly, for example:
Alternatively, components can be specified in "generic form", for example:
The generic form is useful in that it allows Aspire (and/or deployment tooling) to automatically provision and configure an appropriate Dapr component for the generic type. When running locally, for example, the following components will be initialized:
"state"
)"pubsub"
)Options (such as the path to the local configuration file for the component) can also be specified, for example:
Dapr component resources can then be associated with one or more individual projects (that are also associated with Dapr sidecars). In doing so, Aspire will automatically configure the appropriate sidecars to load those components (rather than require explicitly setting the "resources paths" of the sidecar).
For example:
Dapr Component Resource Manifest JSON
Dapr components are written to the manifest as individual resources of type
dapr.component.v0
with specific options in adaprComponent
object.Dapr Sidecar Resource Manifest JSON
Dapr sidecars are written to the manifest as individual resources of type
dapr.v0
with specific options in adapr
object. Of note are theapplication
andcomponents
properties, which store references to the project associated with the sidecar and all the components associated with that project, respectively. Note also that external tools (e.g. deployment tooling) may only use a subset of options, depending on the scenario (e.g.resourcesPath
andrunFIle
are both not applicable to Azure Container Apps).Future work
In cases where no existing Dapr-provisioned state store or pub-sub exists (e.g.
dapr init --slim
), rather than use the limited in-memory components, Aspire might instead explicitly provision a Redis (or similar) container. This makes additional assumptions, however, such as the existence of Docker host.Related to: