From 9b1da7e974658e44120e25726c73a287d6fc3e15 Mon Sep 17 00:00:00 2001 From: Ashish Nair Date: Fri, 8 Sep 2023 11:39:17 -0700 Subject: [PATCH 1/3] Feat: Created a Network Container Status Section to be updated with the latest error code (#2193) * Added Network Conatiner Status to include the latest error code for a Network Container * Updated the crd to have the Status field included into the Network Container * Updated the names and added Status and ErrorText as two fields in NC Status * Fixed the casing and json values for these variables * Added error code to the NC Status and removed the latest prefix from the varibale names * Removed the timestamp variable from the NC Status * Moved the Status object inside the NC Status to be able to accurately define the status of each NC for the node * Changed to having an enum representing the NC Status which DNC-RC will update after inferring the error and CNS can use this field to propagate and NCRequest failures * Made the validation of the new enum optional to keep it backward compatible --- .../api/v1alpha/nodenetworkconfig.go | 22 ++++++++++++++----- .../acn.azure.com_nodenetworkconfigs.yaml | 5 +++++ 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/crd/nodenetworkconfig/api/v1alpha/nodenetworkconfig.go b/crd/nodenetworkconfig/api/v1alpha/nodenetworkconfig.go index 4b96ea5ff6..8f91bf154d 100644 --- a/crd/nodenetworkconfig/api/v1alpha/nodenetworkconfig.go +++ b/crd/nodenetworkconfig/api/v1alpha/nodenetworkconfig.go @@ -58,6 +58,15 @@ const ( Error Status = "Error" ) +// NCStatus indicates the latest NC request status +// +kubebuilder:validation:Enum=SubnetFull +// +kubebuilder:validation:Optional +type NCStatus string + +const ( + NCStatusSubnetFull NCStatus = "SubnetFull" +) + // NodeNetworkConfigStatus defines the observed state of NetworkConfig type NodeNetworkConfigStatus struct { // +kubebuilder:default=0 @@ -107,12 +116,13 @@ type NetworkContainer struct { SubnetAddressSpace string `json:"subnetAddressSpace,omitempty"` // +kubebuilder:default=0 // +kubebuilder:validation:Optional - Version int64 `json:"version"` - NodeIP string `json:"nodeIP,omitempty"` - SubscriptionID string `json:"subcriptionID,omitempty"` - ResourceGroupID string `json:"resourceGroupID,omitempty"` - VNETID string `json:"vnetID,omitempty"` - SubnetID string `json:"subnetID,omitempty"` + Version int64 `json:"version"` + NodeIP string `json:"nodeIP,omitempty"` + SubscriptionID string `json:"subcriptionID,omitempty"` + ResourceGroupID string `json:"resourceGroupID,omitempty"` + VNETID string `json:"vnetID,omitempty"` + SubnetID string `json:"subnetID,omitempty"` + Status NCStatus `json:"status,omitempty"` } // IPAssignment groups an IP address and Name. Name is a UUID set by the the IP address assigner. diff --git a/crd/nodenetworkconfig/manifests/acn.azure.com_nodenetworkconfigs.yaml b/crd/nodenetworkconfig/manifests/acn.azure.com_nodenetworkconfigs.yaml index 901b878280..04b5a01f4d 100644 --- a/crd/nodenetworkconfig/manifests/acn.azure.com_nodenetworkconfigs.yaml +++ b/crd/nodenetworkconfig/manifests/acn.azure.com_nodenetworkconfigs.yaml @@ -115,6 +115,11 @@ spec: type: string resourceGroupID: type: string + status: + description: NCStatus indicates the latest NC request status + enum: + - SubnetFull + type: string subcriptionID: type: string subnetAddressSpace: From 7177c07b0deaded6c4c40a6bc232a98f5a98b599 Mon Sep 17 00:00:00 2001 From: Ashish Nair Date: Thu, 21 Sep 2023 19:50:29 -0700 Subject: [PATCH 2/3] feat: Consume the NCStatus to be able to append subnet is full error to Pod IP reservation failures (#2202) * Added Network Conatiner Status to include the latest error code for a Network Container * Updated the crd to have the Status field included into the Network Container * Updated the names and added Status and ErrorText as two fields in NC Status * Fixed the casing and json values for these variables * Propagated the NC Status inside the CNS and IPAM Monitor pool states * Fixed the lint error of missing comma * Saved the updated NC Status into the CNS statefile * Updated the IP assignment to check and error out subnet is Full when there are no more available IPs for CNS to assign * Fixed a minor compilation issue * Fixed lint failures * Fixed lint failures * Removed the reference from the metastate of the ipam monitor * 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 * Updated the error to use errors pkg instead of fmt * Updating the cns reconcillation logic to skip if there is a failure updating the NC and there are no IPs allocated for the NC * 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 * Addressed the PR comment which helped delete a block of code to store ncIDs and also added more error codes to the NCStatus --------- Signed-off-by: GitHub --- cns/NetworkContainerContract.go | 6 ++++-- cns/kubecontroller/nodenetworkconfig/conversion.go | 1 + cns/kubecontroller/nodenetworkconfig/conversion_linux.go | 3 ++- .../nodenetworkconfig/conversion_windows.go | 3 ++- crd/nodenetworkconfig/api/v1alpha/nodenetworkconfig.go | 8 ++++++-- .../manifests/acn.azure.com_nodenetworkconfigs.yaml | 6 +++++- 6 files changed, 20 insertions(+), 7 deletions(-) diff --git a/cns/NetworkContainerContract.go b/cns/NetworkContainerContract.go index f945a73212..fefc4f9f14 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" ) @@ -92,6 +93,7 @@ type CreateNetworkContainerRequest struct { AllowHostToNCCommunication bool AllowNCToHostCommunication bool EndpointPolicies []NetworkContainerRequestPolicies + NCStatus v1alpha.NCStatus } // CreateNetworkContainerRequest implements fmt.Stringer for logging @@ -99,9 +101,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/kubecontroller/nodenetworkconfig/conversion.go b/cns/kubecontroller/nodenetworkconfig/conversion.go index 808a25e168..68590153c6 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.Status, }, nil } diff --git a/cns/kubecontroller/nodenetworkconfig/conversion_linux.go b/cns/kubecontroller/nodenetworkconfig/conversion_linux.go index 83b44a736f..d0e3a91458 100644 --- a/cns/kubecontroller/nodenetworkconfig/conversion_linux.go +++ b/cns/kubecontroller/nodenetworkconfig/conversion_linux.go @@ -32,5 +32,6 @@ func createNCRequestFromStaticNCHelper(nc v1alpha.NetworkContainer, primaryIPPre IPSubnet: subnet, GatewayIPAddress: nc.DefaultGateway, }, - } + NCStatus: nc.Status, + }, nil } diff --git a/cns/kubecontroller/nodenetworkconfig/conversion_windows.go b/cns/kubecontroller/nodenetworkconfig/conversion_windows.go index d44e59f150..803f7b6124 100644 --- a/cns/kubecontroller/nodenetworkconfig/conversion_windows.go +++ b/cns/kubecontroller/nodenetworkconfig/conversion_windows.go @@ -44,5 +44,6 @@ func createNCRequestFromStaticNCHelper(nc v1alpha.NetworkContainer, primaryIPPre IPSubnet: subnet, GatewayIPAddress: nc.DefaultGateway, }, - } + NCStatus: nc.Status, + }, nil } diff --git a/crd/nodenetworkconfig/api/v1alpha/nodenetworkconfig.go b/crd/nodenetworkconfig/api/v1alpha/nodenetworkconfig.go index 8f91bf154d..c23b6cbf4d 100644 --- a/crd/nodenetworkconfig/api/v1alpha/nodenetworkconfig.go +++ b/crd/nodenetworkconfig/api/v1alpha/nodenetworkconfig.go @@ -59,12 +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" + 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 04b5a01f4d..deeb3246ff 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 From 6285b90381ff8fc16ff33fb892b343b7ce61e234 Mon Sep 17 00:00:00 2001 From: Ashish Nair Date: Mon, 25 Sep 2023 23:04:34 +0000 Subject: [PATCH 3/3] Fixed all the compilation failures in the files after chery-pick --- cns/kubecontroller/nodenetworkconfig/conversion_linux.go | 2 +- cns/kubecontroller/nodenetworkconfig/conversion_windows.go | 2 +- cns/restserver/ipam.go | 5 ++++- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/cns/kubecontroller/nodenetworkconfig/conversion_linux.go b/cns/kubecontroller/nodenetworkconfig/conversion_linux.go index d0e3a91458..25548354b9 100644 --- a/cns/kubecontroller/nodenetworkconfig/conversion_linux.go +++ b/cns/kubecontroller/nodenetworkconfig/conversion_linux.go @@ -33,5 +33,5 @@ func createNCRequestFromStaticNCHelper(nc v1alpha.NetworkContainer, primaryIPPre GatewayIPAddress: nc.DefaultGateway, }, NCStatus: nc.Status, - }, nil + } } diff --git a/cns/kubecontroller/nodenetworkconfig/conversion_windows.go b/cns/kubecontroller/nodenetworkconfig/conversion_windows.go index 803f7b6124..50f9002a6e 100644 --- a/cns/kubecontroller/nodenetworkconfig/conversion_windows.go +++ b/cns/kubecontroller/nodenetworkconfig/conversion_windows.go @@ -45,5 +45,5 @@ func createNCRequestFromStaticNCHelper(nc v1alpha.NetworkContainer, primaryIPPre GatewayIPAddress: nc.DefaultGateway, }, NCStatus: nc.Status, - }, nil + } } diff --git a/cns/restserver/ipam.go b/cns/restserver/ipam.go index fb3eaca631..a028ce1159 100644 --- a/cns/restserver/ipam.go +++ b/cns/restserver/ipam.go @@ -542,7 +542,9 @@ func (service *HTTPRestService) AssignAnyAvailableIPConfig(podInfo cns.PodInfo) service.Lock() defer service.Unlock() + var ncID string for _, ipState := range service.PodIPConfigState { + ncID = ipState.NCID if ipState.GetState() == types.Available { if err := service.assignIPConfig(ipState, podInfo); err != nil { return cns.PodIpInfo{}, err @@ -557,7 +559,8 @@ func (service *HTTPRestService) AssignAnyAvailableIPConfig(podInfo cns.PodInfo) } } //nolint:goerr113 - return cns.PodIpInfo{}, fmt.Errorf("no IPs available, waiting on Azure CNS to allocate more") + return cns.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)) } // If IPConfig is already assigned to pod, it returns that else it returns one of the available ipconfigs.