From d1e1e2cfceadeba6dc9676c200dce6c990976bb3 Mon Sep 17 00:00:00 2001 From: Mathew Merrick Date: Mon, 24 May 2021 13:17:21 -0700 Subject: [PATCH 1/6] release with ip address --- cns/cnsclient/cnsclient.go | 10 +++++++--- cns/cnsclient/cnsclient_test.go | 6 +++--- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/cns/cnsclient/cnsclient.go b/cns/cnsclient/cnsclient.go index efd2070059..1a98eda5c7 100644 --- a/cns/cnsclient/cnsclient.go +++ b/cns/cnsclient/cnsclient.go @@ -220,7 +220,7 @@ func (cnsClient *CNSClient) RequestIPAddress(orchestratorContext []byte) (*cns.I defer func() { if err != nil { - cnsClient.ReleaseIPAddress(orchestratorContext) + cnsClient.ReleaseIPAddress("", orchestratorContext) } }() @@ -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 expressed not in CIDR notation +func (cnsClient *CNSClient) ReleaseIPAddress(ipaddress string, orchestratorContext []byte) error { var ( err error res *http.Response @@ -281,6 +281,10 @@ func (cnsClient *CNSClient) ReleaseIPAddress(orchestratorContext []byte) error { OrchestratorContext: orchestratorContext, } + if len(ipaddress) == 0 { + payload.DesiredIPAddress = ipaddress + } + err = json.NewEncoder(&body).Encode(payload) if err != nil { log.Errorf("encoding json failed with %v", err) diff --git a/cns/cnsclient/cnsclient_test.go b/cns/cnsclient/cnsclient_test.go index 6a90f7039b..3635d5c76b 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("", 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(ipaddresses[0].IPAddress, 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("", orchestratorContext) if err != nil { t.Fatalf("Expected to not fail when releasing IP reservation found with context: %+v", err) } From 223479bc7f089b2e2a969b3e8fd143e7e0c419a2 Mon Sep 17 00:00:00 2001 From: Mathew Merrick Date: Mon, 24 May 2021 13:26:36 -0700 Subject: [PATCH 2/6] CNI contract update --- cns/NetworkContainerContract.go | 1 + cns/cnsclient/cnsclient.go | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/cns/NetworkContainerContract.go b/cns/NetworkContainerContract.go index fab7701983..175f39d06a 100644 --- a/cns/NetworkContainerContract.go +++ b/cns/NetworkContainerContract.go @@ -218,6 +218,7 @@ type HostIPInfo struct { type IPConfigRequest struct { DesiredIPAddress string + ContainerID string OrchestratorContext json.RawMessage } diff --git a/cns/cnsclient/cnsclient.go b/cns/cnsclient/cnsclient.go index 1a98eda5c7..b83f9cf620 100644 --- a/cns/cnsclient/cnsclient.go +++ b/cns/cnsclient/cnsclient.go @@ -266,7 +266,7 @@ func (cnsClient *CNSClient) RequestIPAddress(orchestratorContext []byte) (*cns.I return response, err } -// ReleaseIPAddress calls releaseIPAddress on CNS, ipaddress expressed not in CIDR notation +// ReleaseIPAddress calls releaseIPAddress on CNS, ipaddress ex: (10.0.0.1) func (cnsClient *CNSClient) ReleaseIPAddress(ipaddress string, orchestratorContext []byte) error { var ( err error @@ -281,7 +281,7 @@ func (cnsClient *CNSClient) ReleaseIPAddress(ipaddress string, orchestratorConte OrchestratorContext: orchestratorContext, } - if len(ipaddress) == 0 { + if len(ipaddress) > 0 { payload.DesiredIPAddress = ipaddress } From 97ff8281d18de05184c132135ba842b1ea797c19 Mon Sep 17 00:00:00 2001 From: Mathew Merrick Date: Fri, 28 May 2021 15:23:11 -0700 Subject: [PATCH 3/6] update contract --- cns/NetworkContainerContract.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cns/NetworkContainerContract.go b/cns/NetworkContainerContract.go index 175f39d06a..050946fa5a 100644 --- a/cns/NetworkContainerContract.go +++ b/cns/NetworkContainerContract.go @@ -218,7 +218,8 @@ type HostIPInfo struct { type IPConfigRequest struct { DesiredIPAddress string - ContainerID string + PodInterfaceID string + InfraContainerID string OrchestratorContext json.RawMessage } From ccb5f918d57369fbe168c97761aa469899f868d0 Mon Sep 17 00:00:00 2001 From: Mathew Merrick Date: Tue, 1 Jun 2021 12:58:57 -0700 Subject: [PATCH 4/6] pass endpoint info to invoker delete --- cni/network/invoker.go | 3 ++- cni/network/invoker_azure.go | 4 ++-- cni/network/network.go | 16 ++++++++-------- cns/cnsclient/cnsclient.go | 27 ++++++++++----------------- 4 files changed, 22 insertions(+), 28 deletions(-) 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/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/cnsclient/cnsclient.go b/cns/cnsclient/cnsclient.go index b83f9cf620..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) @@ -267,7 +267,7 @@ func (cnsClient *CNSClient) RequestIPAddress(orchestratorContext []byte) (*cns.I } // ReleaseIPAddress calls releaseIPAddress on CNS, ipaddress ex: (10.0.0.1) -func (cnsClient *CNSClient) ReleaseIPAddress(ipaddress string, orchestratorContext []byte) error { +func (cnsClient *CNSClient) ReleaseIPAddress(ipconfig *cns.IPConfigRequest) error { var ( err error res *http.Response @@ -275,22 +275,15 @@ func (cnsClient *CNSClient) ReleaseIPAddress(ipaddress string, orchestratorConte ) url := cnsClient.connectionURL + cns.ReleaseIPConfig - log.Printf("ReleaseIPAddress url %v", url) - - payload := &cns.IPConfigRequest{ - OrchestratorContext: orchestratorContext, - } - if len(ipaddress) > 0 { - payload.DesiredIPAddress = ipaddress - } - - 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()) From f155174d04dcf39d1b14ec2e999c4cece576816e Mon Sep 17 00:00:00 2001 From: Mathew Merrick Date: Tue, 1 Jun 2021 13:52:36 -0700 Subject: [PATCH 5/6] update test --- cns/NetworkContainerContract.go | 4 ++-- cns/cnsclient/cnsclient_test.go | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/cns/NetworkContainerContract.go b/cns/NetworkContainerContract.go index 050946fa5a..7b28b5502f 100644 --- a/cns/NetworkContainerContract.go +++ b/cns/NetworkContainerContract.go @@ -224,8 +224,8 @@ type IPConfigRequest struct { } 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_test.go b/cns/cnsclient/cnsclient_test.go index 3635d5c76b..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(ipaddresses[0].IPAddress, 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) } From 901e839e01de1241a4e22fb780b6a955febbc9e1 Mon Sep 17 00:00:00 2001 From: Mathew Merrick Date: Tue, 1 Jun 2021 17:47:21 -0700 Subject: [PATCH 6/6] fix merge artifacts --- cni/network/invoker_cns.go | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) 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) }