From c4b3d7d9ae9684df20d3724b2a048c85520f904f Mon Sep 17 00:00:00 2001 From: rjdenney Date: Mon, 24 Oct 2022 16:15:42 -0400 Subject: [PATCH 01/17] Add proposal for Cilium dualstack --- docs/Cilium-dualstack.md | 23 ++++++++++++++++++++++ docs/feature/dualstack/Cilium-dualstack.md | 23 ++++++++++++++++++++++ 2 files changed, 46 insertions(+) create mode 100644 docs/Cilium-dualstack.md create mode 100644 docs/feature/dualstack/Cilium-dualstack.md diff --git a/docs/Cilium-dualstack.md b/docs/Cilium-dualstack.md new file mode 100644 index 0000000000..8040ed5142 --- /dev/null +++ b/docs/Cilium-dualstack.md @@ -0,0 +1,23 @@ +# Cilium Azure-ipam Dualstack Overlay Proposal + +## Purpose + +This document proposes changes to the azure-ipam contract so that we can work with dualstack clusters between Cilium and Azure CNS. + +## Overview + +With the current CNS solution for dualstack we are setting the nnc to pass multiple NCs with CIDR to be unrolled and added to a pool that contains both v6 and v4 ips. Since we are planning to use Cilium CNI for the linux dualstack solution we need to make changes to azure-ipam to allow for a dualstack cluster to get multiple IPs. Currently azure-Ipam handles the [`CmdAdd`](https://github.com/Azure/azure-container-networking/blob/master/azure-ipam/ipam.go) command from Cilium CNI and sends an IP request to the CNS. The CNS hen in overlay then looks through the pool and picks the first IP found that isn’t used. For dualstack we need to change two things: 1) Azure-ipam needs to be able to assign multiple IPs and 2) CNS needs to be able to determine what type of IP it is returning so that we can have one IPv4 and one IPv6. + +## Proposal + +For this issue two solutions are being proposed. + +The first is having azure-ipam send two separate requests for both an IPv4 and an IPv6. The add command would need to be able to recognize that the pod is dualstack, create two [`IPConfigRequest`](https://github.com/Azure/azure-container-networking/blob/master/cns/NetworkContainerContract.go)s with an added type field to specify either v4 or v6. These two IPs would then both be added to the array of IPs in `cniResult` and return them to the Cilium CNI. + +The second solution is to send one request from azure-ipam and have it specify in its `IPConfigRequest` that it wants a dualstack pod. With solution instead of calling CNS twice to request two IPs it would only call the CNS once and would return both IPs requested in an array. This would most likely require a much more extensive code change as requesting an IP would now return an array of IPs instead of just the one IP. + +For both of these proposals there would also need to be some changes to how IPs are assigned in the CNS. When we go through the pool to find an available IP in [`AssignAnyAvailableIPConfig`](https://github.com/Azure/azure-container-networking/blob/master/cns/restserver/ipam.go) a type check would need to be added to see if it is either v4 or v6 and the type that we want to check for can be passed into the function.This type check + +## Further Questions + +For any errors what should we do if we can obtain one ip but not the other? \ No newline at end of file diff --git a/docs/feature/dualstack/Cilium-dualstack.md b/docs/feature/dualstack/Cilium-dualstack.md new file mode 100644 index 0000000000..8040ed5142 --- /dev/null +++ b/docs/feature/dualstack/Cilium-dualstack.md @@ -0,0 +1,23 @@ +# Cilium Azure-ipam Dualstack Overlay Proposal + +## Purpose + +This document proposes changes to the azure-ipam contract so that we can work with dualstack clusters between Cilium and Azure CNS. + +## Overview + +With the current CNS solution for dualstack we are setting the nnc to pass multiple NCs with CIDR to be unrolled and added to a pool that contains both v6 and v4 ips. Since we are planning to use Cilium CNI for the linux dualstack solution we need to make changes to azure-ipam to allow for a dualstack cluster to get multiple IPs. Currently azure-Ipam handles the [`CmdAdd`](https://github.com/Azure/azure-container-networking/blob/master/azure-ipam/ipam.go) command from Cilium CNI and sends an IP request to the CNS. The CNS hen in overlay then looks through the pool and picks the first IP found that isn’t used. For dualstack we need to change two things: 1) Azure-ipam needs to be able to assign multiple IPs and 2) CNS needs to be able to determine what type of IP it is returning so that we can have one IPv4 and one IPv6. + +## Proposal + +For this issue two solutions are being proposed. + +The first is having azure-ipam send two separate requests for both an IPv4 and an IPv6. The add command would need to be able to recognize that the pod is dualstack, create two [`IPConfigRequest`](https://github.com/Azure/azure-container-networking/blob/master/cns/NetworkContainerContract.go)s with an added type field to specify either v4 or v6. These two IPs would then both be added to the array of IPs in `cniResult` and return them to the Cilium CNI. + +The second solution is to send one request from azure-ipam and have it specify in its `IPConfigRequest` that it wants a dualstack pod. With solution instead of calling CNS twice to request two IPs it would only call the CNS once and would return both IPs requested in an array. This would most likely require a much more extensive code change as requesting an IP would now return an array of IPs instead of just the one IP. + +For both of these proposals there would also need to be some changes to how IPs are assigned in the CNS. When we go through the pool to find an available IP in [`AssignAnyAvailableIPConfig`](https://github.com/Azure/azure-container-networking/blob/master/cns/restserver/ipam.go) a type check would need to be added to see if it is either v4 or v6 and the type that we want to check for can be passed into the function.This type check + +## Further Questions + +For any errors what should we do if we can obtain one ip but not the other? \ No newline at end of file From f77b8206e869a632fc4df2e3eb96773456dbf745 Mon Sep 17 00:00:00 2001 From: rjdenney Date: Mon, 24 Oct 2022 16:31:16 -0400 Subject: [PATCH 02/17] adding modifications to proposal --- docs/Cilium-dualstack.md | 23 ---------------------- docs/feature/dualstack/Cilium-dualstack.md | 6 +++--- 2 files changed, 3 insertions(+), 26 deletions(-) delete mode 100644 docs/Cilium-dualstack.md diff --git a/docs/Cilium-dualstack.md b/docs/Cilium-dualstack.md deleted file mode 100644 index 8040ed5142..0000000000 --- a/docs/Cilium-dualstack.md +++ /dev/null @@ -1,23 +0,0 @@ -# Cilium Azure-ipam Dualstack Overlay Proposal - -## Purpose - -This document proposes changes to the azure-ipam contract so that we can work with dualstack clusters between Cilium and Azure CNS. - -## Overview - -With the current CNS solution for dualstack we are setting the nnc to pass multiple NCs with CIDR to be unrolled and added to a pool that contains both v6 and v4 ips. Since we are planning to use Cilium CNI for the linux dualstack solution we need to make changes to azure-ipam to allow for a dualstack cluster to get multiple IPs. Currently azure-Ipam handles the [`CmdAdd`](https://github.com/Azure/azure-container-networking/blob/master/azure-ipam/ipam.go) command from Cilium CNI and sends an IP request to the CNS. The CNS hen in overlay then looks through the pool and picks the first IP found that isn’t used. For dualstack we need to change two things: 1) Azure-ipam needs to be able to assign multiple IPs and 2) CNS needs to be able to determine what type of IP it is returning so that we can have one IPv4 and one IPv6. - -## Proposal - -For this issue two solutions are being proposed. - -The first is having azure-ipam send two separate requests for both an IPv4 and an IPv6. The add command would need to be able to recognize that the pod is dualstack, create two [`IPConfigRequest`](https://github.com/Azure/azure-container-networking/blob/master/cns/NetworkContainerContract.go)s with an added type field to specify either v4 or v6. These two IPs would then both be added to the array of IPs in `cniResult` and return them to the Cilium CNI. - -The second solution is to send one request from azure-ipam and have it specify in its `IPConfigRequest` that it wants a dualstack pod. With solution instead of calling CNS twice to request two IPs it would only call the CNS once and would return both IPs requested in an array. This would most likely require a much more extensive code change as requesting an IP would now return an array of IPs instead of just the one IP. - -For both of these proposals there would also need to be some changes to how IPs are assigned in the CNS. When we go through the pool to find an available IP in [`AssignAnyAvailableIPConfig`](https://github.com/Azure/azure-container-networking/blob/master/cns/restserver/ipam.go) a type check would need to be added to see if it is either v4 or v6 and the type that we want to check for can be passed into the function.This type check - -## Further Questions - -For any errors what should we do if we can obtain one ip but not the other? \ No newline at end of file diff --git a/docs/feature/dualstack/Cilium-dualstack.md b/docs/feature/dualstack/Cilium-dualstack.md index 8040ed5142..e5ec26127c 100644 --- a/docs/feature/dualstack/Cilium-dualstack.md +++ b/docs/feature/dualstack/Cilium-dualstack.md @@ -6,15 +6,15 @@ This document proposes changes to the azure-ipam contract so that we can work wi ## Overview -With the current CNS solution for dualstack we are setting the nnc to pass multiple NCs with CIDR to be unrolled and added to a pool that contains both v6 and v4 ips. Since we are planning to use Cilium CNI for the linux dualstack solution we need to make changes to azure-ipam to allow for a dualstack cluster to get multiple IPs. Currently azure-Ipam handles the [`CmdAdd`](https://github.com/Azure/azure-container-networking/blob/master/azure-ipam/ipam.go) command from Cilium CNI and sends an IP request to the CNS. The CNS hen in overlay then looks through the pool and picks the first IP found that isn’t used. For dualstack we need to change two things: 1) Azure-ipam needs to be able to assign multiple IPs and 2) CNS needs to be able to determine what type of IP it is returning so that we can have one IPv4 and one IPv6. +With the current CNS solution for dualstack we are setting the nnc to pass multiple NCs with CIDR to be unrolled and added to a pool that contains both v6 and v4 ips. Since we are planning to use Cilium CNI for the linux dualstack solution we need to make changes to azure-ipam to allow for a dualstack cluster to get multiple IPs. Currently azure-Ipam handles the [`CmdAdd`](https://github.com/Azure/azure-container-networking/blob/master/azure-ipam/ipam.go) command from Cilium CNI and sends an IP request to the CNS. The CNS then in overlay then looks through the pool and picks the first IP found that isn’t used. For dualstack we need to change two things: 1) Azure-ipam needs to be able to assign multiple IPs and 2) CNS needs to be able to determine what type of IP it is returning so that we can have one IPv4 and one IPv6. ## Proposal For this issue two solutions are being proposed. -The first is having azure-ipam send two separate requests for both an IPv4 and an IPv6. The add command would need to be able to recognize that the pod is dualstack, create two [`IPConfigRequest`](https://github.com/Azure/azure-container-networking/blob/master/cns/NetworkContainerContract.go)s with an added type field to specify either v4 or v6. These two IPs would then both be added to the array of IPs in `cniResult` and return them to the Cilium CNI. +The first is having azure-ipam send two separate requests for both an IPv4 and an IPv6. The add command would need to be able to recognize that the pod is dualstack, create two [`IPConfigRequest`](https://github.com/Azure/azure-container-networking/blob/master/cns/NetworkContainerContract.go)s with an added type field to specify either v4 or v6 and call [`RequestIPAddress`](https://github.com/Azure/azure-container-networking/blob/master/cns/client/client.go) twice (once for each ip type). These two IPs would then both be returned in separate `IPConfigResponse` objects to the azure-ipam and added to the array of IPs in `cniResult` which then get returned to the Cilium CNI. -The second solution is to send one request from azure-ipam and have it specify in its `IPConfigRequest` that it wants a dualstack pod. With solution instead of calling CNS twice to request two IPs it would only call the CNS once and would return both IPs requested in an array. This would most likely require a much more extensive code change as requesting an IP would now return an array of IPs instead of just the one IP. +The second solution is to send one request from azure-ipam and have it specify in its `IPConfigRequest` that it wants a dualstack pod. With solution instead of calling `RequestIPAddress` twice to request two IPs it would only call the CNS once and would return both IPs requested in an array of `PodIpInfo`s in the `IPConfigResponse`. This would most likely require a much more extensive code change as requesting an IP would now return an array of IPs instead of just the one IP. The change would have to make the default situation expect that only one IP should be returned in the array unless we know that the pod is dualstack (or possible future reasons for returning multiple IPs). For both of these proposals there would also need to be some changes to how IPs are assigned in the CNS. When we go through the pool to find an available IP in [`AssignAnyAvailableIPConfig`](https://github.com/Azure/azure-container-networking/blob/master/cns/restserver/ipam.go) a type check would need to be added to see if it is either v4 or v6 and the type that we want to check for can be passed into the function.This type check From da9800e527692b5a0af66f545dffc27e1e822d0c Mon Sep 17 00:00:00 2001 From: rjdenney Date: Tue, 25 Oct 2022 12:49:35 -0400 Subject: [PATCH 03/17] format changes and adding struts --- docs/feature/dualstack/Cilium-dualstack.md | 41 +++++++++++++++++++--- 1 file changed, 37 insertions(+), 4 deletions(-) diff --git a/docs/feature/dualstack/Cilium-dualstack.md b/docs/feature/dualstack/Cilium-dualstack.md index e5ec26127c..3d3b099f63 100644 --- a/docs/feature/dualstack/Cilium-dualstack.md +++ b/docs/feature/dualstack/Cilium-dualstack.md @@ -6,15 +6,48 @@ This document proposes changes to the azure-ipam contract so that we can work wi ## Overview -With the current CNS solution for dualstack we are setting the nnc to pass multiple NCs with CIDR to be unrolled and added to a pool that contains both v6 and v4 ips. Since we are planning to use Cilium CNI for the linux dualstack solution we need to make changes to azure-ipam to allow for a dualstack cluster to get multiple IPs. Currently azure-Ipam handles the [`CmdAdd`](https://github.com/Azure/azure-container-networking/blob/master/azure-ipam/ipam.go) command from Cilium CNI and sends an IP request to the CNS. The CNS then in overlay then looks through the pool and picks the first IP found that isn’t used. For dualstack we need to change two things: 1) Azure-ipam needs to be able to assign multiple IPs and 2) CNS needs to be able to determine what type of IP it is returning so that we can have one IPv4 and one IPv6. +With the current CNS solution for dualstack we are setting the nnc to pass multiple NCs with CIDR to be unrolled and added to a pool that contains both v6 and v4 ips. Since we are planning to use Cilium CNI for the linux dualstack solution we need to make changes to azure-ipam to allow for a dualstack cluster to get multiple IPs. Currently azure-Ipam handles the [`CmdAdd`](https://github.com/Azure/azure-container-networking/blob/master/azure-ipam/ipam.go) command from Cilium CNI and sends an IP request to the CNS. The CNS hen in overlay then looks through the pool and picks the first IP found that isn’t used. For dualstack we need to change two things: +1. Azure-ipam needs to be able to assign multiple IPs +2. CNS needs to be able to determine what type of IP it is returning so that we can have one IPv4 and one IPv6. ## Proposal For this issue two solutions are being proposed. -The first is having azure-ipam send two separate requests for both an IPv4 and an IPv6. The add command would need to be able to recognize that the pod is dualstack, create two [`IPConfigRequest`](https://github.com/Azure/azure-container-networking/blob/master/cns/NetworkContainerContract.go)s with an added type field to specify either v4 or v6 and call [`RequestIPAddress`](https://github.com/Azure/azure-container-networking/blob/master/cns/client/client.go) twice (once for each ip type). These two IPs would then both be returned in separate `IPConfigResponse` objects to the azure-ipam and added to the array of IPs in `cniResult` which then get returned to the Cilium CNI. - -The second solution is to send one request from azure-ipam and have it specify in its `IPConfigRequest` that it wants a dualstack pod. With solution instead of calling `RequestIPAddress` twice to request two IPs it would only call the CNS once and would return both IPs requested in an array of `PodIpInfo`s in the `IPConfigResponse`. This would most likely require a much more extensive code change as requesting an IP would now return an array of IPs instead of just the one IP. The change would have to make the default situation expect that only one IP should be returned in the array unless we know that the pod is dualstack (or possible future reasons for returning multiple IPs). +The first is having azure-ipam send two separate requests for both an IPv4 and an IPv6. The add command would need to be able to recognize that the pod is dualstack, create two [`IPConfigRequest`](https://github.com/Azure/azure-container-networking/blob/master/cns/NetworkContainerContract.go)s with an added type field to specify either v4 or v6. These two IPs would then both be added to the array of IPs in `cniResult` and return them to the Cilium CNI. + +``` +type IPConfigRequest struct { + + DesiredIPAddress string + PodInterfaceID string + InfraContainerID string + OrchestratorContext json.RawMessage + Ifname string // Used by delegated IPAM + IPType string // designating 'IPv4' or 'IPv6'. If not filled than we will return first IP of any type +} +``` + +The second solution is to send one request from azure-ipam and have it specify in its `IPConfigRequest` that it wants a dualstack pod. With solution instead of calling CNS twice to request two IPs it would only call the CNS once and would return both IPs requested in an array. This would most likely require a much more extensive code change as requesting an IP would now return an array of IPs instead of just the one IP. + +``` +type IPConfigRequest struct { + + DesiredIPAddress string + PodInterfaceID string + InfraContainerID string + OrchestratorContext json.RawMessage + Ifname string // Used by delegated IPAM + PodType string // would designate that a pod is 'dualstack' and search for two IPs instead of just one +} +``` + +``` +type IPConfigResponse struct { + PodIpInfo []PodIpInfo + Response Response +} +``` For both of these proposals there would also need to be some changes to how IPs are assigned in the CNS. When we go through the pool to find an available IP in [`AssignAnyAvailableIPConfig`](https://github.com/Azure/azure-container-networking/blob/master/cns/restserver/ipam.go) a type check would need to be added to see if it is either v4 or v6 and the type that we want to check for can be passed into the function.This type check From 3463797ee0743a170a2252fa4ef91a4cb07d8a4c Mon Sep 17 00:00:00 2001 From: rjdenney Date: Tue, 25 Oct 2022 13:00:02 -0400 Subject: [PATCH 04/17] formatting --- docs/feature/dualstack/Cilium-dualstack.md | 28 +++++++++++----------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/docs/feature/dualstack/Cilium-dualstack.md b/docs/feature/dualstack/Cilium-dualstack.md index 3d3b099f63..6b5a7b76b1 100644 --- a/docs/feature/dualstack/Cilium-dualstack.md +++ b/docs/feature/dualstack/Cilium-dualstack.md @@ -19,12 +19,12 @@ The first is having azure-ipam send two separate requests for both an IPv4 and a ``` type IPConfigRequest struct { - DesiredIPAddress string - PodInterfaceID string - InfraContainerID string - OrchestratorContext json.RawMessage - Ifname string // Used by delegated IPAM - IPType string // designating 'IPv4' or 'IPv6'. If not filled than we will return first IP of any type + DesiredIPAddress string + PodInterfaceID string + InfraContainerID string + OrchestratorContext json.RawMessage + Ifname string // Used by delegated IPAM + IPType string // designating 'IPv4' or 'IPv6'. If not filled than we will return first IP of any type } ``` @@ -33,19 +33,19 @@ The second solution is to send one request from azure-ipam and have it specify i ``` type IPConfigRequest struct { - DesiredIPAddress string - PodInterfaceID string - InfraContainerID string - OrchestratorContext json.RawMessage - Ifname string // Used by delegated IPAM - PodType string // would designate that a pod is 'dualstack' and search for two IPs instead of just one + DesiredIPAddress string + PodInterfaceID string + InfraContainerID string + OrchestratorContext json.RawMessage + Ifname string // Used by delegated IPAM + PodType string // would designate that a pod is 'dualstack' and search for two IPs instead of just one } ``` ``` type IPConfigResponse struct { - PodIpInfo []PodIpInfo - Response Response + PodIpInfo []PodIpInfo + Response Response } ``` From 5860b40858d106326cfad6f8432aab2788d7d94e Mon Sep 17 00:00:00 2001 From: rjdenney Date: Tue, 25 Oct 2022 13:02:44 -0400 Subject: [PATCH 05/17] highlight --- docs/feature/dualstack/Cilium-dualstack.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/feature/dualstack/Cilium-dualstack.md b/docs/feature/dualstack/Cilium-dualstack.md index 6b5a7b76b1..6b68613d69 100644 --- a/docs/feature/dualstack/Cilium-dualstack.md +++ b/docs/feature/dualstack/Cilium-dualstack.md @@ -24,7 +24,7 @@ type IPConfigRequest struct { InfraContainerID string OrchestratorContext json.RawMessage Ifname string // Used by delegated IPAM - IPType string // designating 'IPv4' or 'IPv6'. If not filled than we will return first IP of any type + IPType string // designating 'IPv4' or 'IPv6'. If not filled than we will return first IP of any type } ``` @@ -38,13 +38,13 @@ type IPConfigRequest struct { InfraContainerID string OrchestratorContext json.RawMessage Ifname string // Used by delegated IPAM - PodType string // would designate that a pod is 'dualstack' and search for two IPs instead of just one + PodType string // would designate that a pod is 'dualstack' and search for two IPs instead of just one } ``` ``` type IPConfigResponse struct { - PodIpInfo []PodIpInfo + PodIpInfo []PodIpInfo Response Response } ``` From fab116a1635138d7a271d8bb381f48a9079f22e8 Mon Sep 17 00:00:00 2001 From: rjdenney Date: Tue, 25 Oct 2022 13:14:56 -0400 Subject: [PATCH 06/17] adding diff formatting to code block --- docs/feature/dualstack/Cilium-dualstack.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/feature/dualstack/Cilium-dualstack.md b/docs/feature/dualstack/Cilium-dualstack.md index 6b68613d69..a33ebb3186 100644 --- a/docs/feature/dualstack/Cilium-dualstack.md +++ b/docs/feature/dualstack/Cilium-dualstack.md @@ -24,7 +24,7 @@ type IPConfigRequest struct { InfraContainerID string OrchestratorContext json.RawMessage Ifname string // Used by delegated IPAM - IPType string // designating 'IPv4' or 'IPv6'. If not filled than we will return first IP of any type + +IPType string // designating 'IPv4' or 'IPv6'. If not filled than we will return first IP of any type } ``` @@ -38,13 +38,13 @@ type IPConfigRequest struct { InfraContainerID string OrchestratorContext json.RawMessage Ifname string // Used by delegated IPAM - PodType string // would designate that a pod is 'dualstack' and search for two IPs instead of just one + +PodType string // would designate that a pod is 'dualstack' and search for two IPs instead of just one } ``` ``` type IPConfigResponse struct { - PodIpInfo []PodIpInfo + +PodIpInfo []PodIpInfo Response Response } ``` From bc819a72da289bd92d9a2f6169c9940d967d57d8 Mon Sep 17 00:00:00 2001 From: rjdenney Date: Tue, 25 Oct 2022 13:16:25 -0400 Subject: [PATCH 07/17] diff --- docs/feature/dualstack/Cilium-dualstack.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/feature/dualstack/Cilium-dualstack.md b/docs/feature/dualstack/Cilium-dualstack.md index a33ebb3186..c6168b1a83 100644 --- a/docs/feature/dualstack/Cilium-dualstack.md +++ b/docs/feature/dualstack/Cilium-dualstack.md @@ -16,7 +16,7 @@ For this issue two solutions are being proposed. The first is having azure-ipam send two separate requests for both an IPv4 and an IPv6. The add command would need to be able to recognize that the pod is dualstack, create two [`IPConfigRequest`](https://github.com/Azure/azure-container-networking/blob/master/cns/NetworkContainerContract.go)s with an added type field to specify either v4 or v6. These two IPs would then both be added to the array of IPs in `cniResult` and return them to the Cilium CNI. -``` +```diff type IPConfigRequest struct { DesiredIPAddress string @@ -30,7 +30,7 @@ type IPConfigRequest struct { The second solution is to send one request from azure-ipam and have it specify in its `IPConfigRequest` that it wants a dualstack pod. With solution instead of calling CNS twice to request two IPs it would only call the CNS once and would return both IPs requested in an array. This would most likely require a much more extensive code change as requesting an IP would now return an array of IPs instead of just the one IP. -``` +```diff type IPConfigRequest struct { DesiredIPAddress string @@ -42,7 +42,7 @@ type IPConfigRequest struct { } ``` -``` +```diff type IPConfigResponse struct { +PodIpInfo []PodIpInfo Response Response From d3327aaa2dcb957785730104cb991c7ae6dda98e Mon Sep 17 00:00:00 2001 From: rjdenney Date: Tue, 25 Oct 2022 13:19:33 -0400 Subject: [PATCH 08/17] formatting Please enter the commit message for your changes. Lines starting --- docs/feature/dualstack/Cilium-dualstack.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/feature/dualstack/Cilium-dualstack.md b/docs/feature/dualstack/Cilium-dualstack.md index c6168b1a83..b3da355f6d 100644 --- a/docs/feature/dualstack/Cilium-dualstack.md +++ b/docs/feature/dualstack/Cilium-dualstack.md @@ -24,7 +24,7 @@ type IPConfigRequest struct { InfraContainerID string OrchestratorContext json.RawMessage Ifname string // Used by delegated IPAM - +IPType string // designating 'IPv4' or 'IPv6'. If not filled than we will return first IP of any type ++ IPType string // designating 'IPv4' or 'IPv6'. If not filled than we will return first IP of any type } ``` @@ -38,13 +38,13 @@ type IPConfigRequest struct { InfraContainerID string OrchestratorContext json.RawMessage Ifname string // Used by delegated IPAM - +PodType string // would designate that a pod is 'dualstack' and search for two IPs instead of just one ++ PodType string // would designate that a pod is 'dualstack' and search for two IPs instead of just one } ``` ```diff type IPConfigResponse struct { - +PodIpInfo []PodIpInfo ++ PodIpInfo []PodIpInfo Response Response } ``` From ae83a16f36f7e546a00d8d14b98b218ab34dd872 Mon Sep 17 00:00:00 2001 From: rjdenney Date: Tue, 25 Oct 2022 13:21:04 -0400 Subject: [PATCH 09/17] added removed line to IPConfigResponse --- docs/feature/dualstack/Cilium-dualstack.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/feature/dualstack/Cilium-dualstack.md b/docs/feature/dualstack/Cilium-dualstack.md index b3da355f6d..e26695d209 100644 --- a/docs/feature/dualstack/Cilium-dualstack.md +++ b/docs/feature/dualstack/Cilium-dualstack.md @@ -44,6 +44,7 @@ type IPConfigRequest struct { ```diff type IPConfigResponse struct { +- PodIpInfo PodIpInfo + PodIpInfo []PodIpInfo Response Response } From 9d81582eb113c538952767ddd33b9c43d6be78f0 Mon Sep 17 00:00:00 2001 From: rjdenney Date: Tue, 25 Oct 2022 13:28:06 -0400 Subject: [PATCH 10/17] removing new lines to structs --- docs/feature/dualstack/Cilium-dualstack.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/docs/feature/dualstack/Cilium-dualstack.md b/docs/feature/dualstack/Cilium-dualstack.md index e26695d209..113aa5a1fd 100644 --- a/docs/feature/dualstack/Cilium-dualstack.md +++ b/docs/feature/dualstack/Cilium-dualstack.md @@ -18,7 +18,6 @@ The first is having azure-ipam send two separate requests for both an IPv4 and a ```diff type IPConfigRequest struct { - DesiredIPAddress string PodInterfaceID string InfraContainerID string @@ -32,7 +31,6 @@ The second solution is to send one request from azure-ipam and have it specify i ```diff type IPConfigRequest struct { - DesiredIPAddress string PodInterfaceID string InfraContainerID string From 12735214e37885d25390ad3c940e500b43fbc982 Mon Sep 17 00:00:00 2001 From: rjdenney Date: Tue, 25 Oct 2022 16:22:35 -0400 Subject: [PATCH 11/17] adding background --- docs/feature/dualstack/Cilium-dualstack.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/docs/feature/dualstack/Cilium-dualstack.md b/docs/feature/dualstack/Cilium-dualstack.md index 113aa5a1fd..a729a890d0 100644 --- a/docs/feature/dualstack/Cilium-dualstack.md +++ b/docs/feature/dualstack/Cilium-dualstack.md @@ -6,15 +6,19 @@ This document proposes changes to the azure-ipam contract so that we can work wi ## Overview -With the current CNS solution for dualstack we are setting the nnc to pass multiple NCs with CIDR to be unrolled and added to a pool that contains both v6 and v4 ips. Since we are planning to use Cilium CNI for the linux dualstack solution we need to make changes to azure-ipam to allow for a dualstack cluster to get multiple IPs. Currently azure-Ipam handles the [`CmdAdd`](https://github.com/Azure/azure-container-networking/blob/master/azure-ipam/ipam.go) command from Cilium CNI and sends an IP request to the CNS. The CNS hen in overlay then looks through the pool and picks the first IP found that isn’t used. For dualstack we need to change two things: +With the current CNS solution for dualstack we are setting the nnc to pass multiple NCs with CIDR to be unrolled and added to a pool that contains both v6 and v4 ips. Since we are planning to use Cilium CNI for the linux dualstack solution we need to make changes to azure-ipam to allow for a dualstack cluster to get multiple IPs. Currently azure-Ipam handles the [`CmdAdd`](https://github.com/Azure/azure-container-networking/blob/master/azure-ipam/ipam.go#:~:text=func%20(p%20*IPAMPlugin)%20CmdAdd(args%20*cniSkel.CmdArgs)%20error%20%7B) command from Cilium CNI and sends an IP request to the CNS. The CNS then in overlay then looks through the pool and picks the first IP found that isn’t used. For dualstack we need to change two things: 1. Azure-ipam needs to be able to assign multiple IPs 2. CNS needs to be able to determine what type of IP it is returning so that we can have one IPv4 and one IPv6. +## Current Implementation + +When we use Cilium CNI with Azure CNS we use an Azure-ipam plugin to communicate from the Cilium CNI and CNS. This plugin takes in add commands from the Cilium CNI and reads the CNI network config from stdin. The data from stdin then gets parsed into an [`IPConfigRequest`](https://github.com/Azure/azure-container-networking/blob/master/cns/NetworkContainerContract.go) object and is passed to [`RequestIPAddress`](https://github.com/Azure/azure-container-networking/blob/master/cns/client/client.go). From here we call the [`requestIPConfigHandler`](https://github.com/Azure/azure-container-networking/blob/master/cns/restserver/ipam.go#:~:text=func%20(service%20*HTTPRestService)%20requestIPConfigHandler(w%20http.ResponseWriter%2C%20r%20*http.Request)%20%7B) in ipam using which then retrieves and validates the `IPConfigRequest` and then calls the [`requestIPConfigHelper`](https://github.com/Azure/azure-container-networking/blob/master/cns/restserver/ipam.go#:~:text=func%20requestIPConfigHelper(service%20*HTTPRestService%2C%20req%20cns.IPConfigRequest)%20(cns.PodIpInfo%2C%20error)%20%7B). The helper function first checks to see if the `IPConfigRequest` already belongs to a pod and returns the existing pod's information. If it doesn't already exist than it checks if there is a desiredIPaddress or not. If there is a desired IP address than we search for that specific IP and if not it calls +[`AssignAnyAvailableIPConfig`](https://github.com/Azure/azure-container-networking/blob/master/cns/restserver/ipam.go#:~:text=func%20(service%20*HTTPRestService)%20AssignAnyAvailableIPConfig(podInfo%20cns.PodInfo)%20(cns.PodIpInfo%2C%20error)%20%7B) ## Proposal For this issue two solutions are being proposed. -The first is having azure-ipam send two separate requests for both an IPv4 and an IPv6. The add command would need to be able to recognize that the pod is dualstack, create two [`IPConfigRequest`](https://github.com/Azure/azure-container-networking/blob/master/cns/NetworkContainerContract.go)s with an added type field to specify either v4 or v6. These two IPs would then both be added to the array of IPs in `cniResult` and return them to the Cilium CNI. +The first is having azure-ipam send two separate requests for both an IPv4 and an IPv6. The add command would need to be able to recognize that the pod is dualstack, create two `IPConfigRequest`s with an added type field to specify either v4 or v6 and call `RequestIPAddress` twice to get both IPs. These two IPs would then both be added to the array of IPs in `cniResult` and return them to the Cilium CNI. ```diff type IPConfigRequest struct { From 15f9d5c215393ad424f055d71f5e0185b3d6154f Mon Sep 17 00:00:00 2001 From: rjdenney Date: Thu, 5 Jan 2023 20:02:27 -0500 Subject: [PATCH 12/17] adding updates for added flag to NNC to determine IPMode --- docs/feature/dualstack/Cilium-dualstack.md | 48 +++++++++++++--------- 1 file changed, 28 insertions(+), 20 deletions(-) diff --git a/docs/feature/dualstack/Cilium-dualstack.md b/docs/feature/dualstack/Cilium-dualstack.md index a729a890d0..53a6605c63 100644 --- a/docs/feature/dualstack/Cilium-dualstack.md +++ b/docs/feature/dualstack/Cilium-dualstack.md @@ -1,12 +1,12 @@ -# Cilium Azure-ipam Dualstack Overlay Proposal +# Cilium Azure-ipam Dual Stack Overlay Proposal ## Purpose -This document proposes changes to the azure-ipam contract so that we can work with dualstack clusters between Cilium and Azure CNS. +This document proposes changes to the azure-ipam contract so that we can work with dual stack clusters between Cilium and Azure CNS. ## Overview -With the current CNS solution for dualstack we are setting the nnc to pass multiple NCs with CIDR to be unrolled and added to a pool that contains both v6 and v4 ips. Since we are planning to use Cilium CNI for the linux dualstack solution we need to make changes to azure-ipam to allow for a dualstack cluster to get multiple IPs. Currently azure-Ipam handles the [`CmdAdd`](https://github.com/Azure/azure-container-networking/blob/master/azure-ipam/ipam.go#:~:text=func%20(p%20*IPAMPlugin)%20CmdAdd(args%20*cniSkel.CmdArgs)%20error%20%7B) command from Cilium CNI and sends an IP request to the CNS. The CNS then in overlay then looks through the pool and picks the first IP found that isn’t used. For dualstack we need to change two things: +With the current CNS solution for dual stack we are setting the nnc to pass multiple NCs with CIDR to be unrolled and added to a pool that contains both v6 and v4 ips. Since we are planning to use Cilium CNI for the linux dual stack solution we need to make changes to azure-ipam to allow for a dual stack cluster to get multiple IPs. Currently azure-Ipam handles the [`CmdAdd`](https://github.com/Azure/azure-container-networking/blob/master/azure-ipam/ipam.go#:~:text=func%20(p%20*IPAMPlugin)%20CmdAdd(args%20*cniSkel.CmdArgs)%20error%20%7B) command from Cilium CNI and sends an IP request to the CNS. The CNS then in overlay then looks through the pool and picks the first IP found that isn’t used. For dual stack we need to change two things: 1. Azure-ipam needs to be able to assign multiple IPs 2. CNS needs to be able to determine what type of IP it is returning so that we can have one IPv4 and one IPv6. @@ -14,33 +14,45 @@ With the current CNS solution for dualstack we are setting the nnc to pass multi When we use Cilium CNI with Azure CNS we use an Azure-ipam plugin to communicate from the Cilium CNI and CNS. This plugin takes in add commands from the Cilium CNI and reads the CNI network config from stdin. The data from stdin then gets parsed into an [`IPConfigRequest`](https://github.com/Azure/azure-container-networking/blob/master/cns/NetworkContainerContract.go) object and is passed to [`RequestIPAddress`](https://github.com/Azure/azure-container-networking/blob/master/cns/client/client.go). From here we call the [`requestIPConfigHandler`](https://github.com/Azure/azure-container-networking/blob/master/cns/restserver/ipam.go#:~:text=func%20(service%20*HTTPRestService)%20requestIPConfigHandler(w%20http.ResponseWriter%2C%20r%20*http.Request)%20%7B) in ipam using which then retrieves and validates the `IPConfigRequest` and then calls the [`requestIPConfigHelper`](https://github.com/Azure/azure-container-networking/blob/master/cns/restserver/ipam.go#:~:text=func%20requestIPConfigHelper(service%20*HTTPRestService%2C%20req%20cns.IPConfigRequest)%20(cns.PodIpInfo%2C%20error)%20%7B). The helper function first checks to see if the `IPConfigRequest` already belongs to a pod and returns the existing pod's information. If it doesn't already exist than it checks if there is a desiredIPaddress or not. If there is a desired IP address than we search for that specific IP and if not it calls [`AssignAnyAvailableIPConfig`](https://github.com/Azure/azure-container-networking/blob/master/cns/restserver/ipam.go#:~:text=func%20(service%20*HTTPRestService)%20AssignAnyAvailableIPConfig(podInfo%20cns.PodInfo)%20(cns.PodIpInfo%2C%20error)%20%7B) -## Proposal +## New Implementation -For this issue two solutions are being proposed. +### NNC change -The first is having azure-ipam send two separate requests for both an IPv4 and an IPv6. The add command would need to be able to recognize that the pod is dualstack, create two `IPConfigRequest`s with an added type field to specify either v4 or v6 and call `RequestIPAddress` twice to get both IPs. These two IPs would then both be added to the array of IPs in `cniResult` and return them to the Cilium CNI. +For the CNS to know that we are in dual stack we will use a flag that will be passed in using the NNC. This will be filled in from the DNC-RC side and then sent to the CNS over the CRD in a flag called `IPMode`. The IPMode flag will be of type IPMode and will have two options "SingleStack" which when passed in will have the CNS work as it currently does and "Dual Stack" which tell the CNS to look for two IPs instead of one. ```diff -type IPConfigRequest struct { - DesiredIPAddress string - PodInterfaceID string - InfraContainerID string - OrchestratorContext json.RawMessage - Ifname string // Used by delegated IPAM -+ IPType string // designating 'IPv4' or 'IPv6'. If not filled than we will return first IP of any type +// NodeNetworkConfigStatus defines the observed state of NetworkConfig +type NodeNetworkConfigStatus struct { + AssignedIPCount int `json:"assignedIPCount,omitempty"` + Scaler Scaler `json:"scaler,omitempty"` + Status Status `json:"status,omitempty"` ++ IPMode IPMode `json:"IPmode,omitempty"` + NetworkContainers []NetworkContainer `json:"networkContainers,omitempty"` } ``` -The second solution is to send one request from azure-ipam and have it specify in its `IPConfigRequest` that it wants a dualstack pod. With solution instead of calling CNS twice to request two IPs it would only call the CNS once and would return both IPs requested in an array. This would most likely require a much more extensive code change as requesting an IP would now return an array of IPs instead of just the one IP. +``` +type IPMode string -```diff +const ( + SingleStack Status = "SingleStack" + DualStack Status = "DualStack" +) +``` + +In the CNS will flag then be used in the [`AssignAnyAvailableIPConfig`](https://github.com/Azure/azure-container-networking/blob/master/cns/restserver/ipam.go#:~:text=func%20(service%20*HTTPRestService)%20AssignAnyAvailableIPConfig(podInfo%20cns.PodInfo)%20(cns.PodIpInfo%2C%20error)%20%7B) function to determine whether or not are creating a pod in single stack or dual stack. + +### Contract IP change + +We will continue to send only one `IPConfigRequest` when we are running in dual stack but when in dual stack we will expect the repsonse to include a slice of `PodIpInfo`. When running the [`AssignAnyAvailableIPConfig`](https://github.com/Azure/azure-container-networking/blob/master/cns/restserver/ipam.go#:~:text=func%20(service%20*HTTPRestService)%20AssignAnyAvailableIPConfig(podInfo%20cns.PodInfo)%20(cns.PodIpInfo%2C%20error)%20%7B) function with the new flag for dual stack if we are running in dual stack we will have the function pick the first availabe IPv6 and first available IPv4 IP that it can find. These IPs will now be returned as a slice from `AssignAnyAvailableIPConfig` and used to populate the `IPConfigResponse` struct and returned back to the azure-ipam. The azure-ipam will then cycle the length of the slice to populate the message the is sent to the Cilium-CNI. + +``` type IPConfigRequest struct { DesiredIPAddress string PodInterfaceID string InfraContainerID string OrchestratorContext json.RawMessage Ifname string // Used by delegated IPAM -+ PodType string // would designate that a pod is 'dualstack' and search for two IPs instead of just one } ``` @@ -52,8 +64,4 @@ type IPConfigResponse struct { } ``` -For both of these proposals there would also need to be some changes to how IPs are assigned in the CNS. When we go through the pool to find an available IP in [`AssignAnyAvailableIPConfig`](https://github.com/Azure/azure-container-networking/blob/master/cns/restserver/ipam.go) a type check would need to be added to see if it is either v4 or v6 and the type that we want to check for can be passed into the function.This type check - -## Further Questions -For any errors what should we do if we can obtain one ip but not the other? \ No newline at end of file From 94338ae5ed40ad46ab0e275ebf82dcfb67a486fd Mon Sep 17 00:00:00 2001 From: rjdenney Date: Fri, 6 Jan 2023 10:29:18 -0500 Subject: [PATCH 13/17] Changed IPMode example to a diff to show it's being added --- docs/feature/dualstack/Cilium-dualstack.md | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/docs/feature/dualstack/Cilium-dualstack.md b/docs/feature/dualstack/Cilium-dualstack.md index 53a6605c63..a45f57be62 100644 --- a/docs/feature/dualstack/Cilium-dualstack.md +++ b/docs/feature/dualstack/Cilium-dualstack.md @@ -31,13 +31,13 @@ type NodeNetworkConfigStatus struct { } ``` -``` -type IPMode string - -const ( - SingleStack Status = "SingleStack" - DualStack Status = "DualStack" -) +```diff ++type IPMode string ++ ++const ( ++ SingleStack Status = "SingleStack" ++ DualStack Status = "DualStack" ++) ``` In the CNS will flag then be used in the [`AssignAnyAvailableIPConfig`](https://github.com/Azure/azure-container-networking/blob/master/cns/restserver/ipam.go#:~:text=func%20(service%20*HTTPRestService)%20AssignAnyAvailableIPConfig(podInfo%20cns.PodInfo)%20(cns.PodIpInfo%2C%20error)%20%7B) function to determine whether or not are creating a pod in single stack or dual stack. From 0064601b2d81d9f62c22b3fa3a305350b58bb5bf Mon Sep 17 00:00:00 2001 From: rjdenney Date: Thu, 19 Jan 2023 14:00:42 -0500 Subject: [PATCH 14/17] changing to reflect a new API to be added in CNS to get multiple IPs --- docs/feature/dualstack/Cilium-dualstack.md | 81 ++++++++++++---------- 1 file changed, 44 insertions(+), 37 deletions(-) diff --git a/docs/feature/dualstack/Cilium-dualstack.md b/docs/feature/dualstack/Cilium-dualstack.md index a45f57be62..0c923e9d3b 100644 --- a/docs/feature/dualstack/Cilium-dualstack.md +++ b/docs/feature/dualstack/Cilium-dualstack.md @@ -1,67 +1,74 @@ -# Cilium Azure-ipam Dual Stack Overlay Proposal +# Cilium Azure-ipam dualstack Overlay Proposal ## Purpose -This document proposes changes to the azure-ipam contract so that we can work with dual stack clusters between Cilium and Azure CNS. +This document proposes changes to the azure-ipam contract and implement a new API so that we can work with dualstack clusters between Cilium and Azure CNS. ## Overview -With the current CNS solution for dual stack we are setting the nnc to pass multiple NCs with CIDR to be unrolled and added to a pool that contains both v6 and v4 ips. Since we are planning to use Cilium CNI for the linux dual stack solution we need to make changes to azure-ipam to allow for a dual stack cluster to get multiple IPs. Currently azure-Ipam handles the [`CmdAdd`](https://github.com/Azure/azure-container-networking/blob/master/azure-ipam/ipam.go#:~:text=func%20(p%20*IPAMPlugin)%20CmdAdd(args%20*cniSkel.CmdArgs)%20error%20%7B) command from Cilium CNI and sends an IP request to the CNS. The CNS then in overlay then looks through the pool and picks the first IP found that isn’t used. For dual stack we need to change two things: -1. Azure-ipam needs to be able to assign multiple IPs -2. CNS needs to be able to determine what type of IP it is returning so that we can have one IPv4 and one IPv6. +With the current CNS solution for dualstack we are setting the NNC to pass multiple NCs with CIDR to be unrolled and added to a pool that contains both v6 and v4 IPs. Since we are planning to use Cilium CNI for the Linux dualstack solution we need to make add and additional CNS API to allow for a dualstack cluster to get multiple IPs. Currently azure-ipam handles the [`CmdAdd`](https://github.com/Azure/azure-container-networking/blob/master/azure-ipam/ipam.go#:~:text=func%20(p%20*IPAMPlugin)%20CmdAdd(args%20*cniSkel.CmdArgs)%20error%20%7B) command from Cilium CNI and sends an IP request to CNS. In Overlay, CNS then looks through the pool and picks the first IP found that isn’t used. For dualstack we need to change three things: +1. A new contract type to allow for a slice of IPs to be returned +2. A new API that acts similar to the current API but expects uses new contract as a return type +3. Changes need to be made in CNS ipam so that we return an IP for each NC ## Current Implementation -When we use Cilium CNI with Azure CNS we use an Azure-ipam plugin to communicate from the Cilium CNI and CNS. This plugin takes in add commands from the Cilium CNI and reads the CNI network config from stdin. The data from stdin then gets parsed into an [`IPConfigRequest`](https://github.com/Azure/azure-container-networking/blob/master/cns/NetworkContainerContract.go) object and is passed to [`RequestIPAddress`](https://github.com/Azure/azure-container-networking/blob/master/cns/client/client.go). From here we call the [`requestIPConfigHandler`](https://github.com/Azure/azure-container-networking/blob/master/cns/restserver/ipam.go#:~:text=func%20(service%20*HTTPRestService)%20requestIPConfigHandler(w%20http.ResponseWriter%2C%20r%20*http.Request)%20%7B) in ipam using which then retrieves and validates the `IPConfigRequest` and then calls the [`requestIPConfigHelper`](https://github.com/Azure/azure-container-networking/blob/master/cns/restserver/ipam.go#:~:text=func%20requestIPConfigHelper(service%20*HTTPRestService%2C%20req%20cns.IPConfigRequest)%20(cns.PodIpInfo%2C%20error)%20%7B). The helper function first checks to see if the `IPConfigRequest` already belongs to a pod and returns the existing pod's information. If it doesn't already exist than it checks if there is a desiredIPaddress or not. If there is a desired IP address than we search for that specific IP and if not it calls -[`AssignAnyAvailableIPConfig`](https://github.com/Azure/azure-container-networking/blob/master/cns/restserver/ipam.go#:~:text=func%20(service%20*HTTPRestService)%20AssignAnyAvailableIPConfig(podInfo%20cns.PodInfo)%20(cns.PodIpInfo%2C%20error)%20%7B) +To communicate to Cilium CNI from Azure CNS we use the azure-ipam plugin. This plugin takes in add commands from the Cilium CNI and reads the CNI network config from stdin. The data from stdin then gets parsed into an [`IPConfigRequest`](https://github.com/Azure/azure-container-networking/blob/master/cns/NetworkContainerContract.go) object and is passed to [`RequestIPAddress`](https://github.com/Azure/azure-container-networking/blob/master/cns/client/client.go). From here we call the [`requestIPConfigHandler`](https://github.com/Azure/azure-container-networking/blob/master/cns/restserver/ipam.go#:~:text=func%20(service%20*HTTPRestService)%20requestIPConfigHandler(w%20http.ResponseWriter%2C%20r%20*http.Request)%20%7B) in ipam which then retrieves and validates the [`IPConfigRequest`](https://github.com/Azure/azure-container-networking/blob/master/cns/NetworkContainerContract.go) and then calls the [`requestIPConfigHelper`](https://github.com/Azure/azure-container-networking/blob/master/cns/restserver/ipam.go#:~:text=func%20requestIPConfigHelper(service%20*HTTPRestService%2C%20req%20cns.IPConfigRequest)%20(cns.PodIpInfo%2C%20error)%20%7B). The helper function first checks to see if the `IPConfigRequest` already belongs to a pod and returns the existing pod's information. If it doesn't already exist than it checks if there is a desired IP address or not. If there is a desired IP address than we search for that specific IP and if not we call +[`AssignAnyAvailableIPConfig`](https://github.com/Azure/azure-container-networking/blob/master/cns/restserver/ipam.go#:~:text=func%20(service%20*HTTPRestService)%20AssignAnyAvailableIPConfig(podInfo%20cns.PodInfo)%20(cns.PodIpInfo%2C%20error)%20%7B). This function then searches for the first available IP and returns it back up to the handler. The handler then packages the IP into a [`IPConfigResponse`](https://github.com/Azure/azure-container-networking/blob/bd299fe7271a7a23b3d0268d8e14ad812181e076/cns/NetworkContainerContract.go#L419) which gets returned to the client. ## New Implementation -### NNC change +### New Contract Type -For the CNS to know that we are in dual stack we will use a flag that will be passed in using the NNC. This will be filled in from the DNC-RC side and then sent to the CNS over the CRD in a flag called `IPMode`. The IPMode flag will be of type IPMode and will have two options "SingleStack" which when passed in will have the CNS work as it currently does and "Dual Stack" which tell the CNS to look for two IPs instead of one. +To preserve the current contract type but also to allow for the new functionality we will add a new type, `IPConfigsResponse` to the contract. This new type will be similar to the prexisting [`IPConfigResponse`](https://github.com/Azure/azure-container-networking/blob/bd299fe7271a7a23b3d0268d8e14ad812181e076/cns/NetworkContainerContract.go#L419) but replacing the single `PodIpInfo` with a slice. This will allow for a single call to the API to return one IP per NC. ```diff -// NodeNetworkConfigStatus defines the observed state of NetworkConfig -type NodeNetworkConfigStatus struct { - AssignedIPCount int `json:"assignedIPCount,omitempty"` - Scaler Scaler `json:"scaler,omitempty"` - Status Status `json:"status,omitempty"` -+ IPMode IPMode `json:"IPmode,omitempty"` - NetworkContainers []NetworkContainer `json:"networkContainers,omitempty"` +// IPConfigResponse is used in CNS IPAM mode as a response to CNI ADD +type IPConfigResponse struct { + PodIpInfo PodIpInfo + Response Response } + ++// IPConfigsResponse is used in CNS IPAM mode to return a slice of IP configs as a response to CNI ADD ++type IPConfigsResponse struct { ++ PodIpInfo []PodIpInfo ++ Response Response ++} ``` +### New API + +A new API will called `RequestAllIPAddresses` be created for requesting IPs when running in Overlay. This will work similar to the current [`RequestIPAddress`](https://github.com/Azure/azure-container-networking/blob/master/cns/client/client.go) and will accept the same [`IPConfigRequest`](https://github.com/Azure/azure-container-networking/blob/master/cns/NetworkContainerContract.go), the main difference will be that it will return the new contract type `IPConfigsResponse` with the slice of `PodIpInfo`. + ```diff -+type IPMode string -+ -+const ( -+ SingleStack Status = "SingleStack" -+ DualStack Status = "DualStack" -+) ++// RequestAllIPAddresses calls the RequestAllIPConfigs in CNS ++func (c *Client) RequestAllIPAddresses(ctx context.Context, ipconfig cns.IPConfigRequest) (*cns.IPConfigsResponse, error) +} ``` -In the CNS will flag then be used in the [`AssignAnyAvailableIPConfig`](https://github.com/Azure/azure-container-networking/blob/master/cns/restserver/ipam.go#:~:text=func%20(service%20*HTTPRestService)%20AssignAnyAvailableIPConfig(podInfo%20cns.PodInfo)%20(cns.PodIpInfo%2C%20error)%20%7B) function to determine whether or not are creating a pod in single stack or dual stack. +If for some reason this function returns a 404 error we will then try again but use the original [`RequestIPAddress`](https://github.com/Azure/azure-container-networking/blob/master/cns/client/client.go) API instead. The swift case will not use the new API and will continue to use the current one. -### Contract IP change +### IPAM Changes -We will continue to send only one `IPConfigRequest` when we are running in dual stack but when in dual stack we will expect the repsonse to include a slice of `PodIpInfo`. When running the [`AssignAnyAvailableIPConfig`](https://github.com/Azure/azure-container-networking/blob/master/cns/restserver/ipam.go#:~:text=func%20(service%20*HTTPRestService)%20AssignAnyAvailableIPConfig(podInfo%20cns.PodInfo)%20(cns.PodIpInfo%2C%20error)%20%7B) function with the new flag for dual stack if we are running in dual stack we will have the function pick the first availabe IPv6 and first available IPv4 IP that it can find. These IPs will now be returned as a slice from `AssignAnyAvailableIPConfig` and used to populate the `IPConfigResponse` struct and returned back to the azure-ipam. The azure-ipam will then cycle the length of the slice to populate the message the is sent to the Cilium-CNI. +In IPAM we will need to have a new handler that is called by the new API called `RequestAllIpConfigsHandler`. This new handler will act similarly as the current [`requestIPConfigHandler`](https://github.com/Azure/azure-container-networking/blob/master/cns/restserver/ipam.go#:~:text=func%20(service%20*HTTPRestService)%20requestIPConfigHandler(w%20http.ResponseWriter%2C%20r%20*http.Request)%20%7B) handler with the exception of using the new contract `IPConfigsResponse`. This new function will also call [`requestIPConfigHelper`](https://github.com/Azure/azure-container-networking/blob/master/cns/restserver/ipam.go#:~:text=func%20requestIPConfigHelper(service%20*HTTPRestService%2C%20req%20cns.IPConfigRequest)%20(cns.PodIpInfo%2C%20error)%20%7B) to retrieve an IP however the return type will be changed from a single `PodIpInfo` to a slice so that we can get all IPs needed with one call. [`requestIPConfigHelper`](https://github.com/Azure/azure-container-networking/blob/master/cns/restserver/ipam.go#:~:text=func%20requestIPConfigHelper(service%20*HTTPRestService%2C%20req%20cns.IPConfigRequest)%20(cns.PodIpInfo%2C%20error)%20%7B) will still call [`AssignAnyAvailableIPConfig`](https://github.com/Azure/azure-container-networking/blob/master/cns/restserver/ipam.go#:~:text=func%20(service%20*HTTPRestService)%20AssignAnyAvailableIPConfig(podInfo%20cns.PodInfo)%20(cns.PodIpInfo%2C%20error)%20%7B) but will now be expecting for a slice of IPs to be returned. Within [`AssignAnyAvailableIPConfig`](https://github.com/Azure/azure-container-networking/blob/master/cns/restserver/ipam.go#:~:text=func%20(service%20*HTTPRestService)%20AssignAnyAvailableIPConfig(podInfo%20cns.PodInfo)%20(cns.PodIpInfo%2C%20error)%20%7B) instead of just getting the first available IP, it will now fill a slice with one IP from each NC provided on the NNC. This will then get returned back up to the handler function and then sent back to the client. +```diff ++// used to request an IPConfig for each NC from the CNS state ++func (service *HTTPRestService) RequestAllIpConfigsHandler(w http.ResponseWriter, r *http.Request) ``` -type IPConfigRequest struct { - DesiredIPAddress string - PodInterfaceID string - InfraContainerID string - OrchestratorContext json.RawMessage - Ifname string // Used by delegated IPAM -} + +```diff +-func requestIPConfigHelper(service *HTTPRestService, req cns.IPConfigRequest) (cns.PodIpInfo, error) ++func requestIPConfigHelper(service *HTTPRestService, req cns.IPConfigRequest) ([]cns.PodIpInfo, error) ``` ```diff -type IPConfigResponse struct { -- PodIpInfo PodIpInfo -+ PodIpInfo []PodIpInfo - Response Response -} +-func (service *HTTPRestService) AssignAnyAvailableIPConfig(podInfo cns.PodInfo) (cns.PodIpInfo, error) ++func (service *HTTPRestService) AssignAnyAvailableIPConfig(podInfo cns.PodInfo) ([]cns.PodIpInfo, error) ``` +To keep backwards compatibility the current handler, [`requestIPConfigHandler`](https://github.com/Azure/azure-container-networking/blob/master/cns/restserver/ipam.go#:~:text=func%20(service%20*HTTPRestService)%20requestIPConfigHandler(w%20http.ResponseWriter%2C%20r%20*http.Request)%20%7B) will still accept the slice from [`requestIPConfigHelper`](https://github.com/Azure/azure-container-networking/blob/master/cns/restserver/ipam.go#:~:text=func%20requestIPConfigHelper(service%20*HTTPRestService%2C%20req%20cns.IPConfigRequest)%20(cns.PodIpInfo%2C%20error)%20%7B) but will then have to retrieve the first index of the slice so that it can be used in the old version of the contract, [`IPConfigResponse`](https://github.com/Azure/azure-container-networking/blob/bd299fe7271a7a23b3d0268d8e14ad812181e076/cns/NetworkContainerContract.go#L419). + + +### Questions +When searching for one IP per NC is there some where that we save all of the NC names. If I don't know the names ahead of time I will probably need to search through all of the IPs to get all of the NC names at one point and I could save it. \ No newline at end of file From 7726eeb0065fc5b5866b3d68a6d4d42ba33051d7 Mon Sep 17 00:00:00 2001 From: rjdenney Date: Thu, 19 Jan 2023 14:11:41 -0500 Subject: [PATCH 15/17] Updated overview --- docs/feature/dualstack/Cilium-dualstack.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/feature/dualstack/Cilium-dualstack.md b/docs/feature/dualstack/Cilium-dualstack.md index 0c923e9d3b..e72c97a7dc 100644 --- a/docs/feature/dualstack/Cilium-dualstack.md +++ b/docs/feature/dualstack/Cilium-dualstack.md @@ -6,10 +6,10 @@ This document proposes changes to the azure-ipam contract and implement a new AP ## Overview -With the current CNS solution for dualstack we are setting the NNC to pass multiple NCs with CIDR to be unrolled and added to a pool that contains both v6 and v4 IPs. Since we are planning to use Cilium CNI for the Linux dualstack solution we need to make add and additional CNS API to allow for a dualstack cluster to get multiple IPs. Currently azure-ipam handles the [`CmdAdd`](https://github.com/Azure/azure-container-networking/blob/master/azure-ipam/ipam.go#:~:text=func%20(p%20*IPAMPlugin)%20CmdAdd(args%20*cniSkel.CmdArgs)%20error%20%7B) command from Cilium CNI and sends an IP request to CNS. In Overlay, CNS then looks through the pool and picks the first IP found that isn’t used. For dualstack we need to change three things: -1. A new contract type to allow for a slice of IPs to be returned -2. A new API that acts similar to the current API but expects uses new contract as a return type -3. Changes need to be made in CNS ipam so that we return an IP for each NC +With the current DNC/DNC-RC solution for dualstack we are setting the NNC to pass multiple NCs with a CIDR to be unrolled and added to a pool that contains both v6 and v4 IPs. For CNI/CNS we need to be able to ensure that we are able to assign an IP from both of these NCs when creating pods and will do so using the following changes: +1. Create a new contract type to allow for a slice of IPs to be returned +2. Create a new API that acts similar to the current API but uses the new contract as a return type to return multiple IPs +3. Make changes to existing functions in ipam so that one IP is assigned per NC to a pod ## Current Implementation @@ -71,4 +71,4 @@ To keep backwards compatibility the current handler, [`requestIPConfigHandler`]( ### Questions -When searching for one IP per NC is there some where that we save all of the NC names. If I don't know the names ahead of time I will probably need to search through all of the IPs to get all of the NC names at one point and I could save it. \ No newline at end of file +When searching for one IP per NC is there some where that we save all of the NC names? If we don't know the names ahead of time I will probably need to search through all of the IPs to get all of the NC names at one point and I could save it. \ No newline at end of file From 984665db72a0a6d6de22581f566f9f0493e3d9f9 Mon Sep 17 00:00:00 2001 From: rjdenney Date: Thu, 19 Jan 2023 14:13:28 -0500 Subject: [PATCH 16/17] add link for IPConfigRequest --- docs/feature/dualstack/Cilium-dualstack.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/feature/dualstack/Cilium-dualstack.md b/docs/feature/dualstack/Cilium-dualstack.md index e72c97a7dc..ad40686e1c 100644 --- a/docs/feature/dualstack/Cilium-dualstack.md +++ b/docs/feature/dualstack/Cilium-dualstack.md @@ -13,7 +13,7 @@ With the current DNC/DNC-RC solution for dualstack we are setting the NNC to pas ## Current Implementation -To communicate to Cilium CNI from Azure CNS we use the azure-ipam plugin. This plugin takes in add commands from the Cilium CNI and reads the CNI network config from stdin. The data from stdin then gets parsed into an [`IPConfigRequest`](https://github.com/Azure/azure-container-networking/blob/master/cns/NetworkContainerContract.go) object and is passed to [`RequestIPAddress`](https://github.com/Azure/azure-container-networking/blob/master/cns/client/client.go). From here we call the [`requestIPConfigHandler`](https://github.com/Azure/azure-container-networking/blob/master/cns/restserver/ipam.go#:~:text=func%20(service%20*HTTPRestService)%20requestIPConfigHandler(w%20http.ResponseWriter%2C%20r%20*http.Request)%20%7B) in ipam which then retrieves and validates the [`IPConfigRequest`](https://github.com/Azure/azure-container-networking/blob/master/cns/NetworkContainerContract.go) and then calls the [`requestIPConfigHelper`](https://github.com/Azure/azure-container-networking/blob/master/cns/restserver/ipam.go#:~:text=func%20requestIPConfigHelper(service%20*HTTPRestService%2C%20req%20cns.IPConfigRequest)%20(cns.PodIpInfo%2C%20error)%20%7B). The helper function first checks to see if the `IPConfigRequest` already belongs to a pod and returns the existing pod's information. If it doesn't already exist than it checks if there is a desired IP address or not. If there is a desired IP address than we search for that specific IP and if not we call +To communicate to Cilium CNI from Azure CNS we use the azure-ipam plugin. This plugin takes in add commands from the Cilium CNI and reads the CNI network config from stdin. The data from stdin then gets parsed into an [`IPConfigRequest`](https://github.com/Azure/azure-container-networking/blob/master/cns/NetworkContainerContract.go) object and is passed to [`RequestIPAddress`](https://github.com/Azure/azure-container-networking/blob/master/cns/client/client.go). From here we call the [`requestIPConfigHandler`](https://github.com/Azure/azure-container-networking/blob/master/cns/restserver/ipam.go#:~:text=func%20(service%20*HTTPRestService)%20requestIPConfigHandler(w%20http.ResponseWriter%2C%20r%20*http.Request)%20%7B) in ipam which then retrieves and validates the [`IPConfigRequest`](https://github.com/Azure/azure-container-networking/blob/master/cns/NetworkContainerContract.go) and then calls the [`requestIPConfigHelper`](https://github.com/Azure/azure-container-networking/blob/master/cns/restserver/ipam.go#:~:text=func%20requestIPConfigHelper(service%20*HTTPRestService%2C%20req%20cns.IPConfigRequest)%20(cns.PodIpInfo%2C%20error)%20%7B). The helper function first checks to see if the [`IPConfigRequest`](https://github.com/Azure/azure-container-networking/blob/master/cns/NetworkContainerContract.go) already belongs to a pod and returns the existing pod's information. If it doesn't already exist than it checks if there is a desired IP address or not. If there is a desired IP address than we search for that specific IP and if not we call [`AssignAnyAvailableIPConfig`](https://github.com/Azure/azure-container-networking/blob/master/cns/restserver/ipam.go#:~:text=func%20(service%20*HTTPRestService)%20AssignAnyAvailableIPConfig(podInfo%20cns.PodInfo)%20(cns.PodIpInfo%2C%20error)%20%7B). This function then searches for the first available IP and returns it back up to the handler. The handler then packages the IP into a [`IPConfigResponse`](https://github.com/Azure/azure-container-networking/blob/bd299fe7271a7a23b3d0268d8e14ad812181e076/cns/NetworkContainerContract.go#L419) which gets returned to the client. ## New Implementation From 69ecb7de1e799f2a3f7a2a48061d8723e9720564 Mon Sep 17 00:00:00 2001 From: rjdenney Date: Wed, 1 Feb 2023 15:33:17 -0500 Subject: [PATCH 17/17] updates and suggested changes --- docs/feature/dualstack/Cilium-dualstack.md | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/docs/feature/dualstack/Cilium-dualstack.md b/docs/feature/dualstack/Cilium-dualstack.md index ad40686e1c..12f6dbfff4 100644 --- a/docs/feature/dualstack/Cilium-dualstack.md +++ b/docs/feature/dualstack/Cilium-dualstack.md @@ -1,4 +1,4 @@ -# Cilium Azure-ipam dualstack Overlay Proposal +# CNS and Azure-ipam dualstack Overlay Proposal ## Purpose @@ -37,23 +37,23 @@ type IPConfigResponse struct { ### New API -A new API will called `RequestAllIPAddresses` be created for requesting IPs when running in Overlay. This will work similar to the current [`RequestIPAddress`](https://github.com/Azure/azure-container-networking/blob/master/cns/client/client.go) and will accept the same [`IPConfigRequest`](https://github.com/Azure/azure-container-networking/blob/master/cns/NetworkContainerContract.go), the main difference will be that it will return the new contract type `IPConfigsResponse` with the slice of `PodIpInfo`. +A new API will called `RequestIPs` be created for requesting IPs when running in Overlay. This will work similar to the current [`RequestIPAddress`](https://github.com/Azure/azure-container-networking/blob/master/cns/client/client.go) and will accept the same [`IPConfigRequest`](https://github.com/Azure/azure-container-networking/blob/master/cns/NetworkContainerContract.go), the main difference will be that it will return the new contract type `IPConfigsResponse` with the slice of `PodIpInfo`. ```diff -+// RequestAllIPAddresses calls the RequestAllIPConfigs in CNS -+func (c *Client) RequestAllIPAddresses(ctx context.Context, ipconfig cns.IPConfigRequest) (*cns.IPConfigsResponse, error) ++// RequestIPs calls the RequestIPConfigs in CNS ++func (c *Client) RequestIPs(ctx context.Context, ipconfig cns.IPConfigRequest) (*cns.IPConfigsResponse, error) } ``` -If for some reason this function returns a 404 error we will then try again but use the original [`RequestIPAddress`](https://github.com/Azure/azure-container-networking/blob/master/cns/client/client.go) API instead. The swift case will not use the new API and will continue to use the current one. +If for some reason this function returns a 404 error we will then try again but use the original [`RequestIPAddress`](https://github.com/Azure/azure-container-networking/blob/master/cns/client/client.go) API instead. This will also make it so that the new azure-ipam will be backwards compatible with previous versions of CNS that do not have the new API . ### IPAM Changes -In IPAM we will need to have a new handler that is called by the new API called `RequestAllIpConfigsHandler`. This new handler will act similarly as the current [`requestIPConfigHandler`](https://github.com/Azure/azure-container-networking/blob/master/cns/restserver/ipam.go#:~:text=func%20(service%20*HTTPRestService)%20requestIPConfigHandler(w%20http.ResponseWriter%2C%20r%20*http.Request)%20%7B) handler with the exception of using the new contract `IPConfigsResponse`. This new function will also call [`requestIPConfigHelper`](https://github.com/Azure/azure-container-networking/blob/master/cns/restserver/ipam.go#:~:text=func%20requestIPConfigHelper(service%20*HTTPRestService%2C%20req%20cns.IPConfigRequest)%20(cns.PodIpInfo%2C%20error)%20%7B) to retrieve an IP however the return type will be changed from a single `PodIpInfo` to a slice so that we can get all IPs needed with one call. [`requestIPConfigHelper`](https://github.com/Azure/azure-container-networking/blob/master/cns/restserver/ipam.go#:~:text=func%20requestIPConfigHelper(service%20*HTTPRestService%2C%20req%20cns.IPConfigRequest)%20(cns.PodIpInfo%2C%20error)%20%7B) will still call [`AssignAnyAvailableIPConfig`](https://github.com/Azure/azure-container-networking/blob/master/cns/restserver/ipam.go#:~:text=func%20(service%20*HTTPRestService)%20AssignAnyAvailableIPConfig(podInfo%20cns.PodInfo)%20(cns.PodIpInfo%2C%20error)%20%7B) but will now be expecting for a slice of IPs to be returned. Within [`AssignAnyAvailableIPConfig`](https://github.com/Azure/azure-container-networking/blob/master/cns/restserver/ipam.go#:~:text=func%20(service%20*HTTPRestService)%20AssignAnyAvailableIPConfig(podInfo%20cns.PodInfo)%20(cns.PodIpInfo%2C%20error)%20%7B) instead of just getting the first available IP, it will now fill a slice with one IP from each NC provided on the NNC. This will then get returned back up to the handler function and then sent back to the client. +In IPAM we will need to have a new handler that is called by the new API called `requestIPsHandler`. This new handler will act similarly as the current [`requestIPConfigHandler`](https://github.com/Azure/azure-container-networking/blob/master/cns/restserver/ipam.go#:~:text=func%20(service%20*HTTPRestService)%20requestIPConfigHandler(w%20http.ResponseWriter%2C%20r%20*http.Request)%20%7B) handler with the exception of using the new contract `IPConfigsResponse`. This new function will also call [`requestIPConfigHelper`](https://github.com/Azure/azure-container-networking/blob/master/cns/restserver/ipam.go#:~:text=func%20requestIPConfigHelper(service%20*HTTPRestService%2C%20req%20cns.IPConfigRequest)%20(cns.PodIpInfo%2C%20error)%20%7B) to retrieve an IP however the return type will be changed from a single `PodIpInfo` to a slice so that we can get all IPs needed with one call. [`requestIPConfigHelper`](https://github.com/Azure/azure-container-networking/blob/master/cns/restserver/ipam.go#:~:text=func%20requestIPConfigHelper(service%20*HTTPRestService%2C%20req%20cns.IPConfigRequest)%20(cns.PodIpInfo%2C%20error)%20%7B) will still call [`AssignAnyAvailableIPConfig`](https://github.com/Azure/azure-container-networking/blob/master/cns/restserver/ipam.go#:~:text=func%20(service%20*HTTPRestService)%20AssignAnyAvailableIPConfig(podInfo%20cns.PodInfo)%20(cns.PodIpInfo%2C%20error)%20%7B) but will now be expecting for a slice of IPs to be returned. Within [`AssignAnyAvailableIPConfig`](https://github.com/Azure/azure-container-networking/blob/master/cns/restserver/ipam.go#:~:text=func%20(service%20*HTTPRestService)%20AssignAnyAvailableIPConfig(podInfo%20cns.PodInfo)%20(cns.PodIpInfo%2C%20error)%20%7B) instead of just getting the first available IP, it will now fill a slice with one IP from each NC provided on the NNC. This will then get returned back up to the handler function and then sent back to the client.[`GetExistingIPConfig`](https://github.com/Azure/azure-container-networking/blob/9dc035473b01ea4b72f5c3a3cb21d5a4fbc3d9cd/cns/restserver/ipam.go#L481) and [`AssignDesiredIPConfig`](https://github.com/Azure/azure-container-networking/blob/9dc035473b01ea4b72f5c3a3cb21d5a4fbc3d9cd/cns/restserver/ipam.go#L504) will also be updated to support multiple IPs on a pod by being able to return a slice of IP configs for an existing pod and for allowing a slice of IPs to be desired when creating a pod respectively. ```diff +// used to request an IPConfig for each NC from the CNS state -+func (service *HTTPRestService) RequestAllIpConfigsHandler(w http.ResponseWriter, r *http.Request) ++func (service *HTTPRestService) requestIPsHandler(w http.ResponseWriter, r *http.Request) ``` ```diff @@ -67,8 +67,3 @@ In IPAM we will need to have a new handler that is called by the new API called ``` To keep backwards compatibility the current handler, [`requestIPConfigHandler`](https://github.com/Azure/azure-container-networking/blob/master/cns/restserver/ipam.go#:~:text=func%20(service%20*HTTPRestService)%20requestIPConfigHandler(w%20http.ResponseWriter%2C%20r%20*http.Request)%20%7B) will still accept the slice from [`requestIPConfigHelper`](https://github.com/Azure/azure-container-networking/blob/master/cns/restserver/ipam.go#:~:text=func%20requestIPConfigHelper(service%20*HTTPRestService%2C%20req%20cns.IPConfigRequest)%20(cns.PodIpInfo%2C%20error)%20%7B) but will then have to retrieve the first index of the slice so that it can be used in the old version of the contract, [`IPConfigResponse`](https://github.com/Azure/azure-container-networking/blob/bd299fe7271a7a23b3d0268d8e14ad812181e076/cns/NetworkContainerContract.go#L419). - - -### Questions - -When searching for one IP per NC is there some where that we save all of the NC names? If we don't know the names ahead of time I will probably need to search through all of the IPs to get all of the NC names at one point and I could save it. \ No newline at end of file