Reduce Environment variables

[richlander]

Seems funny after #426.

I got to thinking ... with DOTNET_RUNNING_IN_CONTAINER in place, can we provide different defaults for dotnet watch and for Kestrel?

Would be excellent if the defaults made DOTNET_USE_POLLING_FILE_WATCHER and ASPNETCORE_URLS=http://+:80 unnecessary when running in containers, when DOTNET_RUNNING_IN_CONTAINER was set to true??? Shouldn't that be the case?

/cc @natemcmaster @MichaelSimons @glennc

[natemcmaster]

We could make DOTNET_RUNNING_IN_CONTAINER imply DOTNET_USE_POLLING_FILE_WATCHER, but not ASPNETCORE_URLS. Users may have configured their endpoints differently, for example ASPNETCORE_URLS=https://+:443

[MichaelSimons]

@natemcmaster - I would think you could make DOTNET_RUNNING_IN_CONTAINER imply both. There would have to be an override mechanism however (e.g. any explicit variables would need to override "implied" variables).

That aside, the implied logic is a drawback of sorts as it does complicate things - implementation and documentation.

[MichaelSimons]

Just to make sure this is known, there are other ways to determine if running in a container (see https://github.com/dotnet/cli/blob/master/src/dotnet/Telemetry/DockerContainerDetectorForTelemetry). The main reason I like DOTNET_RUNNING_IN_CONTAINER is because it is platform agnostic in addition to being easy to check for.

[natemcmaster]

How would dotnet-watch tell the different between an implied and explicit variable? I can't think of a way to make implied vs explicit values for ASPNETCORE_URLS work with dotnet-watch and dotnet-run.

[richlander]

Isn't it something like this:

HarvestEnvironmentVariables();
if (DOTNET_RUNNING_IN_CONTAINER && ASPNETCORE_URLS == "")
{
 ASPNETCORE_URLS = "http://+:80";
}

I think about 443 this way:

• Our current behavior doesn't help 443.
• This is a q of default. Once we switch to secure by default, then maybe 443 becomes the default on not 80.

[tmds]

DOTNET_RUNNING_IN_CONTAINER +1

The Dockerfiles should have an EXPOSE directive to document what port gets used, and imo, it makes sense to set ASPNETCORE_URLS in the Dockerfile to make clear it matches the exposed port.

On the topic of secure by default: it is common to have http only in containers. See this issue on how ASP.NET Core default template fails when deployed on a container platform: aspnet/Templating#404

[natemcmaster]

@richlander so, my thinking on this hasn't changed. We still need ASPNETCORE_URLS to be defined separately for usage outside of dotnet-watch. If you want to condense DOTNET_USE_POLLING_FILE_WATCHER and DOTNET_RUNNING_IN_CONTAINER, I can make the two-line change to dotnet-watch to support polling when DOTNET_RUNNING_IN_CONTAINER is set.

[richlander]

Good on DOTNET_USE_POLLING_FILE_WATCHER and DOTNET_RUNNING_IN_CONTAINER. Filed aspnet/DotNetTools#415 for that.

I'll see if @davidfowl is wiling to take a change in aspnet/Hosting to make running in containers easier.

Thoughts @glennc?

[davidfowl]

What's the change to hosting?

[tmds]

What's the change to hosting?

@davidfowl I don't know where the 'fix' should be. It's about aspnet/Templating#404, aspnet/BasicMiddleware#315

[richlander]

We're not going to make this change. The EVs in the Dockerfiles are fine as-is.

[tmds]

@richlander you are not adding 'DOTNET_RUNNING_IN_CONTAINER'?
The samples seem to depend on it:

dotnet-docker/samples/dotnetapp/Directory.Build.props

Line 8 in 79844a3

<PropertyGroup Condition="'$(DOTNET_RUNNING_IN_CONTAINER)' == 'true'">