Streamlining OVS Driver Installation and Upgrade for Windows Container

[XinShuYang]

Describe the problem/challenge you have
I am encountering some challenges with the installation and upgrade of the OVS driver on Windows when OVS is running inside the container. There are two main related issues and I would like to discuss potential solutions or workarounds.

1. Installation of Own signed OVS Drivers
   Currently, we support the automatic installation of signed OVS drivers during running OVS container. However, for the own signed driver, developers are required to manually run Install-OVS.PS1 to import the certificate(https://github.com/gran-vmv/antrea/blob/ipam-e2e-k8s/docs/windows.md#1-optional-install-ovs-provided-by-antrea-or-your-own). To streamline this process, maybe we can potentially automate the certificate import step inside the container. But I am not sure how to efficiently manage and possibly clean up any redundant certificates that may accumulate over time. Also I am not sure if cleaning these certificates is necessary, or how it could be implemented effectively.

2. Upgrading OVS Drivers on the Same Windows Host
   In scenarios where the OVS container image is upgraded on the same Windows host, we currently run netcfg -q ovsext to check if the OVS driver is installed.

   antrea/build/yamls/antrea-windows-with-ovs.yml

   Lines 51 to 55 in 023f000

   	$driverStatus = netcfg -q ovsext
   	if ($driverStatus -like '*not installed*') {
   	# Install OVS Driver
   	netcfg -l $OVSDriverDir/ovsext.inf -c s -i OVSExt
   	}

   But this command doesn't indicate whether the OVS driver needs to be reinstalled during an upgrade. In fact, after the OVS driver is installed for the first time, this check will always return the result that the driver is already installed, so the driver in the new OVS image is never installed. I think we need a better method to determine if the driver installation is necessary for the upgrade.

Describe the solution you'd like
I haven't found a good solutions for these two issues yet, want to involve more people for discussion and suggestions. @antoninbas @wenyingd @rajnkamr

[antoninbas]

For 1, I just want to confirm my understanding. I assume you are referring to the test-signed driver case, where users have to run .\Install-OVS.ps1 -InstallUserspace $false? If yes, I agree it would be nice to remove the requirement in this case (i.e., users no longer have to run .\Install-OVS.ps1 directly). However, the script doesn't just import the certificate, it also downloads OVS. Can we handle this without running the script?

If you are referring to what we discussed in #5789 (comment) (also tracked in #5624 (comment)), then it seems you were implying that we could download and install the OVS driver as part of the DaemonSet (not just load the certificate).

[rajnkamr]

• To streamline this process, maybe we can potentially automate the certificate import step inside the container. But I am not sure how to efficiently manage and possibly clean up any redundant certificates that may accumulate over time. Also I am not sure if cleaning these certificates is necessary, or how it could be implemented effectively.

For Issue 1, IMO, It is preferable to streamline certificate installation whether it is signed or unsigned (test-signed),
To ensure the script not only checks for redundant certificates but also manages the addition of new certificates effectively, we need to enhance it to handle both cases
Check if the certificate already exists before importing.
Identify and remove redundant /additional certificates periodically or after every import.

2. But this command doesn't indicate whether the OVS driver needs to be reinstalled during an upgrade. In fact, after the OVS driver is installed for the first time, this check will always return the result that the driver is already installed, so the driver in the new OVS image is never installed.

For issue 2,
We could run pre upgrade check and compare with currently installed version with to be upgrade version of driver in current package before going for driver installation check "netcfg -q ovsext" !

[XinShuYang]

For 1, I just want to confirm my understanding. I assume you are referring to the test-signed driver case, where users have to run .\Install-OVS.ps1 -InstallUserspace $false? If yes, I agree it would be nice to remove the requirement in this case (i.e., users no longer have to run .\Install-OVS.ps1 directly). However, the script doesn't just import the certificate, it also downloads OVS. Can we handle this without running the script?

If you are referring to what we discussed in #5789 (comment) (also tracked in #5624 (comment)), then it seems you were implying that we could download and install the OVS driver as part of the DaemonSet (not just load the certificate).

For the user-provided signed OVS driver, I prefer the user should place it into a custom antrea-ovs image to be compatible with our existing DaemonSet manifest. When the DaemonSet starts, it can check for the existence of a certificate in the target path to decide whether to import the existing certificate or generate a new one. What do you think? @antoninbas @wenyingd

[XinShuYang]

2. But this command doesn't indicate whether the OVS driver needs to be reinstalled during an upgrade. In fact, after the OVS driver is installed for the first time, this check will always return the result that the driver is already installed, so the driver in the new OVS image is never installed.

For issue 2, We could run pre upgrade check and compare with currently installed version with to be upgrade version of driver in current package before going for driver installation check "netcfg -q ovsext" !

For issue2, netcfg -q ovsext can‘t provide sufficient information about the installed driver status, @wenyingd investigated this issue and had an enhancement PR in #6383

[antoninbas]

@XinShuYang

For the user-provided signed OVS driver, I prefer the user should place it into a custom antrea-ovs image to be compatible with our existing DaemonSet manifest.

I don't think this relates to my question?

Maybe the confusion comes from the fact that your issue refers to the "unsigned driver" case, but I don't think there is such a thing. Drivers are always signed, but it can be an actual signature or a test signature.
Maybe you could clearly identify all 4 scenarios from the table in https://github.com/antrea-io/antrea/blob/main/docs/windows.md#1-optional-install-ovs-provided-by-antrea-or-your-own, and describe whether you want to change how we handle that scenario, and if yes, how.

For the user-provided signed OVS driver, I prefer the user should place it into a custom antrea-ovs image to be compatible with our existing DaemonSet manifest. When the DaemonSet starts, it can check for the existence of a certificate in the target path to decide whether to import the existing certificate or generate a new one.

For this case (second row of the table?), I think I agree. At the moment, we tells users they don't need to run .\Install-OVS.ps1 in this case, but that means they are responsible for manually installing the OVS driver on every Windows Node. If we can provide a script that "mutates" the upstream antrea-windows image to add a local file containing the OVS driver to it, I think that could be a good solution, especially if the script itself can be run on Linux / macOS (Docker Desktop). At a later time we could also consider supporting PersistentVolume/PersistentVolumeClaim as a way to make a signed driver file accessible to the DaemonSet.

[XinShuYang]

@XinShuYang

For the user-provided signed OVS driver, I prefer the user should place it into a custom antrea-ovs image to be compatible with our existing DaemonSet manifest.

I don't think this relates to my question?

Maybe the confusion comes from the fact that your issue refers to the "unsigned driver" case, but I don't think there is such a thing. Drivers are always signed, but it can be an actual signature or a test signature. Maybe you could clearly identify all 4 scenarios from the table in https://github.com/antrea-io/antrea/blob/main/docs/windows.md#1-optional-install-ovs-provided-by-antrea-or-your-own, and describe whether you want to change how we handle that scenario, and if yes, how.

Got your point, when I mentioned "unsigned driver," I was referring to the "own signed driver" mentioned in the document (I am not sure if there's a more accurate way to express it..). I have updated the relevant description in the issue.

For the user-provided signed OVS driver, I prefer the user should place it into a custom antrea-ovs image to be compatible with our existing DaemonSet manifest. When the DaemonSet starts, it can check for the existence of a certificate in the target path to decide whether to import the existing certificate or generate a new one.

For this case (second row of the table?), I think I agree. At the moment, we tells users they don't need to run .\Install-OVS.ps1 in this case, but that means they are responsible for manually installing the OVS driver on every Windows Node. If we can provide a script that "mutates" the upstream antrea-windows image to add a local file containing the OVS driver to it, I think that could be a good solution, especially if the script itself can be run on Linux / macOS (Docker Desktop). At a later time we could also consider supporting PersistentVolume/PersistentVolumeClaim as a way to make a signed driver file accessible to the DaemonSet.

Regarding this proposal, @wenyingd and I have some modifications: we plan to no longer provide users with the option to import their own generated certificate files. Instead, we will run Get-AuthenticodeSignature to check whether the driver is own signed. If it is, we will generate a new certificate and import it directly; otherwise(for test-signed driver), we will install the driver directly (since test-signed drivers can be installed without importing the certificate). You can see the specific implementation in #6383 (https://github.com/wenyingd/antrea/blob/76e5613a6902238d4494bfe21d18e88bc2f5f76c/hack/windows/OVSHelper.psm1#L52-L60). Do you have any other concerns or questions? @antoninbas

[XinShuYang]

Close this issue as #6383 has been merged, thank you for the discussion.