Support proxyless singleton containers

[bwateratmsft]

/cc @karolz-ms

This change works with a corresponding change in DCP to support proxyless aka headless services. The ApplicationExecutor will choose containers which consume no services and start them up without a proxy in front. The Service object assumes the address and port of the container itself and no proxy process is started.

[davidfowl]

Don't we need to handle versioning now?

[eerhardt]

Don't we need to handle versioning now?

Versioning of what? Supporting the preview1 DCP with a preview2 Hosting package?

[davidfowl]

I'm assuming this feature of Hosting requires a new DCP API (or option on the API), if that option is specified when an older DCP is running what will happen? Is it a noop?

[bwateratmsft]

An old version of DCP given this new argument will throw an error.

[karolz-ms]

@davidfowl what is your expectation? That the user gets a "you need new DCP" error if they try to run the application?

[davidfowl]

Wonderful, then we get a chance to practice versioning. How do we make this experience smooth for people moving from preview1 to preview2 without exploding 😄. Can we detect the version and do good things? (like not set this option?)

[karolz-ms]

@DamianEdwards FYI

[eerhardt]

@davidfowl what is your expectation? That the user gets a "you need new DCP" error if they try to run the application?

This would be a good experience IMO. As long as the error message wasn't cryptic and explicitly told me I needed a new version of the workload. (I don't think any customer should ever see or hear of "DCP").

[davidfowl]

While I think that's OK in this case, I do think we want to be able to light up features like this depending on the version running. So its a forcing function to build some infrastructure into DAB to handle versioning between DCP and DAB and take the appropriate action (which I think in this case should just not do the extra work), but I'm OK starting with an error.

[karolz-ms]

While I think that's OK in this case, I do think we want to be able to light up features like this depending on the version running. So its a forcing function to build some infrastructure into DAB to handle versioning between DCP and DAB and take the appropriate action (which I think in this case should just not do the extra work), but I'm OK starting with an error.

Agreed we should have a way to do conditional logic based on DCP version. In this particular case though I think everyone will benefit for proxyless singleton containers and it does not make sense to retain support of preview 1 version of DCP.

[davidfowl]

Was the new verison of dcp checked in? Will this explode in the repo or work?

[karolz-ms]

Yes, the new version of DCP has been published earlier (last week, actually), so everything should still work, you just need to update the Aspire workload.

[davidfowl]

@karolz-ms that happens automagically when you are using the aspire repo itself though right? This repo uses DCP from a package, not from the workload.

[karolz-ms]

@karolz-ms that happens automagically when you are using the aspire repo itself though right? This repo uses DCP from a package, not from the workload.

Ehm, I am confused--DCP comes strictly from the workload; it is not part of any (NuGet) package...

[davidfowl]

@bwateratmsft this PR seems to have broken the ability for container to use WithReference because services aren't created before containers are used. I'm not sure ServicesConsumed is ever non-empty, so containers are being created before anything else.

When I tried to remove this, it just hangs in DCP.

[bwateratmsft]

@karolz-ms do you know why ServicesConsumed would be empty even when the container is referencing services?

[davidfowl]

It was never populated AFAIK. What was the reasoning behind the logic for assigning proxyless containers?

[bwateratmsft]

https://github.com/microsoft/usvc/issues/109 is the original issue for it. As I understand it the reasoning was twofold--first, a small perf/simplicity gain of not running a proxy in one of the scenarios where it wasn't necessary; second, a higher chance that "singleton" containers would start before the project executables consuming them, reducing errors during startup.

[davidfowl]

We don’t store enough information to know that containers are “dependency less”

[karolz-ms]

@bwateratmsft is right, with the clarification that it is not so much about containers starting early (they do not necessarily start before Executables consuming them, or they might start, but not be ready to consume requests for a while).

The main benefit apart from perf is that we do not proxy requests to them, so the clients either can't connect, or get a response. With the proxy there is a third possibility: can connect, but the connection gets dropped (because the backend is not ready), which many clients do not deal very well with.

[eerhardt]

which many clients do not deal very well with.

For example the Redis client. See https://github.com/dotnet/aspire/pull/597#issuecomment-1787977577

[davidfowl]

I think we're going to have to revert this change until we are able to map dependencies. This change assumes containers have none which breaks other things.

[karolz-ms]

... because up until now (specifically the attempt to enable `WithReference() with containers) the assumption was valid.

We should expedite the map dependencies change to the model. It will open up other scenarios (e.g. around deployment) too.