Skip to content

Commit df8f231

Browse files
tssuryaopenshift-cherrypick-robot
authored andcommitted
Change the capacity struct from int to ptrOfInt
Today, in CNCC we store the capacity values as integers: type capacity struct { IPv4 int `json:"ipv4,omitempty"` IPv6 int `json:"ipv6,omitempty"` IP int `json:"ip,omitempty"` } When capacity is full, CNCC sets the value to 0. Also, depending on the platform it also ignores setting fields it doesn't care about (example AWS doesn't use IP, gcp and azure don't use IPv4 and IPv6). However given we have omitempty set, this was omitting the zero value in the annotation. When OVN-Kubernetes reads this annotation it was then setting the capacity to unlimited: nodeEgressIPConfig := []nodeEgressIPConfiguration{ { Capacity: Capacity{ IP: UnlimitedNodeCapacity, IPv4: UnlimitedNodeCapacity, --> we set this to maxint32 IPv6: UnlimitedNodeCapacity, }, }, } which is causing all EgressIPs to be assigned to this node leading to: status: conditions: - lastTransitionTime: "2025-10-06T19:24:24Z" message: "Error processing cloud assignment request, err: PrivateIpAddressLimitExceeded: Number of private addresses will exceed limit.\n\tstatus code: 400, request id: 457f4332-e9c4-44c9-bfcf-deeb5e7e43ce" In this fix, what we really want is to remove omitempty so that the zero capacity gets reflected correctly, however doing so also means fields that are unset will also be zero which can lead to confusion. Basically we are not able to distinguish between unset field and 0 value fields. Hence we are changing the capacity struct to be pointer type values so that null/nil means unset and 0 means full capacity. We still keep the omitempty since we don't need to do anything with unset fields - there is no behaviour change there and OVN-Kubernetes will continue to treat that as unlimited capacity. Upgrades: CNCC upon reboot seems to call: func (n *NodeController) SyncHandler(key string) error { .... // Filter out cloudPrivateIPConfigs assigned to node (key) and write the entry // into same slice starting from index 0, finally chop off unwanted entries // when passing it into GetNodeEgressIPConfiguration. index := 0 for _, cloudPrivateIPConfig := range cloudPrivateIPConfigs { if isAssignedCloudPrivateIPConfigOnNode(cloudPrivateIPConfig, key) { cloudPrivateIPConfigs[index] = cloudPrivateIPConfig index++ } } nodeEgressIPConfigs, err := n.cloudProviderClient.GetNodeEgressIPConfiguration(node, cloudPrivateIPConfigs[:index]) if err != nil { return fmt.Errorf("error retrieving the private IP configuration for node: %s, err: %v", node.Name, err) } return n.SetNodeEgressIPConfigAnnotation(node, nodeEgressIPConfigs) } // SetCloudPrivateIPConfigAnnotationOnNode annotates the corev1.Node with the cloud subnet information and capacity func (n *NodeController) SetNodeEgressIPConfigAnnotation(node *corev1.Node, nodeEgressIPConfigs []*cloudprovider.NodeEgressIPConfiguration) error { annotation, err := n.generateAnnotation(nodeEgressIPConfigs) if err != nil { return err } klog.Infof("Setting annotation: '%s: %s' on node: %s", nodeEgressIPConfigAnnotationKey, annotation, node.Name) return retry.RetryOnConflict(retry.DefaultRetry, func() error { ctx, cancel := context.WithTimeout(n.ctx, controller.ClientTimeout) defer cancel() // See: updateCloudPrivateIPConfigStatus nodeLatest, err := n.kubeClient.CoreV1().Nodes().Get(ctx, node.Name, metav1.GetOptions{}) if err != nil { return err } existingAnnotations := nodeLatest.Annotations existingAnnotations[nodeEgressIPConfigAnnotationKey] = annotation nodeLatest.SetAnnotations(existingAnnotations) _, err = n.kubeClient.CoreV1().Nodes().Update(ctx, nodeLatest, metav1.UpdateOptions{}) return err }) } and we seem to be overwriting the annotation - so we should be good on upgrades in changing from older annotations to new annotations - where 0 valued fields will appear for full capacity nodes. Once that happens, OVN-Kubernetes should overrite the UnlimitedValue to value 0 tat indicates 0 capacity and we should enter: if eNode.egressIPConfig.Capacity.IP < util.UnlimitedNodeCapacity { if eNode.egressIPConfig.Capacity.IP-len(eNode.allocations) <= 0 { klog.V(5).Infof("Additional allocation on Node: %s exhausts it's IP capacity, trying another node", eNode.name) continue } } if eNode.egressIPConfig.Capacity.IPv4 < util.UnlimitedNodeCapacity && utilnet.IsIPv4(eIP) { if eNode.egressIPConfig.Capacity.IPv4-getIPFamilyAllocationCount(eNode.allocations, false) <= 0 { klog.V(5).Infof("Additional allocation on Node: %s exhausts it's IPv4 capacity, trying another node", eNode.name) continue } } if eNode.egressIPConfig.Capacity.IPv6 < util.UnlimitedNodeCapacity && utilnet.IsIPv6(eIP) { if eNode.egressIPConfig.Capacity.IPv6-getIPFamilyAllocationCount(eNode.allocations, true) <= 0 { klog.V(5).Infof("Additional allocation on Node: %s exhausts it's IPv6 capacity, trying another node", eNode.name) continue } } these desired conditions correctly. Signed-off-by: Surya Seetharaman <[email protected]>
1 parent 467e50f commit df8f231

File tree

6 files changed

+31
-21
lines changed

6 files changed

+31
-21
lines changed

pkg/cloudprovider/aws.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"k8s.io/apimachinery/pkg/util/wait"
2323
"k8s.io/klog/v2"
2424
utilnet "k8s.io/utils/net"
25+
"k8s.io/utils/ptr"
2526
)
2627

