From 1f7460c15dd74b353f8d61d2a6698d0ed4211057 Mon Sep 17 00:00:00 2001 From: Shuyang Xin Date: Thu, 22 Feb 2024 15:47:45 +0800 Subject: [PATCH] Remove Docker Support for Antrea Windows This commit removes the support for Docker from the Antrea Windows Agent. The specific changes made in this commit include modifying the CNI call to return an error when the runtime is identified as Docker on Windows. Signed-off-by: Shuyang Xin --- .../interface_configuration_linux.go | 4 - .../interface_configuration_windows.go | 56 ++----- pkg/agent/cniserver/pod_configuration.go | 15 +- .../cniserver/pod_configuration_windows.go | 11 -- pkg/agent/cniserver/server.go | 16 ++ pkg/agent/cniserver/server_linux.go | 6 + pkg/agent/cniserver/server_windows.go | 6 + pkg/agent/cniserver/server_windows_test.go | 140 ++---------------- 8 files changed, 56 insertions(+), 198 deletions(-) diff --git a/pkg/agent/cniserver/interface_configuration_linux.go b/pkg/agent/cniserver/interface_configuration_linux.go index da83ff6569f..822f6dcc9dd 100644 --- a/pkg/agent/cniserver/interface_configuration_linux.go +++ b/pkg/agent/cniserver/interface_configuration_linux.go @@ -647,7 +647,3 @@ func isVeth(link netlink.Link) bool { _, isVeth := link.(*netlink.Veth) return isVeth } - -func getOVSInterfaceType(ovsPortName string) int { - return defaultOVSInterfaceType -} diff --git a/pkg/agent/cniserver/interface_configuration_windows.go b/pkg/agent/cniserver/interface_configuration_windows.go index a083354369d..c4bc721b005 100644 --- a/pkg/agent/cniserver/interface_configuration_windows.go +++ b/pkg/agent/cniserver/interface_configuration_windows.go @@ -51,7 +51,6 @@ var ( getNamespaceEndpointIDsFunc = hcn.GetNamespaceEndpointIds hotAttachEndpointFunc = hcsshim.HotAttachEndpoint attachEndpointInNamespaceFunc = attachEndpointInNamespace - isContainerAttachOnEndpointFunc = isContainerAttachOnEndpoint getHcnEndpointByIDFunc = hcn.GetEndpointByID deleteHnsEndpointFunc = deleteHnsEndpoint removeEndpointFromNamespaceFunc = hcn.RemoveNamespaceEndpoint @@ -227,42 +226,18 @@ func attachContainerLink(ep *hcsshim.HNSEndpoint, containerID, sandbox, containe var attached bool var err error var hcnEp *hcn.HostComputeEndpoint - if isDockerContainer(sandbox) { - // Docker runtime - attached, err = isContainerAttachOnEndpointFunc(ep, containerID) - if err != nil { - return nil, err - } - } else { - // containerd runtime - if hcnEp, err = getHcnEndpointByIDFunc(ep.Id); err != nil { - return nil, err - } - attachedEpIds, err := getNamespaceEndpointIDsFunc(sandbox) - if err != nil { - return nil, err - } - for _, existingEP := range attachedEpIds { - if existingEP == hcnEp.Id { - attached = true - break - } - } + + // containerd runtime + if hcnEp, err = getHcnEndpointByIDFunc(ep.Id); err != nil { + return nil, err } - if attached { - klog.V(2).Infof("HNS Endpoint %s already attached on container %s", ep.Id, containerID) + if hcnEp == nil { + return nil, fmt.Errorf("failed to get hcnEp on container %s", containerID) } else { - if hcnEp == nil { - // Docker runtime - if err := hotAttachEndpointFunc(containerID, ep.Id); err != nil { - return nil, err - } - } else { - // containerd runtime - if err := attachEndpointInNamespaceFunc(hcnEp, sandbox); err != nil { - return nil, err - } + // containerd runtime + if err := attachEndpointInNamespaceFunc(hcnEp, sandbox); err != nil { + return nil, err } } containerIface := ¤t.Interface{ @@ -273,10 +248,6 @@ func attachContainerLink(ep *hcsshim.HNSEndpoint, containerID, sandbox, containe return containerIface, nil } -func isContainerAttachOnEndpoint(endpoint *hcsshim.HNSEndpoint, containerID string) (bool, error) { - return endpoint.IsAttached(containerID) -} - func attachEndpointInNamespace(hcnEp *hcn.HostComputeEndpoint, sandbox string) error { return hcnEp.NamespaceAttach(sandbox) } @@ -485,15 +456,6 @@ func (ic *ifConfigurator) getInterceptedInterfaces( return nil, nil, errors.New("getInterceptedInterfaces is unsupported on Windows") } -// getOVSInterfaceType returns "internal". Windows uses internal OVS interface for container vNIC. -func getOVSInterfaceType(ovsPortName string) int { - ifaceName := fmt.Sprintf("vEthernet (%s)", ovsPortName) - if !hostInterfaceExistsFunc(ifaceName) { - return defaultOVSInterfaceType - } - return internalOVSInterfaceType -} - func (ic *ifConfigurator) addPostInterfaceCreateHook(containerID, endpointName string, containerAccess *containerAccessArbitrator, hook postInterfaceCreateHook) error { if containerAccess == nil { return fmt.Errorf("container lock cannot be null") diff --git a/pkg/agent/cniserver/pod_configuration.go b/pkg/agent/cniserver/pod_configuration.go index 351b13c19bd..120c74e04a6 100644 --- a/pkg/agent/cniserver/pod_configuration.go +++ b/pkg/agent/cniserver/pod_configuration.go @@ -56,7 +56,6 @@ const ( const ( defaultOVSInterfaceType int = iota //nolint suppress deadcode check for windows - internalOVSInterfaceType defaultIFDevName = "eth0" ) @@ -265,15 +264,11 @@ func (pc *podConfigurator) configureInterfacesCommon( func (pc *podConfigurator) createOVSPort(ovsPortName string, ovsAttachInfo map[string]interface{}, vlanID uint16) (string, error) { var portUUID string var err error - switch getOVSInterfaceType(ovsPortName) { - case internalOVSInterfaceType: - portUUID, err = pc.ovsBridgeClient.CreateInternalPort(ovsPortName, 0, "", ovsAttachInfo) - default: - if vlanID == 0 { - portUUID, err = pc.ovsBridgeClient.CreatePort(ovsPortName, ovsPortName, ovsAttachInfo) - } else { - portUUID, err = pc.ovsBridgeClient.CreateAccessPort(ovsPortName, ovsPortName, ovsAttachInfo, vlanID) - } + + if vlanID == 0 { + portUUID, err = pc.ovsBridgeClient.CreatePort(ovsPortName, ovsPortName, ovsAttachInfo) + } else { + portUUID, err = pc.ovsBridgeClient.CreateAccessPort(ovsPortName, ovsPortName, ovsAttachInfo, vlanID) } if err != nil { klog.Errorf("Failed to add OVS port %s, remove from local cache: %v", ovsPortName, err) diff --git a/pkg/agent/cniserver/pod_configuration_windows.go b/pkg/agent/cniserver/pod_configuration_windows.go index 8842a6bf416..d2a5981d377 100644 --- a/pkg/agent/cniserver/pod_configuration_windows.go +++ b/pkg/agent/cniserver/pod_configuration_windows.go @@ -26,7 +26,6 @@ import ( "antrea.io/antrea/pkg/agent/cniserver/ipam" "antrea.io/antrea/pkg/agent/interfacestore" "antrea.io/antrea/pkg/agent/types" - "antrea.io/antrea/pkg/agent/util" "antrea.io/antrea/pkg/util/k8s" ) @@ -72,20 +71,10 @@ func (pc *podConfigurator) connectInterfaceToOVS( // Use the outer veth interface name as the OVS port name. ovsPortName := hostIface.Name containerConfig := buildContainerConfig(ovsPortName, containerID, podName, podNamespace, containerIface, ips, vlanID) - hostIfAlias := util.VirtualAdapterName(ovsPortName) // - For containerd runtime, the container interface is created after CNI replying the network setup result. // So for such case we need to use asynchronous way to wait for interface to be created: we create the OVS port // and set the OVS Interface type "" first, and change the OVS Interface type to "internal" to connect to the // container interface after it is created. After OVS connects to the container interface, an OFPort is allocated. - // - For Docker runtime, the container interface is created after antrea-agent attaches the HNSEndpoint to the - // sandbox container, so we create OVS port synchronously. - // - Here antrea-agent determines the way of OVS port creation by checking if container interface is yet created. - // If one day containerd runtime changes the behavior and container interface can be created when attaching - // HNSEndpoint/HostComputeEndpoint, the current implementation will still work. It will choose the synchronized - // way to create OVS port. - if hostInterfaceExistsFunc(hostIfAlias) { - return containerConfig, pc.connectInterfaceToOVSCommon(ovsPortName, netNS, containerConfig) - } klog.V(2).Infof("Adding OVS port %s for container %s", ovsPortName, containerID) ovsAttachInfo := BuildOVSPortExternalIDs(containerConfig) portUUID, err := pc.createOVSPort(ovsPortName, ovsAttachInfo, containerConfig.VLANID) diff --git a/pkg/agent/cniserver/server.go b/pkg/agent/cniserver/server.go index cec67536c14..961cc3c21a8 100644 --- a/pkg/agent/cniserver/server.go +++ b/pkg/agent/cniserver/server.go @@ -442,6 +442,10 @@ func (s *CNIServer) CmdAdd(ctx context.Context, request *cnipb.CniCmdRequest) (* result := &ipam.IPAMResult{Result: current.Result{CNIVersion: current.ImplementedSpecVersion}} netNS := s.hostNetNsPath(cniConfig.Netns) + if !validateRuntime(netNS) { + // Support for Docker runtime has been deprecated in Antrea 2.0. + return nil, fmt.Errorf("failed to process CmdAdd request because Docker runtime is not supported after Antrea 2.0") + } isInfraContainer := isInfraContainer(netNS) success := false @@ -563,6 +567,12 @@ func (s *CNIServer) CmdDel(ctx context.Context, request *cnipb.CniCmdRequest) (* return response, nil } + netNS := s.hostNetNsPath(cniConfig.Netns) + if !validateRuntime(netNS) { + // Support for Docker runtime has been deprecated in Antrea 2.0. + return nil, fmt.Errorf("failed to process CmdDel request because Docker runtime is not supported after Antrea 2.0") + } + return s.cmdDel(ctx, cniConfig) } @@ -575,6 +585,12 @@ func (s *CNIServer) CmdCheck(_ context.Context, request *cnipb.CniCmdRequest) ( return response, nil } + netNS := s.hostNetNsPath(cniConfig.Netns) + if !validateRuntime(netNS) { + // Support for Docker runtime has been deprecated in Antrea 2.0. + return nil, fmt.Errorf("failed to process CmdCheck request because Docker runtime is not supported after Antrea 2.0") + } + infraContainer := cniConfig.getInfraContainer() s.containerAccess.lockContainer(infraContainer) defer s.containerAccess.unlockContainer(infraContainer) diff --git a/pkg/agent/cniserver/server_linux.go b/pkg/agent/cniserver/server_linux.go index d9f380608ea..6a45824936a 100644 --- a/pkg/agent/cniserver/server_linux.go +++ b/pkg/agent/cniserver/server_linux.go @@ -37,6 +37,12 @@ func isInfraContainer(netNS string) bool { return true } +// validateRuntime returns true if the container runtime is supported by Antrea. +// Always return true on Linux platform, because both Docker and Containerd are supported. +func validateRuntime(netNS string) bool { + return true +} + // getInfraContainer returns the sandbox container ID of a Pod. // On Linux, it's always the ContainerID in the request. func (c *CNIConfig) getInfraContainer() string { diff --git a/pkg/agent/cniserver/server_windows.go b/pkg/agent/cniserver/server_windows.go index e1158ce9883..532afe9abb1 100644 --- a/pkg/agent/cniserver/server_windows.go +++ b/pkg/agent/cniserver/server_windows.go @@ -64,6 +64,12 @@ func isDockerContainer(netNS string) bool { return netNS == dockerInfraContainerNetNS || strings.Contains(netNS, ":") } +// validateRuntime returns false if a container is created by Docker with the provided network namespace +// because the Docker support has been removed since Antrea 2.0. +func validateRuntime(netNS string) bool { + return !isDockerContainer(netNS) +} + func getInfraContainer(containerID, netNS string) string { if isInfraContainer(netNS) { return containerID diff --git a/pkg/agent/cniserver/server_windows_test.go b/pkg/agent/cniserver/server_windows_test.go index ba562bae762..0e81a94090b 100644 --- a/pkg/agent/cniserver/server_windows_test.go +++ b/pkg/agent/cniserver/server_windows_test.go @@ -219,10 +219,6 @@ func (t *hnsTestUtil) hotAttachEndpoint(containerID string, epID string) error { return t.endpointAttachErr } -func (t *hnsTestUtil) isContainerAttachOnEndpoint(ep *hcsshim.HNSEndpoint, containerID string) (bool, error) { - return t.isAttached, nil -} - func (t *hnsTestUtil) getHcnEndpointByID(epID string) (*hcn.HostComputeEndpoint, error) { return t.hcnEndpoint, nil } @@ -249,7 +245,6 @@ func (t *hnsTestUtil) setFunctions() { getNamespaceEndpointIDsFunc = t.getNamespaceEndpointIDs hotAttachEndpointFunc = t.hotAttachEndpoint attachEndpointInNamespaceFunc = t.attachEndpointInNamespace - isContainerAttachOnEndpointFunc = t.isContainerAttachOnEndpoint getHcnEndpointByIDFunc = t.getHcnEndpointByID deleteHnsEndpointFunc = t.deleteHnsEndpoint removeEndpointFromNamespaceFunc = t.removeEndpointFromNamespace @@ -261,7 +256,6 @@ func (t *hnsTestUtil) restore() { getNamespaceEndpointIDsFunc = hcn.GetNamespaceEndpointIds hotAttachEndpointFunc = hcsshim.HotAttachEndpoint attachEndpointInNamespaceFunc = attachEndpointInNamespace - isContainerAttachOnEndpointFunc = isContainerAttachOnEndpoint getHcnEndpointByIDFunc = hcn.GetEndpointByID deleteHnsEndpointFunc = deleteHnsEndpoint removeEndpointFromNamespaceFunc = hcn.RemoveNamespaceEndpoint @@ -321,9 +315,6 @@ func TestCmdAdd(t *testing.T) { oriIPAMResult := &ipam.IPAMResult{Result: *ipamResult} ctx := context.TODO() - dockerInfraContainer := "261a1970-5b6c-11ed-8caf-000c294e5d03" - dockerWorkContainer := "261e579a-5b6c-11ed-8caf-000c294e5d03" - unknownInfraContainer := generateUUID() containerdInfraContainer := generateUUID() defer mockHostInterfaceExists()() @@ -352,97 +343,6 @@ func TestCmdAdd(t *testing.T) { expectedErr error }{ { - name: "docker-infra-create-failure", - podName: "pod0", - containerID: dockerInfraContainer, - infraContainerID: dockerInfraContainer, - netns: "none", - ipamAdd: true, - ipamDel: true, - hnsEndpointCreateErr: fmt.Errorf("unable to create HnsEndpoint"), - errResponse: &cnipb.CniCmdResponse{ - Error: &cnipb.Error{ - Code: cnipb.ErrorCode_CONFIG_INTERFACE_FAILURE, - Message: "unable to create HnsEndpoint", - }, - }, - }, { - name: "docker-infra-attach-failure", - podName: "pod1", - containerID: dockerInfraContainer, - infraContainerID: dockerInfraContainer, - netns: "none", - ipamAdd: true, - ipamDel: true, - endpointAttachErr: fmt.Errorf("unable to attach HnsEndpoint"), - errResponse: &cnipb.CniCmdResponse{ - Error: &cnipb.Error{ - Code: cnipb.ErrorCode_CONFIG_INTERFACE_FAILURE, - Message: "failed to configure container IP: unable to attach HnsEndpoint", - }, - }, - }, { - name: "docker-infra-success", - podName: "pod2", - containerID: dockerInfraContainer, - infraContainerID: dockerInfraContainer, - netns: "none", - ipamAdd: true, - connectOVS: true, - containerIfaceExist: true, - }, { - name: "docker-workload-allocate-ip-failure", - podName: "pod3", - containerID: dockerWorkContainer, - infraContainerID: unknownInfraContainer, - netns: fmt.Sprintf("container:%s", unknownInfraContainer), - expectedErr: fmt.Errorf("allocated IP address not found"), - }, { - name: "docker-workload-no-endpoint", - podName: "pod4", - containerID: dockerWorkContainer, - infraContainerID: dockerInfraContainer, - netns: fmt.Sprintf("container:%s", dockerInfraContainer), - oriIPAMResult: oriIPAMResult, - errResponse: &cnipb.CniCmdResponse{ - Error: &cnipb.Error{ - Code: cnipb.ErrorCode_CONFIG_INTERFACE_FAILURE, - Message: "failed to find HNSEndpoint: pod4-6631b7", - }, - }, - }, { - name: "docker-workload-attach-failure", - podName: "pod5", - containerID: dockerWorkContainer, - infraContainerID: dockerInfraContainer, - netns: fmt.Sprintf("container:%s", dockerInfraContainer), - oriIPAMResult: oriIPAMResult, - endpointAttachErr: fmt.Errorf("unable to attach HnsEndpoint"), - endpointExists: true, - errResponse: &cnipb.CniCmdResponse{ - Error: &cnipb.Error{ - Code: cnipb.ErrorCode_CONFIG_INTERFACE_FAILURE, - Message: "failed to configure container IP: unable to attach HnsEndpoint", - }, - }, - }, { - name: "docker-workload-success", - podName: "pod6", - containerID: dockerWorkContainer, - infraContainerID: dockerInfraContainer, - netns: fmt.Sprintf("container:%s", dockerInfraContainer), - oriIPAMResult: oriIPAMResult, - endpointExists: true, - }, { - name: "docker-workload-already-attached", - podName: "pod7", - containerID: dockerWorkContainer, - infraContainerID: dockerInfraContainer, - netns: fmt.Sprintf("container:%s", dockerInfraContainer), - isAttached: true, - endpointExists: true, - oriIPAMResult: oriIPAMResult, - }, { name: "containerd-success", podName: "pod8", containerID: containerdInfraContainer, @@ -500,14 +400,9 @@ func TestCmdAdd(t *testing.T) { } ovsPortID := generateUUID() if tc.connectOVS { - if isDocker { - mockOVSBridgeClient.EXPECT().CreateInternalPort(ovsPortName, int32(0), gomock.Any(), gomock.Any()).Return(ovsPortID, nil).Times(1) - mockOVSBridgeClient.EXPECT().GetOFPort(ovsPortName, false).Return(int32(100), nil).Times(1) - } else { - mockOVSBridgeClient.EXPECT().CreatePort(ovsPortName, ovsPortName, gomock.Any()).Return(ovsPortID, nil).Times(1) - mockOVSBridgeClient.EXPECT().SetInterfaceType(ovsPortName, "internal").Return(nil).Times(1) - mockOVSBridgeClient.EXPECT().GetOFPort(ovsPortName, true).Return(int32(100), nil).Times(1) - } + mockOVSBridgeClient.EXPECT().CreatePort(ovsPortName, ovsPortName, gomock.Any()).Return(ovsPortID, nil).Times(1) + mockOVSBridgeClient.EXPECT().SetInterfaceType(ovsPortName, "internal").Return(nil).Times(1) + mockOVSBridgeClient.EXPECT().GetOFPort(ovsPortName, true).Return(int32(100), nil).Times(1) mockOFClient.EXPECT().InstallPodFlows(ovsPortName, gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) mockRoute.EXPECT().AddLocalAntreaFlexibleIPAMPodRule(gomock.Any()).Return(nil).Times(1) } @@ -577,22 +472,14 @@ func TestCmdDel(t *testing.T) { ifaceExists bool errResponse *cnipb.CniCmdResponse }{ - { - name: "docker-infra-success", - netns: "none", - ipamDel: true, - disconnectOVS: true, - endpointExists: true, - ifaceExists: true, - }, { name: "interface-not-exist", - netns: "none", + netns: generateUUID(), ipamDel: true, }, { name: "ipam-delete-failure", - netns: "none", + netns: generateUUID(), ipamDel: true, ipamError: fmt.Errorf("unable to delete IP"), disconnectOVS: true, @@ -667,6 +554,7 @@ func TestCmdCheck(t *testing.T) { ipam.ResetIPAMDriver(ipamType, ipamMock) ctx := context.TODO() + containerNetns := generateUUID() containerID := "261a1970-5b6c-11ed-8caf-000c294e5d03" mac, _ := net.ParseMAC("11:22:33:44:33:22") containerIP, containerIPNet, _ := net.ParseCIDR("10.1.2.100/24") @@ -709,11 +597,11 @@ func TestCmdCheck(t *testing.T) { { name: "check-success", podName: "pod0", - netns: "none", + netns: containerNetns, containerID: containerID, prevResult: wrapperIPAMResult(*ipamResult, []*current.Interface{ {Name: "pod0-6631b7", Mac: "11:22:33:44:33:22", Sandbox: ""}, - {Name: "pod0-6631b7_eth0", Mac: "11:22:33:44:33:22", Sandbox: "none"}, + {Name: "pod0-6631b7_eth0", Mac: "11:22:33:44:33:22", Sandbox: containerNetns}, }), existingIface: wrapperContainerInterface("pod0-6631b7", containerID, "pod0", generateUUID(), mac, containerIP), netInterface: &net.Interface{ @@ -725,7 +613,7 @@ func TestCmdCheck(t *testing.T) { }, { name: "pod-namespace-mismatch", podName: "pod1", - netns: "none", + netns: containerNetns, containerID: containerID, prevResult: wrapperIPAMResult(*ipamResult, []*current.Interface{ {Name: "pod1-6631b7", Mac: "11:22:33:44:33:22", Sandbox: ""}, @@ -741,17 +629,17 @@ func TestCmdCheck(t *testing.T) { errResponse: &cnipb.CniCmdResponse{ Error: &cnipb.Error{ Code: cnipb.ErrorCode_CHECK_INTERFACE_FAILURE, - Message: "sandbox in prevResult invalid-namespace doesn't match configured netns: none", + Message: fmt.Sprintf("sandbox in prevResult invalid-namespace doesn't match configured netns: %s", containerNetns), }, }, }, { name: "container-host-names-mismatch", podName: "pod2", - netns: "none", + netns: containerNetns, containerID: containerID, prevResult: wrapperIPAMResult(*ipamResult, []*current.Interface{ {Name: "pod2-6631b7", Mac: "11:22:33:44:33:22", Sandbox: ""}, - {Name: "eth0", Mac: "11:22:33:44:33:22", Sandbox: "none"}, + {Name: "eth0", Mac: "11:22:33:44:33:22", Sandbox: containerNetns}, }), existingIface: wrapperContainerInterface("pod2-6631b7", containerID, "pod2", generateUUID(), mac, containerIP), netInterface: &net.Interface{ @@ -769,11 +657,11 @@ func TestCmdCheck(t *testing.T) { }, { name: "container-host-MAC-mismatch", podName: "pod3", - netns: "none", + netns: containerNetns, containerID: containerID, prevResult: wrapperIPAMResult(*ipamResult, []*current.Interface{ {Name: "pod3-6631b7", Mac: "11:22:33:44:33:22", Sandbox: ""}, - {Name: "pod3-6631b7_eth0", Mac: "11:22:33:44:33:33", Sandbox: "none"}, + {Name: "pod3-6631b7_eth0", Mac: "11:22:33:44:33:33", Sandbox: containerNetns}, }), existingIface: wrapperContainerInterface("pod3-6631b7", containerID, "pod3", generateUUID(), mac, containerIP), netInterface: &net.Interface{