Skip to content
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

Set the ASPNETCORE_URLS to match the container port. #34

Merged
merged 1 commit into from
Jan 19, 2017

Conversation

tmds
Copy link
Member

@tmds tmds commented Jan 18, 2017

ASP.NET Core uses port 5000 by default while the image is exposing port 8080.
We set the ASPNETCORE_URLS environment variable to make ASP.NET Core use the port exposed by the container.
We can't change the port exposed by the container since this would break applications that explicitly force their port to 8080 using WebHostBuilder.UseUrls.

ASP.NET Core uses port 5000 by default while the image is exposing port 8080.
We set the ASPNETCORE_URLS environment variable to make ASP.NET Core use the port exposed by the container.
We can't change the port exposed by the container since this would break applications that explicitly force their port to 8080 using `WebHostBuilder.UseUrls`.
@tmds
Copy link
Member Author

tmds commented Jan 18, 2017

@omajid @jerboaa ptal

Copy link
Contributor

@jerboaa jerboaa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Thanks!

@omajid
Copy link
Member

omajid commented Mar 7, 2017

I have had questions asking about the upstream default of 5000 not working with our images. Is there a reason to not switch the default container export port to the upstream default port? That is, switch to 5000 rather than 8080?

@tmds
Copy link
Member Author

tmds commented Mar 7, 2017

Is there a reason to not switch the default container export port to the upstream default port?

@omajid I put this in my first comment:

We can't change the port exposed by the container since this would break applications that explicitly force their port to 8080 using WebHostBuilder.UseUrls.

I like us setting ASPNETCORE_URLS since it is the official way to inform the webserver of the port to use. We can change the port of the container when we do a breaking release, but Kestrel should pick it up already from the environment variable.

@omajid
Copy link
Member

omajid commented Mar 7, 2017

Just to make sure I am understanding this correctly, we can't change the default because it would break existing containers? Would changing the default be okay for the next breaking release (probably 2.0)?

@tmds
Copy link
Member Author

tmds commented Mar 7, 2017

Just to make sure I am understanding this correctly, we can't change the default because it would break existing containers?

Yes.
Setting ASPNETCORE_URLS fixes the issue in a non-breaking way.

Would changing the default be okay for the next breaking release (probably 2.0)?

Correct.

@jerboaa
Copy link
Contributor

jerboaa commented Mar 8, 2017

@omajid I'm not sure why you'd want this changed for 2.0. It's being set as env in the container and from a user perspective it shouldn't matter whether it's 5000 or 8080. They don't need code changes. I understand why this has been asked for the old images. Those don't set the env var, but for the new ones I don't see much gain in switching to 5000. Is there a compelling reason? Sounds like added work for not much gain to me.

My $0.02

@omajid
Copy link
Member

omajid commented Mar 8, 2017

@jerboaa The default port used by ASP.NET Core is 5000. It seems to me sticking to the upstream default would be more obvious for users. Otherwise they have to remember that upstream uses 5000 but our defaults are 8080.

@jerboaa
Copy link
Contributor

jerboaa commented Mar 9, 2017

@omajid Fair enough.

@tmds
Copy link
Member Author

tmds commented Mar 9, 2017

As long as we set ASPNETCORE_URLS it's fine for me.

I just had a look, Microsoft is using port 80 on their images: https://github.com/aspnet/aspnet-docker/blob/master/1.1/jessie/runtime/Dockerfile#L4

@jerboaa
Copy link
Contributor

jerboaa commented Mar 9, 2017

What @tmds said. Also, what if they change the default? ;-)

FWIW, it's not like it's not documented:
https://github.com/redhat-developer/s2i-dotnetcore/blob/master/1.1/s2i/bin/usage#L15

In the OpenShift case it does not even matter, as the route will be 80 and mapped to the exposed port of the container. That is it's mapped to 5000 or mapped to 8080 of the exposed container port. Neither is really visible to the user.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants