chore(win-bridge): text related

- format function names
- add/remove comments
- adjust message of error

Signed-off-by: thxcode <thxcode0824@gmail.com>

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
+// ConstructEndpointName constructs endpoint id which is used to identify an endpoint from HNS/HCN.
-// 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
 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
+// RemoveHnsEndpoint detaches the given name endpoint from container specified by containerID,
-// For shared endpoint, ContainerDetach is used
+// or removes the given name endpoint completely.
-// for removing the endpoint completely, HotDetachEndpoint is used
+func RemoveHnsEndpoint(epName string, netns string, containerID string) error {
-func DeprovisionEndpoint(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.
+// AddHnsEndpoint attaches an HNSEndpoint to a container specified by containerID.
-// If an endpoint already exists, the endpoint is reused.
+func AddHnsEndpoint(epName string, expectedNetworkId string, containerID string, netns string, makeEndpoint HnsEndpointMakerFunc) (*hcsshim.HNSEndpoint, error) {
-// This call is idempotent
+	// for shared endpoint, we expect that the endpoint already exists.
-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.
 	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,
+// AddHcnEndpoint attaches a HostComputeEndpoint to the given namespace.
-	makeEndpoint HcnEndpointMakerFunc) (*hcn.HostComputeEndpoint, error) {
+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
+// ConstructHnsResult constructs the CNI result for the HNSEndpoint.
-func ConstructResult(hnsNetwork *hcsshim.HNSNetwork, hnsEndpoint *hcsshim.HNSEndpoint) (*current.Result, error) {
+func ConstructHnsResult(hnsNetwork *hcsshim.HNSNetwork, hnsEndpoint *hcsshim.HNSEndpoint) (*current.Result, error) {
 	resultInterface := &current.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 := &current.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,
 		},

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
+// MarshalPolicies converts the HNSEndpoint policies in Policies
-// to HNS specific policies as Json raw bytes
+// 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
+// ApplyOutboundNatPolicy applies the sNAT policy in HNS/HCN and configures the given CIDR as an exception.
-// Simultaneously an exception is added for the network that has to be Nat'd
 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

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) {
+	hnsEndpoint, err := hns.AddHnsEndpoint(epName, hnsNetwork.Id, args.ContainerID, args.Netns, func() (*hcsshim.HNSEndpoint, 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 = 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 {

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 {