2728
const (
@@ -218,8 +219,9 @@ func (a *AWS) GetNodeEgressIPConfiguration(node *corev1.Node, cloudPrivateIPConf
218219
return nil, err
219220
}
220221
config.Capacity = capacity{
221-
IPv4: capV4,
222-
IPv6: capV6,
222+
IPv4: ptr.To(capV4),
223+
IPv6: ptr.To(capV6),
224+
// IP field not used by AWS (uses per-IP-family capacity)
223225
}
224226
return []*NodeEgressIPConfiguration{config}, nil
225227
}

pkg/cloudprovider/azure.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,15 @@ package cloudprovider
33
import (
44
"context"
55
"fmt"
6-
"github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime"
7-
"k8s.io/utils/ptr"
86
"net"
97
"os"
108
"strings"
119
"sync"
1210
"time"
1311

12+
"github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime"
13+
"k8s.io/utils/ptr"
14+
1415
"github.com/Azure/azure-sdk-for-go/sdk/azcore"
1516
"github.com/Azure/azure-sdk-for-go/sdk/azcore/arm"
1617
"github.com/Azure/azure-sdk-for-go/sdk/azcore/cloud"
@@ -351,7 +352,8 @@ func (a *Azure) GetNodeEgressIPConfiguration(node *corev1.Node, cloudPrivateIPCo
351352
config.IFAddr.IPv6 = v6Subnet.String()
352353
}
353354
config.Capacity = capacity{
354-
IP: a.getCapacity(networkInterface, len(cloudPrivateIPConfigs)),
355+
// IPv4 and IPv6 fields not used by Azure (uses IP-family-agnostic capacity)
356+
IP: ptr.To(a.getCapacity(networkInterface, len(cloudPrivateIPConfigs))),
355357
}
356358
return []*NodeEgressIPConfiguration{config}, nil
357359
}

pkg/cloudprovider/cloudprovider.go

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,15 @@ import (
44
"context"
55
"errors"
66
"fmt"
7-
apifeatures "github.com/openshift/api/features"
8-
configv1 "github.com/openshift/api/config/v1"
9-
"github.com/openshift/library-go/pkg/operator/configobserver/featuregates"
107
"net"
118
"os"
129
"path/filepath"
1310
"sync"
1411

12+
configv1 "github.com/openshift/api/config/v1"
13+
apifeatures "github.com/openshift/api/features"
14+
"github.com/openshift/library-go/pkg/operator/configobserver/featuregates"
15+
1516
v1 "github.com/openshift/api/cloudnetwork/v1"
1617
corev1 "k8s.io/api/core/v1"
1718
)
@@ -102,9 +103,9 @@ type ifAddr struct {
102103
}
103104

104105
type capacity struct {
105-
IPv4 int `json:"ipv4,omitempty"`
106-
IPv6 int `json:"ipv6,omitempty"`
107-
IP int `json:"ip,omitempty"`
106+
IPv4 *int `json:"ipv4,omitempty"`
107+
IPv6 *int `json:"ipv6,omitempty"`
108+
IP *int `json:"ip,omitempty"`
108109
}
109110

110111
// NodeEgressIPConfiguration stores details - specific to each cloud - which are

pkg/cloudprovider/gcp.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"google.golang.org/api/option"
1313
corev1 "k8s.io/api/core/v1"
1414
utilnet "k8s.io/utils/net"
15+
"k8s.io/utils/ptr"
1516
)
1617

