Add support for Windows OVS Containerisation in Install-OVS script #5055
Conversation
There was a problem hiding this comment.
I am a bit confused on the parameter "ContaineriseOVS", it looks when the flag is set as $true, no usespace processes is installed. Instead, when it is set as "$false", the OVS usespace service are installed. If this is your idea, I would prefer to modify this parameter with something like "InstallUserspace", and the default value is $true. The steps to install OVS userspace processes are ignored only when its value is set as $false.
I used the parameter name containeriseOVS so that user can decide whether they want to run ovs processes inside Container or on the host , hence the default value is false.When true it does not install the processes and user can then apply the updated antrea-windows-containerd.yaml to run these processes inside the container.I have updated relevant information in the windows.md doc file and in the comments section in Install OVS script. |
I got your idea, but my point is a parameter should always tell what to do but not "what not to do". The parameter "ContaineriseOVS" is telling the script not to install OVS Services. That is strange. So I suggested to use paramter to tell the script to install service with a default value $true. |
Okay I got your point. I have made the changes. Can you check whether anything else needs to be changed |
luolanzone
added
the
action/release-note
luolanzone
added
the
area/OS/windows
|
@XinShuYang in last sync meeting, you mentioned that there are three PRs need to be merged into one to pass CI tests, could you point out the PR numbers? thanks. |
NamanAg30
force-pushed
the
ovsscript
branch
4 times, most recently
from
6050c6f to
e48ca1f
Compare
|
@wenyingd Anything else that needs to be changed in this patch? |
For antrea-io#4952 Signed-off-by: Naman Agarwal <naman.agarwal75@gmail.com>
|
|
||
| .PARAMETER InstallUserspace | ||
| Specifies whether OVS userspace processes are included in the installation. If false, these processes will not | ||
| be installed as Windows service on the host. |
| @@ -22,6 +22,10 @@ Install OVS | |||
| .PARAMETER NodeIP | |||
| The node ip used by kubelet | |||
|
|
|||
| .PARAMETER InstallOVSUserspace | |||
| Specifies whether OVS userspace processes is included in the installation. If false, these processes will not | |||
| be installed as Windows service on the host. | |||
| @@ -23,6 +23,7 @@ The following components should be configured and run on the Windows Node. | |||
|
|
|||
| antrea-agent and kube-proxy run as processes on host and are managed by | |||
| management Pods. It is recommended to run OVS daemons as Windows services. | |||
| We also support running OVS processes inside container. | |||
There was a problem hiding this comment.
Could you adjust lines to wrap at column 80?
| @@ -302,6 +303,14 @@ get-service ovsdb-server | |||
| get-service ovs-vswitchd | |||
| ``` | |||
|
|
|||
| If you want to containerise OVS for containerd runtime, OVS userspace processes is | |||
There was a problem hiding this comment.
"containerize" is used more?
| @@ -302,6 +303,14 @@ get-service ovsdb-server | |||
| get-service ovs-vswitchd | |||
| ``` | |||
|
|
|||
| If you want to containerise OVS for containerd runtime, OVS userspace processes is | |||
| not required on host and hence you can use the `InstallUserspace` parameter | |||
There was a problem hiding this comment.
"not required" -> "not run"?
on the host
| @@ -302,6 +303,14 @@ get-service ovsdb-server | |||
| get-service ovs-vswitchd | |||
| ``` | |||
|
|
|||
| If you want to containerise OVS for containerd runtime, OVS userspace processes is | |||
| not required on host and hence you can use the `InstallUserspace` parameter | |||
| as false. | |||
There was a problem hiding this comment.
set the InstallUserspace parameter to false
rajnkamr
removed
area/OS/windows
|
This PR is stale because it has been open 90 days with no activity. Remove stale label or comment, or this will be closed in 90 days |
github-actions
bot
added
the
lifecycle/stale
For #4952