From ec75bb8587443894405d0c0d8ee328b8dbaba761 Mon Sep 17 00:00:00 2001 From: thxcode Date: Mon, 19 Apr 2021 10:47:57 +0800 Subject: [PATCH] chore(win-bridge): text related - format function names - add/remove comments - adjust message of error Signed-off-by: thxcode --- pkg/hns/endpoint_windows.go | 66 ++++++++---------- pkg/hns/netconf_windows.go | 15 ++-- .../windows/win-bridge/win-bridge_windows.go | 68 ++++++++----------- .../win-overlay/win-overlay_windows.go | 8 +-- 4 files changed, 70 insertions(+), 87 deletions(-) diff --git a/pkg/hns/endpoint_windows.go b/pkg/hns/endpoint_windows.go index 9dfa41d9..d9a72f4d 100644 --- a/pkg/hns/endpoint_windows.go +++ b/pkg/hns/endpoint_windows.go @@ -24,6 +24,7 @@ import ( "github.com/containernetworking/cni/pkg/types" current "github.com/containernetworking/cni/pkg/types/100" + "github.com/containernetworking/plugins/pkg/errors" ) @@ -40,7 +41,7 @@ type EndpointInfo struct { IpAddress net.IP } -// GetSandboxContainerID returns the sandbox ID of this pod +// GetSandboxContainerID returns the sandbox ID of this pod. func GetSandboxContainerID(containerID string, netNs string) string { if len(netNs) != 0 && netNs != pauseContainerNetNS { splits := strings.SplitN(netNs, ":", 2) @@ -52,7 +53,7 @@ func GetSandboxContainerID(containerID string, netNs string) string { return containerID } -// short function so we know when to return "" for a string +// GetIpString returns the given IP in string. func GetIpString(ip *net.IP) string { if len(*ip) == 0 { return "" @@ -61,6 +62,7 @@ func GetIpString(ip *net.IP) string { } } +// GenerateHnsEndpoint generates an HNSEndpoint with given info and config. func GenerateHnsEndpoint(epInfo *EndpointInfo, n *NetConf) (*hcsshim.HNSEndpoint, error) { // run the IPAM plugin and get back the config to apply hnsEndpoint, err := hcsshim.GetHNSEndpointByName(epInfo.EndpointName) @@ -95,6 +97,7 @@ func GenerateHnsEndpoint(epInfo *EndpointInfo, n *NetConf) (*hcsshim.HNSEndpoint return hnsEndpoint, nil } +// GenerateHcnEndpoint generates a HostComputeEndpoint with given info and config. func GenerateHcnEndpoint(epInfo *EndpointInfo, n *NetConf) (*hcn.HostComputeEndpoint, error) { // run the IPAM plugin and get back the config to apply hcnEndpoint, err := hcn.GetEndpointByName(epInfo.EndpointName) @@ -155,17 +158,14 @@ func GenerateHcnEndpoint(epInfo *EndpointInfo, n *NetConf) (*hcn.HostComputeEndp return hcnEndpoint, nil } -// ConstructEndpointName constructs enpointId which is used to identify an endpoint from HNS -// There is a special consideration for netNs name here, which is required for Windows Server 1709 -// containerID is the Id of the container on which the endpoint is worked on +// ConstructEndpointName constructs endpoint id which is used to identify an endpoint from HNS/HCN. func ConstructEndpointName(containerID string, netNs string, networkName string) string { return GetSandboxContainerID(containerID, netNs) + "_" + networkName } -// DeprovisionEndpoint removes an endpoint from the container by sending a Detach request to HNS -// For shared endpoint, ContainerDetach is used -// for removing the endpoint completely, HotDetachEndpoint is used -func DeprovisionEndpoint(epName string, netns string, containerID string) error { +// RemoveHnsEndpoint detaches the given name endpoint from container specified by containerID, +// or removes the given name endpoint completely. +func RemoveHnsEndpoint(epName string, netns string, containerID string) error { if len(netns) == 0 { return nil } @@ -178,29 +178,24 @@ func DeprovisionEndpoint(epName string, netns string, containerID string) error return errors.Annotatef(err, "failed to find HNSEndpoint %s", epName) } + // for shared endpoint, detach it from the container. if netns != pauseContainerNetNS { - // Shared endpoint removal. Do not remove the endpoint. hnsEndpoint.ContainerDetach(containerID) return nil } - // Do not consider this as failure, else this would leak endpoints + // for removing the endpoint completely, hot detach is used at first. hcsshim.HotDetachEndpoint(containerID, hnsEndpoint.Id) - - // Do not return error hnsEndpoint.Delete() return nil } -type EndpointMakerFunc func() (*hcsshim.HNSEndpoint, error) +type HnsEndpointMakerFunc func() (*hcsshim.HNSEndpoint, error) -// ProvisionEndpoint provisions an endpoint to a container specified by containerID. -// If an endpoint already exists, the endpoint is reused. -// This call is idempotent -func ProvisionEndpoint(epName string, expectedNetworkId string, containerID string, netns string, makeEndpoint EndpointMakerFunc) (*hcsshim.HNSEndpoint, error) { - // On the second add call we expect that the endpoint already exists. If it - // does not then we should return an error. +// AddHnsEndpoint attaches an HNSEndpoint to a container specified by containerID. +func AddHnsEndpoint(epName string, expectedNetworkId string, containerID string, netns string, makeEndpoint HnsEndpointMakerFunc) (*hcsshim.HNSEndpoint, error) { + // for shared endpoint, we expect that the endpoint already exists. if netns != pauseContainerNetNS { _, err := hcsshim.GetHNSEndpointByName(epName) if err != nil { @@ -235,15 +230,15 @@ func ProvisionEndpoint(epName string, expectedNetworkId string, containerID stri // hot attach if err := hcsshim.HotAttachEndpoint(containerID, hnsEndpoint.Id); err != nil { if createEndpoint { - err := DeprovisionEndpoint(epName, netns, containerID) + err := RemoveHnsEndpoint(epName, netns, containerID) if err != nil { - return nil, errors.Annotatef(err, "failed to Deprovsion after HotAttach failure") + return nil, errors.Annotatef(err, "failed to remove the new HNSEndpoint %s after attaching container %s failure", hnsEndpoint.Id, containerID) } } if hcsshim.ErrComputeSystemDoesNotExist == err { return hnsEndpoint, nil } - return nil, err + return nil, errors.Annotatef(err, "failed to attach container %s to HNSEndpoint %s", containerID, hnsEndpoint.Id) } return hnsEndpoint, nil @@ -251,32 +246,30 @@ func ProvisionEndpoint(epName string, expectedNetworkId string, containerID stri type HcnEndpointMakerFunc func() (*hcn.HostComputeEndpoint, error) -func AddHcnEndpoint(epName string, expectedNetworkId string, namespace string, - makeEndpoint HcnEndpointMakerFunc) (*hcn.HostComputeEndpoint, error) { - +// AddHcnEndpoint attaches a HostComputeEndpoint to the given namespace. +func AddHcnEndpoint(epName string, expectedNetworkId string, namespace string, makeEndpoint HcnEndpointMakerFunc) (*hcn.HostComputeEndpoint, error) { hcnEndpoint, err := makeEndpoint() if err != nil { - return nil, errors.Annotate(err, "failed to make a new HNSEndpoint") + return nil, errors.Annotate(err, "failed to make a new HostComputeEndpoint") } if hcnEndpoint, err = hcnEndpoint.Create(); err != nil { - return nil, errors.Annotate(err, "failed to create the new HNSEndpoint") + return nil, errors.Annotate(err, "failed to create the new HostComputeEndpoint") } err = hcn.AddNamespaceEndpoint(namespace, hcnEndpoint.Id) if err != nil { err := RemoveHcnEndpoint(epName) if err != nil { - return nil, errors.Annotatef(err, "failed to Remove Endpoint after AddNamespaceEndpoint failure") + return nil, errors.Annotatef(err, "failed to remote the new HostComputeEndpoint %s after adding HostComputeNamespace %s failure", epName, namespace) } - return nil, errors.Annotate(err, "failed to Add endpoint to namespace") + return nil, errors.Annotatef(err, "failed to add HostComputeEndpoint %s to HostComputeNamespace %s", epName, namespace) } return hcnEndpoint, nil - } -// ConstructResult constructs the CNI result for the endpoint -func ConstructResult(hnsNetwork *hcsshim.HNSNetwork, hnsEndpoint *hcsshim.HNSEndpoint) (*current.Result, error) { +// ConstructHnsResult constructs the CNI result for the HNSEndpoint. +func ConstructHnsResult(hnsNetwork *hcsshim.HNSNetwork, hnsEndpoint *hcsshim.HNSEndpoint) (*current.Result, error) { resultInterface := ¤t.Interface{ Name: hnsEndpoint.Name, Mac: hnsEndpoint.MacAddress, @@ -296,7 +289,7 @@ func ConstructResult(hnsNetwork *hcsshim.HNSNetwork, hnsEndpoint *hcsshim.HNSEnd CNIVersion: current.ImplementedSpecVersion, Interfaces: []*current.Interface{resultInterface}, IPs: []*current.IPConfig{resultIPConfig}, - DNS: types.DNS{ + DNS: types.DNS{ Search: strings.Split(hnsEndpoint.DNSSuffix, ","), Nameservers: strings.Split(hnsEndpoint.DNSServerList, ","), }, @@ -305,7 +298,7 @@ func ConstructResult(hnsNetwork *hcsshim.HNSNetwork, hnsEndpoint *hcsshim.HNSEnd return result, nil } -// This version follows the v2 workflow of removing the endpoint from the namespace and deleting it +// RemoveHcnEndpoint removes the given name endpoint from namespace. func RemoveHcnEndpoint(epName string) error { hcnEndpoint, err := hcn.GetEndpointByName(epName) if hcn.IsNotFoundError(err) { @@ -323,6 +316,7 @@ func RemoveHcnEndpoint(epName string) error { return nil } +// ConstructHcnResult constructs the CNI result for the HostComputeEndpoint. func ConstructHcnResult(hcnNetwork *hcn.HostComputeNetwork, hcnEndpoint *hcn.HostComputeEndpoint) (*current.Result, error) { resultInterface := ¤t.Interface{ Name: hcnEndpoint.Name, @@ -344,7 +338,7 @@ func ConstructHcnResult(hcnNetwork *hcn.HostComputeNetwork, hcnEndpoint *hcn.Hos CNIVersion: current.ImplementedSpecVersion, Interfaces: []*current.Interface{resultInterface}, IPs: []*current.IPConfig{resultIPConfig}, - DNS: types.DNS{ + DNS: types.DNS{ Search: hcnEndpoint.Dns.Search, Nameservers: hcnEndpoint.Dns.ServerList, }, diff --git a/pkg/hns/netconf_windows.go b/pkg/hns/netconf_windows.go index 01974333..6785d859 100644 --- a/pkg/hns/netconf_windows.go +++ b/pkg/hns/netconf_windows.go @@ -19,7 +19,6 @@ import ( "encoding/json" "fmt" "net" - "strings" "github.com/Microsoft/hcsshim/hcn" @@ -72,6 +71,7 @@ func GetDefaultDestinationPrefix(ip *net.IP) string { return destinationPrefix } +// ApplyLoopbackDSR configures the given IP to support loopback DSR. func (n *NetConf) ApplyLoopbackDSR(ip *net.IP) { value := fmt.Sprintf(`"Destinations" : ["%s"]`, ip.String()) if n.ApiVersion == 2 { @@ -89,7 +89,7 @@ func (n *NetConf) ApplyLoopbackDSR(ip *net.IP) { } } -// If runtime dns values are there use that else use cni conf supplied dns +// GetDNS returns the DNS values if they are there use that else use netconf supplied DNS. func (n *NetConf) GetDNS() types.DNS { dnsResult := n.DNS if len(n.RuntimeConfig.DNS.Nameservers) > 0 { @@ -101,8 +101,8 @@ func (n *NetConf) GetDNS() types.DNS { return dnsResult } -// MarshalPolicies converts the Endpoint policies in Policies -// to HNS specific policies as Json raw bytes +// MarshalPolicies converts the HNSEndpoint policies in Policies +// to HNS specific policies as Json raw bytes. func (n *NetConf) MarshalPolicies() []json.RawMessage { if n.Policies == nil { n.Policies = make([]policy, 0) @@ -120,8 +120,7 @@ func (n *NetConf) MarshalPolicies() []json.RawMessage { return result } -// ApplyOutboundNatPolicy applies NAT Policy in VFP using HNS -// Simultaneously an exception is added for the network that has to be Nat'd +// ApplyOutboundNatPolicy applies the sNAT policy in HNS/HCN and configures the given CIDR as an exception. func (n *NetConf) ApplyOutboundNatPolicy(nwToNat string) { if n.Policies == nil { n.Policies = make([]policy, 0) @@ -183,7 +182,7 @@ func (n *NetConf) ApplyOutboundNatPolicy(nwToNat string) { }) } -// ApplyDefaultPAPolicy is used to configure a endpoint PA policy in HNS +// ApplyDefaultPAPolicy applies an endpoint PA policy in HNS/HCN. func (n *NetConf) ApplyDefaultPAPolicy(paAddress string) { if n.Policies == nil { n.Policies = make([]policy, 0) @@ -217,7 +216,7 @@ func (n *NetConf) ApplyDefaultPAPolicy(paAddress string) { }) } -// ApplyPortMappingPolicy is used to configure HostPort<>ContainerPort mapping in HNS +// ApplyPortMappingPolicy applies the host/container port mapping policies in HNS/HCN. func (n *NetConf) ApplyPortMappingPolicy(portMappings []PortMapEntry) { if portMappings == nil { return diff --git a/plugins/main/windows/win-bridge/win-bridge_windows.go b/plugins/main/windows/win-bridge/win-bridge_windows.go index 36f1035a..9ecf0456 100644 --- a/plugins/main/windows/win-bridge/win-bridge_windows.go +++ b/plugins/main/windows/win-bridge/win-bridge_windows.go @@ -55,21 +55,22 @@ func loadNetConf(bytes []byte) (*NetConf, string, error) { return n, n.CNIVersion, nil } -func ProcessEndpointArgs(args *skel.CmdArgs, n *NetConf) (*hns.EndpointInfo, error) { +func processEndpointArgs(args *skel.CmdArgs, n *NetConf) (*hns.EndpointInfo, error) { epInfo := new(hns.EndpointInfo) epInfo.NetworkName = n.Name epInfo.EndpointName = hns.ConstructEndpointName(args.ContainerID, args.Netns, epInfo.NetworkName) - // It's not necessary to have have an IPAM in windows as hns can provide IP/GW + + // it's not necessary to have have an IPAM in windows as HNS can provide IP/GW if n.IPAM.Type != "" { r, err := ipam.ExecAdd(n.IPAM.Type, args.StdinData) if err != nil { - return nil, errors.Annotatef(err, "error while ipam.ExecAdd") + return nil, errors.Annotatef(err, "error while executing IPAM addition") } - // Convert whatever the IPAM result was into the current Result type + // convert whatever the IPAM result was into the current result result, err := current.NewResultFromResult(r) if err != nil { - return nil, errors.Annotatef(err, "error while NewResultFromResult") + return nil, errors.Annotatef(err, "error while converting the result from IPAM addition") } else { if len(result.IPs) == 0 { return nil, fmt.Errorf("IPAM plugin return is missing IP config") @@ -81,12 +82,13 @@ func ProcessEndpointArgs(args *skel.CmdArgs, n *NetConf) (*hns.EndpointInfo, err epInfo.Gateway[len(epInfo.Gateway)-1] += 2 } } - // NAT based on the the configured cluster network + + // configure sNAT exception if len(n.IPMasqNetwork) != 0 { n.ApplyOutboundNatPolicy(n.IPMasqNetwork) } - // Add HostPort mapping if any present + // add port mapping if any present n.ApplyPortMappingPolicy(n.RuntimeConfig.PortMaps) epInfo.DNS = n.GetDNS() @@ -98,40 +100,37 @@ func cmdHnsAdd(args *skel.CmdArgs, n *NetConf) (*current.Result, error) { networkName := n.Name hnsNetwork, err := hcsshim.GetHNSNetworkByName(networkName) if err != nil { - return nil, errors.Annotatef(err, "error while GETHNSNewtorkByName(%s)", networkName) + return nil, errors.Annotatef(err, "error while getting network %v", networkName) } - if hnsNetwork == nil { - return nil, fmt.Errorf("network %v not found", networkName) + return nil, fmt.Errorf("network %v is not found", networkName) } - if !strings.EqualFold(hnsNetwork.Type, "L2Bridge") && !strings.EqualFold(hnsNetwork.Type, "L2Tunnel") { - return nil, fmt.Errorf("network %v is of an unexpected type: %v", networkName, hnsNetwork.Type) + return nil, fmt.Errorf("network %v is of unexpected type: %v", networkName, hnsNetwork.Type) } epName := hns.ConstructEndpointName(args.ContainerID, args.Netns, n.Name) - hnsEndpoint, err := hns.ProvisionEndpoint(epName, hnsNetwork.Id, args.ContainerID, args.Netns, func() (*hcsshim.HNSEndpoint, error) { - epInfo, err := ProcessEndpointArgs(args, n) + hnsEndpoint, err := hns.AddHnsEndpoint(epName, hnsNetwork.Id, args.ContainerID, args.Netns, func() (*hcsshim.HNSEndpoint, error) { + epInfo, err := processEndpointArgs(args, n) if err != nil { - return nil, errors.Annotatef(err, "error while ProcessEndpointArgs") + return nil, errors.Annotate(err, "error while processing endpoint args") } epInfo.NetworkId = hnsNetwork.Id hnsEndpoint, err := hns.GenerateHnsEndpoint(epInfo, &n.NetConf) if err != nil { - return nil, errors.Annotatef(err, "error while GenerateHnsEndpoint") + return nil, errors.Annotate(err, "error while generating HNSEndpoint") } return hnsEndpoint, nil }) if err != nil { - return nil, errors.Annotatef(err, "error while ProvisionEndpoint(%v,%v,%v)", epName, hnsNetwork.Id, args.ContainerID) + return nil, errors.Annotate(err, "error while adding HNSEndpoint") } - result, err := hns.ConstructResult(hnsNetwork, hnsEndpoint) + result, err := hns.ConstructHnsResult(hnsNetwork, hnsEndpoint) if err != nil { - return nil, errors.Annotatef(err, "error while constructResult") + return nil, errors.Annotate(err, "error while constructing HNSEndpoint addition result") } - return result, nil } @@ -139,48 +138,44 @@ func cmdHcnAdd(args *skel.CmdArgs, n *NetConf) (*current.Result, error) { networkName := n.Name hcnNetwork, err := hcn.GetNetworkByName(networkName) if err != nil { - return nil, errors.Annotatef(err, "error while GetNetworkByName(%s)", networkName) + return nil, errors.Annotatef(err, "error while getting network %v", networkName) } - if hcnNetwork == nil { - return nil, fmt.Errorf("network %v not found", networkName) + return nil, fmt.Errorf("network %v is not found", networkName) } - if hcnNetwork.Type != hcn.L2Bridge && hcnNetwork.Type != hcn.L2Tunnel { return nil, fmt.Errorf("network %v is of unexpected type: %v", networkName, hcnNetwork.Type) } epName := hns.ConstructEndpointName(args.ContainerID, args.Netns, n.Name) - hcnEndpoint, err := hns.AddHcnEndpoint(epName, hcnNetwork.Id, args.Netns, func() (*hcn.HostComputeEndpoint, error) { - epInfo, err := ProcessEndpointArgs(args, n) + epInfo, err := processEndpointArgs(args, n) if err != nil { - return nil, errors.Annotatef(err, "error while ProcessEndpointArgs") + return nil, errors.Annotate(err, "error while processing endpoint args") } epInfo.NetworkId = hcnNetwork.Id hcnEndpoint, err := hns.GenerateHcnEndpoint(epInfo, &n.NetConf) if err != nil { - return nil, errors.Annotatef(err, "error while GenerateHcnEndpoint") + return nil, errors.Annotate(err, "error while generating HostComputeEndpoint") } return hcnEndpoint, nil }) if err != nil { - return nil, errors.Annotatef(err, "error while AddHcnEndpoint(%v,%v,%v)", epName, hcnNetwork.Id, args.Netns) + return nil, errors.Annotate(err, "error while adding HostComputeEndpoint") } result, err := hns.ConstructHcnResult(hcnNetwork, hcnEndpoint) if err != nil { - return nil, errors.Annotatef(err, "error while ConstructHcnResult") + return nil, errors.Annotate(err, "error while constructing HostComputeEndpoint addition result") } - return result, nil } func cmdAdd(args *skel.CmdArgs) error { n, cniVersion, err := loadNetConf(args.StdinData) if err != nil { - return errors.Annotate(err, "error while loadNetConf") + return err } var result *current.Result @@ -189,15 +184,11 @@ func cmdAdd(args *skel.CmdArgs) error { } else { result, err = cmdHnsAdd(args, n) } - if err != nil { ipam.ExecDel(n.IPAM.Type, args.StdinData) - return errors.Annotate(err, "error while executing ADD command") + return err } - if result == nil { - return fmt.Errorf("result for ADD not populated correctly") - } return types.PrintResult(result, cniVersion) } @@ -216,9 +207,8 @@ func cmdDel(args *skel.CmdArgs) error { if n.ApiVersion == 2 { return hns.RemoveHcnEndpoint(epName) - } else { - return hns.DeprovisionEndpoint(epName, args.Netns, args.ContainerID) } + return hns.RemoveHnsEndpoint(epName, args.Netns, args.ContainerID) } func cmdCheck(_ *skel.CmdArgs) error { diff --git a/plugins/main/windows/win-overlay/win-overlay_windows.go b/plugins/main/windows/win-overlay/win-overlay_windows.go index 77bb1714..b7e0325a 100644 --- a/plugins/main/windows/win-overlay/win-overlay_windows.go +++ b/plugins/main/windows/win-overlay/win-overlay_windows.go @@ -86,7 +86,7 @@ func cmdAdd(args *skel.CmdArgs) error { epName := hns.ConstructEndpointName(args.ContainerID, args.Netns, n.Name) - hnsEndpoint, err := hns.ProvisionEndpoint(epName, hnsNetwork.Id, args.ContainerID, args.Netns, func() (*hcsshim.HNSEndpoint, error) { + hnsEndpoint, err := hns.AddHnsEndpoint(epName, hnsNetwork.Id, args.ContainerID, args.Netns, func() (*hcsshim.HNSEndpoint, error) { // run the IPAM plugin and get back the config to apply r, err := ipam.ExecAdd(n.IPAM.Type, args.StdinData) if err != nil { @@ -140,10 +140,10 @@ func cmdAdd(args *skel.CmdArgs) error { } }() if err != nil { - return errors.Annotatef(err, "error while ProvisionEndpoint(%v,%v,%v)", epName, hnsNetwork.Id, args.ContainerID) + return errors.Annotatef(err, "error while AddHnsEndpoint(%v,%v,%v)", epName, hnsNetwork.Id, args.ContainerID) } - result, err := hns.ConstructResult(hnsNetwork, hnsEndpoint) + result, err := hns.ConstructHnsResult(hnsNetwork, hnsEndpoint) if err != nil { return errors.Annotatef(err, "error while constructResult") } @@ -164,7 +164,7 @@ func cmdDel(args *skel.CmdArgs) error { epName := hns.ConstructEndpointName(args.ContainerID, args.Netns, n.Name) - return hns.DeprovisionEndpoint(epName, args.Netns, args.ContainerID) + return hns.RemoveHnsEndpoint(epName, args.Netns, args.ContainerID) } func cmdCheck(_ *skel.CmdArgs) error {