Stop relying on ReleaseId/DisplayVersion registry keys

[slonopotamus]

Instead, we now use Windows build numbers to identify host system.

This is a preparation step to add support for Windows Server 2022 LTSC,
whose DisplayVersion (21H2) collides with Windows 10 21H2 and Windows 11 21H2.

This commit also removes EOLed Windows 1903 and 1909 in order to reduce lookup tables that we have to hardcode.

[TBBle]

Windows 10 2004 hits EOL in December, and 20H2 EOLs in May next year. So if we just disappear them entirely, then we lose support for any Windows Client platforms.

I'd keep the EOL support around until we either decide to drop Windows 10 process isolation support once there's no supported base images available, or something changes on the MS side, or some other intervening idea occurs.

Better to say "Yes, we recognise this, it's EOL, are you sure you want to continue" when we can recognise a version.

Also, these two versions are only blacklisted for Docker versions before 19.03.6, so this change actually promotes them from EOL warning with user-override to "supported", i.e. silently accepted.

[slonopotamus]

The trick is that they're blacklisted as a host and EOL as a target. We silently allow building supported versions on EOL hosts even before this commit.

[slonopotamus]

Windows 10 2004 hits EOL in December, and 20H2 EOLs in May next year. So if we just disappear them entirely, then we lose support for any Windows Client platforms.

OK, agreed.

[TBBle]

Ah, good point. I'm fine with not complaining about using an EOL host to build a supported target.

