cloudfoundry / dotnet-core-buildpack

Cloud Foundry buildpack for .NET Core on Linux
http://docs.cloudfoundry.org/buildpacks/
Apache License 2.0
91 stars 90 forks source link

Runtime version being forced to what's found in runtimeconfig.json #184

Closed rellis-of-rhindleton closed 6 years ago

rellis-of-rhindleton commented 6 years ago

What version of Cloud Foundry and CF CLI are you using? (i.e. What is the output of running cf curl /v2/info && cf version?

cf version 6.35.1+bfbe5f0e8.2018-03-14

What version of the buildpack you are using?

1.0.33

If you were attempting to accomplish a task, what was it you were attempting to do?

Deploy and execute an ASP.NET Core application compiled with SDK 2.0.6.

What did you expect to happen?

The buildpack has runtime version 2.0.3, so that runtime should be used.

What was the actual behavior?

Runtime 2.0.0 was used. This warning was generated:

build   17-May-2018 11:13:56    -----> Installing .NET SDK
build   17-May-2018 11:13:56           .NET SDK version: 2.0.3
build   17-May-2018 11:13:58           file:///tmp/buildpacks/4d121f46911b7520ea3b3f0a05bbb13b/dependencies/https___buildpacks.cloudfoundry.org_dependencies_dotnet_dotnet.2.0.3.linux-amd64-b56d13fc.tar.xz
... (about 17 seconds later)
build   17-May-2018 11:14:13           Detected .NET Core runtime version 2.0.0 in /tmp/app/MoveMe-API.runtimeconfig.json
build   17-May-2018 11:14:13    -----> Installing required .NET Core runtime(s)
build   17-May-2018 11:14:13           Downloading and installing .NET Core runtime 2.0.0
build   17-May-2018 11:14:14           file:///tmp/buildpacks/4d121f46911b7520ea3b3f0a05bbb13b/dependencies/https___buildpacks.cloudfoundry.org_dependencies_dotnet-framework_dotnet-framework.2.0.0.linux-amd64-13cb2a76.tar.xz
build   17-May-2018 11:14:14           **WARNING** A newer version of dotnet-framework is available in this buildpack. Please adjust your app to use version 2.0.3 instead of version 2.0.0 as soon as possible. Old versions of dotnet-framework are only provided to assist in migrating to newer versions.

Please confirm where necessary:

Please forgive me if this has already been fixed in later buildpack versions -- our hosts are updating to 2.0.6 this weekend -- or if this stuff isn't coming from the buildpack at all. This entire environment is new to me and I'm having a hard time telling what's coming from where. I've done a cursory search in the repository but didn't find anything that looks remotely similar to what I'm seeing, so this may be a wild goose chase.

All I know is that the versioning rules around .NET Core allow the latest patch version of a runtime to be used. If our runtimeconfig.json specifies 2.0.0, that merely indicates the minimum required runtime version for the SDK we compiled it with. Runtime versions 2.0.* are all valid choices; the default behavior is to use the latest patch version installed. In fact since 2.0.3 of the SDK is already being installed, the best choice runtime should already be on the system.

cf-gitbot commented 6 years ago

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/157671649

The labels on this github issue will be updated when the story is started.

mfjerome commented 6 years ago

+1 for this issue. I was about to submit it too. The buildpack should not only use the runtimeconfig.json to detect which specific patch version of the dotnet core runtime to download and install. It should:

AND

Then, you let the app decide which version to use (automatic roll-foward is enabled by default). The developper may specify "applyPatches:false" in the runtime.config to block the roll-forward.

macsux commented 6 years ago

More info on expected behavior when processing runtime.config can be found here https://github.com/dotnet/cli/blob/v2.0.0/Documentation/specs/runtime-configuration-file.md

sclevine commented 6 years ago

Apologies for the delay. We're looking into this now. Can someone confirm if this is the same underlying issue as #177?

Also, could someone provide a sample app that demonstrates the failure with the latest version of the buildpack?

mfjerome commented 6 years ago

@sclevine I don't think this is the same issue. I'm not sure but #177 might have been raised at a time where the buildpack did not offer a precise runtime version at all, thus why it could not be found by the application.

In our current issue, the buildpack offer the latest version that our application would need, but it is not being downloaded because of the wrong logic used to detect it in the runtime.config.

I hope my explanation makes sense..

macsux commented 6 years ago

I've wrote sample app to test this issue. I've actually discovered that buildpack does what it supposed to happen - it applies minor version when applyPatches is not specified, but the message it gives out (warning) is incorrect (making it seems like it's forcing lower runtime).

Demo project: https://github.com/macsux/dotnetcorebuildpackIssue184Demo

Results: http://issue184.cfapps.io/ <--- master branch http://issue184applypatches.cfapps.io/ <--- applyPatches branch

timiles commented 6 years ago

It appears I'm getting this same error when following the tutorial here: https://docs.microsoft.com/en-us/azure/app-service/containers/quickstart-dotnetcore - pretty bad first experience of Azure if you get stuck on the first tutorial :(

(Edit: regarding the tutorial, it works if you downgrade Microsoft.AspNetCore.All to 2.0.7, which has Microsoft.AspNetCore.Antiforgery 2.0.2. Microsoft.AspNetCore.All 2.0.8 with dependency on Microsoft.AspNetCore.Antiforgery 2.0.3 fails due to the runtime not being the latest.)

mfjerome commented 6 years ago

Not 100% sure but I am sharing my thoughts/analysis on the matter to help the discussion.

When looking at the logs when I push the app (provided by @macsux ), it seems the buildpack download its latest known .Net Core SDK (which includes its corresponding runtime v2.0.6 ). Then later, it reads the runtime.config, detects it requires v2.0.0 of the runtime, downloads and installs it. Since it is an old runtime, the warning message is raised.

In the end, the apps works correctly and use the latest runtime provided by the SDK. The other runtime is ignored.

If my logic is right, two things seems in opposition here:

1 – The buildpack always install the latest SDK (which is a bit counter-intuitive since you normally don't install a SDK on a production host, and don’t really need it if you deploy self-contained app or if you deploy portable app which only requires the correct runtimes). It is needed if you push codebase that needs to be compiled and published. 2 – The buildpack tries to detect and install a runtime required by the app, which is only useful if you don’t have the SDK.

So whatever the decision, one these 2 steps should be dropped. Either skip the SDK, or skip the runtime installation.

Edit: Here is a great article freshly written by Andrew Lock, explaining the same notions, applied to docker containers with .NET Core 2.1 (SDK versus runtime): https://andrewlock.net/exploring-the-net-core-2-1-docker-files-dotnet-runtime-vs-aspnetcore-runtime-vs-sdk/

It explains the 4 docker images offered my Microsoft that represents the 4 following scenarios:


Logs when trying to push your sample. Notice the "using the default SDK -----> Installing dotnet 2.1.104" and the "Required dotnetframework versions: [2.0.0] -----> Installing dotnet-framework 2.0.0"

Staging app and tracing logs...
   Creating container
   Successfully created container
   Downloading app package...
   Downloaded app package (44.1K)
   -----> Download go 1.9.1
   -----> Running go build supply
   -----> Dotnet-Core Buildpack version 2.0.6
   -----> Supplying Dotnet Core
   -----> Installing libunwind 1.2.1
          Download [https://buildpacks.cloudfoundry.org/dependencies/manual-binaries/dotnet/libunwind-1.2.1-linux-x64-80af276a.tgz]
          using the default SDK
   -----> Installing dotnet 2.1.104
          Download [https://buildpacks.cloudfoundry.org/dependencies/dotnet/dotnet.2.1.104.linux-amd64-4c4036d4.tar.xz]
   -----> Running go build finalize
   -----> Finalizing Dotnet Core
          Required dotnetframework versions: [2.0.0]
   -----> Installing dotnet-framework 2.0.0
          Download [https://buildpacks.cloudfoundry.org/dependencies/dotnet-framework/dotnet-framework.2.0.0.linux-amd64-13cb2a76.tar.xz]
          **WARNING** A newer version of dotnet-framework is available in this buildpack. Please adjust your app to use version 2.0.6 instead of version 2.0.0 as soon as possible. Old versions of dotnet-framework are only provided to assist in migrating to newer versions.
   -----> Cleaning staging area
   Exit status 0

Logs when trying to push a self-contained app (which doesn’t require any .net core components on the host, it would even work with a simple binary buildpack) to show that the SDK is always installed.

Staging app and tracing logs...
   Creating container
   Successfully created container
   Downloading build artifacts cache...
   Downloading app package...
   Downloaded app package (38M)
   Downloaded build artifacts cache (111.4M)
   -----> Download go 1.9.1
   -----> Running go build supply
   -----> Dotnet-Core Buildpack version 2.0.6
   -----> Supplying Dotnet Core
   -----> Installing libunwind 1.2.1
          Copy [/tmp/cache/final/dependencies/10049b6ebf7e832b899ad805e311c53dfb50e6987726fb32743145262e3f3a74/libunwind-1.2.1-linux-x64-80af276a.tgz]
          using the default SDK
   -----> Installing dotnet 2.1.104
          Copy [/tmp/cache/final/dependencies/fa2f6809207874883247b9b85f37cf35c86f9340efc052defd633a31b90bf9dd/dotnet.2.1.104.linux-amd64-4c4036d4.tar.xz]
   -----> Running go build finalize
   -----> Finalizing Dotnet Core
   -----> Cleaning staging area
          Removing dotnet
   Exit status 0
   Uploading droplet, build artifacts cache...
   Uploading build artifacts cache...
   Uploading droplet...
   Uploaded build artifacts cache (111.4M)
   Uploaded droplet (37.5M)
   Uploading complete
   Stopping instance 5d85d81e-b457-4fcb-93c8-300b3aa0d9d6
   Destroying container
   Successfully destroyed container
sclevine commented 6 years ago

There is a fix for this on the develop branch, if anyone would like to try it out before the next release.

mfjerome commented 6 years ago

With the new buildpack, the warning is no longer present and the latest runtime is installed. This precise issue could be closed. But another one could be raised because the app provided by @macsux crashes as it falls into this use case that is not covered by the buildpack:

The ASP.NET Core runtime (Microsoft.AspNetCore.All shared store) v2.0.x is missing.

sclevine commented 6 years ago

@mfjerome This should be fixed as well. The issue is that the runtime package store is provided by the SDK, and the store is also a dependency of framework-dependent apps. If you use the latest SDK (v2.1.x) the v2.0.x assemblies are missing. .NET Core doesn't provide an easy way to lock to a particular line of SDK versions, so we introduced a buildpack.yml file that lets you do this. Try adding buildpack.yml to your app root with these contents:

dotnet-core:
   sdk: 2.0.x
macsux commented 6 years ago

@sclevine please consider building custom store to include prior sdk packages including aspnet packages. Those are currently not being added. See the following link for info on how to create custom store https://docs.microsoft.com/en-us/dotnet/core/deploying/runtime-store cc: @mfjerome

sclevine commented 6 years ago

The buildpack needs to work in offline environments, and currently cannot contain more than 1 GB of dependencies. We think the ability to lock the SDK version to a particular dependency line should be sufficient here, with the caveat that SDK v2.1.201 must be treated as a separate line because it contains 2.0.x assemblies. Before we look into creating a custom store, can you confirm that the following configuration doesn't work:

  1. applyPatches: true
  2. sdk: 2.0.x (or whatever SDK version line you use to create the FDD app)
macsux commented 6 years ago

I just tried against PWS and it failed. I just pushed app that is targeting 2.1 but is using aspnet 2.0 - a fairly common scenario.

This is a poor solution as you're working under assumption as I'm ONLY using packages from SDK 2.0.x. This means that I need packages from both SDKs. If we're working under the assumption that buildpack works only in offline then it needs to be able to carry what is essentially an offline nuget repo of packages that were included with previous version.

  2018-06-28T14:55:10.73-0400 [APP/PROC/WEB/0] ERR Error:
   2018-06-28T14:55:10.73-0400 [APP/PROC/WEB/0] ERR   An assembly specified in the application dependencies manifest (Articulate.deps.json) was not found:
   2018-06-28T14:55:10.73-0400 [APP/PROC/WEB/0] ERR     package: 'Microsoft.ApplicationInsights.AspNetCore', version: '2.1.1'
   2018-06-28T14:55:10.73-0400 [APP/PROC/WEB/0] ERR     path: 'lib/netstandard1.6/Microsoft.ApplicationInsights.AspNetCore.dll'
   2018-06-28T14:55:10.73-0400 [APP/PROC/WEB/0] ERR   This assembly was expected to be in the local runtime store as the application was published using the following target manifest files:
   2018-06-28T14:55:10.73-0400 [APP/PROC/WEB/0] ERR     aspnetcore-store-2.0.0-linux-x64.xml;aspnetcore-store-2.0.0-osx-x64.xml;aspnetcore-store-2.0.0-win7-x64.xml;aspnetcore-store-2.0.0-win7-x86.xml

Secondly, having developer specify SDK to direct the buildpack really breaks the whole concept of the buildpack - here's a bunch of my code, please figure out how to run it. If I have to guide it in a very cf specific kind of way, I might as well make a SCD (self contained deployment) negating the need to have a buildpack entirely.

Can we look at possibly finding a way to get around the 1GB limitation, as with the speed .NET Core is evolving we going to find an entire array of combinations of target runtime / packages. Possibly mounting an external package store into the container. I would also explore the option of publishing the custom store's manifest that developers can target as per article I published in the previous link.

macsux commented 6 years ago

Here's the sample app you can try, it also prints out the runtime version https://github.com/Pivotal-Field-Engineering/pcf-ers-dotnetcore-demo

Edit csproj and remove this <PublishWithAspNetCoreTargetManifest>false</PublishWithAspNetCoreTargetManifest> to make it NOT bundle asp.net dependencies with it (which should be introduced by the buildpack). After that just do regular dotnet publish and cf push

sclevine commented 6 years ago

I just tried against PWS and it failed. I just pushed app that is targeting 2.1 but is using aspnet 2.0 - a fairly common scenario.

The sample you provided works with the suggested changes to buildpack.yml. (sdk: 2.0.x) Note that the value contains a literal x -- it does not require you to specify the exact SDK version.

This is a poor solution as you're working under assumption as I'm ONLY using packages from SDK 2.0.x. This means that I need packages from both SDKs.

We noticed that SDK 2.1.201 includes both 2.0.x and 2.1.x assemblies. Aside from that case, could you help us by describing the workflow you've observed that creates an FDD app with a mixture of assemblies from different SDKs, where a setting like sdk: 2.0.x would not work?

Secondly, having developer specify SDK to direct the buildpack really breaks the whole concept of the buildpack - here's a bunch of my code, please figure out how to run it

It's common for buildpack apps to require configuration that keeps them locked to a version line. Otherwise we have no way of providing backwards-compatible changes across buildpack updates. If specifying the SDK version line (not the exact version) along with applyPatches: true is actually sufficient to keep apps working, developers would only need to change this file when they building their FDD app with an entirely new SDK version line. This follows the convention of the rest of the buildpacks.

It's worth noting that FDD apps are an unusual case for buildpacks because you don't push source code. The buildpack also supports a source-based workflow, but this workflow is difficult to use in offline environments (without access to a nuget repo) because .NET Core doesn't support vendoring source code.

If I have to guide it in a very cf specific kind of way, I might as well make a SCD (self contained deployment) negating the need to have a buildpack entirely.

Buildpacks provide value by keeping your dependencies up-to-date (and thus secure) without requiring changes to source code. Using an SCD prevents the buildpack from updating the runtime.

Can we look at possibly finding a way to get around the 1GB limitation, as with the speed .NET Core is evolving we going to find an entire array of combinations of target runtime / packages. Possibly mounting an external package store into the container.

The current limitation is due to a blob store upload setting on many foundations. We would like to increase this limit, but it must be changed individually by each foundation operator, so this process may take a significant amount of time. That said, buildpacks are already mounted into the staging container, and significant platform changes would be required to volume mount an additional store during staging, so we believe raising the limit would be the easiest path forward.

I would also explore the option of publishing the custom store's manifest that developers can target as per article I published in the previous link.

We would need to provide all of the assemblies from this store in the buildpack, correct? If we removed any from the buildpack, we would break existing apps.

astrieanna commented 6 years ago

Fixed by https://github.com/cloudfoundry/dotnet-core-buildpack/commit/806bdbdc944c1d3d92293663f410a9cc451f7b2f