Skip to content

Commit

Permalink
Adjust CIDRPool to use int32 instead of uint fields
Browse files Browse the repository at this point in the history
Signed-off-by: Vasilis Remmas <[email protected]>
  • Loading branch information
vasrem committed Aug 6, 2024
1 parent ef6ff29 commit f069e36
Show file tree
Hide file tree
Showing 10 changed files with 52 additions and 42 deletions.
49 changes: 24 additions & 25 deletions api/v1alpha1/cidrpool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
gomegaTypes "github.com/onsi/gomega/types"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/utils/ptr"

"github.com/Mellanox/nvidia-k8s-ipam/api/v1alpha1"
)
Expand All @@ -39,18 +40,14 @@ func validatePoolAndCheckErr(pool *v1alpha1.CIDRPool, isValid bool, errMatcher .
}
}

func getUintPtr(i uint) *uint {
return &i
}

var _ = Describe("CIDRPool", func() {
It("Valid IPv4 pool", func() {
cidrPool := v1alpha1.CIDRPool{
ObjectMeta: metav1.ObjectMeta{Name: "test"},
Spec: v1alpha1.CIDRPoolSpec{
CIDR: "192.168.0.0/16",
PerNodeNetworkPrefix: 24,
GatewayIndex: getUintPtr(100),
GatewayIndex: ptr.To[int32](100),
Exclusions: []v1alpha1.ExcludeRange{
{StartIP: "192.168.0.10", EndIP: "192.168.0.20"},
{StartIP: "192.168.0.25", EndIP: "192.168.0.25"},
Expand Down Expand Up @@ -78,7 +75,7 @@ var _ = Describe("CIDRPool", func() {
Spec: v1alpha1.CIDRPoolSpec{
CIDR: "fdf8:6aef:d1fe::/48",
PerNodeNetworkPrefix: 120,
GatewayIndex: getUintPtr(5),
GatewayIndex: ptr.To[int32](5),
Exclusions: []v1alpha1.ExcludeRange{
{StartIP: "fdf8:6aef:d1fe::5", EndIP: "fdf8:6aef:d1fe::5"},
},
Expand All @@ -98,7 +95,7 @@ var _ = Describe("CIDRPool", func() {
validatePoolAndCheckErr(&cidrPool, true)
})
DescribeTable("CIDR",
func(cidr string, prefix uint, isValid bool) {
func(cidr string, prefix int32, isValid bool) {
cidrPool := v1alpha1.CIDRPool{
ObjectMeta: metav1.ObjectMeta{Name: "test"},
Spec: v1alpha1.CIDRPoolSpec{
Expand All @@ -107,15 +104,15 @@ var _ = Describe("CIDRPool", func() {
}}
validatePoolAndCheckErr(&cidrPool, isValid, ContainSubstring("spec.cidr"))
},
Entry("empty", "", uint(30), false),
Entry("invalid value", "aaaa", uint(30), false),
Entry("/32", "192.168.1.1/32", uint(32), false),
Entry("/128", "2001:db8:3333:4444::0/128", uint(128), false),
Entry("valid ipv4", "192.168.1.0/24", uint(30), true),
Entry("valid ipv6", "2001:db8:3333:4444::0/64", uint(120), true),
Entry("empty", "", int32(30), false),
Entry("invalid value", "aaaa", int32(30), false),
Entry("/32", "192.168.1.1/32", int32(32), false),
Entry("/128", "2001:db8:3333:4444::0/128", int32(128), false),
Entry("valid ipv4", "192.168.1.0/24", int32(30), true),
Entry("valid ipv6", "2001:db8:3333:4444::0/64", int32(120), true),
)
DescribeTable("PerNodeNetworkPrefix",
func(cidr string, prefix uint, isValid bool) {
func(cidr string, prefix int32, isValid bool) {
cidrPool := v1alpha1.CIDRPool{
ObjectMeta: metav1.ObjectMeta{Name: "test"},
Spec: v1alpha1.CIDRPoolSpec{
Expand All @@ -124,12 +121,13 @@ var _ = Describe("CIDRPool", func() {
}}
validatePoolAndCheckErr(&cidrPool, isValid, ContainSubstring("spec.perNodeNetworkPrefix"))
},
Entry("not set", "192.168.0.0/16", uint(0), false),
Entry("larger than CIDR", "192.168.0.0/16", uint(8), false),
Entry("smaller than 31 for IPv4 pool", "192.168.0.0/16", uint(32), false),
Entry("smaller than 127 for IPv6 pool", "2001:db8:3333:4444::0/64", uint(128), false),
Entry("match CIDR prefix size - ipv4", "192.168.0.0/16", uint(16), true),
Entry("match CIDR prefix size - ipv6", "2001:db8:3333:4444::0/64", uint(64), true),
Entry("not set", "192.168.0.0/16", int32(0), false),
Entry("negative", "192.168.0.0/16", int32(-10), false),
Entry("larger than CIDR", "192.168.0.0/16", int32(8), false),
Entry("smaller than 31 for IPv4 pool", "192.168.0.0/16", int32(32), false),
Entry("smaller than 127 for IPv6 pool", "2001:db8:3333:4444::0/64", int32(128), false),
Entry("match CIDR prefix size - ipv4", "192.168.0.0/16", int32(16), true),
Entry("match CIDR prefix size - ipv6", "2001:db8:3333:4444::0/64", int32(64), true),
)
DescribeTable("NodeSelector",
func(nodeSelector *corev1.NodeSelector, isValid bool, errMatcher ...gomegaTypes.GomegaMatcher) {
Expand All @@ -156,7 +154,7 @@ var _ = Describe("CIDRPool", func() {
}, false, ContainSubstring("spec.nodeSelectorTerms[0].matchExpressions[0].operator")),
)
DescribeTable("GatewayIndex",
func(gatewayIndex uint, isValid bool, errMatcher ...gomegaTypes.GomegaMatcher) {
func(gatewayIndex int32, isValid bool, errMatcher ...gomegaTypes.GomegaMatcher) {
cidrPool := v1alpha1.CIDRPool{
ObjectMeta: metav1.ObjectMeta{Name: "test"},
Spec: v1alpha1.CIDRPoolSpec{
Expand All @@ -167,9 +165,10 @@ var _ = Describe("CIDRPool", func() {
}
validatePoolAndCheckErr(&cidrPool, isValid, ContainSubstring("spec.gatewayIndex"))
},
Entry("too large", uint(255), false),
Entry("index 1 is valid for point to point", uint(1), true),
Entry("index 2 is valid for point to point", uint(2), true),
Entry("negative", int32(-10), false),
Entry("too large", int32(255), false),
Entry("index 1 is valid for point to point", int32(1), true),
Entry("index 2 is valid for point to point", int32(2), true),
)
DescribeTable("Exclusions",
func(exclusions []v1alpha1.ExcludeRange, isValid bool, errMatcher ...gomegaTypes.GomegaMatcher) {
Expand Down Expand Up @@ -273,7 +272,7 @@ var _ = Describe("CIDRPoolAllocation", func() {
Spec: v1alpha1.CIDRPoolSpec{
CIDR: "192.168.0.0/16",
PerNodeNetworkPrefix: 24,
GatewayIndex: getUintPtr(100),
GatewayIndex: ptr.To[int32](100),
StaticAllocations: []v1alpha1.CIDRPoolStaticAllocation{
{NodeName: "node1", Prefix: "192.168.1.0/24", Gateway: "192.168.1.10"},
{NodeName: "node2", Prefix: "192.168.2.0/24"},
Expand Down
4 changes: 2 additions & 2 deletions api/v1alpha1/cidrpool_type.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,10 @@ type CIDRPoolSpec struct {
// and distributed between matching nodes
CIDR string `json:"cidr"`
// use IP with this index from the host prefix as a gateway, skip gateway configuration if the value not set
GatewayIndex *uint `json:"gatewayIndex,omitempty"`
GatewayIndex *int32 `json:"gatewayIndex,omitempty"`
// size of the network prefix for each host, the network defined in "cidr" field will be split to multiple networks
// with this size.
PerNodeNetworkPrefix uint `json:"perNodeNetworkPrefix"`
PerNodeNetworkPrefix int32 `json:"perNodeNetworkPrefix"`
// contains reserved IP addresses that should not be allocated by nv-ipam
Exclusions []ExcludeRange `json:"exclusions,omitempty"`
// static allocations for the pool
Expand Down
15 changes: 13 additions & 2 deletions api/v1alpha1/cidrpool_validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,20 @@ func (r *CIDRPool) validateCIDR() field.ErrorList {
return field.ErrorList{field.Invalid(
field.NewPath("spec", "cidr"), r.Spec.CIDR, "single IP prefixes are not supported")}
}

if r.Spec.GatewayIndex != nil && *r.Spec.GatewayIndex < 0 {
return field.ErrorList{field.Invalid(
field.NewPath("spec", "gatewayIndex"), r.Spec.GatewayIndex, "must not be negative")}
}

if r.Spec.PerNodeNetworkPrefix < 0 {
return field.ErrorList{field.Invalid(
field.NewPath("spec", "perNodeNetworkPrefix"), r.Spec.PerNodeNetworkPrefix, "must not be negative")}
}

if r.Spec.PerNodeNetworkPrefix == 0 ||
r.Spec.PerNodeNetworkPrefix >= uint(bitsTotal) ||
r.Spec.PerNodeNetworkPrefix < uint(setBits) {
r.Spec.PerNodeNetworkPrefix >= int32(bitsTotal) ||
r.Spec.PerNodeNetworkPrefix < int32(setBits) {
return field.ErrorList{field.Invalid(
field.NewPath("spec", "perNodeNetworkPrefix"),
r.Spec.PerNodeNetworkPrefix, "must be less or equal than network prefix size in the \"cidr\" field")}
Expand Down
2 changes: 1 addition & 1 deletion api/v1alpha1/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import (
// - subnet 192.168.0.0/24, index 10, gateway = 102.168.1.10
// - subnet 192.168.1.2/31, index 0, gateway = 192.168.1.2 (point to point network can use network IP as a gateway)
// - subnet 192.168.33.0/24, index 900, gateway = "" (invalid config, index too large)
func GetGatewayForSubnet(subnet *net.IPNet, index uint) string {
func GetGatewayForSubnet(subnet *net.IPNet, index int32) string {
setBits, bitsTotal := subnet.Mask.Size()
maxIndex := GetPossibleIPCount(subnet)

Expand Down
2 changes: 1 addition & 1 deletion api/v1alpha1/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ var _ = Describe("Helpers", func() {
func(subnet string, index int, gw string) {
_, network, err := net.ParseCIDR(subnet)
Expect(err).NotTo(HaveOccurred())
ret := v1alpha1.GetGatewayForSubnet(network, uint(index))
ret := v1alpha1.GetGatewayForSubnet(network, int32(index))
Expect(ret).To(Equal(gw))
},
Entry("ipv4 - start range", "192.168.1.0/24", 1, "192.168.1.1"),
Expand Down
2 changes: 1 addition & 1 deletion api/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 4 additions & 6 deletions cmd/ipam-controller/app/app_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/klog/v2"
"k8s.io/utils/ptr"

ipamv1alpha1 "github.com/Mellanox/nvidia-k8s-ipam/api/v1alpha1"
"github.com/Mellanox/nvidia-k8s-ipam/cmd/ipam-controller/app"
Expand Down Expand Up @@ -306,15 +307,14 @@ var _ = Describe("App", func() {

It("CIDRPool Basic tests", func(ctx SpecContext) {
By("Create valid pools")
pool1gatewayIndex := uint(1)
pool1 := &ipamv1alpha1.CIDRPool{
ObjectMeta: metav1.ObjectMeta{
Name: pool1Name,
Namespace: TestNamespace,
},
Spec: ipamv1alpha1.CIDRPoolSpec{
CIDR: "10.10.0.0/16",
GatewayIndex: &pool1gatewayIndex,
GatewayIndex: ptr.To[int32](1),
PerNodeNetworkPrefix: 24,
Exclusions: []ipamv1alpha1.ExcludeRange{
{StartIP: "10.10.1.22", EndIP: "10.10.1.30"},
Expand All @@ -329,15 +329,14 @@ var _ = Describe("App", func() {
}
Expect(k8sClient.Create(ctx, pool1)).NotTo(HaveOccurred())

pool2gatewayIndex := uint(1)
pool2 := &ipamv1alpha1.CIDRPool{
ObjectMeta: metav1.ObjectMeta{
Name: pool2Name,
Namespace: TestNamespace,
},
Spec: ipamv1alpha1.CIDRPoolSpec{
CIDR: "192.168.0.0/16",
GatewayIndex: &pool2gatewayIndex,
GatewayIndex: ptr.To[int32](1),
PerNodeNetworkPrefix: 31,
},
}
Expand Down Expand Up @@ -415,15 +414,14 @@ var _ = Describe("App", func() {

By("Create Pool3, with selector which ignores all nodes")
// ranges for two nodes only can be allocated
pool3gatewayIndex := uint(1)
pool3 := &ipamv1alpha1.CIDRPool{
ObjectMeta: metav1.ObjectMeta{
Name: pool3Name,
Namespace: TestNamespace,
},
Spec: ipamv1alpha1.CIDRPoolSpec{
CIDR: "10.10.0.0/24",
GatewayIndex: &pool3gatewayIndex,
GatewayIndex: ptr.To[int32](1),
PerNodeNetworkPrefix: 25,
NodeSelector: &corev1.NodeSelector{
NodeSelectorTerms: []corev1.NodeSelectorTerm{
Expand Down
4 changes: 2 additions & 2 deletions cmd/ipam-node/app/app_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/klog/v2"
"k8s.io/utils/ptr"

nodev1 "github.com/Mellanox/nvidia-k8s-ipam/api/grpc/nvidia/ipam/node/v1"
ipamv1alpha1 "github.com/Mellanox/nvidia-k8s-ipam/api/v1alpha1"
Expand Down Expand Up @@ -84,12 +85,11 @@ func createTestIPPools() {
}

func createTestCIDRPools() {
pool1GatewayIndex := uint(1)
pool1 := &ipamv1alpha1.CIDRPool{
ObjectMeta: metav1.ObjectMeta{Name: testPoolName1, Namespace: testNamespace},
Spec: ipamv1alpha1.CIDRPoolSpec{
CIDR: "192.100.0.0/16",
GatewayIndex: &pool1GatewayIndex,
GatewayIndex: ptr.To[int32](1),
PerNodeNetworkPrefix: 24,
Exclusions: []ipamv1alpha1.ExcludeRange{
{StartIP: "192.100.0.1", EndIP: "192.100.0.10"},
Expand Down
2 changes: 2 additions & 0 deletions deploy/crds/nv-ipam.nvidia.com_cidrpools.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ spec:
gatewayIndex:
description: use IP with this index from the host prefix as a gateway,
skip gateway configuration if the value not set
format: int32
type: integer
nodeSelector:
description: selector for nodes, if empty match all nodes
Expand Down Expand Up @@ -157,6 +158,7 @@ spec:
description: size of the network prefix for each host, the network
defined in "cidr" field will be split to multiple networks with
this size.
format: int32
type: integer
staticAllocations:
description: static allocations for the pool
Expand Down
4 changes: 2 additions & 2 deletions pkg/ip/cidr.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,9 +178,9 @@ func LastIP(network *net.IPNet) net.IP {
// println(gen().String()) // 192.168.1.0/25
// println(gen().String()) // 192.168.1.128/25
// println(gen().String()) // <nil> - no more ranges available
func GetSubnetGen(network *net.IPNet, prefixSize uint) func() *net.IPNet {
func GetSubnetGen(network *net.IPNet, prefixSize int32) func() *net.IPNet {
networkOnes, netBitsTotal := network.Mask.Size()
if prefixSize < uint(networkOnes) || prefixSize > uint(netBitsTotal) {
if prefixSize < int32(networkOnes) || prefixSize > int32(netBitsTotal) {
return func() *net.IPNet { return nil }
}
isIPv6 := false
Expand Down

0 comments on commit f069e36

Please sign in to comment.