Dotnet-docker: 2.0.7 images break some ASP.NET apps

[sebastienros]

With this change
https://github.com/dotnet/dotnet-docker/pull/484/files#diff-11d7304b9ce2f1a56720f63ea2a7fdf7R22

If the ASP.NET application is configured to use environment variables with priority over command line, or even if they were not using the command line argument at all using the default Kestrel port (5000 for 2.0) then it is breaking existing apps.

For instance my application started listening on 80 instead of the one I was configuring previously as there were no ENV defined, and I didn't expect one to be defined.

[natemcmaster]

@richlander @MichaelSimons - I would consider this a breaking change in a patch release. Should we revert the addition of ASPNETCORE_URLS to the base images?

[kgorlick]

I had the same issue. All of the sudden one of our containers based on microsoft/dotnet:2.0-sdk stopped working today. I just updated my DockerFile to use a multi-stage build, with the first stage being what you had in your Dockerfile yesterday and the second stage based on the first. My application is working again.

To be clear my solution was to remove https://github.com/dotnet/dotnet-docker/blob/master/2.0/sdk/stretch/amd64/Dockerfile#L31-L38.

[glennc]

I'm not 100% clear on how you are setting the port that allows you to access your app and still gets overridden by the env var. What's the code/config look like?

[kgorlick]

@glennc, if that question is for me I am mapping ports in my docker-compose file. To be honest I did not spend anytime trying to figure out why the change broke my system (it was very late and I was quite done with sleuthing for the day).

[MichaelSimons]

I too would like to see the code/config. Without knowing the details, a simple workaround may be to add ENV ASPNETCORE_URLS= in your Dockerfile or compose file. Alternatively you could set the variable to the explicit port you are dependent on ASPNETCORE_URLS=httP://+:xyz

Unfortunately reverting the change once it is released can also have an impact on customers who have already taken a dependency on this. If this is having an impact on you please respond and include details about your code/config. Our plan as of now is to leave the change in place. We will refrain from making this type of change in the future during a service release - we do apologize.

[sebastienros]

The workaround is easy, just override the new value that was set. Yet it's still breaking more people now than it would be if it was reverted.

It's changing the defaults, so anyone using the defaults will be broken. It's harsh to expect users to override all the defaults in case they might change, in a minor update, without announcement.

[natemcmaster]

What is the calculus here for not reverting a breaking change in a patch? We've previously always revert changes that have wide impact and break people on a patch update. What was the reasoning for not reverting on this one?

Unfortunately reverting the change once it is released can also have an impact on customers who have already taken a dependency on this.

Presumably, reverting now would mean users to need to go back to the state they were in on Monday, right? That seems like a fair tradeoff given that anyone who has taken dependency on this is actively checking and updating their definitions. IMO it seems like the bigger risk here are systems that are automatically pulling updates and don't get serviced everyday. Those services may fail as soon as they pull this patch.

[kgorlick]

That is what happened to me @natemcmaster

[MichaelSimons]

We've previously always revert changes that have wide impact and break people on a patch update.

Not true - we unfortunately have had a case in the past (a change to the SHELL in microsoft/dotnet-framework) where the consensus from the us/community was to stay the course.

What was the reasoning for not reverting on this one?

We are not getting a read from this issue that this has a measurable impact at this point. As I previously mentioned, reverting changes also has an impact so we are trying to not cause too many ripples at this point.

@richlander - Thoughts?

[MichaelSimons]

Discussing this with others, a decision was made to revert the ASPNETCORE_URLS change. I will make this PR right away.

[MichaelSimons]

The 2.0 images have been updated - please re-pull if necessary.

[richlander]

For clarity, we intend to keep this change in .NET Core 2.1. The revert is for .NET Core 2.0.

[richlander]

Thanks @MichaelSimons !

[kgorlick]

@MichaelSimons, the updated image fixed my issue. Thank you.