Enable buildpipeline build and add test apps for 4.7 and 4.7.1 #16

rakeshsinghranchi · 2017-11-22T18:01:37Z

This takes care of the following :

Adding missing 4.7 and 4.7.1 test cases
Simplifies dockerfile using multi-stage feature
Enables ltsc2016 and 1709 tagging
Enables buildpipeline
Enables manifest based tagging

This makes PR #15 obsolete.

Pipeline build definition : https://devdiv.visualstudio.com/DefaultCollection/DevDiv/_apps/hub/ms.vss-ciworkflow.build-ci-hub?_a=edit-build-definition&id=7948

MichaelSimons · 2017-11-27T15:49:40Z

dotnetapp-4.7/Dockerfile

@@ -0,0 +1,26 @@
+FROM microsoft/dotnet-framework:4.7 as builder


Last week microsoft/dotnet-framework-docker#37 was merged. These new build images should be used here. It would greatly simplify the Dockerfiles and the time to build them. Should be as simple as:

microsoft/dotnet-framework-build:4.7.1 WORKDIR /app COPY . ./ RUN ["msbuild.exe", "dotnetapp-4.7.csproj", "/p:Configuration=Release"]

This applies to all the samples except 3.5.

MichaelSimons · 2017-11-27T15:58:11Z

manifest.json