I'm ambivalent about complaining when we try to use an EOL target; I'd lean towards keeping the warning around based on the calculated base tag (i.e. after applying the user's specified base tag), but that isn't really comprehensive or reliable (e.g., basetag 1909-KB4574727-amd64 would not be recognised), so I don't mind if we drop it now.

My personal use-cases are 100% process-isolation-based (and Windows Server LTSC-based 95% of the time), so I haven't had any value from the EOL warning myself.

I'd be interested to know if @adamrehn has any concerns about dropping this (or use-cases where it prevents a bad outcome), but if not, then I'm happy to resolve this thread.

[slonopotamus]

basetag 1909-KB4574727-amd64 would not be recognised

ue4-docker doesn't support building targets that are not hardcoded in valid tags list at all. #166 wanted to change that, but kinda failed.

---

I thought about 20H2 again. I very doubt that anyone would want to build ue4-docker images for it after it gets EOLed. If they're on a newer host, they would need to use Hyper-V isolation. And if you use Hyper-V isolation, why not just use a supported target version? Alternatively, they still run EOLed 20H2 on their host. Well, if you use EOLed Windows, also use EOLed version of ue4-docker that has 20H2 support, no?

[slonopotamus]

I think I broke the logic that checks if host is older than target again (as it attempted to happen in #166).

[TBBle]

I think I broke the logic that checks if host is older than target again (as it attempted to happen in #166).

Does making _tagsByBuildNumber an OrderedDict resolve it? At a brief glance, isNewerBaseTag depends on _validTags being ordered.

This whole test relies heavily on basetag<=>Build equivalence, but unless we extract the Windows OS version from the manifest of the base container image, we have no other way of knowing that a given base tag is for an older, newer, or matching kernel version compared to the host, so guessing by base tag is the best option I can see.

That said, if the user provides a base tag, e.g., they're deliberately building an EOL target container that we no longer know about, the WindowsUtils.isNewerBaseTag failure should probably be a warning (or skipped entirely... If the user provides a base tag, we have to drop a lot of assumptions about the format of that tag).

[slonopotamus]

Does making _tagsByBuildNumber an OrderedDict resolve it? At a brief glance, isNewerBaseTag depends on _validTags being ordered.

In Python, plain dict is insertion-ordered since Python 3.6. I have to check what is our oldest supported Python version... UPD: we're using f-strings that only appeared in 3.6, so older Python cannot run ue4-docker anyway.

isNewerBaseTag doesn't properly work for unknown things.

or skipped entirely... If the user provides a base tag, we have to drop a lot of assumptions about the format of that tag.

Aaand... That's exactly what I did in #166 :D

[TBBle]

And as I hid in my last wall-of-text comment on #166, I think sorting/ordering is fighting a losing battle. We can catch some of the problems, some of the time. We cannot correctly handle unknown things by SAC-style tags (i.e. ReleaseVersion registry value) because Windows Client 21H2 is both newer (Windows 11: 22000) and older (Windows 10: 19044) than Windows Server 21H2 (20348). That said, none of those three exist as images with a 21H2 tag, and this is a new situation compared to last time we looked at this, but I think that just strengthens my idea that we can't handle unknown things, because we definitely would have guessed wrong last time.

We can know the host build number, but for base tags, we can only know by either inspecting the manifest (which assumes we have the manifest at the time of running ue4-docker, and we can't assume that), or by knowing that certain images (path + tag) map to certain kernel builds, and that goes out the window as soon as I locally modify ue4-docker to use a local pull-through on-site Artifactory proxy in front of mcr.microsoft.com (or to target Insider builds, or both), or need to target a specific immutable version tag with -basetag, e.g. because I need a base image before the Feb 2020 security update to run on a not-updated LTSC 2019 server.

What if we drop the newer/older/etc. testing, and instead (not in this PR...) actually do a docker run <baseimage> -- echo "It works!" to verify functionality first. Then we can definitely say "Chosen base image doesn't work on this host", even if we cannot definitively say why.

We could try guessing, and perhaps even inspect the image (I think we can get that metadata) but that's a bonus at that point.

[slonopotamus]

So, let's agree that we want to extend current "we only accept things that we know" to "we accept whatever user gave as input" and just disable our checks for unknown tags?

[adamrehn]

Regarding version ordering, there are really only two scenarios where we actually care about the version relationship between the host and the target:

• When copying DLL files from the host system (no longer an issue after Use mcr.microsoft.com/windows image as a source for DLLs instead of host system #187.)

• When determining whether the host is capable of running target containers via either process isolation or Hyper-V isolation.

All of the ordering checks were implemented with DLL copying in mind and they've resulted in a number of limitations, such as awkwardness when using Insider Builds of Windows. With the DLL copying issue resolved, I'm all in favour of eliminating the ordering checks and performing a simple test to verify the ability to run containers as described by @TBBle above.

[TBBle]

We could try guessing, and perhaps even inspect the image (I think we can get that metadata) but that's a bonus at that point.

I checked in-passing, and docker image inspect will include the Windows build version the image is targeting. So if we go this route, then once we've docker run mcr... and it's failed, we can inspect that mcr... and hence have the relevant build numbers available.

We could in-fact auto-switch to Hyper-V isolation and/or reject the image if we first pull the base image, and then inspect it for the relevant metadata (I think it's marked as 'variant' in the Docker image metadata) before proceeding further.

Obviously if we're not going to actually do the build now, we shouldn't be pulling or inspecting stuff, as the host system may not be the final place it's being run anyway.

Anyway, it's a future thing to do, but at least it seems feasible.

[slonopotamus]

Okay, I think I did it this time.

We now:

1. Do not touch ReleaseId/DisplayVersion at all and use lookup table based on host build number.
2. Allow usage of any servercore base tag. The only caveat is that we skip checking whether that tag is newer/older and just issue a warning.

[slonopotamus]

@TBBle did you have a chance to look at updated changes?

[TBBle]

Sorry, I accidentally marked this read, forgot I had an outstanding review.

[TBBle]

Non-blocking nit-pick: I'm reasonably unfond of Optional[bool], but don't have a better option, except letting the ValueError bubble up, or throwing a custom exception, and catching that in the caller.

[slonopotamus]

Dunno, these both look equally ugly to me :D

I'd actually make a enum { Newer, Older, Unknown } but don't think it will improve anything at all given that we're in Python that doesn't even have switch/case. Okay, it got it in 3.10, but that's too much new yet to use.