From 57b42a7b995f23ee74d6879da5b49bb65f8b740a Mon Sep 17 00:00:00 2001 From: Nathan Gieseker Date: Tue, 12 Feb 2019 19:29:17 -0800 Subject: [PATCH] Windows: Adds HCS Calls and Bug Fixes Move the windows plugin to use the Host Compute (v2) APIs, as well as clean-up the code. Allows win-bridge to use either the old API or Host Compute (v2) api depending on a conf parameter. Fixes a leaked endpoint issue on windows for the v1 flow, and removes the hns/pkg from the linux test run. --- Godeps/Godeps.json | 4 +- pkg/hns/endpoint_windows.go | 219 +++++++++++++++++- pkg/hns/hns_suite_windows_test.go | 13 ++ ..._test.go => netconf_suite_windows_test.go} | 0 pkg/hns/{netconf.go => netconf_windows.go} | 10 +- ...etconf_test.go => netconf_windows_test.go} | 0 .../windows/win-bridge/win-bridge_windows.go | 154 ++++++++---- .../win-overlay/win-overlay_windows.go | 2 +- test_linux.sh | 2 +- 9 files changed, 349 insertions(+), 55 deletions(-) create mode 100644 pkg/hns/hns_suite_windows_test.go rename pkg/hns/{netconf_suite_test.go => netconf_suite_windows_test.go} (100%) rename pkg/hns/{netconf.go => netconf_windows.go} (95%) rename pkg/hns/{netconf_test.go => netconf_windows_test.go} (100%) diff --git a/Godeps/Godeps.json b/Godeps/Godeps.json index ffd50756..d1c2e2df 100644 --- a/Godeps/Godeps.json +++ b/Godeps/Godeps.json @@ -13,8 +13,8 @@ }, { "ImportPath": "github.com/Microsoft/hcsshim", - "Comment": "v0.7.4", - "Rev": "e44e499d29527b244d6858772f1b9090eeaddc4e" + "Comment": "v0.7.6", + "Rev": "6efef912cc0ecd8778bab95d105662d4f73f8ccd" }, { "ImportPath": "github.com/Microsoft/hcsshim/internal/guid", diff --git a/pkg/hns/endpoint_windows.go b/pkg/hns/endpoint_windows.go index 9d9185fa..84604700 100644 --- a/pkg/hns/endpoint_windows.go +++ b/pkg/hns/endpoint_windows.go @@ -20,6 +20,8 @@ import ( "strings" "github.com/Microsoft/hcsshim" + "github.com/Microsoft/hcsshim/hcn" + "github.com/containernetworking/cni/pkg/types" "github.com/containernetworking/cni/pkg/types/current" "github.com/juju/errors" ) @@ -28,6 +30,16 @@ const ( pauseContainerNetNS = "none" ) +type EndpointInfo struct { + EndpointName string + DnsSearch []string + NetworkName string + NetworkId string + Gateway net.IP + IpAddress net.IP + Nameservers []string +} + // GetSandboxContainerID returns the sandbox ID of this pod func GetSandboxContainerID(containerID string, netNs string) string { if len(netNs) != 0 && netNs != pauseContainerNetNS { @@ -40,6 +52,111 @@ func GetSandboxContainerID(containerID string, netNs string) string { return containerID } +// short function so we know when to return "" for a string +func GetIpString(ip *net.IP) string { + if len(*ip) == 0 { + return "" + } else { + return ip.String() + } +} + +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) + if err != nil && !hcsshim.IsNotExist(err) { + return nil, errors.Annotatef(err, "Attempt to get endpoint \"%v\" failed", epInfo.EndpointName) + } + + if hnsEndpoint != nil { + if hnsEndpoint.VirtualNetwork != epInfo.NetworkId { + _, err = hnsEndpoint.Delete() + if err != nil { + return nil, errors.Annotatef(err, "Failed to delete endpoint %v", epInfo.EndpointName) + } + hnsEndpoint = nil + } + } + + if hnsEndpoint == nil { + hnsEndpoint = &hcsshim.HNSEndpoint{ + Name: epInfo.EndpointName, + VirtualNetwork: epInfo.NetworkId, + DNSServerList: strings.Join(epInfo.Nameservers, ","), + DNSSuffix: strings.Join(epInfo.DnsSearch, ","), + GatewayAddress: GetIpString(&epInfo.Gateway), + IPAddress: epInfo.IpAddress, + Policies: n.MarshalPolicies(), + } + } + return hnsEndpoint, nil +} + +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) + if err != nil && (err != hcn.EndpointNotFoundError{EndpointName: epInfo.EndpointName}) { + return nil, errors.Annotatef(err, "Attempt to get endpoint \"%v\" failed", epInfo.EndpointName) + } + + if hcnEndpoint != nil { + // If the endpont already exists, then we should return error unless + // the endpoint is based on a different network then delete + // should that fail return error + if hcnEndpoint.HostComputeNetwork != epInfo.NetworkId { + err = hcnEndpoint.Delete() + if err != nil { + return nil, errors.Annotatef(err, "Failed to delete endpoint %v", epInfo.EndpointName) + hcnEndpoint = nil + + } + } else { + return nil, fmt.Errorf("Endpoint \"%v\" already exits", epInfo.EndpointName) + } + } + + if hcnEndpoint == nil { + routes := []hcn.Route{ + { + NextHop: GetIpString(&epInfo.Gateway), + DestinationPrefix: func() string { + destinationPrefix := "0.0.0.0/0" + if ipv6 := epInfo.Gateway.To4(); ipv6 == nil { + destinationPrefix = "::/0" + } + return destinationPrefix + }(), + }, + } + + hcnDns := hcn.Dns{ + Search: epInfo.DnsSearch, + ServerList: epInfo.Nameservers, + } + + hcnIpConfig := hcn.IpConfig{ + IpAddress: GetIpString(&epInfo.IpAddress), + } + ipConfigs := []hcn.IpConfig{hcnIpConfig} + + hcnEndpoint = &hcn.HostComputeEndpoint{ + SchemaVersion: hcn.Version{Major: 2}, + Name: epInfo.EndpointName, + HostComputeNetwork: epInfo.NetworkId, + Dns: hcnDns, + Routes: routes, + IpConfigurations: ipConfigs, + Policies: func() []hcn.EndpointPolicy { + if n.HcnPolicyArgs == nil { + n.HcnPolicyArgs = []hcn.EndpointPolicy{} + } + return n.HcnPolicyArgs + }(), + } + } + 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 @@ -80,7 +197,16 @@ 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) { +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 { + return nil, errors.Annotatef(err, "failed to find HNSEndpoint %s", epName) + } + } + // check if endpoint already exists createEndpoint := true hnsEndpoint, err := hcsshim.GetHNSEndpointByName(epName) @@ -107,16 +233,47 @@ 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) + if err != nil { + return nil, errors.Annotatef(err, "failed to Deprovsion after HotAttach failure") + } + } if hcsshim.ErrComputeSystemDoesNotExist == err { return hnsEndpoint, nil } - return nil, err } return hnsEndpoint, nil } +type HcnEndpointMakerFunc func() (*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") + } + + if hcnEndpoint, err = hcnEndpoint.Create(); err != nil { + return nil, errors.Annotate(err, "failed to create the new HNSEndpoint") + } + + 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 Add endpoint to namespace") + } + return hcnEndpoint, nil + +} + // ConstructResult constructs the CNI result for the endpoint func ConstructResult(hnsNetwork *hcsshim.HNSNetwork, hnsEndpoint *hcsshim.HNSEndpoint) (*current.Result, error) { resultInterface := ¤t.Interface{ @@ -147,6 +304,64 @@ func ConstructResult(hnsNetwork *hcsshim.HNSNetwork, hnsEndpoint *hcsshim.HNSEnd result := ¤t.Result{} result.Interfaces = []*current.Interface{resultInterface} result.IPs = []*current.IPConfig{resultIPConfig} + result.DNS = types.DNS{ + Search: strings.Split(hnsEndpoint.DNSSuffix, ","), + Nameservers: strings.Split(hnsEndpoint.DNSServerList, ","), + } + + return result, nil +} + +// This version follows the v2 workflow of removing the endpoint from the namespace and deleting it +func RemoveHcnEndpoint(epName string) error { + hcnEndpoint, err := hcn.GetEndpointByName(epName) + if err != nil { + _ = fmt.Errorf("[win-cni] Failed to find endpoint %v, err:%v", epName, err) + return err + } + if hcnEndpoint != nil { + err = hcnEndpoint.Delete() + if err != nil { + return fmt.Errorf("[win-cni] Failed to delete endpoint %v, err:%v", epName, err) + } + } + return nil +} + +func ConstructHcnResult(hcnNetwork *hcn.HostComputeNetwork, hcnEndpoint *hcn.HostComputeEndpoint) (*current.Result, error) { + resultInterface := ¤t.Interface{ + Name: hcnEndpoint.Name, + Mac: hcnEndpoint.MacAddress, + } + _, ipSubnet, err := net.ParseCIDR(hcnNetwork.Ipams[0].Subnets[0].IpAddressPrefix) + if err != nil { + return nil, err + } + + var ipVersion string + ipAddress := net.ParseIP(hcnEndpoint.IpConfigurations[0].IpAddress) + if ipv4 := ipAddress.To4(); ipv4 != nil { + ipVersion = "4" + } else if ipv6 := ipAddress.To16(); ipv6 != nil { + ipVersion = "6" + } else { + return nil, fmt.Errorf("[win-cni] The IPAddress of hnsEndpoint isn't a valid ipv4 or ipv6 Address.") + } + + resultIPConfig := ¤t.IPConfig{ + Version: ipVersion, + Address: net.IPNet{ + IP: ipAddress, + Mask: ipSubnet.Mask}, + Gateway: net.ParseIP(hcnEndpoint.Routes[0].NextHop), + } + result := ¤t.Result{} + result.Interfaces = []*current.Interface{resultInterface} + result.IPs = []*current.IPConfig{resultIPConfig} + result.DNS = types.DNS{ + Search: hcnEndpoint.Dns.Search, + Nameservers: hcnEndpoint.Dns.ServerList, + } return result, nil } diff --git a/pkg/hns/hns_suite_windows_test.go b/pkg/hns/hns_suite_windows_test.go new file mode 100644 index 00000000..af24522d --- /dev/null +++ b/pkg/hns/hns_suite_windows_test.go @@ -0,0 +1,13 @@ +package hns_test + +import ( + "testing" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" +) + +func TestHns(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Hns Suite") +} diff --git a/pkg/hns/netconf_suite_test.go b/pkg/hns/netconf_suite_windows_test.go similarity index 100% rename from pkg/hns/netconf_suite_test.go rename to pkg/hns/netconf_suite_windows_test.go diff --git a/pkg/hns/netconf.go b/pkg/hns/netconf_windows.go similarity index 95% rename from pkg/hns/netconf.go rename to pkg/hns/netconf_windows.go index 003ffde9..b73c972c 100644 --- a/pkg/hns/netconf.go +++ b/pkg/hns/netconf_windows.go @@ -15,19 +15,19 @@ package hns import ( - "encoding/json" - "strings" - "bytes" + "encoding/json" + "github.com/Microsoft/hcsshim/hcn" "github.com/buger/jsonparser" "github.com/containernetworking/cni/pkg/types" + "strings" ) // NetConf is the CNI spec type NetConf struct { types.NetConf - - Policies []policy `json:"policies,omitempty"` + HcnPolicyArgs []hcn.EndpointPolicy `json:"HcnPolicyArgs,omitempty"` + Policies []policy `json:"policies,omitempty"` } type policy struct { diff --git a/pkg/hns/netconf_test.go b/pkg/hns/netconf_windows_test.go similarity index 100% rename from pkg/hns/netconf_test.go rename to pkg/hns/netconf_windows_test.go diff --git a/plugins/main/windows/win-bridge/win-bridge_windows.go b/plugins/main/windows/win-bridge/win-bridge_windows.go index 13439742..af3325ed 100644 --- a/plugins/main/windows/win-bridge/win-bridge_windows.go +++ b/plugins/main/windows/win-bridge/win-bridge_windows.go @@ -21,8 +21,8 @@ import ( "strings" "github.com/juju/errors" - "github.com/Microsoft/hcsshim" + "github.com/Microsoft/hcsshim/hcn" "github.com/containernetworking/cni/pkg/skel" "github.com/containernetworking/cni/pkg/types" "github.com/containernetworking/cni/pkg/types/current" @@ -34,7 +34,8 @@ import ( type NetConf struct { hns.NetConf - IPMasqNetwork string `json:"ipMasqNetwork,omitempty"` + IPMasqNetwork string `json:"ipMasqNetwork,omitempty"` + ApiVersion int `json:"ApiVersion"` } func init() { @@ -52,11 +53,44 @@ func loadNetConf(bytes []byte) (*NetConf, string, error) { return n, n.CNIVersion, nil } -func cmdAdd(args *skel.CmdArgs) error { - n, cniVersion, err := loadNetConf(args.StdinData) - if err != nil { - return errors.Annotate(err, "error while loadNetConf") +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 + 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") + } + + // Convert whatever the IPAM result was into the current Result type + result, err := current.NewResultFromResult(r) + if err != nil { + return nil, errors.Annotatef(err, "error while NewResultFromResult") + } else { + if len(result.IPs) == 0 { + return nil, errors.New("IPAM plugin return is missing IP config") + } + epInfo.IpAddress = result.IPs[0].Address.IP + epInfo.Gateway = result.IPs[0].Address.IP.Mask(result.IPs[0].Address.Mask) + + // Calculate gateway for bridge network (needs to be x.2) + epInfo.Gateway[len(epInfo.Gateway)-1] += 2 + } } + // NAT based on the the configured cluster network + if len(n.IPMasqNetwork) != 0 { + n.ApplyOutboundNatPolicy(n.IPMasqNetwork) + } + + epInfo.DnsSearch = n.DNS.Search + epInfo.Nameservers = n.DNS.Nameservers + + return epInfo, nil +} + +func cmdHnsAdd(args *skel.CmdArgs, n *NetConf, cniVersion *string) error { networkName := n.Name hnsNetwork, err := hcsshim.GetHNSNetworkByName(networkName) @@ -74,44 +108,16 @@ func cmdAdd(args *skel.CmdArgs) error { epName := hns.ConstructEndpointName(args.ContainerID, args.Netns, n.Name) - hnsEndpoint, err := hns.ProvisionEndpoint(epName, hnsNetwork.Id, args.ContainerID, func() (*hcsshim.HNSEndpoint, error) { - // run the IPAM plugin and get back the config to apply - r, err := ipam.ExecAdd(n.IPAM.Type, args.StdinData) + hnsEndpoint, err := hns.ProvisionEndpoint(epName, hnsNetwork.Id, args.ContainerID, args.Netns, func() (*hcsshim.HNSEndpoint, error) { + epInfo, err := ProcessEndpointArgs(args, n) + epInfo.NetworkId = hnsNetwork.Id if err != nil { - return nil, errors.Annotatef(err, "error while ipam.ExecAdd") + return nil, errors.Annotatef(err, "error while ProcessEndpointArgs") } - - // Convert whatever the IPAM result was into the current Result type - result, err := current.NewResultFromResult(r) + hnsEndpoint, err := hns.GenerateHnsEndpoint(epInfo, &n.NetConf) if err != nil { - return nil, errors.Annotatef(err, "error while NewResultFromResult") + return nil, errors.Annotatef(err, "error while GenerateHnsEndpoint") } - - if len(result.IPs) == 0 { - return nil, errors.New("IPAM plugin return is missing IP config") - } - - // Calculate gateway for bridge network (needs to be x.2) - gw := result.IPs[0].Address.IP.Mask(result.IPs[0].Address.Mask) - gw[len(gw)-1] += 2 - - // NAT based on the the configured cluster network - if len(n.IPMasqNetwork) != 0 { - n.ApplyOutboundNatPolicy(n.IPMasqNetwork) - } - - result.DNS = n.DNS - - hnsEndpoint := &hcsshim.HNSEndpoint{ - Name: epName, - VirtualNetwork: hnsNetwork.Id, - DNSServerList: strings.Join(result.DNS.Nameservers, ","), - DNSSuffix: strings.Join(result.DNS.Search, ","), - GatewayAddress: gw.String(), - IPAddress: result.IPs[0].Address.IP, - Policies: n.MarshalPolicies(), - } - return hnsEndpoint, nil }) if err != nil { @@ -123,7 +129,64 @@ func cmdAdd(args *skel.CmdArgs) error { return errors.Annotatef(err, "error while constructResult") } - return types.PrintResult(result, cniVersion) + return types.PrintResult(result, *cniVersion) + +} + +func cmdHcnAdd(args *skel.CmdArgs, n *NetConf, cniVersion *string) error { + networkName := n.Name + hcnNetwork, err := hcn.GetNetworkByName(networkName) + if err != nil { + return errors.Annotatef(err, "error while GetNetworkByName(%s)", networkName) + } + + if hcnNetwork == nil { + return fmt.Errorf("network %v not found", networkName) + } + + if hcnNetwork.Type != hcn.L2Bridge { + return 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) + if err != nil { + return nil, errors.Annotatef(err, "error while ProcessEndpointArgs") + } + epInfo.NetworkId = hcnNetwork.Id + + hcnEndpoint, err := hns.GenerateHcnEndpoint(epInfo, &n.NetConf) + if err != nil { + return nil, errors.Annotatef(err, "error while GenerateHcnEndpoint") + } + return hcnEndpoint, nil + }) + if err != nil { + return errors.Annotatef(err, "error while AddHcnEndpoint(%v,%v,%v)", epName, hcnNetwork.Id, args.Netns) + } + + result, err := hns.ConstructHcnResult(hcnNetwork, hcnEndpoint) + if err != nil { + return errors.Annotatef(err, "error while ConstructHcnResult") + } + + return types.PrintResult(result, *cniVersion) +} + +func cmdAdd(args *skel.CmdArgs) error { + n, cniVersion, err := loadNetConf(args.StdinData) + if err != nil { + return errors.Annotate(err, "error while loadNetConf") + } + + if n.ApiVersion == 2 { + err = cmdHcnAdd(args, n, &cniVersion) + } else { + err = cmdHnsAdd(args, n, &cniVersion) + } + return err } func cmdDel(args *skel.CmdArgs) error { @@ -135,10 +198,13 @@ func cmdDel(args *skel.CmdArgs) error { if err := ipam.ExecDel(n.IPAM.Type, args.StdinData); err != nil { return err } - epName := hns.ConstructEndpointName(args.ContainerID, args.Netns, n.Name) - - return hns.DeprovisionEndpoint(epName, args.Netns, args.ContainerID) + + if n.ApiVersion == 2 { + return hns.RemoveHcnEndpoint(epName) + } else { + return hns.DeprovisionEndpoint(epName, args.Netns, args.ContainerID) + } } func cmdGet(_ *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 cefe68a6..047b6a71 100644 --- a/plugins/main/windows/win-overlay/win-overlay_windows.go +++ b/plugins/main/windows/win-overlay/win-overlay_windows.go @@ -83,7 +83,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, func() (*hcsshim.HNSEndpoint, error) { + hnsEndpoint, err := hns.ProvisionEndpoint(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 { diff --git a/test_linux.sh b/test_linux.sh index 34b64f80..c5e4d6fa 100755 --- a/test_linux.sh +++ b/test_linux.sh @@ -10,7 +10,7 @@ source ./build_linux.sh echo "Running tests" -GINKGO_FLAGS="-p --randomizeAllSpecs --randomizeSuites --failOnPending --progress" +GINKGO_FLAGS="-p --randomizeAllSpecs --randomizeSuites --failOnPending --progress --skipPackage=pkg/hns" # user has not provided PKG override if [ -z "$PKG" ]; then