diff --git a/api/v1alpha1/ippool_test.go b/api/v1alpha1/ippool_test.go index 1523a70..90bc4d6 100644 --- a/api/v1alpha1/ippool_test.go +++ b/api/v1alpha1/ippool_test.go @@ -65,6 +65,16 @@ var _ = Describe("Validate", func() { } Expect(ipPool.Validate()).To(BeEmpty()) }) + It("Valid - no Gateway", func() { + ipPool := v1alpha1.IPPool{ + ObjectMeta: metav1.ObjectMeta{Name: "test"}, + Spec: v1alpha1.IPPoolSpec{ + Subnet: "192.168.0.0/16", + PerNodeBlockSize: 128, + }, + } + Expect(ipPool.Validate()).To(BeEmpty()) + }) It("Empty object", func() { ipPool := v1alpha1.IPPool{} Expect(ipPool.Validate().ToAggregate().Error()). @@ -72,7 +82,6 @@ var _ = Describe("Validate", func() { ContainSubstring("metadata.name"), ContainSubstring("spec.subnet"), ContainSubstring("spec.perNodeBlockSize"), - ContainSubstring("gateway"), )) }) It("Invalid - perNodeBlockSize is too large", func() { diff --git a/api/v1alpha1/ippool_type.go b/api/v1alpha1/ippool_type.go index 834921a..3df519e 100644 --- a/api/v1alpha1/ippool_type.go +++ b/api/v1alpha1/ippool_type.go @@ -40,7 +40,7 @@ type IPPoolSpec struct { // must be less than amount of available IPs in the subnet PerNodeBlockSize int `json:"perNodeBlockSize"` // gateway for the pool - Gateway string `json:"gateway"` + Gateway string `json:"gateway,omitempty"` // selector for nodes, if empty match all nodes NodeSelector *corev1.NodeSelector `json:"nodeSelector,omitempty"` } diff --git a/api/v1alpha1/ippool_validate.go b/api/v1alpha1/ippool_validate.go index f4d7daa..265dbaa 100644 --- a/api/v1alpha1/ippool_validate.go +++ b/api/v1alpha1/ippool_validate.go @@ -52,11 +52,14 @@ func (r *IPPool) Validate() field.ErrorList { "is larger then amount of IPs available in the subnet")) } } - parsedGW := net.ParseIP(r.Spec.Gateway) - if len(parsedGW) == 0 { - errList = append(errList, field.Invalid( - field.NewPath("spec", "gateway"), r.Spec.Gateway, - "is invalid IP address")) + var parsedGW net.IP + if r.Spec.Gateway != "" { + parsedGW = net.ParseIP(r.Spec.Gateway) + if len(parsedGW) == 0 { + errList = append(errList, field.Invalid( + field.NewPath("spec", "gateway"), r.Spec.Gateway, + "is invalid IP address")) + } } if network != nil && len(parsedGW) != 0 && !network.Contains(parsedGW) { diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index f74af3e..f8b97d4 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -19,7 +19,7 @@ package v1alpha1 import ( "k8s.io/api/core/v1" - runtime "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime" ) // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. diff --git a/deploy/crds/nv-ipam.nvidia.com_ippools.yaml b/deploy/crds/nv-ipam.nvidia.com_ippools.yaml index 0190190..c786905 100644 --- a/deploy/crds/nv-ipam.nvidia.com_ippools.yaml +++ b/deploy/crds/nv-ipam.nvidia.com_ippools.yaml @@ -139,7 +139,6 @@ spec: description: subnet of the pool type: string required: - - gateway - perNodeBlockSize - subnet type: object diff --git a/pkg/cni/plugin/plugin.go b/pkg/cni/plugin/plugin.go index 3a2d730..9aed698 100644 --- a/pkg/cni/plugin/plugin.go +++ b/pkg/cni/plugin/plugin.go @@ -168,21 +168,24 @@ func grpcRespToResult(resp *nodev1.AllocateResponse) (*current.Result, error) { if alloc.Ip == "" { return nil, logErr("IP can't be empty") } - if alloc.Gateway == "" { - return nil, logErr("Gateway can't be empty") - } ipAddr, netAddr, err := net.ParseCIDR(alloc.Ip) if err != nil { return nil, logErr(fmt.Sprintf("unexpected IP address format, received value: %s", alloc.Ip)) } - gwIP := net.ParseIP(alloc.Gateway) - if gwIP == nil { - return nil, logErr(fmt.Sprintf("unexpected Gateway address format, received value: %s", alloc.Gateway)) - } - result.IPs = append(result.IPs, ¤t.IPConfig{ + + ipConfig := ¤t.IPConfig{ Address: net.IPNet{IP: ipAddr, Mask: netAddr.Mask}, - Gateway: gwIP, - }) + } + + if alloc.Gateway != "" { + gwIP := net.ParseIP(alloc.Gateway) + if gwIP == nil { + return nil, logErr(fmt.Sprintf("unexpected Gateway address format, received value: %s", alloc.Gateway)) + } + ipConfig.Gateway = gwIP + } + + result.IPs = append(result.IPs, ipConfig) } return result, nil diff --git a/pkg/ipam-controller/config/config_test.go b/pkg/ipam-controller/config/config_test.go index 5ee053b..f6e4c65 100644 --- a/pkg/ipam-controller/config/config_test.go +++ b/pkg/ipam-controller/config/config_test.go @@ -44,12 +44,6 @@ var _ = Describe("Config", func() { cfg := &config.Config{Pools: map[string]config.PoolConfig{"pool1": poolConfig}} Expect(cfg.Validate()).To(HaveOccurred()) }) - It("Invalid pool: no gw", func() { - poolConfig := getValidPool() - poolConfig.Gateway = "" - cfg := &config.Config{Pools: map[string]config.PoolConfig{"pool1": poolConfig}} - Expect(cfg.Validate()).To(HaveOccurred()) - }) It("Invalid pool: gw outside of the subnet", func() { poolConfig := getValidPool() poolConfig.Gateway = "10.10.10.1" diff --git a/pkg/ipam-node/allocator/allocator.go b/pkg/ipam-node/allocator/allocator.go index d5c0a41..c3fb5ae 100644 --- a/pkg/ipam-node/allocator/allocator.go +++ b/pkg/ipam-node/allocator/allocator.go @@ -139,7 +139,7 @@ func (i *RangeIter) Next() (*net.IPNet, net.IP) { if i.cur == nil { i.cur = r.RangeStart i.startIP = i.cur - if i.cur.Equal(r.Gateway) { + if r.Gateway != nil && i.cur.Equal(r.Gateway) { return i.Next() } return &net.IPNet{IP: i.cur, Mask: r.Subnet.Mask}, r.Gateway @@ -165,7 +165,7 @@ func (i *RangeIter) Next() (*net.IPNet, net.IP) { return nil, nil } - if i.cur.Equal(r.Gateway) { + if r.Gateway != nil && i.cur.Equal(r.Gateway) { return i.Next() } diff --git a/pkg/ipam-node/allocator/allocator_test.go b/pkg/ipam-node/allocator/allocator_test.go index aae6a7f..b9adfe9 100644 --- a/pkg/ipam-node/allocator/allocator_test.go +++ b/pkg/ipam-node/allocator/allocator_test.go @@ -26,6 +26,7 @@ import ( cniTypes "github.com/containernetworking/cni/pkg/types" current "github.com/containernetworking/cni/pkg/types/100" + "github.com/Mellanox/nvidia-k8s-ipam/pkg/ip" "github.com/Mellanox/nvidia-k8s-ipam/pkg/ipam-node/allocator" storePkg "github.com/Mellanox/nvidia-k8s-ipam/pkg/ipam-node/store" "github.com/Mellanox/nvidia-k8s-ipam/pkg/ipam-node/types" @@ -46,7 +47,7 @@ type AllocatorTestCase struct { func mkAlloc(session storePkg.Session) allocator.IPAllocator { p := allocator.RangeSet{ - allocator.Range{Subnet: mustSubnet("192.168.1.0/29")}, + allocator.Range{Subnet: mustSubnet("192.168.1.0/29"), Gateway: net.ParseIP("192.168.1.1")}, } Expect(p.Canonicalize()).NotTo(HaveOccurred()) return allocator.NewIPAllocator(&p, testPoolName, session) @@ -69,7 +70,7 @@ func (t AllocatorTestCase) run(idx int, session storePkg.Session) (*current.IPCo if err != nil { return nil, err } - p = append(p, allocator.Range{Subnet: cniTypes.IPNet(*subnet)}) + p = append(p, allocator.Range{Subnet: cniTypes.IPNet(*subnet), Gateway: ip.NextIP(subnet.IP)}) } for r := range t.ipMap { Expect(session.Reserve(testPoolName, r, "net1", @@ -377,4 +378,24 @@ var _ = Describe("allocator", func() { checkAlloc(a, "0", net.IP{192, 168, 1, 0}) }) }) + Context("no gateway", func() { + It("should use the first IP of the range", func() { + session, err := storePkg.New( + filepath.Join(GinkgoT().TempDir(), "test_store")).Open(context.Background()) + Expect(err).NotTo(HaveOccurred()) + defer func() { + _ = session.Commit() + }() + p := allocator.RangeSet{ + allocator.Range{Subnet: mustSubnet("192.168.1.0/30")}, + allocator.Range{Subnet: mustSubnet("192.168.1.4/30")}, + } + Expect(p.Canonicalize()).NotTo(HaveOccurred()) + a := allocator.NewIPAllocator(&p, testPoolName, session) + // get range iterator and do the first Next + checkAlloc(a, "0", net.IP{192, 168, 1, 1}) + checkAlloc(a, "1", net.IP{192, 168, 1, 2}) + checkAlloc(a, "2", net.IP{192, 168, 1, 5}) + }) + }) }) diff --git a/pkg/ipam-node/allocator/range.go b/pkg/ipam-node/allocator/range.go index ce9c58d..9cecb38 100644 --- a/pkg/ipam-node/allocator/range.go +++ b/pkg/ipam-node/allocator/range.go @@ -56,13 +56,8 @@ func (r *Range) Canonicalize() error { "For a subnet mask of length %d the network address is %s", ones, networkIP.String()) } - // If the gateway is nil, claim .1 - if r.Gateway == nil { - r.Gateway = ip.NextIP(r.Subnet.IP) - if r.Gateway == nil { - return fmt.Errorf("computed Gateway for the subnet is not a valid IP") - } - } else { + // validate Gateway only if set + if r.Gateway != nil { if err := CanonicalizeIP(&r.Gateway); err != nil { return err } diff --git a/pkg/ipam-node/allocator/range_test.go b/pkg/ipam-node/allocator/range_test.go index b4e2c15..671e9bb 100644 --- a/pkg/ipam-node/allocator/range_test.go +++ b/pkg/ipam-node/allocator/range_test.go @@ -37,7 +37,6 @@ var _ = Describe("IP ranges", func() { Subnet: networkSubnet(subnetStr), RangeStart: net.IP{192, 0, 2, 1}, RangeEnd: net.IP{192, 0, 2, 254}, - Gateway: net.IP{192, 0, 2, 1}, })) }) It("should generate sane defaults for a smaller ipv4 subnet", func() { @@ -51,7 +50,6 @@ var _ = Describe("IP ranges", func() { Subnet: networkSubnet(subnetStr), RangeStart: net.IP{192, 0, 2, 1}, RangeEnd: net.IP{192, 0, 2, 126}, - Gateway: net.IP{192, 0, 2, 1}, })) }) It("should reject ipv4 subnet using a masked address", func() { @@ -97,7 +95,6 @@ var _ = Describe("IP ranges", func() { Subnet: networkSubnet(subnetStr), RangeStart: net.ParseIP("2001:DB8:1::1"), RangeEnd: net.ParseIP("2001:DB8:1::ffff:ffff:ffff:ffff"), - Gateway: net.ParseIP("2001:DB8:1::1"), })) }) diff --git a/pkg/ipam-node/handlers/allocate.go b/pkg/ipam-node/handlers/allocate.go index 9a9813d..d411863 100644 --- a/pkg/ipam-node/handlers/allocate.go +++ b/pkg/ipam-node/handlers/allocate.go @@ -54,11 +54,14 @@ func (h *Handlers) Allocate(ctx context.Context, req *nodev1.AllocateRequest) (* } resp := &nodev1.AllocateResponse{} for _, r := range result { - resp.Allocations = append(resp.Allocations, &nodev1.AllocationInfo{ - Pool: r.Pool, - Ip: r.Address.String(), - Gateway: r.Gateway.String(), - }) + allocationInfo := &nodev1.AllocationInfo{ + Pool: r.Pool, + Ip: r.Address.String(), + } + if r.Gateway != nil { + allocationInfo.Gateway = r.Gateway.String() + } + resp.Allocations = append(resp.Allocations, allocationInfo) } return resp, nil } @@ -108,15 +111,11 @@ func (h *Handlers) allocateInPool(pool string, reqLog logr.Logger, if err != nil || subnet == nil || subnet.IP == nil || subnet.Mask == nil { return PoolAlloc{}, poolCfgError(poolLog, pool, "invalid subnet") } - gateway := net.ParseIP(poolCfg.Gateway) - if gateway == nil { - return PoolAlloc{}, poolCfgError(poolLog, pool, "invalid gateway") - } rangeSet := &allocator.RangeSet{allocator.Range{ RangeStart: rangeStart, RangeEnd: rangeEnd, Subnet: cniTypes.IPNet(*subnet), - Gateway: gateway, + Gateway: net.ParseIP(poolCfg.Gateway), }} if err := rangeSet.Canonicalize(); err != nil { return PoolAlloc{}, poolCfgError(poolLog, pool,