fix #7916 by using DOTNET_RUNNING_IN_CONTAINERS env var to detect container #9903
Conversation
…tecting container environment
| @@ -11,13 +11,13 @@ namespace Microsoft.AspNetCore.DataProtection.Internal | |||
| { | |||
| internal static class DockerUtils | |||
There was a problem hiding this comment.
As long as we're at it, rename to ContainerUtils?
There was a problem hiding this comment.
Yeah, I thought about that. I'll do it :)
|
This comment was made automatically. If there is a problem contact [email protected]. I've triaged the above build. I've created/commented on the following issue(s) |
:( Will retrigger when Azure is feeling less under-the-weather |
|
This comment was made automatically. If there is a problem contact [email protected]. I've triaged the above build. I've created/commented on the following issue(s) |
ghost
deleted the
| { | ||
| // Official .NET Core images (Windows and Linux) set this. So trust it if it's there. | ||
| if(string.Equals(Environment.GetEnvironmentVariable("DOTNET_RUNNING_IN_CONTAINERS"), "true", StringComparison.OrdinalIgnoreCase)) |
There was a problem hiding this comment.
There's a typo here, instead of DOTNET_RUNNING_IN_CONTAINERS it should be DOTNET_RUNNING_IN_CONTAINER.
See, e.g. https://github.com/dotnet/dotnet-docker/blob/master/2.1/runtime-deps/stretch-slim/amd64/Dockerfile#L21 for where it is set, or e.g. https://www.hanselman.com/blog/DetectingThatANETCoreAppIsRunningInADockerContainerAndSkippableFactsInXUnit.aspx for another source.
There was a problem hiding this comment.
Looks like the environment variable isn't consistent... See https://github.com/dotnet/dotnet-docker/blob/4fd3b74fc5b7444f39f3fdcffa45eef467bccb24/2.1/aspnet/nanoserver-1803/amd64/Dockerfile
There was a problem hiding this comment.
Perhaps we should just check both then :)
There was a problem hiding this comment.
Never mind, I see that the plural is deprecated. Will update!
Fixes #7916
I was assigned this back before @natemcmaster got back :). Finally just decided to do it rather than punting it around. Testing this looks tricky, I'll do a quick manual pass once it lands in the SDK. It's a pretty simple change.
All the official .NET Core images set this environment variable (Windows and Linux) so it should serve as a reasonable first thing to look for and make us a little more resiliant to running in other container runtimes.