[Sample] Update C# hello world serving sample according to Microsoft docs, to multistep build

[mattwelke]

Proposed Changes

• Update C# hello world sample to reflect current best practices in Microsoft docs (https://docs.microsoft.com/en-us/aspnet/core/host-and-deploy/docker/building-net-docker-images?view=aspnetcore-2.2#build-and-deploy-manually)
• Uses a multi stage build which results in the image being ~200 MB instead of ~2 GB and image can be built without SDK installed on dev machine
• Removes .dockerignore file because copied files are explicit with multi stage build
• Updates README file to reflect new Dockerfile and docs links

Tested by building the image before and after this Dockerfile change and ensuring they both worked the same way (responded to requests on a port specified by PORT env var).

[knative-prow-robot]

Hi @Welkie. Thanks for your PR.

I'm waiting for a knative member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[grayside]

Hi Welkie, thanks for the PR!

I'm not familiar enough with C# to evaluate the practice here but in looking at the linked article, it makes sense.

Two minor points of feedback:

1. Please restore the .dockerignore file, as it's purpose is to help ensure deterministic builds regardless of what happens in local development or changes to the Dockerfile.
2. Please restore the comments in the Dockerfile, as they provided a bit more context and had consistency of more formal grammar across the sample code.

I see this change also switches the Container Registry away from Docker Hub. I'm not sure if there is a policy on this in the knative project, however, the comment referring to the container URL in Docker Hub seems like it should be updated if this is kept.

/assign grayside
/ok-to-test

[mattwelke]

@grayside

Changes made. The comments should reflect the rest of the samples now.

Note that the reason I originally linked to the Docker Hub page at the top of the Dockerfile is because that still appears to be how Microsoft intends people to "discover" the image.

https://hub.docker.com/_/microsoft-dotnet-core-aspnet/

You can see how they give a sample docker pull command that grabs the image from the registry mcr.microsoft.com. It's a bit odd because usually if an image is listed on Docker Hub it's downloaded from Docker Hub too. I've updated the Dockerfile comment to have a link to the Microsoft docs page as you requested.

[grayside]

Hi Welkie, thanks for making the changes. If Microsoft prefers to direct people to Docker Hub, I'm okay with keeping that link. Can we add a similar link for the second stage? Also, the README needs to be updated to match the Dockerfile.

Sorry for asking for so many minor changes! I've tested the new Dockerfile locally and this works.

[mattwelke]

@grayside No problem. This should be good now. It's got the link at the top of each stage of the build in the Dockerfile, and they're Docker Hub links. The readme is up to date now too.

[mattwelke]

@grayside Ready for a review. Made the changes you requested and included a fix for an issue @PicardParis caught.

[grayside]

/lgtm

[mattwelke]

Do you need me to integrate the latest changes on the base branch before this can be merged?

[samodell]

/approved

Sorry for the wait, @mattwelke ! Thanks for your contribution.

[samodell]

I meant --

/approve

[knative-prow-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mattwelke, samodell

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [samodell]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment