fix: Removed dependency on 'ReleaseId' for WS2022 - #4659
Conversation
|
/azp run pr-e2e |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
fyi on the |
|
/azp run pr-e2e |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Commenter does not have sufficient privileges for PR 4659 in repo Azure/aks-engine |
| $windowsReleaseId = (Get-ItemProperty "HKLM:\SOFTWARE\Microsoft\Windows NT\CurrentVersion").ReleaseId | ||
| $windowsVersion = "" | ||
|
|
||
| # Starting with 20H2 tags used to publish container images may not match the 'ReleaseId' |
There was a problem hiding this comment.
It won't work at all for WS 2022 is that right?
There was a problem hiding this comment.
ReleaseId returns '2009' for anything 20H2 and beyond.
WS2022 is covered by the buildNumber switch.
20348 => ltsc2022
The ReleaseId fallback is for anything that the switch doesn't have a case for.
There was a problem hiding this comment.
I think our switch covers all known versions BuildId => image tags. This will cause strange runtime errors for anything newer than 20H2 as you experienced. It maybe be better to fail explicitly here. wdyt?
There was a problem hiding this comment.
If we could somehow cause aks-engine to hit an exception here, that would be ideal. It would say "Current Build not recognized" or something like that. This way, nobody will be rooting around in logs after-the-fact
There was a problem hiding this comment.
Are we testing versions that aren't captured in the switch? Would we have to update the corresponding unit tests?
There was a problem hiding this comment.
unfortunately this is done at cluster bring up time not the cluster template generation. Even though CSE would fail and some one would have to go get the logs to figure it out, I think that would be ideal over the cluster coming online but having strange behavior when deploying pods and trying to root cause it.
The reason it needs the version id here instead of just using a multi-arch image is the experimental Hyper-v support doesn't work with multi-arch images. You end up in a similar scenario where the cluster is online but pods can't be deployed.
|
/azp run pr-e2e |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run pr-e2e |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Nicolas Belovoskey (@nick5616) did you sign a contributor license agreement? |
|
There was some sort of error with the CLA back in September. Rebasing seems to have fixed it :) Mark Rossetti (@marosset) |
|
/lgtm |
|
/azp run pr-e2e |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run pr-e2e |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
James Sturtevant (@jsturtevant) can you review this again? |
|
Congrats on merging your first pull request! 🎉🎉🎉 |
|
Mark Rossetti (@marosset) James Sturtevant (@jsturtevant) will incorporating this change into a release require a new Windows VHD image? |
|
I don't think so as it's in the CSE files not the provisioning scripts |
6 participants
Reason for Change:
The behavior of ReleaseId replaced the ws2022 pause image with a 20h2 pause image, which prevented any ws2022 pods from starting.
Issue Fixed:
Credit Where Due:
Does this change contain code from or inspired by another project?
If "Yes," did you notify that project's maintainers and provide attribution?
Requirements:
Notes:
This is still an issue for any version of windows that does not have build number 20348 but is later than 20H2.