Merge pull request #39 from ykulazhenkov/pr-empty-gateway

Allow to skip gateway configuration

api/v1alpha1/ippool_test.go

@@ -65,14 +65,23 @@ 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()).
 			To(And(
 				ContainSubstring("metadata.name"),
 				ContainSubstring("spec.subnet"),
 				ContainSubstring("spec.perNodeBlockSize"),
-				ContainSubstring("gateway"),
 			))
 	})
 	It("Invalid - perNodeBlockSize is too large", func() {

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"`
 }

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) {

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

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, &current.IPConfig{
+
+		ipConfig := &current.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

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"

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()
 	}
 

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})
+		})
+	})
 })

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
 		}

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"),
 		}))
 	})
 

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,