diff --git a/cni/network/invoker.go b/cni/network/invoker.go index 47a4508733..3afc4a95d8 100644 --- a/cni/network/invoker.go +++ b/cni/network/invoker.go @@ -4,6 +4,7 @@ import ( "net" "github.com/Azure/azure-container-networking/cni" + "github.com/Azure/azure-container-networking/network" cniTypesCurr "github.com/containernetworking/cni/pkg/types/current" ) @@ -16,5 +17,5 @@ type IPAMInvoker interface { Add(nwCfg *cni.NetworkConfig, subnetPrefix *net.IPNet, options map[string]interface{}) (*cniTypesCurr.Result, *cniTypesCurr.Result, error) //Delete calls to the invoker source, and returns error. Returning an error here will fail the CNI Delete call. - Delete(address *net.IPNet, nwCfg *cni.NetworkConfig, options map[string]interface{}) error + Delete(address *net.IPNet, nwCfg *cni.NetworkConfig, epInfo *network.EndpointInfo, options map[string]interface{}) error } diff --git a/cni/network/invoker_azure.go b/cni/network/invoker_azure.go index cbf19651bb..bb3c99361c 100644 --- a/cni/network/invoker_azure.go +++ b/cni/network/invoker_azure.go @@ -50,7 +50,7 @@ func (invoker *AzureIPAMInvoker) Add(nwCfg *cni.NetworkConfig, subnetPrefix *net defer func() { if err != nil { if len(result.IPs) > 0 { - invoker.plugin.ipamInvoker.Delete(&result.IPs[0].Address, nwCfg, options) + invoker.plugin.ipamInvoker.Delete(&result.IPs[0].Address, nwCfg, nil, options) } else { err = fmt.Errorf("No IP's to delete on error: %v", err) } @@ -79,7 +79,7 @@ func (invoker *AzureIPAMInvoker) Add(nwCfg *cni.NetworkConfig, subnetPrefix *net return result, resultV6, err } -func (invoker *AzureIPAMInvoker) Delete(address *net.IPNet, nwCfg *cni.NetworkConfig, options map[string]interface{}) error { +func (invoker *AzureIPAMInvoker) Delete(address *net.IPNet, nwCfg *cni.NetworkConfig, _ *network.EndpointInfo, options map[string]interface{}) error { if nwCfg == nil { return invoker.plugin.Errorf("nil nwCfg passed to CNI ADD, stack: %+v", string(debug.Stack())) diff --git a/cni/network/invoker_cns.go b/cni/network/invoker_cns.go index 99f4502255..89a173cde4 100644 --- a/cni/network/invoker_cns.go +++ b/cni/network/invoker_cns.go @@ -168,7 +168,7 @@ func setHostOptions(nwCfg *cni.NetworkConfig, hostSubnetPrefix *net.IPNet, ncSub } // Delete calls into the releaseipconfiguration API in CNS -func (invoker *CNSIPAMInvoker) Delete(address *net.IPNet, nwCfg *cni.NetworkConfig, options map[string]interface{}) error { +func (invoker *CNSIPAMInvoker) Delete(address *net.IPNet, nwCfg *cni.NetworkConfig, epInfo *network.EndpointInfo, options map[string]interface{}) error { // Parse Pod arguments. podInfo := cns.KubernetesPodInfo{PodName: invoker.podName, PodNamespace: invoker.podNamespace} @@ -178,5 +178,22 @@ func (invoker *CNSIPAMInvoker) Delete(address *net.IPNet, nwCfg *cni.NetworkConf return err } - return invoker.cnsClient.ReleaseIPAddress(orchestratorContext) + req := cns.IPConfigRequest{ + OrchestratorContext: orchestratorContext, + } + + if address != nil { + req.DesiredIPAddress = address.IP.String() + } else { + log.Printf("CNS invoker called with empty IP address") + } + + if epInfo != nil { + req.PodInterfaceID = epInfo.Id + req.InfraContainerID = epInfo.ContainerID + } else { + log.Printf("CNS invoker called with empty endpoint information") + } + + return invoker.cnsClient.ReleaseIPAddress(&req) } diff --git a/cni/network/network.go b/cni/network/network.go index d8c3f011d0..6a91f9913b 100644 --- a/cni/network/network.go +++ b/cni/network/network.go @@ -482,10 +482,10 @@ func (plugin *netPlugin) Add(args *cniSkel.CmdArgs) error { defer func() { if err != nil { if result != nil && len(result.IPs) > 0 { - plugin.ipamInvoker.Delete(&result.IPs[0].Address, nwCfg, options) + plugin.ipamInvoker.Delete(&result.IPs[0].Address, nwCfg, epInfo, options) } if resultV6 != nil && len(resultV6.IPs) > 0 { - plugin.ipamInvoker.Delete(&resultV6.IPs[0].Address, nwCfg, options) + plugin.ipamInvoker.Delete(&resultV6.IPs[0].Address, nwCfg, epInfo, options) } } }() @@ -586,10 +586,10 @@ func (plugin *netPlugin) Add(args *cniSkel.CmdArgs) error { defer func() { if err != nil { if result != nil && len(result.IPs) > 0 { - plugin.ipamInvoker.Delete(&result.IPs[0].Address, nwCfg, nwInfo.Options) + plugin.ipamInvoker.Delete(&result.IPs[0].Address, nwCfg, epInfo, nwInfo.Options) } if resultV6 != nil && len(resultV6.IPs) > 0 { - plugin.ipamInvoker.Delete(&resultV6.IPs[0].Address, nwCfg, nwInfo.Options) + plugin.ipamInvoker.Delete(&resultV6.IPs[0].Address, nwCfg, epInfo, nwInfo.Options) } } }() @@ -889,7 +889,7 @@ func (plugin *netPlugin) Delete(args *cniSkel.CmdArgs) error { if !nwCfg.MultiTenancy { // attempt to release address associated with this Endpoint id // This is to ensure clean up is done even in failure cases - err = plugin.ipamInvoker.Delete(nil, nwCfg, nwInfo.Options) + err = plugin.ipamInvoker.Delete(nil, nwCfg, epInfo, nwInfo.Options) if err != nil { log.Printf("Network not found, attempted to release address with error: %v", err) } @@ -908,7 +908,7 @@ func (plugin *netPlugin) Delete(args *cniSkel.CmdArgs) error { // attempt to release address associated with this Endpoint id // This is to ensure clean up is done even in failure cases log.Printf("release ip ep not found") - if err = plugin.ipamInvoker.Delete(nil, nwCfg, nwInfo.Options); err != nil { + if err = plugin.ipamInvoker.Delete(nil, nwCfg, epInfo, nwInfo.Options); err != nil { log.Printf("Endpoint not found, attempted to release address with error: %v", err) } } @@ -931,7 +931,7 @@ func (plugin *netPlugin) Delete(args *cniSkel.CmdArgs) error { // Call into IPAM plugin to release the endpoint's addresses. for _, address := range epInfo.IPAddresses { log.Printf("release ip:%s", address.IP.String()) - err = plugin.ipamInvoker.Delete(&address, nwCfg, nwInfo.Options) + err = plugin.ipamInvoker.Delete(&address, nwCfg, epInfo, nwInfo.Options) if err != nil { err = plugin.Errorf("Failed to release address %v with error: %v", address, err) return err @@ -940,7 +940,7 @@ func (plugin *netPlugin) Delete(args *cniSkel.CmdArgs) error { } else if epInfo.EnableInfraVnet { nwCfg.Ipam.Subnet = nwInfo.Subnets[0].Prefix.String() nwCfg.Ipam.Address = epInfo.InfraVnetIP.IP.String() - err = plugin.ipamInvoker.Delete(nil, nwCfg, nwInfo.Options) + err = plugin.ipamInvoker.Delete(nil, nwCfg, epInfo, nwInfo.Options) if err != nil { log.Printf("Failed to release address: %v", err) err = plugin.Errorf("Failed to release address %v with error: %v", nwCfg.Ipam.Address, err) diff --git a/cns/NetworkContainerContract.go b/cns/NetworkContainerContract.go index fab7701983..7b28b5502f 100644 --- a/cns/NetworkContainerContract.go +++ b/cns/NetworkContainerContract.go @@ -218,12 +218,14 @@ type HostIPInfo struct { type IPConfigRequest struct { DesiredIPAddress string + PodInterfaceID string + InfraContainerID string OrchestratorContext json.RawMessage } func (i IPConfigRequest) String() string { - return fmt.Sprintf("[IPConfigRequest: DesiredIPAddress %s, OrchestratorContext %s]", - i.DesiredIPAddress, string(i.OrchestratorContext)) + return fmt.Sprintf("[IPConfigRequest: DesiredIPAddress %s, PodInterfaceID %s, InfraContainerID %s, OrchestratorContext %s]", + i.DesiredIPAddress, i.PodInterfaceID, i.InfraContainerID, string(i.OrchestratorContext)) } // IPConfigResponse is used in CNS IPAM mode as a response to CNI ADD diff --git a/cns/cnsclient/cnsclient.go b/cns/cnsclient/cnsclient.go index efd2070059..e59c6c6a4e 100644 --- a/cns/cnsclient/cnsclient.go +++ b/cns/cnsclient/cnsclient.go @@ -218,12 +218,6 @@ func (cnsClient *CNSClient) RequestIPAddress(orchestratorContext []byte) (*cns.I response *cns.IPConfigResponse ) - defer func() { - if err != nil { - cnsClient.ReleaseIPAddress(orchestratorContext) - } - }() - var body bytes.Buffer url := cnsClient.connectionURL + cns.RequestIPConfig @@ -232,6 +226,12 @@ func (cnsClient *CNSClient) RequestIPAddress(orchestratorContext []byte) (*cns.I OrchestratorContext: orchestratorContext, } + defer func() { + if err != nil { + cnsClient.ReleaseIPAddress(payload) + } + }() + err = json.NewEncoder(&body).Encode(payload) if err != nil { log.Errorf("encoding json failed with %v", err) @@ -266,8 +266,8 @@ func (cnsClient *CNSClient) RequestIPAddress(orchestratorContext []byte) (*cns.I return response, err } -// ReleaseIPAddress calls releaseIPAddress on CNS -func (cnsClient *CNSClient) ReleaseIPAddress(orchestratorContext []byte) error { +// ReleaseIPAddress calls releaseIPAddress on CNS, ipaddress ex: (10.0.0.1) +func (cnsClient *CNSClient) ReleaseIPAddress(ipconfig *cns.IPConfigRequest) error { var ( err error res *http.Response @@ -275,18 +275,15 @@ func (cnsClient *CNSClient) ReleaseIPAddress(orchestratorContext []byte) error { ) url := cnsClient.connectionURL + cns.ReleaseIPConfig - log.Printf("ReleaseIPAddress url %v", url) - - payload := &cns.IPConfigRequest{ - OrchestratorContext: orchestratorContext, - } - err = json.NewEncoder(&body).Encode(payload) + err = json.NewEncoder(&body).Encode(ipconfig) if err != nil { log.Errorf("encoding json failed with %v", err) return err } + log.Printf("Releasing ipconfig %s", string(body.Bytes())) + res, err = cnsClient.httpc.Post(url, contentTypeJSON, &body) if err != nil { log.Errorf("[Azure CNSClient] HTTP Post returned error %v", err.Error()) diff --git a/cns/cnsclient/cnsclient_test.go b/cns/cnsclient/cnsclient_test.go index 6a90f7039b..0185baeb85 100644 --- a/cns/cnsclient/cnsclient_test.go +++ b/cns/cnsclient/cnsclient_test.go @@ -227,7 +227,7 @@ func TestCNSClientRequestAndRelease(t *testing.T) { } // no IP reservation found with that context, expect no failure. - err = cnsClient.ReleaseIPAddress(orchestratorContext) + err = cnsClient.ReleaseIPAddress(&cns.IPConfigRequest{OrchestratorContext: orchestratorContext}) if err != nil { t.Fatalf("Release ip idempotent call failed: %+v", err) } @@ -278,7 +278,7 @@ func TestCNSClientRequestAndRelease(t *testing.T) { t.Log(ipaddresses) // release requested IP address, expect success - err = cnsClient.ReleaseIPAddress(orchestratorContext) + err = cnsClient.ReleaseIPAddress(&cns.IPConfigRequest{DesiredIPAddress: ipaddresses[0].IPAddress, OrchestratorContext: orchestratorContext}) if err != nil { t.Fatalf("Expected to not fail when releasing IP reservation found with context: %+v", err) } @@ -318,7 +318,7 @@ func TestCNSClientPodContextApi(t *testing.T) { t.Log(podcontext) // release requested IP address, expect success - err = cnsClient.ReleaseIPAddress(orchestratorContext) + err = cnsClient.ReleaseIPAddress(&cns.IPConfigRequest{OrchestratorContext: orchestratorContext}) if err != nil { t.Fatalf("Expected to not fail when releasing IP reservation found with context: %+v", err) }