+        {
+          "sharedTags": {
+          },
+          "platforms": [


This manifest doesn't look correct? Have you run the image builder tool on this manifest? I think it will fail the validation phase.

Each .NET version should be listed as a separate Image. 3.5 and 4.7.1 should each have two platforms and the 3.5 and 4.7.1 tags should be shared tags and not platform tags. Furthermore, I would consider the standalone versions to be shared tags for all versions even though we are not building them for both server versions. This would better support adding both versions if needed in the future.

MichaelSimons · 2017-11-27T15:59:53Z

README.DockerHub.md

+- [`4.7-windowsservercore-ltsc2016`, `4.7` (*dotnetapp-4.7/Dockerfile*)](https://github.com/Microsoft/dotnet-framework-docker-samples/blob/dockerhub/dotnetapp-4.7/Dockerfile)
+- [`4.6.2-windowsservercore-ltsc2016`, `4.6.2` (*dotnetapp-4.6.2/Dockerfile*)](https://github.com/Microsoft/dotnet-framework-docker-samples/blob/dockerhub/dotnetapp-4.7.1/Dockerfile)
+- [`3.5-windowsservercore-ltsc2016`, `3.5` (*dotnetapp-3.5/Dockerfile*)](https://github.com/Microsoft/dotnet-framework-docker-samples/blob/dockerhub/dotnetapp-3.5/Dockerfile)
+


I think the following comment should be added which exists in our other repos (e.g. https://github.com/Microsoft/dotnet-framework-docker/blob/master/README.md).

Note: .NET Core multi-arch tags, such as 2.0-runtime, have been updated to use nanoserver-1709 images if your host is Windows Server 1709 or higher or Windows 10 Fall Creators Update (Version 1709) or higher. You need Docker 17.10 or later to take advantage of these updated tags.

rakeshsinghranchi · 2017-11-28T19:36:47Z

@MichaelSimons , updated the PR with code review comments. PTAL. Test pipe build : https://devdiv.visualstudio.com/DevDiv/_build/index?buildId=1175441&_a=summary

richlander · 2017-11-28T21:51:20Z

dotnetapp-3.5/Dockerfile

@@ -1,4 +1,4 @@
-FROM microsoft/dotnet-framework:3.5
+FROM microsoft/dotnet-framework:3.5 as builder


This suggests to me that we need to create a 3.5 builder as well. I don't want to ship our asymmetric samples between 3.5 and 4.x (not that these samples are the decision maker!).

Agreed . Issue: #17 tracks the related work.

richlander · 2017-11-28T21:52:26Z

dotnetapp-4.7.1/dotnetapp-4.7.1.csproj

@@ -0,0 +1,64 @@
+<?xml version="1.0" encoding="utf-8"?>


I am wondering if we should switch all of our projects to SDK style. That would seem to be a good idea.

This is a good idea. Issue : #18 tracks this issue now.

@MichaelSimons ,Does this also require updating the build image ?

rakeshsinghranchi · 2017-11-29T00:45:51Z

@MichaelSimons @richlander , do you have any further comments for this PR ?

richlander · 2017-11-29T00:48:12Z

That's it for me. What's your game plan?

rakeshsinghranchi · 2017-11-29T01:06:04Z

I plan to complete this PR as is and enable the build pipeline ASAP. Next step will be working on improvements as noted in review comments and tracked by issues. Let me know if you have any suggestions.

richlander · 2017-11-29T01:23:31Z

By "complete", I assume you mean "merge". Is having these changes in master useful as part of enabling the build pipeline?

rakeshsinghranchi · 2017-11-29T02:31:26Z

Yes, I meant "merge". These changes are for dockerhub branch specifically for producing dotnet-framework-samples images. Yes,having single branch will greatly improve the discoverability of the source code. Having these changes in master will also increase & require sample apps parity ( including aspnet,wcf apps ) . I would suggest bringing these changes in phases instead of the major overhaul.

MichaelSimons · 2017-11-29T04:30:18Z

With the addition of multi-stage builds and the introduction of the build images, I think we should reconsider the need for the separate dockerhub branch. I think it would be beneficial to have the images we publish on Docker Hub come from the main examples. This is beneficial for customers as well as us (less maintenance). This is the pattern used by the .NET Core samples.

I also am not sure it is necessary to have a sample on Docker Hub for each version. For .NET Core, we only push the latest version.

rakeshsinghranchi · 2017-11-29T04:46:30Z

Master branch examples are Sdk styled projects for 4.x sample apps. Do we know build images have necessary toolset to build these Sdk based projects?

richlander · 2017-11-29T05:04:37Z

Yes, one sample for 4.latest and one for 3.5 is sufficient.

richlander · 2017-11-29T05:39:31Z

Question: Now that we're supporting multiple DH repos in the dotnet-framework-docker GH repo, should we just collapse this one into it, too?

rakeshsinghranchi · 2017-11-29T09:16:43Z

Do you mean enabling dotnet-framework-docker GH repo to build dotnet-framework-docker-samples images or follow the same suggested pattern for dotnet-docker & dotnet-docker-samples as well ? To me , building multiple DH repo images is not very intuitive for users and may complicate the manifest file layout.

MichaelSimons · 2017-11-29T15:01:47Z

Question: Now that we're supporting multiple DH repos in the dotnet-framework-docker GH repo, should we just collapse this one into it, too?

My initial reaction is no.

Like @rakeshsinghranchi mentions I think it will complicate the official repo and potentially make it more confusing. It will certainly compound the current issue we are trying to solve on limiting the number of times images get build which are triggered by source changes.
The samples repo contains samples that span the Full Fx/.Net Core. For example the Full Fx samples repo contains a wcfapp sample. The .NET Core samples contain an ASP.NET Core sample. I think an ASP.NET sample should be added to the Full Fx samples.

MichaelSimons · 2017-11-29T15:22:42Z

Master branch examples are Sdk styled projects for 4.x sample apps. Do we know build images have necessary toolset to build these Sdk based projects?

The build images currently don't include the .NET Core sdk. The SDK will add another 282 MB assuming we skip the local package cache which I don't think would be needed in this full fx context. Are there other parts of the the sdk which could be trimmed down for this usage? This is where the work to break down the .NET Core sdk into components would be desirable.

MichaelSimons · 2017-11-29T16:19:44Z

I logged microsoft/dotnet-framework-docker#49 to track support for Sdk styled projects.

rakeshsinghranchi · 2017-11-29T20:23:10Z

My understanding is as follow :

Publish .NET 3.5 and .NET 4.7.1 samples image only in DH . Does keeping .NET 4.latest mean deleting older .NET 4.x versions? If not, we will end up leaving images that are not patched.
Preference is to have single branch ( move away from dockerhub to master ) . This will require updating the existing SDK styled samples to old styled samples. Eventually,we want to move to SDK styled samples once Build images don't support building SDK styled projects dotnet-framework-docker#49 is resolved.
We do not want to consolidate dotnet-framework-docker-samples images built from dotnet-framework-docker GH. This will be complicate official repo and confuse users.

What is your take on #1 and #2 ? If we all agree, I will go ahead and merge this PR as is and once microsoft/dotnet-framework-docker#49 is available, will switch from dockerhub to master then.

MichaelSimons · 2017-11-29T22:27:07Z

Publish .NET 3.5 and .NET 4.7.1 samples image only in DH . Does keeping .NET 4.latest mean deleting older .NET 4.x versions? If not, we will end up leaving images that are not patched.

I was thinking that once a new .NET Fx version is available, we would publish a new sample image to DH, tag it with a new tag that matches the runtime, add it to the readme and remove the previous tag from the readme. The tag would still exist as is for eternity. My view is this is reasonable behavior for samples.

Preference is to have single branch ( move away from dockerhub to master ) . This will require updating the existing SDK styled samples to old styled samples. Eventually,we want to move to SDK styled samples once microsoft/dotnet-framework-docker#49 is resolved.

We should defer the branch consolidation until the build image supports the SDK based projects. We shouldn't be changing the samples in master to the old style. I think this is what you are proposing If we all agree, I will go ahead and merge this PR as is and once Microsoft/dotnet-framework-docker#49 is available, will switch from dockerhub to master then.

rakeshsinghranchi · 2017-11-30T21:52:34Z

We should defer the branch consolidation until the build image supports the SDK based projects. We shouldn't be changing the samples in master to the old style. I think this is what you are proposing If we all agree, I will go ahead and merge this PR as is and once microsoft/dotnet-framework-docker#49 is available, will switch from dockerhub to master then.
Yes. @MichaelSimons, do you have any further comments. Will wait for your sign-off before merging this.

MichaelSimons · 2017-11-30T21:57:39Z

@rakeshsinghranchi - were you going to update the PR to only contain the 3.5 and 4.7.1 samples?

rakeshsinghranchi · 2017-12-01T00:17:01Z

@rakeshsinghranchi - were you going to update the PR to only contain the 3.5 and 4.7.1 samples?
Pushed another commit to remove 4.6.2 and 4.7 samples

MichaelSimons

Looks good - just one minor comment.

MichaelSimons · 2017-12-01T02:12:46Z

dotnetapp-3.5/Dockerfile

+FROM microsoft/dotnet-framework:3.5
+
+WORKDIR /app
+


Nit: I don't think all of the white space between every instruction is necessary. Consider reducing and making it consistent with the 4.7.1 Dockerfile.

Enable buildpipeline build and add test apps for 4.7 and 4.7.1

3f1f69a

rakeshsinghranchi requested review from MichaelSimons, richlander and kendrahavens November 22, 2017 18:01

MichaelSimons suggested changes Nov 27, 2017

View reviewed changes

Updated per feedback comments

5ae37ef

richlander reviewed Nov 28, 2017

View reviewed changes

This was referenced Nov 28, 2017

Add readme for microsoft/dotnet-framework-build repo microsoft/dotnet-framework-docker#44

Merged

Add .NET Framework 3.5 Builder Image microsoft/dotnet-framework-docker#48

Closed

Remove .NET 4.6.2 and .NET 4.7 sample applications

3625b4b

MichaelSimons approved these changes Dec 1, 2017

View reviewed changes

update per feedback

b6bd6d5

rakeshsinghranchi merged commit f0280cf into microsoft:dockerhub Dec 1, 2017

This was referenced Dec 1, 2017

Build and publish images using buildpipeline and image builder tool #14

Closed

Support new tagging scheme and new samples (4.7 and 4.7.1) #15

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable buildpipeline build and add test apps for 4.7 and 4.7.1 #16

Enable buildpipeline build and add test apps for 4.7 and 4.7.1 #16

rakeshsinghranchi commented Nov 22, 2017

MichaelSimons Nov 27, 2017

MichaelSimons Nov 27, 2017

MichaelSimons Nov 27, 2017

rakeshsinghranchi commented Nov 28, 2017 •

edited

Loading

richlander Nov 28, 2017 •

edited

Loading

MichaelSimons Nov 28, 2017

rakeshsinghranchi Nov 29, 2017

richlander Nov 28, 2017

rakeshsinghranchi Nov 29, 2017

rakeshsinghranchi commented Nov 29, 2017

richlander commented Nov 29, 2017

rakeshsinghranchi commented Nov 29, 2017

richlander commented Nov 29, 2017

rakeshsinghranchi commented Nov 29, 2017 •

edited

Loading

MichaelSimons commented Nov 29, 2017

rakeshsinghranchi commented Nov 29, 2017

richlander commented Nov 29, 2017

richlander commented Nov 29, 2017

rakeshsinghranchi commented Nov 29, 2017

MichaelSimons commented Nov 29, 2017

MichaelSimons commented Nov 29, 2017

MichaelSimons commented Nov 29, 2017

rakeshsinghranchi commented Nov 29, 2017

MichaelSimons commented Nov 29, 2017

rakeshsinghranchi commented Nov 30, 2017

MichaelSimons commented Nov 30, 2017

rakeshsinghranchi commented Dec 1, 2017

MichaelSimons left a comment

MichaelSimons Dec 1, 2017

		@@ -0,0 +1,26 @@
		FROM microsoft/dotnet-framework:4.7 as builder

		@@ -1,4 +1,4 @@
		FROM microsoft/dotnet-framework:3.5
		FROM microsoft/dotnet-framework:3.5 as builder

Enable buildpipeline build and add test apps for 4.7 and 4.7.1 #16

Enable buildpipeline build and add test apps for 4.7 and 4.7.1 #16

Conversation

rakeshsinghranchi commented Nov 22, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rakeshsinghranchi commented Nov 28, 2017 • edited Loading

richlander Nov 28, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rakeshsinghranchi commented Nov 29, 2017

richlander commented Nov 29, 2017

rakeshsinghranchi commented Nov 29, 2017

richlander commented Nov 29, 2017

rakeshsinghranchi commented Nov 29, 2017 • edited Loading

MichaelSimons commented Nov 29, 2017

rakeshsinghranchi commented Nov 29, 2017

richlander commented Nov 29, 2017

richlander commented Nov 29, 2017

rakeshsinghranchi commented Nov 29, 2017

MichaelSimons commented Nov 29, 2017

MichaelSimons commented Nov 29, 2017

MichaelSimons commented Nov 29, 2017

rakeshsinghranchi commented Nov 29, 2017

MichaelSimons commented Nov 29, 2017

rakeshsinghranchi commented Nov 30, 2017

MichaelSimons commented Nov 30, 2017

rakeshsinghranchi commented Dec 1, 2017

MichaelSimons left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rakeshsinghranchi commented Nov 28, 2017 •

edited

Loading

richlander Nov 28, 2017 •

edited

Loading

rakeshsinghranchi commented Nov 29, 2017 •

edited

Loading