Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow to skip gateway configuration #39

Merged
merged 1 commit into from
Jun 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading