From 2757bbf5dabf71220ba2b7cda640091d162075c4 Mon Sep 17 00:00:00 2001 From: Ashish Nair Date: Wed, 30 Aug 2023 00:03:16 +0000 Subject: [PATCH 01/17] Added Network Conatiner Status to include the latest error code for a Network Container --- .../api/v1alpha/nodenetworkconfig.go | 15 ++++++++++---- .../api/v1alpha/zz_generated.deepcopy.go | 20 +++++++++++++++++++ .../acn.azure.com_nodenetworkconfigs.yaml | 11 ++++++++++ 3 files changed, 42 insertions(+), 4 deletions(-) diff --git a/crd/nodenetworkconfig/api/v1alpha/nodenetworkconfig.go b/crd/nodenetworkconfig/api/v1alpha/nodenetworkconfig.go index 4fca708fb3..daa18ae9b9 100644 --- a/crd/nodenetworkconfig/api/v1alpha/nodenetworkconfig.go +++ b/crd/nodenetworkconfig/api/v1alpha/nodenetworkconfig.go @@ -71,10 +71,17 @@ const ( type NodeNetworkConfigStatus struct { // +kubebuilder:default=0 // +kubebuilder:validation:Optional - AssignedIPCount int `json:"assignedIPCount"` - Scaler Scaler `json:"scaler,omitempty"` - Status Status `json:"status,omitempty"` - NetworkContainers []NetworkContainer `json:"networkContainers,omitempty"` + AssignedIPCount int `json:"assignedIPCount"` + Scaler Scaler `json:"scaler,omitempty"` + Status Status `json:"status,omitempty"` + NetworkContainers []NetworkContainer `json:"networkContainers,omitempty"` + NetworkContainerStatuses []NetworkContainerStatus `json:"networkContainerStatuses,omitempty"` +} + +// NetworkContainerStatus defines the status of the Network Containers +type NetworkContainerStatus struct { + NCID string `json:"id,omitempty"` + LatestErrorCode string `json:"latestErrorCode,omitempty"` } // Scaler groups IP request params together diff --git a/crd/nodenetworkconfig/api/v1alpha/zz_generated.deepcopy.go b/crd/nodenetworkconfig/api/v1alpha/zz_generated.deepcopy.go index ac4c0df1ec..685e3d74d7 100644 --- a/crd/nodenetworkconfig/api/v1alpha/zz_generated.deepcopy.go +++ b/crd/nodenetworkconfig/api/v1alpha/zz_generated.deepcopy.go @@ -43,6 +43,21 @@ func (in *NetworkContainer) DeepCopy() *NetworkContainer { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *NetworkContainerStatus) DeepCopyInto(out *NetworkContainerStatus) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NetworkContainerStatus. +func (in *NetworkContainerStatus) DeepCopy() *NetworkContainerStatus { + if in == nil { + return nil + } + out := new(NetworkContainerStatus) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *NodeNetworkConfig) DeepCopyInto(out *NodeNetworkConfig) { *out = *in @@ -133,6 +148,11 @@ func (in *NodeNetworkConfigStatus) DeepCopyInto(out *NodeNetworkConfigStatus) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.NetworkContainerStatuses != nil { + in, out := &in.NetworkContainerStatuses, &out.NetworkContainerStatuses + *out = make([]NetworkContainerStatus, len(*in)) + copy(*out, *in) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NodeNetworkConfigStatus. diff --git a/crd/nodenetworkconfig/manifests/acn.azure.com_nodenetworkconfigs.yaml b/crd/nodenetworkconfig/manifests/acn.azure.com_nodenetworkconfigs.yaml index c9a8949715..3729a4356f 100644 --- a/crd/nodenetworkconfig/manifests/acn.azure.com_nodenetworkconfigs.yaml +++ b/crd/nodenetworkconfig/manifests/acn.azure.com_nodenetworkconfigs.yaml @@ -81,6 +81,17 @@ spec: assignedIPCount: default: 0 type: integer + networkContainerStatuses: + items: + description: NetworkContainerStatus defines the status of the Network + Containers + properties: + id: + type: string + latestErrorCode: + type: string + type: object + type: array networkContainers: items: description: NetworkContainer defines the structure of a Network From 7a107bfa834bc9487af45fb101a8598db282e30a Mon Sep 17 00:00:00 2001 From: Ashish Nair Date: Wed, 30 Aug 2023 20:26:48 +0000 Subject: [PATCH 02/17] Updated the crd to have the Status field included into the Network Container --- .../api/v1alpha/nodenetworkconfig.go | 13 ++++++------ .../api/v1alpha/zz_generated.deepcopy.go | 7 ++----- .../acn.azure.com_nodenetworkconfigs.yaml | 21 +++++++++---------- 3 files changed, 18 insertions(+), 23 deletions(-) diff --git a/crd/nodenetworkconfig/api/v1alpha/nodenetworkconfig.go b/crd/nodenetworkconfig/api/v1alpha/nodenetworkconfig.go index daa18ae9b9..9f58dc5c98 100644 --- a/crd/nodenetworkconfig/api/v1alpha/nodenetworkconfig.go +++ b/crd/nodenetworkconfig/api/v1alpha/nodenetworkconfig.go @@ -71,17 +71,16 @@ const ( type NodeNetworkConfigStatus struct { // +kubebuilder:default=0 // +kubebuilder:validation:Optional - AssignedIPCount int `json:"assignedIPCount"` - Scaler Scaler `json:"scaler,omitempty"` - Status Status `json:"status,omitempty"` - NetworkContainers []NetworkContainer `json:"networkContainers,omitempty"` - NetworkContainerStatuses []NetworkContainerStatus `json:"networkContainerStatuses,omitempty"` + AssignedIPCount int `json:"assignedIPCount"` + Scaler Scaler `json:"scaler,omitempty"` + Status Status `json:"status,omitempty"` + NetworkContainers []NetworkContainer `json:"networkContainers,omitempty"` } // NetworkContainerStatus defines the status of the Network Containers type NetworkContainerStatus struct { - NCID string `json:"id,omitempty"` - LatestErrorCode string `json:"latestErrorCode,omitempty"` + Timestamp metav1.Time `json:"timestamp,omitempty"` + LatestStatusCode string `json:"latestStatusCode,omitempty"` } // Scaler groups IP request params together diff --git a/crd/nodenetworkconfig/api/v1alpha/zz_generated.deepcopy.go b/crd/nodenetworkconfig/api/v1alpha/zz_generated.deepcopy.go index 685e3d74d7..7bf275c750 100644 --- a/crd/nodenetworkconfig/api/v1alpha/zz_generated.deepcopy.go +++ b/crd/nodenetworkconfig/api/v1alpha/zz_generated.deepcopy.go @@ -31,6 +31,7 @@ func (in *NetworkContainer) DeepCopyInto(out *NetworkContainer) { *out = make([]IPAssignment, len(*in)) copy(*out, *in) } + in.UpdateStatus.DeepCopyInto(&out.UpdateStatus) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NetworkContainer. @@ -46,6 +47,7 @@ func (in *NetworkContainer) DeepCopy() *NetworkContainer { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *NetworkContainerStatus) DeepCopyInto(out *NetworkContainerStatus) { *out = *in + in.Timestamp.DeepCopyInto(&out.Timestamp) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NetworkContainerStatus. @@ -148,11 +150,6 @@ func (in *NodeNetworkConfigStatus) DeepCopyInto(out *NodeNetworkConfigStatus) { (*in)[i].DeepCopyInto(&(*out)[i]) } } - if in.NetworkContainerStatuses != nil { - in, out := &in.NetworkContainerStatuses, &out.NetworkContainerStatuses - *out = make([]NetworkContainerStatus, len(*in)) - copy(*out, *in) - } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NodeNetworkConfigStatus. diff --git a/crd/nodenetworkconfig/manifests/acn.azure.com_nodenetworkconfigs.yaml b/crd/nodenetworkconfig/manifests/acn.azure.com_nodenetworkconfigs.yaml index 3729a4356f..a76d22724d 100644 --- a/crd/nodenetworkconfig/manifests/acn.azure.com_nodenetworkconfigs.yaml +++ b/crd/nodenetworkconfig/manifests/acn.azure.com_nodenetworkconfigs.yaml @@ -81,17 +81,6 @@ spec: assignedIPCount: default: 0 type: integer - networkContainerStatuses: - items: - description: NetworkContainerStatus defines the status of the Network - Containers - properties: - id: - type: string - latestErrorCode: - type: string - type: object - type: array networkContainers: items: description: NetworkContainer defines the structure of a Network @@ -144,6 +133,16 @@ spec: description: NCType is the specific type of network this NC represents. type: string + updateStatus: + description: NetworkContainerStatus defines the status of the + Network Containers + properties: + latestStatusCode: + type: string + timestamp: + format: date-time + type: string + type: object version: default: 0 format: int64 From ed8dcca07f9091e65d75cdd3247bb164b588f6c5 Mon Sep 17 00:00:00 2001 From: Ashish Nair Date: Thu, 31 Aug 2023 00:21:11 +0000 Subject: [PATCH 03/17] Updated the names and added Status and ErrorText as two fields in NC Status --- crd/nodenetworkconfig/api/v1alpha/nodenetworkconfig.go | 5 +++-- .../manifests/acn.azure.com_nodenetworkconfigs.yaml | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/crd/nodenetworkconfig/api/v1alpha/nodenetworkconfig.go b/crd/nodenetworkconfig/api/v1alpha/nodenetworkconfig.go index 9f58dc5c98..74f35174a8 100644 --- a/crd/nodenetworkconfig/api/v1alpha/nodenetworkconfig.go +++ b/crd/nodenetworkconfig/api/v1alpha/nodenetworkconfig.go @@ -79,8 +79,9 @@ type NodeNetworkConfigStatus struct { // NetworkContainerStatus defines the status of the Network Containers type NetworkContainerStatus struct { - Timestamp metav1.Time `json:"timestamp,omitempty"` - LatestStatusCode string `json:"latestStatusCode,omitempty"` + Timestamp metav1.Time `json:"timestamp,omitempty"` + latestStatus string `json:"latestStatus,omitempty"` + latestErrorText string `json:"latestStatus,omitempty"` } // Scaler groups IP request params together diff --git a/crd/nodenetworkconfig/manifests/acn.azure.com_nodenetworkconfigs.yaml b/crd/nodenetworkconfig/manifests/acn.azure.com_nodenetworkconfigs.yaml index a76d22724d..c0562fd151 100644 --- a/crd/nodenetworkconfig/manifests/acn.azure.com_nodenetworkconfigs.yaml +++ b/crd/nodenetworkconfig/manifests/acn.azure.com_nodenetworkconfigs.yaml @@ -137,7 +137,7 @@ spec: description: NetworkContainerStatus defines the status of the Network Containers properties: - latestStatusCode: + latestStatus: type: string timestamp: format: date-time From 28d4371f18fec48572a005b932e9d7b19019c005 Mon Sep 17 00:00:00 2001 From: Ashish Nair Date: Thu, 31 Aug 2023 01:08:53 +0000 Subject: [PATCH 04/17] Fixed the casing and json values for these variables --- crd/nodenetworkconfig/api/v1alpha/nodenetworkconfig.go | 4 ++-- .../manifests/acn.azure.com_nodenetworkconfigs.yaml | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/crd/nodenetworkconfig/api/v1alpha/nodenetworkconfig.go b/crd/nodenetworkconfig/api/v1alpha/nodenetworkconfig.go index 74f35174a8..2b6c8c653f 100644 --- a/crd/nodenetworkconfig/api/v1alpha/nodenetworkconfig.go +++ b/crd/nodenetworkconfig/api/v1alpha/nodenetworkconfig.go @@ -80,8 +80,8 @@ type NodeNetworkConfigStatus struct { // NetworkContainerStatus defines the status of the Network Containers type NetworkContainerStatus struct { Timestamp metav1.Time `json:"timestamp,omitempty"` - latestStatus string `json:"latestStatus,omitempty"` - latestErrorText string `json:"latestStatus,omitempty"` + LatestStatus string `json:"latestStatus,omitempty"` + LatestErrorText string `json:"latestErrorText,omitempty"` } // Scaler groups IP request params together diff --git a/crd/nodenetworkconfig/manifests/acn.azure.com_nodenetworkconfigs.yaml b/crd/nodenetworkconfig/manifests/acn.azure.com_nodenetworkconfigs.yaml index c0562fd151..0adb8410f9 100644 --- a/crd/nodenetworkconfig/manifests/acn.azure.com_nodenetworkconfigs.yaml +++ b/crd/nodenetworkconfig/manifests/acn.azure.com_nodenetworkconfigs.yaml @@ -137,6 +137,8 @@ spec: description: NetworkContainerStatus defines the status of the Network Containers properties: + latestErrorText: + type: string latestStatus: type: string timestamp: From 28cbe8e1379d9fd77ad87dac4ca72728da3cf0e9 Mon Sep 17 00:00:00 2001 From: Ashish Nair Date: Tue, 5 Sep 2023 18:23:48 +0000 Subject: [PATCH 05/17] Propagated the NC Status inside the CNS and IPAM Monitor pool states --- cns/NetworkContainerContract.go | 2 ++ cns/ipampool/monitor.go | 2 ++ cns/kubecontroller/nodenetworkconfig/conversion.go | 1 + cns/kubecontroller/nodenetworkconfig/conversion_linux.go | 1 + cns/kubecontroller/nodenetworkconfig/conversion_windows.go | 1 + 5 files changed, 7 insertions(+) diff --git a/cns/NetworkContainerContract.go b/cns/NetworkContainerContract.go index 9d041a6cfe..6c49cde62c 100644 --- a/cns/NetworkContainerContract.go +++ b/cns/NetworkContainerContract.go @@ -8,6 +8,7 @@ import ( "strings" "github.com/Azure/azure-container-networking/cns/types" + "github.com/Azure/azure-container-networking/crd/nodenetworkconfig/api/v1alpha" "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" ) @@ -95,6 +96,7 @@ type CreateNetworkContainerRequest struct { AllowHostToNCCommunication bool AllowNCToHostCommunication bool EndpointPolicies []NetworkContainerRequestPolicies + NCStatus v1alpha.NetworkContainerStatus } // CreateNetworkContainerRequest implements fmt.Stringer for logging diff --git a/cns/ipampool/monitor.go b/cns/ipampool/monitor.go index 18caf00e03..cba20f71af 100644 --- a/cns/ipampool/monitor.go +++ b/cns/ipampool/monitor.go @@ -46,6 +46,7 @@ type metaState struct { subnet string subnetARMID string subnetCIDR string + nncStatus v1alpha.NetworkContainerStatus } type Options struct { @@ -122,6 +123,7 @@ func (pm *Monitor) Start(ctx context.Context) error { pm.metastate.subnet = nnc.Status.NetworkContainers[0].SubnetName pm.metastate.subnetCIDR = nnc.Status.NetworkContainers[0].SubnetAddressSpace pm.metastate.subnetARMID = GenerateARMID(&nnc.Status.NetworkContainers[0]) + pm.metastate.nncStatus = nnc.Status.NetworkContainers[0].UpdateStatus } pm.metastate.primaryIPAddresses = make(map[string]struct{}) // Add Primary IP to Map, if not present. diff --git a/cns/kubecontroller/nodenetworkconfig/conversion.go b/cns/kubecontroller/nodenetworkconfig/conversion.go index 7e025380c6..8b65659ae2 100644 --- a/cns/kubecontroller/nodenetworkconfig/conversion.go +++ b/cns/kubecontroller/nodenetworkconfig/conversion.go @@ -66,6 +66,7 @@ func CreateNCRequestFromDynamicNC(nc v1alpha.NetworkContainer) (*cns.CreateNetwo IPSubnet: subnet, GatewayIPAddress: nc.DefaultGateway, }, + NCStatus: nc.UpdateStatus, }, nil } diff --git a/cns/kubecontroller/nodenetworkconfig/conversion_linux.go b/cns/kubecontroller/nodenetworkconfig/conversion_linux.go index ab1039f474..af024c2f9f 100644 --- a/cns/kubecontroller/nodenetworkconfig/conversion_linux.go +++ b/cns/kubecontroller/nodenetworkconfig/conversion_linux.go @@ -56,5 +56,6 @@ func createNCRequestFromStaticNCHelper(nc v1alpha.NetworkContainer, primaryIPPre IPSubnet: subnet, GatewayIPAddress: nc.DefaultGateway, }, + NCStatus: nc.UpdateStatus, }, nil } diff --git a/cns/kubecontroller/nodenetworkconfig/conversion_windows.go b/cns/kubecontroller/nodenetworkconfig/conversion_windows.go index c0696ec3ef..0263c7744a 100644 --- a/cns/kubecontroller/nodenetworkconfig/conversion_windows.go +++ b/cns/kubecontroller/nodenetworkconfig/conversion_windows.go @@ -66,5 +66,6 @@ func createNCRequestFromStaticNCHelper(nc v1alpha.NetworkContainer, primaryIPPre IPSubnet: subnet, GatewayIPAddress: nc.DefaultGateway, }, + NCStatus: nc.UpdateStatus }, nil } From 196e1c26c69a8e086560611dbb2f23d204b6c7bb Mon Sep 17 00:00:00 2001 From: Ashish Nair Date: Tue, 5 Sep 2023 18:28:38 +0000 Subject: [PATCH 06/17] Fixed the lint error of missing comma --- cns/kubecontroller/nodenetworkconfig/conversion_windows.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cns/kubecontroller/nodenetworkconfig/conversion_windows.go b/cns/kubecontroller/nodenetworkconfig/conversion_windows.go index 0263c7744a..ce66ba275a 100644 --- a/cns/kubecontroller/nodenetworkconfig/conversion_windows.go +++ b/cns/kubecontroller/nodenetworkconfig/conversion_windows.go @@ -66,6 +66,6 @@ func createNCRequestFromStaticNCHelper(nc v1alpha.NetworkContainer, primaryIPPre IPSubnet: subnet, GatewayIPAddress: nc.DefaultGateway, }, - NCStatus: nc.UpdateStatus + NCStatus: nc.UpdateStatus, }, nil } From 56a35e0b2d11683f1fbcca1af11ca7a2af1e1ead Mon Sep 17 00:00:00 2001 From: Ashish Nair Date: Sat, 9 Sep 2023 08:04:16 +0000 Subject: [PATCH 07/17] Saved the updated NC Status into the CNS statefile --- cns/NetworkContainerContract.go | 6 +++--- cns/ipampool/monitor.go | 1 - .../nodenetworkconfig/conversion.go | 2 +- .../nodenetworkconfig/conversion_linux.go | 2 +- .../nodenetworkconfig/conversion_windows.go | 2 +- .../api/v1alpha/nodenetworkconfig.go | 7 ------- .../api/v1alpha/zz_generated.deepcopy.go | 17 ----------------- .../acn.azure.com_nodenetworkconfigs.yaml | 12 ------------ 8 files changed, 6 insertions(+), 43 deletions(-) diff --git a/cns/NetworkContainerContract.go b/cns/NetworkContainerContract.go index 6c49cde62c..4255658aa1 100644 --- a/cns/NetworkContainerContract.go +++ b/cns/NetworkContainerContract.go @@ -96,7 +96,7 @@ type CreateNetworkContainerRequest struct { AllowHostToNCCommunication bool AllowNCToHostCommunication bool EndpointPolicies []NetworkContainerRequestPolicies - NCStatus v1alpha.NetworkContainerStatus + NCStatus v1alpha.NCStatus } // CreateNetworkContainerRequest implements fmt.Stringer for logging @@ -104,9 +104,9 @@ func (req *CreateNetworkContainerRequest) String() string { return fmt.Sprintf("CreateNetworkContainerRequest"+ "{Version: %s, NetworkContainerType: %s, NetworkContainerid: %s, PrimaryInterfaceIdentifier: %s, "+ "LocalIPConfiguration: %+v, IPConfiguration: %+v, SecondaryIPConfigs: %+v, MultitenancyInfo: %+v, "+ - "AllowHostToNCCommunication: %t, AllowNCToHostCommunication: %t}", + "AllowHostToNCCommunication: %t, AllowNCToHostCommunication: %t, NCStatus: %s}", req.Version, req.NetworkContainerType, req.NetworkContainerid, req.PrimaryInterfaceIdentifier, req.LocalIPConfiguration, - req.IPConfiguration, req.SecondaryIPConfigs, req.MultiTenancyInfo, req.AllowHostToNCCommunication, req.AllowNCToHostCommunication) + req.IPConfiguration, req.SecondaryIPConfigs, req.MultiTenancyInfo, req.AllowHostToNCCommunication, req.AllowNCToHostCommunication, string(req.NCStatus)) } // NetworkContainerRequestPolicies - specifies policies associated with create network request diff --git a/cns/ipampool/monitor.go b/cns/ipampool/monitor.go index cba20f71af..5642c89ea6 100644 --- a/cns/ipampool/monitor.go +++ b/cns/ipampool/monitor.go @@ -123,7 +123,6 @@ func (pm *Monitor) Start(ctx context.Context) error { pm.metastate.subnet = nnc.Status.NetworkContainers[0].SubnetName pm.metastate.subnetCIDR = nnc.Status.NetworkContainers[0].SubnetAddressSpace pm.metastate.subnetARMID = GenerateARMID(&nnc.Status.NetworkContainers[0]) - pm.metastate.nncStatus = nnc.Status.NetworkContainers[0].UpdateStatus } pm.metastate.primaryIPAddresses = make(map[string]struct{}) // Add Primary IP to Map, if not present. diff --git a/cns/kubecontroller/nodenetworkconfig/conversion.go b/cns/kubecontroller/nodenetworkconfig/conversion.go index 8b65659ae2..8f6346ce52 100644 --- a/cns/kubecontroller/nodenetworkconfig/conversion.go +++ b/cns/kubecontroller/nodenetworkconfig/conversion.go @@ -66,7 +66,7 @@ func CreateNCRequestFromDynamicNC(nc v1alpha.NetworkContainer) (*cns.CreateNetwo IPSubnet: subnet, GatewayIPAddress: nc.DefaultGateway, }, - NCStatus: nc.UpdateStatus, + NCStatus: nc.Status, }, nil } diff --git a/cns/kubecontroller/nodenetworkconfig/conversion_linux.go b/cns/kubecontroller/nodenetworkconfig/conversion_linux.go index af024c2f9f..a22dbd4bd8 100644 --- a/cns/kubecontroller/nodenetworkconfig/conversion_linux.go +++ b/cns/kubecontroller/nodenetworkconfig/conversion_linux.go @@ -56,6 +56,6 @@ func createNCRequestFromStaticNCHelper(nc v1alpha.NetworkContainer, primaryIPPre IPSubnet: subnet, GatewayIPAddress: nc.DefaultGateway, }, - NCStatus: nc.UpdateStatus, + NCStatus: nc.Status, }, nil } diff --git a/cns/kubecontroller/nodenetworkconfig/conversion_windows.go b/cns/kubecontroller/nodenetworkconfig/conversion_windows.go index ce66ba275a..0a41ccd41c 100644 --- a/cns/kubecontroller/nodenetworkconfig/conversion_windows.go +++ b/cns/kubecontroller/nodenetworkconfig/conversion_windows.go @@ -66,6 +66,6 @@ func createNCRequestFromStaticNCHelper(nc v1alpha.NetworkContainer, primaryIPPre IPSubnet: subnet, GatewayIPAddress: nc.DefaultGateway, }, - NCStatus: nc.UpdateStatus, + NCStatus: nc.Status, }, nil } diff --git a/crd/nodenetworkconfig/api/v1alpha/nodenetworkconfig.go b/crd/nodenetworkconfig/api/v1alpha/nodenetworkconfig.go index 2b6c8c653f..4fca708fb3 100644 --- a/crd/nodenetworkconfig/api/v1alpha/nodenetworkconfig.go +++ b/crd/nodenetworkconfig/api/v1alpha/nodenetworkconfig.go @@ -77,13 +77,6 @@ type NodeNetworkConfigStatus struct { NetworkContainers []NetworkContainer `json:"networkContainers,omitempty"` } -// NetworkContainerStatus defines the status of the Network Containers -type NetworkContainerStatus struct { - Timestamp metav1.Time `json:"timestamp,omitempty"` - LatestStatus string `json:"latestStatus,omitempty"` - LatestErrorText string `json:"latestErrorText,omitempty"` -} - // Scaler groups IP request params together type Scaler struct { BatchSize int64 `json:"batchSize,omitempty"` diff --git a/crd/nodenetworkconfig/api/v1alpha/zz_generated.deepcopy.go b/crd/nodenetworkconfig/api/v1alpha/zz_generated.deepcopy.go index 7bf275c750..ac4c0df1ec 100644 --- a/crd/nodenetworkconfig/api/v1alpha/zz_generated.deepcopy.go +++ b/crd/nodenetworkconfig/api/v1alpha/zz_generated.deepcopy.go @@ -31,7 +31,6 @@ func (in *NetworkContainer) DeepCopyInto(out *NetworkContainer) { *out = make([]IPAssignment, len(*in)) copy(*out, *in) } - in.UpdateStatus.DeepCopyInto(&out.UpdateStatus) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NetworkContainer. @@ -44,22 +43,6 @@ func (in *NetworkContainer) DeepCopy() *NetworkContainer { return out } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *NetworkContainerStatus) DeepCopyInto(out *NetworkContainerStatus) { - *out = *in - in.Timestamp.DeepCopyInto(&out.Timestamp) -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NetworkContainerStatus. -func (in *NetworkContainerStatus) DeepCopy() *NetworkContainerStatus { - if in == nil { - return nil - } - out := new(NetworkContainerStatus) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *NodeNetworkConfig) DeepCopyInto(out *NodeNetworkConfig) { *out = *in diff --git a/crd/nodenetworkconfig/manifests/acn.azure.com_nodenetworkconfigs.yaml b/crd/nodenetworkconfig/manifests/acn.azure.com_nodenetworkconfigs.yaml index 0adb8410f9..c9a8949715 100644 --- a/crd/nodenetworkconfig/manifests/acn.azure.com_nodenetworkconfigs.yaml +++ b/crd/nodenetworkconfig/manifests/acn.azure.com_nodenetworkconfigs.yaml @@ -133,18 +133,6 @@ spec: description: NCType is the specific type of network this NC represents. type: string - updateStatus: - description: NetworkContainerStatus defines the status of the - Network Containers - properties: - latestErrorText: - type: string - latestStatus: - type: string - timestamp: - format: date-time - type: string - type: object version: default: 0 format: int64 From 5830ed1acb816b7004e23b0da3f2e8bcd6941f96 Mon Sep 17 00:00:00 2001 From: Ashish Nair Date: Sun, 10 Sep 2023 18:53:18 +0000 Subject: [PATCH 08/17] Updated the IP assignment to check and error out subnet is Full when there are no more available IPs for CNS to assign --- cns/restserver/ipam.go | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/cns/restserver/ipam.go b/cns/restserver/ipam.go index d148692c66..fd85a84eec 100644 --- a/cns/restserver/ipam.go +++ b/cns/restserver/ipam.go @@ -14,6 +14,7 @@ import ( "github.com/Azure/azure-container-networking/cns/logger" "github.com/Azure/azure-container-networking/cns/types" "github.com/Azure/azure-container-networking/common" + "github.com/Azure/azure-container-networking/crd/nodenetworkconfig/api/v1alpha" "github.com/pkg/errors" ) @@ -790,6 +791,12 @@ func (service *HTTPRestService) AssignAvailableIPConfigs(podInfo cns.PodInfo) ([ if numOfNCs == 0 { return nil, ErrNoNCs } + // slice to store NCIDs + ncIDs := make([]string, numOfNCs) + for key, _ := range service.state.ContainerStatus { + ncIDs := append(ncIDs, key) + } + service.Lock() defer service.Unlock() // Creates a slice of PodIpInfo with the size as number of NCs to hold the result for assigned IP configs @@ -816,8 +823,16 @@ func (service *HTTPRestService) AssignAvailableIPConfigs(podInfo cns.PodInfo) ([ // Checks to make sure we found one IP for each NC if len(ipsToAssign) != numOfNCs { - //nolint:goerr113 // return error - return podIPInfo, fmt.Errorf("not enough IPs available, waiting on Azure CNS to allocate more") + for _, ncID := range ncIDs { + if _, found := ipsToAssign[ncID]; found { + continue + } + if service.state.ContainerStatus[ncID].CreateNetworkContainerRequest.NCStatus == v1alpha.NCStatusSubnetFull { + return podIPInfo, fmt.Errorf("not enough IPs available for %s, waiting on Azure CNS to allocate more but the Subnet is full", ncID) + } + //nolint:goerr113 // return error + return podIPInfo, fmt.Errorf("not enough IPs available for %s, waiting on Azure CNS to allocate more", ncID) + } } failedToAssignIP := false From 1d104fe8b444d068b3ff801fc1c1267d8ec9c755 Mon Sep 17 00:00:00 2001 From: Ashish Nair Date: Sun, 10 Sep 2023 18:55:38 +0000 Subject: [PATCH 09/17] Fixed a minor compilation issue --- cns/restserver/ipam.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cns/restserver/ipam.go b/cns/restserver/ipam.go index fd85a84eec..d829f26d68 100644 --- a/cns/restserver/ipam.go +++ b/cns/restserver/ipam.go @@ -794,7 +794,7 @@ func (service *HTTPRestService) AssignAvailableIPConfigs(podInfo cns.PodInfo) ([ // slice to store NCIDs ncIDs := make([]string, numOfNCs) for key, _ := range service.state.ContainerStatus { - ncIDs := append(ncIDs, key) + ncIDs = append(ncIDs, key) } service.Lock() From 534ab9dc5901d93a45f9d399555572512a2815d0 Mon Sep 17 00:00:00 2001 From: Ashish Nair Date: Sun, 10 Sep 2023 19:06:11 +0000 Subject: [PATCH 10/17] Fixed lint failures --- cns/restserver/ipam.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cns/restserver/ipam.go b/cns/restserver/ipam.go index d829f26d68..cd3ce620ec 100644 --- a/cns/restserver/ipam.go +++ b/cns/restserver/ipam.go @@ -792,8 +792,8 @@ func (service *HTTPRestService) AssignAvailableIPConfigs(podInfo cns.PodInfo) ([ return nil, ErrNoNCs } // slice to store NCIDs - ncIDs := make([]string, numOfNCs) - for key, _ := range service.state.ContainerStatus { + ncIDs := make([]string, 0) + for key := range service.state.ContainerStatus { ncIDs = append(ncIDs, key) } From a14cf895d2c85e0910d9305de430c6ee9571c674 Mon Sep 17 00:00:00 2001 From: Ashish Nair Date: Sun, 10 Sep 2023 19:09:50 +0000 Subject: [PATCH 11/17] Fixed lint failures --- cns/restserver/ipam.go | 1 + 1 file changed, 1 insertion(+) diff --git a/cns/restserver/ipam.go b/cns/restserver/ipam.go index cd3ce620ec..8ef4aa9048 100644 --- a/cns/restserver/ipam.go +++ b/cns/restserver/ipam.go @@ -828,6 +828,7 @@ func (service *HTTPRestService) AssignAvailableIPConfigs(podInfo cns.PodInfo) ([ continue } if service.state.ContainerStatus[ncID].CreateNetworkContainerRequest.NCStatus == v1alpha.NCStatusSubnetFull { + //nolint:goerr113 // return error return podIPInfo, fmt.Errorf("not enough IPs available for %s, waiting on Azure CNS to allocate more but the Subnet is full", ncID) } //nolint:goerr113 // return error From 3b00dc64e39e270c6abba2b7c8edc35b0497d1e2 Mon Sep 17 00:00:00 2001 From: Ashish Nair Date: Mon, 11 Sep 2023 06:02:29 +0000 Subject: [PATCH 12/17] Removed the reference from the metastate of the ipam monitor --- cns/ipampool/monitor.go | 1 - 1 file changed, 1 deletion(-) diff --git a/cns/ipampool/monitor.go b/cns/ipampool/monitor.go index 5642c89ea6..18caf00e03 100644 --- a/cns/ipampool/monitor.go +++ b/cns/ipampool/monitor.go @@ -46,7 +46,6 @@ type metaState struct { subnet string subnetARMID string subnetCIDR string - nncStatus v1alpha.NetworkContainerStatus } type Options struct { From 299836031448f7be199714739a1d8ee8e4881423 Mon Sep 17 00:00:00 2001 From: Ashish Nair Date: Mon, 11 Sep 2023 06:08:52 +0000 Subject: [PATCH 13/17] Added Update Success and Update Failed statuses to the NC Status to be able to clearly indicate response status inside the NNC from DNC-RC --- crd/nodenetworkconfig/api/v1alpha/nodenetworkconfig.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crd/nodenetworkconfig/api/v1alpha/nodenetworkconfig.go b/crd/nodenetworkconfig/api/v1alpha/nodenetworkconfig.go index 4fca708fb3..8c9b5909ab 100644 --- a/crd/nodenetworkconfig/api/v1alpha/nodenetworkconfig.go +++ b/crd/nodenetworkconfig/api/v1alpha/nodenetworkconfig.go @@ -65,6 +65,8 @@ type NCStatus string const ( NCStatusSubnetFull NCStatus = "SubnetFull" + NCUpdateSuccess NCStatus = "UpdateSuccess" + NCUpdateFailed NCStatus = "UpdateFailed" ) // NodeNetworkConfigStatus defines the observed state of NetworkConfig From 938a53a4cc32235b81573fe9b67c1dc702667bee Mon Sep 17 00:00:00 2001 From: Ashish Nair Date: Mon, 18 Sep 2023 18:07:10 +0000 Subject: [PATCH 14/17] Updated the error to use errors pkg instead of fmt --- cns/restserver/ipam.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/cns/restserver/ipam.go b/cns/restserver/ipam.go index 8ef4aa9048..4939b91b3e 100644 --- a/cns/restserver/ipam.go +++ b/cns/restserver/ipam.go @@ -828,11 +828,9 @@ func (service *HTTPRestService) AssignAvailableIPConfigs(podInfo cns.PodInfo) ([ continue } if service.state.ContainerStatus[ncID].CreateNetworkContainerRequest.NCStatus == v1alpha.NCStatusSubnetFull { - //nolint:goerr113 // return error - return podIPInfo, fmt.Errorf("not enough IPs available for %s, waiting on Azure CNS to allocate more but the Subnet is full", ncID) + return podIPInfo, errors.Errorf("not enough IPs available for %s, waiting on Azure CNS to allocate more but the Subnet is full", ncID) } - //nolint:goerr113 // return error - return podIPInfo, fmt.Errorf("not enough IPs available for %s, waiting on Azure CNS to allocate more", ncID) + return podIPInfo, errors.Errorf("not enough IPs available for %s, waiting on Azure CNS to allocate more", ncID) } } From f5531a0a1d93844827f5cc1fd1db1574ca840e84 Mon Sep 17 00:00:00 2001 From: Ashish Nair Date: Tue, 19 Sep 2023 23:06:50 +0000 Subject: [PATCH 15/17] Updating the cns reconcillation logic to skip if there is a failure updating the NC and there are no IPs allocated for the NC --- cns/kubecontroller/nodenetworkconfig/reconciler.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/cns/kubecontroller/nodenetworkconfig/reconciler.go b/cns/kubecontroller/nodenetworkconfig/reconciler.go index a21772cb55..7382b97d21 100644 --- a/cns/kubecontroller/nodenetworkconfig/reconciler.go +++ b/cns/kubecontroller/nodenetworkconfig/reconciler.go @@ -86,6 +86,14 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco // for each NC, parse it in to a CreateNCRequest and forward it to the appropriate Listener for i := range nnc.Status.NetworkContainers { + // if the NC status has not been updated successfully skip the reconcilliation + noIPs := nnc.Status.NetworkContainers[i].IPAssignments == nil || len(nnc.Status.NetworkContainers[i].IPAssignments) == 0 + if nnc.Status.NetworkContainers[i].Status != v1alpha.NCUpdateSuccess && noIPs { + logger.Printf("[cns-rc] skipping network container %s found in NNC because NC update status has the error %v", + nnc.Status.NetworkContainers[i].ID, nnc.Status.NetworkContainers[i].Status) + continue + } + // check if this NC matches the Node IP if we have one to check against if r.nodeIP != "" { if r.nodeIP != nnc.Status.NetworkContainers[i].NodeIP { From c14cd0776063b1307f998a46f7fe26f04e63d3a0 Mon Sep 17 00:00:00 2001 From: Ashish Nair Date: Wed, 20 Sep 2023 18:30:22 +0000 Subject: [PATCH 16/17] Handled PR comments: * Updated the code to have the NC status be part of the error directly so that it can be consumed by containerD and cx can perform actions on it. * Code update to not use dynamic slices. * Removed the logic which handled 0 IPs allocated to NNC in CNS reconcile Signed-off-by: GitHub --- .../nodenetworkconfig/reconciler.go | 8 -------- cns/restserver/ipam.go | 15 +++++++-------- 2 files changed, 7 insertions(+), 16 deletions(-) diff --git a/cns/kubecontroller/nodenetworkconfig/reconciler.go b/cns/kubecontroller/nodenetworkconfig/reconciler.go index 7382b97d21..a21772cb55 100644 --- a/cns/kubecontroller/nodenetworkconfig/reconciler.go +++ b/cns/kubecontroller/nodenetworkconfig/reconciler.go @@ -86,14 +86,6 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco // for each NC, parse it in to a CreateNCRequest and forward it to the appropriate Listener for i := range nnc.Status.NetworkContainers { - // if the NC status has not been updated successfully skip the reconcilliation - noIPs := nnc.Status.NetworkContainers[i].IPAssignments == nil || len(nnc.Status.NetworkContainers[i].IPAssignments) == 0 - if nnc.Status.NetworkContainers[i].Status != v1alpha.NCUpdateSuccess && noIPs { - logger.Printf("[cns-rc] skipping network container %s found in NNC because NC update status has the error %v", - nnc.Status.NetworkContainers[i].ID, nnc.Status.NetworkContainers[i].Status) - continue - } - // check if this NC matches the Node IP if we have one to check against if r.nodeIP != "" { if r.nodeIP != nnc.Status.NetworkContainers[i].NodeIP { diff --git a/cns/restserver/ipam.go b/cns/restserver/ipam.go index 4939b91b3e..db784d8ecc 100644 --- a/cns/restserver/ipam.go +++ b/cns/restserver/ipam.go @@ -14,7 +14,6 @@ import ( "github.com/Azure/azure-container-networking/cns/logger" "github.com/Azure/azure-container-networking/cns/types" "github.com/Azure/azure-container-networking/common" - "github.com/Azure/azure-container-networking/crd/nodenetworkconfig/api/v1alpha" "github.com/pkg/errors" ) @@ -792,9 +791,11 @@ func (service *HTTPRestService) AssignAvailableIPConfigs(podInfo cns.PodInfo) ([ return nil, ErrNoNCs } // slice to store NCIDs - ncIDs := make([]string, 0) - for key := range service.state.ContainerStatus { - ncIDs = append(ncIDs, key) + ncIDs := make([]string, numOfNCs) + i := 0 + for ncID := range service.state.ContainerStatus { + ncIDs[i] = ncID + i++ } service.Lock() @@ -827,10 +828,8 @@ func (service *HTTPRestService) AssignAvailableIPConfigs(podInfo cns.PodInfo) ([ if _, found := ipsToAssign[ncID]; found { continue } - if service.state.ContainerStatus[ncID].CreateNetworkContainerRequest.NCStatus == v1alpha.NCStatusSubnetFull { - return podIPInfo, errors.Errorf("not enough IPs available for %s, waiting on Azure CNS to allocate more but the Subnet is full", ncID) - } - return podIPInfo, errors.Errorf("not enough IPs available for %s, waiting on Azure CNS to allocate more", ncID) + return podIPInfo, errors.Errorf("not enough IPs available for %s, waiting on Azure CNS to allocate more with NC Status: %s", + ncID, string(service.state.ContainerStatus[ncID].CreateNetworkContainerRequest.NCStatus)) } } From 8e9ba9890cdbfaab27feb0200cc268aca499900a Mon Sep 17 00:00:00 2001 From: Ashish Nair Date: Wed, 20 Sep 2023 19:08:51 +0000 Subject: [PATCH 17/17] Addressed the PR comment which helped delete a block of code to store ncIDs and also added more error codes to the NCStatus --- cns/restserver/ipam.go | 10 +--------- crd/nodenetworkconfig/api/v1alpha/nodenetworkconfig.go | 10 ++++++---- .../manifests/acn.azure.com_nodenetworkconfigs.yaml | 6 +++++- 3 files changed, 12 insertions(+), 14 deletions(-) diff --git a/cns/restserver/ipam.go b/cns/restserver/ipam.go index db784d8ecc..93984a8578 100644 --- a/cns/restserver/ipam.go +++ b/cns/restserver/ipam.go @@ -790,14 +790,6 @@ func (service *HTTPRestService) AssignAvailableIPConfigs(podInfo cns.PodInfo) ([ if numOfNCs == 0 { return nil, ErrNoNCs } - // slice to store NCIDs - ncIDs := make([]string, numOfNCs) - i := 0 - for ncID := range service.state.ContainerStatus { - ncIDs[i] = ncID - i++ - } - service.Lock() defer service.Unlock() // Creates a slice of PodIpInfo with the size as number of NCs to hold the result for assigned IP configs @@ -824,7 +816,7 @@ func (service *HTTPRestService) AssignAvailableIPConfigs(podInfo cns.PodInfo) ([ // Checks to make sure we found one IP for each NC if len(ipsToAssign) != numOfNCs { - for _, ncID := range ncIDs { + for ncID := range service.state.ContainerStatus { if _, found := ipsToAssign[ncID]; found { continue } diff --git a/crd/nodenetworkconfig/api/v1alpha/nodenetworkconfig.go b/crd/nodenetworkconfig/api/v1alpha/nodenetworkconfig.go index 8c9b5909ab..02add068c0 100644 --- a/crd/nodenetworkconfig/api/v1alpha/nodenetworkconfig.go +++ b/crd/nodenetworkconfig/api/v1alpha/nodenetworkconfig.go @@ -59,14 +59,16 @@ const ( ) // NCStatus indicates the latest NC request status -// +kubebuilder:validation:Enum=SubnetFull +// +kubebuilder:validation:Enum=NCUpdateSubnetFullError;NCUpdateInternalServerError;NCUpdateUnauthorizedError;NCUpdateSuccess;NCUpdateFailed // +kubebuilder:validation:Optional type NCStatus string const ( - NCStatusSubnetFull NCStatus = "SubnetFull" - NCUpdateSuccess NCStatus = "UpdateSuccess" - NCUpdateFailed NCStatus = "UpdateFailed" + NCUpdateSubnetFull NCStatus = "NCUpdateSubnetFullError" + NCUpdateInternalServerError NCStatus = "NCUpdateInternalServerError" + NCUpdateUnauthorizedError NCStatus = "NCUpdateUnauthorizedError" + NCUpdateSuccess NCStatus = "NCUpdateSuccess" + NCUpdateFailed NCStatus = "NCUpdateFailed" ) // NodeNetworkConfigStatus defines the observed state of NetworkConfig diff --git a/crd/nodenetworkconfig/manifests/acn.azure.com_nodenetworkconfigs.yaml b/crd/nodenetworkconfig/manifests/acn.azure.com_nodenetworkconfigs.yaml index c9a8949715..00f8a8a0d4 100644 --- a/crd/nodenetworkconfig/manifests/acn.azure.com_nodenetworkconfigs.yaml +++ b/crd/nodenetworkconfig/manifests/acn.azure.com_nodenetworkconfigs.yaml @@ -118,7 +118,11 @@ spec: status: description: NCStatus indicates the latest NC request status enum: - - SubnetFull + - NCUpdateSubnetFullError + - NCUpdateInternalServerError + - NCUpdateUnauthorizedError + - NCUpdateSuccess + - NCUpdateFailed type: string subcriptionID: type: string