Skip to content

Conversation

@mburke5678
Copy link
Contributor

@mburke5678 mburke5678 commented May 2, 2022

https://issues.redhat.com/browse/OSDOCS-3575
For 5.1.0:
2022 support for vSphere, and 20H2 still supported
no changes to other platforms

Preview:
WINC prereqs:
Supported Windows Server version
Supported networking
Creating the vSphere Windows VM golden image. Updated Step 1

@netlify
Copy link

netlify bot commented May 2, 2022

Deploy Preview for osdocs ready!

Name Link
🔨 Latest commit b9e250873109cc2937deab94ed9b71dd53a016e9
🔍 Latest deploy log https://app.netlify.com/sites/osdocs/deploys/627acc941c56780009c3fc51
😎 Deploy Preview https://deploy-preview-45238--osdocs.netlify.app/openshift-enterprise/latest/windows_containers/creating_windows_machinesets/creating-windows-machineset-vsphere
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@openshift-ci openshift-ci bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 2, 2022
@openshift-ci openshift-ci bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels May 9, 2022
@mburke5678
Copy link
Contributor Author

@rrasouli Are you the QE resource for Windwos 2022 support in WMCO 5.1.0? If so, can you PTAL? Thank you in advance.

@rrasouli
Copy link

rrasouli commented May 10, 2022

@mburke5678 yes it is me, looks good to me. one comment to manually install the KB, should be mentioned there, with the PS command for the patch update. Get-WindowsUpdate -Install -KBArticleId 'KB5012637'

@mburke5678
Copy link
Contributor Author

mburke5678 commented May 10, 2022

@rrasouli
Where should we add this command?

one comment to manually install the KB, should be mentioned there, with the PS command for the patch update. Get-WindowsUpdate -Install -KBArticleId 'KB5012637'

Is this what you expect?

@mburke5678
Copy link
Contributor Author

@rrasouli Can you please take a look at the change I made at your suggestion? Thank you in advance.

@rrasouli
Copy link

I would like @jfrancoa to review as well
/lgtm looks good to me

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@saifshaikh48 I'm a little bit confused by this line and the block after line 74 (Install Microsoft container networking patch KB5012637)
So, here we are suggesting to install the KB patch via Get-WindowsUpdate, but later we are suggesting to install it again by downloading it directly? Or are these two different steps required to get the full patch installed?
I believe we did in our golden image what's indicated in line 26 and so far it has worked fine.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like 2 different approaches to get the same result. In the dev golden image, we performed the steps in the block after line 74. Seems like we can suggest either or both approaches, I have no specific preference.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the clarification @saifshaikh48, I'll propose a modification to @mburke5678 then.

Copy link
Contributor Author

@mburke5678 mburke5678 May 18, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jfrancoa Thanks for catching this. It does seem redundant. Please let me know which of the two instances we should remove. Probably should remove the second instance, lines 74 to 90?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my humble opinion I would leave the direct download and installation via script (lines 74 to 90) and either remove or make a "short" note on the existence of Get-WindowsUpdate command. It does look simpler to use, but I remember that we had issues to get the Get-WindowsUpdate command available and we had to install some extra module (PSWindowsUpdate)...however, the steps from 74 to 90 look bulletproof. My fear is that the user would get lost in the process of installing extra stuff to get the KB patch installed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jfrancoa @saifshaikh48 Any thoughts on this ^^

@openshift-ci openshift-ci bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 27, 2022
@mburke5678 mburke5678 force-pushed the OSDOCS-3575-windows-2022-winc-51 branch from e8f2ffc to 390a836 Compare June 1, 2022 21:08
@openshift-ci openshift-ci bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jun 1, 2022
@mburke5678 mburke5678 closed this Jun 1, 2022
@mburke5678 mburke5678 reopened this Jun 1, 2022
Copy link

@jfrancoa jfrancoa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I had this review in pending state for several days already and I forgot to submit it.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would clarify here that the steps to install this patch can be followed if the command suggested in line 26 was not executed. If, on the contrary, it was then we can simply skip it.
Or, you can remove:

"For example, you can use the following PowerShell command to download and install the patch:
+
[source,posh]
----
PS C:\> Get-WindowsUpdate -Install -KBArticleId 'KB5012637'"

from step 1 and move it into step 7, stating it as:

You can use the following PowerShell command to download and install the patch:
+
[source,posh]
----
PS C:\> Get-WindowsUpdate -Install -KBArticleId 'KB5012637'

Or you can manually download the patch files from the link blabla and execute the script

This is a good approach as the Get-WindowsUpdate command doesn't come out of the box, so some users might prefer one or the other.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jfrancoa @rrasouli Do we need to include details on how to download and install the patch? We didn't include the steps previously.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good point...probably it's not really needed. It's helpful for the user, though..but it's clearly out of the scope from this guideline. And if historically this hasn't been explained (and nobody has complained about it so far..), maybe the best is to just update the KB number and leave it as it was.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my humble opinion I would leave the direct download and installation via script (lines 74 to 90) and either remove or make a "short" note on the existence of Get-WindowsUpdate command. It does look simpler to use, but I remember that we had issues to get the Get-WindowsUpdate command available and we had to install some extra module (PSWindowsUpdate)...however, the steps from 74 to 90 look bulletproof. My fear is that the user would get lost in the process of installing extra stuff to get the KB patch installed.

@openshift-ci openshift-ci bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jun 3, 2022
@jfrancoa
Copy link

jfrancoa commented Jun 6, 2022

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jun 6, 2022
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Jun 6, 2022
@openshift-ci
Copy link

openshift-ci bot commented Jun 6, 2022

New changes are detected. LGTM label has been removed.


|Default VXLAN port
|Windows Server Long-Term Servicing Channel (LTSC): Windows Server 2019
Windows Server 2019 (version 1809)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Windows Server 2019 (version 1809)
|Windows Server 2019 (version 1809)

@mburke5678
Copy link
Contributor Author

Closing in favor of #46460

@mburke5678 mburke5678 closed this Jun 8, 2022
@mburke5678 mburke5678 deleted the OSDOCS-3575-windows-2022-winc-51 branch June 8, 2022 20:22
@kalexand-rh kalexand-rh removed this from the Future Release milestone Aug 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

branch/enterprise-4.10 size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants