[Windows] ContainerD support #1781

ruicao93 · 2021-01-25T15:21:08Z

Use HCS API for container network configuration.
Create OVS port asynchronously. When using ContainerD as runtime,
network interface is created after sandbox container creation
complete. So Antrea needs to wait for network interface realized
before creating OVS port.
Use HNSEndpoint to cache params which are used to create OVS port.
During boot stage, antrea-agent could leverage the cached params
to create missing OVS port.

Signed-off-by: Rui Cao rcao@vmware.com

codecov-io · 2021-01-25T15:50:20Z

Codecov Report

❗ No coverage uploaded for pull request base (main@d5634b5). Click here to learn what that means.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1781   +/-   ##
=======================================
  Coverage        ?   42.79%           
=======================================
  Files           ?      109           
  Lines           ?    13605           
  Branches        ?        0           
=======================================
  Hits            ?     5822           
  Misses          ?     7296           
  Partials        ?      487

Flag	Coverage Δ
unit-tests	`42.79% <0.00%> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

1. Use HCS API for container network configuration. 2. Create OVS port asynchronously. When using ContainerD as runtime, network interface is created after sandbox container creation complete. So Antrea needs to wait for network interface realized before creating OVS port. 3. Use HNSEndpoint to cache params which are used to create OVS port. During boot stage, antrea-agent could leverage the cached params to create missed OVS port. Signed-off-by: Rui Cao <rcao@vmware.com>

ruicao93 · 2021-01-25T16:36:39Z

/test-all

ruicao93 · 2021-01-25T17:41:04Z

/test-conformance

antoninbas · 2021-01-25T19:19:24Z

@ruicao93 could you link it to the appropriate issue ? #1679 I believe

ruicao93 · 2021-01-26T12:35:50Z

@ruicao93 could you link it to the appropriate issue ? #1679 I believe

Thanks Antonin. Add the issue link.

pkg/agent/cniserver/server_windows.go

pkg/agent/cniserver/pod_configuration_windows.go

pkg/agent/cniserver/pod_configuration_linux.go

pkg/agent/cniserver/pod_configuration_windows.go

pkg/agent/cniserver/interface_configuration_windows.go

pkg/agent/cniserver/pod_configuration.go

pkg/agent/cniserver/pod_configuration_windows.go

pkg/agent/cniserver/interface_configuration_windows.go

Signed-off-by: Rui Cao <rcao@vmware.com>

ruicao93 · 2021-01-27T06:50:05Z

/test-all

pkg/agent/cniserver/pod_configuration_windows.go

pkg/agent/cniserver/interface_configuration_windows.go

pkg/agent/cniserver/pod_configuration.go

pkg/agent/cniserver/pod_configuration_linux.go

Signed-off-by: Rui Cao <rcao@vmware.com>

pkg/agent/cniserver/interface_configuration_windows.go

go.sum

pkg/agent/cniserver/interface_configuration_windows.go

pkg/agent/cniserver/pod_configuration_windows.go

Signed-off-by: Rui Cao <rcao@vmware.com>

jianjuns · 2021-01-30T00:10:12Z

pkg/agent/util/net_windows.go

+	if _, err := net.InterfaceByName(ifaceName); err == nil {
+		return true
+	}
+	cmd := fmt.Sprintf(`Get-NetAdapter -InterfaceAlias "%s"`, ifaceName)


Could you add some comments to explain why need to run the PS command to check?

Sure. Add comments:

// Some kinds of interface cannot be retrieved by "net.InterfaceByName" such as // container vnic. // So if a interface cannot be found by above function, use powershell command // "Get-NetAdapter" to check if it exists.

pkg/agent/cniserver/server_windows.go

pkg/agent/cniserver/interface_configuration_windows.go

jianjuns · 2021-01-30T00:24:09Z

pkg/agent/cniserver/interface_configuration_windows.go

+	//     The name is same as the OVS port name and HNSEndpoint name.
+	//   - containerID: Used as key for goroutine lock to avoid concurrency issue.
+	//   - podName and PodNamespace: Used to identify the owner of the HNSEndpoint.
+	//   - dummyMac: the MAC address of the HNSEndpoint is unknown before we create it.


Sorry, then why we need to save dummyMAC? To indicate the real MAC is not known?

We save a dummyMAC to avoid printing error in parseConfig(). The parseConfig fuinction will print error if MAC address not exists.

I would change ParseOVSPortInterfaceConfig() in this case, like a flag to indicate whether MAC should be present, or change to let its callers check whether MAC is present.

Sounds good. I add a checkMac param to ParseOVSPortInterfaceConfig() to determine if print error msg after parse MAC.

antoninbas

a couple more minor comments, otherwise LGTM

pkg/agent/cniserver/server_windows.go

pkg/agent/cniserver/interface_configuration_windows.go

antoninbas · 2021-01-30T00:24:43Z

pkg/agent/cniserver/interface_configuration_windows.go

 	} else {
-		if err := hcsshim.HotAttachEndpoint(containerID, ep.Id); err != nil {
-			if isInfraContainer(sandbox) || hcsshim.ErrComputeSystemDoesNotExist != err {
+		if hcnEp == nil {


maybe use an else if "statement" to avoid one level of indentation

btw the hcnEp == nil case means that the container runtime is docker correct?

Yes, hcnEp == nil means the runtime is Docker.

I want to keep them in same level.

if hcnEp == nil { // docker ops } else { // containerd ops }

antoninbas · 2021-01-30T00:33:02Z

pkg/agent/cniserver/interface_configuration_windows.go

+		if hcnEndpoint != nil && hcnEndpoint.HostComputeNamespace != "" {
+			err := hcn.RemoveNamespaceEndpoint(hcnEndpoint.HostComputeNamespace, hcnEndpoint.Id)
+			if err != nil {
+				klog.Errorf("Failed to remove HostComputeEndpoint %s from HostComputeNameSpace %s: %v", hcnEndpoint.Name, hcnEndpoint.HostComputeNamespace, err)


I see, thanks for pointing it out.

Then maybe let's always log the error in the goroutine (also for the call to endpoint.Delete below), assuming it is an actual error (and not "not found").

Then, also assuming it is an actual error, we can write it to the deleteCh channel. To avoid double logging, we can
change the select case for deleteCh to:

case err := <-deleteCh: if err != nil { return err }

What do you think?

pkg/agent/cniserver/pod_configuration_windows.go

Signed-off-by: Rui Cao <rcao@vmware.com>

ruicao93 · 2021-02-01T23:37:48Z

Then, also assuming it is an actual error, we can write it to the deleteCh channel. To avoid double logging, we can
change the select case for deleteCh to:

	case err := <-deleteCh:
		if err != nil {
			return err
		}
What do you think?

@antoninbas , sounds great. I move the error check and logging in gorutine, and only return result here.

Signed-off-by: Rui Cao <rcao@vmware.com>

ruicao93 · 2021-02-02T11:26:50Z

/test-all

jianjuns

LGTM.

vmwclabot added the cla-not-required label Jan 25, 2021

ruicao93 changed the title ~~[Windows] ContainerD support~~ [WIP][Windows] ContainerD support Jan 25, 2021

ruicao93 force-pushed the containerd-workaround-dev2 branch from 080a7f4 to 02ff879 Compare January 25, 2021 15:45

ruicao93 force-pushed the containerd-workaround-dev2 branch from 02ff879 to f8074f7 Compare January 25, 2021 16:15

Base automatically changed from master to main January 26, 2021 00:00

ruicao93 changed the title ~~[WIP][Windows] ContainerD support~~ [Windows] ContainerD support Jan 26, 2021

ruicao93 requested review from tnqn and antoninbas January 26, 2021 11:49

jianjuns reviewed Jan 26, 2021

View reviewed changes

antoninbas reviewed Jan 27, 2021

View reviewed changes

Address comments

c3cf0d2

Signed-off-by: Rui Cao <rcao@vmware.com>

ruicao93 force-pushed the containerd-workaround-dev2 branch from 07db605 to c3cf0d2 Compare January 27, 2021 03:21

jianjuns reviewed Jan 28, 2021

View reviewed changes

Address comments

9a2d766

Signed-off-by: Rui Cao <rcao@vmware.com>

antoninbas reviewed Jan 28, 2021

View reviewed changes

Address comments

198ff65

Signed-off-by: Rui Cao <rcao@vmware.com>

jianjuns reviewed Jan 30, 2021

View reviewed changes

antoninbas reviewed Jan 30, 2021

View reviewed changes

Address comments

c25a24e

Signed-off-by: Rui Cao <rcao@vmware.com>

Address comments

df3c8bf

Signed-off-by: Rui Cao <rcao@vmware.com>

ruicao93 force-pushed the containerd-workaround-dev2 branch from adf8769 to df3c8bf Compare February 1, 2021 23:51

jianjuns approved these changes Feb 2, 2021

View reviewed changes

ruicao93 merged commit 1a53d76 into antrea-io:main Feb 3, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Windows] ContainerD support #1781

[Windows] ContainerD support #1781

ruicao93 commented Jan 25, 2021 •

edited

Loading

codecov-io commented Jan 25, 2021 •

edited

Loading

ruicao93 commented Jan 25, 2021

ruicao93 commented Jan 25, 2021

antoninbas commented Jan 25, 2021 •

edited

Loading

ruicao93 commented Jan 26, 2021

ruicao93 commented Jan 27, 2021

jianjuns Jan 30, 2021

ruicao93 Feb 1, 2021

jianjuns Jan 30, 2021

ruicao93 Feb 1, 2021

jianjuns Feb 1, 2021

ruicao93 Feb 1, 2021

antoninbas left a comment

antoninbas Jan 30, 2021

ruicao93 Feb 1, 2021

antoninbas Jan 30, 2021

ruicao93 commented Feb 1, 2021

ruicao93 commented Feb 2, 2021

jianjuns left a comment

[Windows] ContainerD support #1781

[Windows] ContainerD support #1781

Conversation

ruicao93 commented Jan 25, 2021 • edited Loading

codecov-io commented Jan 25, 2021 • edited Loading

Codecov Report

ruicao93 commented Jan 25, 2021

ruicao93 commented Jan 25, 2021

antoninbas commented Jan 25, 2021 • edited Loading

ruicao93 commented Jan 26, 2021

ruicao93 commented Jan 27, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

antoninbas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ruicao93 commented Feb 1, 2021

ruicao93 commented Feb 2, 2021

jianjuns left a comment

Choose a reason for hiding this comment

ruicao93 commented Jan 25, 2021 •

edited

Loading

codecov-io commented Jan 25, 2021 •

edited

Loading

antoninbas commented Jan 25, 2021 •

edited

Loading