1718
const (
@@ -182,7 +183,8 @@ func (g *GCP) GetNodeEgressIPConfiguration(node *corev1.Node, cloudPrivateIPConf
182183
config.IFAddr.IPv6 = v6Subnet.String()
183184
}
184185
config.Capacity = capacity{
185-
IP: g.getCapacity(networkInterface, len(cloudPrivateIPConfigs)),
186+
// IPv4 and IPv6 fields not used by GCP (uses IP-family-agnostic capacity)
187+
IP: ptr.To(g.getCapacity(networkInterface, len(cloudPrivateIPConfigs))),
186188
}
187189
return []*NodeEgressIPConfiguration{config}, nil //nolint:staticcheck
188190
}

pkg/cloudprovider/openstack.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import (
3131
"k8s.io/client-go/util/retry"
3232
"k8s.io/klog/v2"
3333
utilnet "k8s.io/utils/net"
34+
"k8s.io/utils/ptr"
3435
)
3536

3637
const (
@@ -608,7 +609,8 @@ func (o *OpenStack) getNeutronPortNodeEgressIPConfiguration(p neutronports.Port,
608609
IPv6: ipv6,
609610
},
610611
Capacity: capacity{
611-
IP: c,
612+
// IPv4 and IPv6 fields not used by OpenStack (uses IP-family-agnostic capacity)
613+
IP: ptr.To(c),
612614
},
613615
}, nil
614616
}

pkg/cloudprovider/openstack_test.go

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
corev1 "k8s.io/api/core/v1"
2323
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2424
"k8s.io/apimachinery/pkg/util/sets"
25+
"k8s.io/utils/ptr"
2526
)
2627

2728
const (
@@ -677,7 +678,7 @@ func TestOpenStackPlugin(t *testing.T) {
677678
IPv6: "2000::/64",
678679
},
679680
Capacity: capacity{
680-
IP: openstackMaxCapacity,
681+
IP: ptr.To(openstackMaxCapacity),
681682
},
682683
},
683684
}
@@ -804,7 +805,7 @@ func TestGetNodeEgressIPConfiguration(t *testing.T) {
804805
IPv6: "2000::/64",
805806
},
806807
Capacity: capacity{
807-
IP: openstackMaxCapacity,
808+
IP: ptr.To(openstackMaxCapacity),
808809
},
809810
},
810811
},
@@ -840,7 +841,7 @@ func TestGetNodeEgressIPConfiguration(t *testing.T) {
840841
IPv6: "2001::/64",
841842
},
842843
Capacity: capacity{
843-
IP: openstackMaxCapacity,
844+
IP: ptr.To(openstackMaxCapacity),
844845
},
845846
},
846847
},
@@ -884,7 +885,7 @@ func TestGetNodeEgressIPConfiguration(t *testing.T) {
884885
IPv6: "2000::/64",
885886
},
886887
Capacity: capacity{
887-
IP: openstackMaxCapacity,
888+
IP: ptr.To(openstackMaxCapacity),
888889
},
889890
},
890891
},
@@ -931,7 +932,7 @@ func TestGetNodeEgressIPConfiguration(t *testing.T) {
931932
IPv6: "2000::/64",
932933
},
933934
Capacity: capacity{
934-
IP: openstackMaxCapacity,
935+
IP: ptr.To(openstackMaxCapacity),
935936
},
936937
},
937938
},
@@ -943,7 +944,7 @@ func TestGetNodeEgressIPConfiguration(t *testing.T) {
943944
IPv6: "2001::/64",
944945
},
945946
Capacity: capacity{
946-
IP: openstackMaxCapacity,
947+
IP: ptr.To(openstackMaxCapacity),
947948
},
948949
},
949950
},
@@ -1028,7 +1029,7 @@ func TestGetNeutronPortNodeEgressIPConfiguration(t *testing.T) {
10281029
IPv6: "2000::/64",
10291030
},
10301031
Capacity: capacity{
1031-
IP: openstackMaxCapacity - 2, // 2 allowed_address_pairs configured on the port.
1032+
IP: ptr.To(openstackMaxCapacity - 2), // 2 allowed_address_pairs configured on the port.
10321033
},
10331034
},
10341035
},
@@ -1041,7 +1042,7 @@ func TestGetNeutronPortNodeEgressIPConfiguration(t *testing.T) {
10411042
IPv6: "2000::/64",
10421043
},
10431044
Capacity: capacity{
1044-
IP: openstackMaxCapacity + 3 - 2, // excluding 2 allowed_address_pairs configured on the port.
1045+
IP: ptr.To(openstackMaxCapacity + 3 - 2), // excluding 2 allowed_address_pairs configured on the port.
10451046
},
10461047
},
10471048
// Configure cloudPrivateIPConfigs with 3 ips are within neutron subnet, 1 ip outside neutron subnet.

0 commit comments

Comments
 (0)