Skip to content

Commit

Permalink
Merge pull request #39 from ykulazhenkov/pr-empty-gateway
Browse files Browse the repository at this point in the history
Allow to skip gateway configuration
  • Loading branch information
ykulazhenkov authored Jun 10, 2024
2 parents 279a309 + 371d21e commit 18fc6a0
Show file tree
Hide file tree
Showing 12 changed files with 69 additions and 49 deletions.
11 changes: 10 additions & 1 deletion api/v1alpha1/ippool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
2 changes: 1 addition & 1 deletion api/v1alpha1/ippool_type.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
}
Expand Down
13 changes: 8 additions & 5 deletions api/v1alpha1/ippool_validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
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.

1 change: 0 additions & 1 deletion deploy/crds/nv-ipam.nvidia.com_ippools.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,6 @@ spec:
description: subnet of the pool
type: string
required:
- gateway
- perNodeBlockSize
- subnet
type: object
Expand Down
23 changes: 13 additions & 10 deletions pkg/cni/plugin/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 0 additions & 6 deletions pkg/ipam-controller/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
4 changes: 2 additions & 2 deletions pkg/ipam-node/allocator/allocator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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()
}

Expand Down
25 changes: 23 additions & 2 deletions pkg/ipam-node/allocator/allocator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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)
Expand All @@ -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",
Expand Down Expand Up @@ -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})
})
})
})
9 changes: 2 additions & 7 deletions pkg/ipam-node/allocator/range.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
3 changes: 0 additions & 3 deletions pkg/ipam-node/allocator/range_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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() {
Expand Down Expand Up @@ -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"),
}))
})

Expand Down
19 changes: 9 additions & 10 deletions pkg/ipam-node/handlers/allocate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 18fc6a0

Please sign in to comment.