Review comments

Author: madhanrm

pkg/hns/endpoint_windows.go

@@ -1,3 +1,19 @@
+// +build windows
+
+// Copyright 2017 CNI authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
 package hns
 
 import (
@@ -8,6 +24,9 @@ import (
 	"strings"
 )
 
+// 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
 func ConstructEndpointName(containerID string, netNs string, networkName string) string {
 	if netNs != "" {
 		splits := strings.Split(netNs, ":")
@@ -19,6 +38,9 @@ func ConstructEndpointName(containerID string, netNs string, networkName string)
 	return epName
 }
 
+// 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 {
 	hnsEndpoint, err := hcsshim.GetHNSEndpointByName(epName)
 	if err != nil {
@@ -52,6 +74,9 @@ func DeprovisionEndpoint(epName string, netns string, containerID string) error
 
 type EndpointMakerFunc 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, makeEndpoint EndpointMakerFunc) (*hcsshim.HNSEndpoint, error) {
 	// check if endpoint already exists
 	createEndpoint := true
@@ -87,6 +112,7 @@ func ProvisionEndpoint(epName string, expectedNetworkId string, containerID stri
 	return hnsEndpoint, nil
 }
 
+// ConstructResult constructs the CNI result for the endpoint
 func ConstructResult(hnsNetwork *hcsshim.HNSNetwork, hnsEndpoint *hcsshim.HNSEndpoint) (*current.Result, error) {
 	resultInterface := &current.Interface{
 		Name: hnsEndpoint.Name,

pkg/hns/netconf_windows.go

@@ -1,22 +1,41 @@
+// +build windows
+
+// Copyright 2017 CNI authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
 package hns
 
 import (
-	"strings"
 	"encoding/json"
 	"github.com/containernetworking/cni/pkg/types"
+	"strings"
 )
 
+// NetConf is the CNI spec
 type NetConf struct {
 	types.NetConf
 
-	additionalArgs    []policyArgument `json:"AdditionalArgs,omitempty"`
+	additionalArgs []policyArgument `json:"AdditionalArgs,omitempty"`
 }
 
 type policyArgument struct {
-	Type string
+	Type  string
 	Value map[string]interface{}
 }
 
+// MarshalPolicies converts the Endpoint policies in AdditionalArgs
+// to HNS specific policies as Json raw bytes
 func (n *NetConf) MarshalPolicies() []json.RawMessage {
 	if n.additionalArgs == nil {
 		n.additionalArgs = []policyArgument{}
@@ -32,6 +51,8 @@ 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
 func (n *NetConf) ApplyOutboundNatPolicy(nwToNat string) {
 	if n.additionalArgs == nil {
 		n.additionalArgs = []policyArgument{}
@@ -84,9 +105,10 @@ func (n *NetConf) ApplyOutboundNatPolicy(nwToNat string) {
 	n.additionalArgs = append(n.additionalArgs, natEntry)
 }
 
+// ApplyDefaultPAPolicy is used to configure a endpoint PA policy in HNS
 func (n *NetConf) ApplyDefaultPAPolicy(paAddress string) {
 	if n.additionalArgs == nil {
-		 n.additionalArgs = []policyArgument{}
+		n.additionalArgs = []policyArgument{}
 	}
 
 	// if its already present, leave untouched
@@ -118,4 +140,3 @@ func hasKey(m map[string]interface{}, k string) bool {
 	_, ok := m[k]
 	return ok
 }
-

plugins/meta/flannel/flannel.go

@@ -20,7 +20,6 @@ package main
 
 import (
 	"bufio"
-	"encoding/binary"
 	"encoding/json"
 	"fmt"
 	"io/ioutil"
@@ -31,7 +30,6 @@ import (
 	"strconv"
 	"strings"
 
-	"errors"
 	"github.com/containernetworking/cni/pkg/invoke"
 	"github.com/containernetworking/cni/pkg/skel"
 	"github.com/containernetworking/cni/pkg/types"
@@ -267,11 +265,10 @@ func cmdAddWindows(containerID string, n *NetConf, fenv *subnetEnv) error {
 	}
 
 	n.Delegate["ipam"] = map[string]interface{}{
-		"type":       "host-local",
-		"subnet":     fenv.sn.String(),
+		"type":   "host-local",
+		"subnet": fenv.sn.String(),
 	}
 
-
 	return delegateAdd(containerID, n.DataDir, n.Delegate)
 }