Fix restart error during build process for Windows

[RadekRDK]

When trying to install the .NET Framework manually, the image build process fails because .NET demands a system restart after installation. This interrupts the build process entirely, and no image can be built. Instead, a base image should be used that already has the .NET environment included in it, so that the framework installation process is skipped entirely.

Additionally, the command instruction has incorrect syntax, precisely the escaped quotation marks which should not be there.

[neomantra]

Thanks for this PR. Since we are making changes here, is there any reason not to move to 2022 images?

I felt like we had to add that quoting to make it work, but that was a while ago and I certainly felt like a chimpanzee flinging things when I was doing it... ;) If it works without, then great!

[RadekRDK]

There is nothing keeping us from moving to 2022 images really. Only point against this that I can think of is to keep backwards compatibility with older Windows versions, since you can just use Hyper-V isolation to run older container versions on a newer host version, whereas (as far as I know) you are out of luck if it's the other way around. However, I am poor at judging whether it makes sense to worry about this, so take this with a grain of salt. Otherwise, we can easily just move to the 2022 tag for each of the base images.

[neomantra]

I was talking about it with my .NET friend to help ground me. I think there's three things here:

1. quoting fix (✅ )
2. 2019 vs 2022 (don't break things, but advance)
3. current or dotnet (why bloat for those don't need?)

(Also maybe 2022 fixes the restarting problem? But maybe moot.)

Either way, 2 and 3 are just build-arg injection. It's pretty easy to have 4 flavors of large categories like so (appveyor):

- ps: docker build --pull -t openresty:windows-2019 -t openresty:windows  -f windows/Dockerfile .
- ps: docker build --pull -t openresty:windows-2019-dotnet -f windows/Dockerfile --build-arg RESTY_INSTALL_BASE="mcr.microsoft.com/dotnet/framework/runtime" --build-arg RESTY_INSTALL_TAG="4.8-windowsservercore-ltsc2019" .
- ps: docker build --pull -t openresty:windows-2022 -f windows/Dockerfile --build-arg --build-arg RESTY_INSTALL_TAG="ltsc2022" .
- ps: docker build --pull -t openresty:windows-2022-dotnet -f windows/Dockerfile --build-arg RESTY_INSTALL_BASE="mcr.microsoft.com/dotnet/framework/runtime" --build-arg RESTY_INSTALL_TAG="4.8-windowsservercore-ltsc2022" .

Thoughts on this? I haven't tested that at all and perhaps the args aren't right. Can you test those commands out on a Windows box and update your PR with the correct ones?

[t-beckmann]

The docker file parser escape sequence at the top does not seem to affect the CMD instruction when supplying a JSON array in more recent docker versions. As a result, the backtick causes a broken command in the resulting image. However, the escaping is not needed because the CMD array is passed as individual command line arguments anyways so that removal of that broken escapes makes total sense.

If I understand correctly, the RESTY_INSTALL_BASE image is used for the downloader only. The 4.8 .NET framework here is required for running chocolatey only. The resulting target image used at OpenRESTY runtime is based on the nanoserver and does not contain the .NET runtime.

Both changes made result in a workable windows container image...

[RadekRDK]

The install arguments are only used for installing Chocolatey in the first build stage. Which install image is used makes no difference for the runtime build, so setting them for each separate windows version build is redundant.

I have tested this further, and it turns out that we do not even need the dotnet base image, as the server core image from the ltsc2022 tag already seems to have a dotnet runtime included in it. It is certainly not affected by the restart error anymore. Simply switching RESTY_INSTALL_TAG from ltsc2019 to ltsc2022 therefore sufficed.

[neomantra]

OK, then it seems reasonable to just make that change. Thank you both for the discussion and verification.

@RadekRDK Can you update your PR with just changing the RESTY_INSTALL_TAG and the quoting Also please update the README (with the new default value) and CHANGELOG?

[RadekRDK]

Updating the install tag while keeping the server core image breaks the appveyor pipeline for some reason. Possibly the runner does not support 2022 images yet. Only solution that I can give for this is switching back to the dotnet image.

[neomantra]

Thanks for these changes. I'll poke at Appveyor later today and try to get this out.

[neomantra]

I think this can be fixed by changing this image line in appveyor.yml to say 2022 instead of 2019. This is a list of available build environments.

Can you add that change to your PR and push it? While you are at it, please change the 2016 in the lower part to 2022 ? Thanks!

[RadekRDK]

I have managed to establish a functioning appveyor pipeline with my current changes. Changing the appveyor image to 2022 unfortunately did not work, so instead I am resorting to the dotnet image instead. I guess it's the simplest solution and also guarantees that the install image has a .NET runtime available in it at all times.

[neomantra]

As I got back to upgrading this for 1.25.3.1, I realized this PR was still open..... can you clarify what specific code change you needed for success?

[RadekRDK]

For pipeline success, changing the RESTY_INSTALL_BASE and RESTY_INSTALL_TAG build arguments to the dotnet image was the necessary change. This skips the step of installing a dotnet runtime during the image build.

[neomantra]

Thanks for updating the PR -- code is clearer than prose.

[RadekRDK]

I am done making changes to this PR. Is there anything else I need to do for this to become merge ready?

[neomantra]

Merged this with some minor tweaks, thanks!