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.

Sign up for GitHub

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

Jump to bottom

ipam: Fix init flow in case there are sticky ips in the system #4823

Merged
merged 1 commit into from
Nov 14, 2024
+32 −17
Merged

ipam: Fix init flow in case there are sticky ips in the system #4823

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
21 changes: 14 additions & 7 deletions go-controller/pkg/allocator/ip/subnet/allocator_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,14 @@ import (

ipam "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/allocator/ip"
ovntest "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/testing"
"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/util"

"github.com/onsi/ginkgo/v2"
"github.com/onsi/gomega"
)

var _ = ginkgo.Describe("Subnet IP allocator operations", func() {
const subnetName = "subnet1"
var (
allocator Allocator
)
Expand All @@ -21,7 +23,6 @@ var _ = ginkgo.Describe("Subnet IP allocator operations", func() {

ginkgo.Context("when adding subnets", func() {
ginkgo.It("creates each IPAM and reserves IPs correctly", func() {
subnetName := "subnet1"
subnets := []string{
"10.1.1.0/24",
"2000::/64",
Expand All @@ -40,7 +41,6 @@ var _ = ginkgo.Describe("Subnet IP allocator operations", func() {
})

ginkgo.It("handles updates to the subnets correctly", func() {
subnetName := "subnet1"
subnets := []string{
"10.1.1.0/24",
"2000::/64",
Expand Down Expand Up @@ -69,7 +69,6 @@ var _ = ginkgo.Describe("Subnet IP allocator operations", func() {
})

ginkgo.It("excludes subnets correctly", func() {
subnetName := "subnet1"
subnets := []string{
"10.1.1.0/24",
}
Expand All @@ -93,7 +92,6 @@ var _ = ginkgo.Describe("Subnet IP allocator operations", func() {

ginkgo.Context("when allocating IP addresses", func() {
ginkgo.It("IPAM for each subnet allocates IPs contiguously", func() {
subnetName := "subnet1"
subnets := []string{
"10.1.1.0/24",
"2000::/64",
Expand All @@ -116,7 +114,6 @@ var _ = ginkgo.Describe("Subnet IP allocator operations", func() {
})

ginkgo.It("IPAM allocates, releases, and reallocates IPs correctly", func() {
subnetName := "subnet1"
subnets := []string{
"10.1.1.0/24",
}
Expand All @@ -141,8 +138,19 @@ var _ = ginkgo.Describe("Subnet IP allocator operations", func() {
}
})

ginkgo.It("fails to allocate multiple IPs from the same subnet", func() {
subnets := []string{"10.1.1.0/24", "2000::/64"}

gomega.Expect(allocator.AddOrUpdateSubnet(subnetName, ovntest.MustParseIPNets(subnets...))).To(gomega.Succeed())

ips, err := util.ParseIPNets([]string{"10.1.1.1/24", "10.1.1.2/24"})
gomega.Expect(err).NotTo(gomega.HaveOccurred())
gomega.Expect(allocator.AllocateIPs(subnetName, ips)).To(gomega.MatchError(
"failed to allocate IP 10.1.1.2 for subnet1: attempted to reserve multiple IPs in the same IPAM instance",
))
})

ginkgo.It("releases IPs for other subnets when any other subnet allocation fails", func() {
subnetName := "subnet1"
subnets := []string{
"10.1.1.0/24",
"10.1.2.0/29",
Expand Down Expand Up @@ -184,7 +192,6 @@ var _ = ginkgo.Describe("Subnet IP allocator operations", func() {
})

ginkgo.It("fails correctly when trying to block a previously allocated IP", func() {
subnetName := "subnet1"
subnets := []string{
"10.1.1.0/24",
"2000::/64",
Expand Down
8 changes: 7 additions & 1 deletion go-controller/pkg/clustermanager/secondary_network_unit_test.go
View file
Open in desktop
oshoval marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -359,10 +359,12 @@ var _ = ginkgo.Describe("Cluster Controller Manager", func() {
ginkgo.Context("persistent IP allocations", func() {
const (
claimName = "claim1"
claimName2 = "claim2"
namespace = "ns"
networkName = "blue"
subnetCIDR = "192.168.200.0/24"
subnetIP = "192.168.200.2/24"
subnetIP2 = "192.168.200.3/24"
)

var netInfo util.NetInfo
Expand All @@ -387,6 +389,7 @@ var _ = ginkgo.Describe("Cluster Controller Manager", func() {
KubeClient: fake.NewSimpleClientset(),
IPAMClaimsClient: fakeipamclaimclient.NewSimpleClientset(
ipamClaimWithIPAddr(claimName, namespace, networkName, subnetIP),
ipamClaimWithIPAddr(claimName2, namespace, networkName, subnetIP2),
oshoval marked this conversation as resolved.
Show resolved Hide resolved
),
NetworkAttchDefClient: fakenadclient.NewSimpleClientset(),
}
Expand Down Expand Up @@ -420,9 +423,12 @@ var _ = ginkgo.Describe("Cluster Controller Manager", func() {

ips, err := util.ParseIPNets([]string{subnetIP})
gomega.Expect(err).NotTo(gomega.HaveOccurred())

gomega.Expect(nc.subnetAllocator.AllocateIPs(netInfo.GetNetworkName(), ips)).To(gomega.Equal(ip.ErrAllocated))

ips2, err := util.ParseIPNets([]string{subnetIP2})
gomega.Expect(err).NotTo(gomega.HaveOccurred())
gomega.Expect(nc.subnetAllocator.AllocateIPs(netInfo.GetNetworkName(), ips2)).To(gomega.Equal(ip.ErrAllocated))

return nil
}

Expand Down
16 changes: 8 additions & 8 deletions go-controller/pkg/persistentips/allocator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,17 @@ package persistentips
import (
"errors"
"fmt"
ipam "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/allocator/ip"
"k8s.io/klog/v2"
"net"

"k8s.io/klog/v2"

"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"

ipamclaimsapi "github.com/k8snetworkplumbingwg/ipamclaims/pkg/crd/ipamclaims/v1alpha1"
ipamclaimslister "github.com/k8snetworkplumbingwg/ipamclaims/pkg/crd/ipamclaims/v1alpha1/apis/listers/ipamclaims/v1alpha1"

ipam "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/allocator/ip"
"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/kube"
ovnktypes "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/types"
"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/util"
Expand Down Expand Up @@ -150,7 +151,6 @@ func (icr *IPAMClaimReconciler) FindIPAMClaim(claimName string, namespace string
// the cluster. For live pods, therse are already allocated, so no error will
// be thrown (e.g. we ignore the `ipam.IsErrAllocated` error
func (icr *IPAMClaimReconciler) Sync(objs []interface{}, ipAllocator IPAllocator) error {
var ips []*net.IPNet
for _, obj := range objs {
ipamClaim, ok := obj.(*ipamclaimsapi.IPAMClaim)
if !ok {
Expand All @@ -169,11 +169,11 @@ func (icr *IPAMClaimReconciler) Sync(objs []interface{}, ipAllocator IPAllocator
if err != nil {
return fmt.Errorf("failed at parsing IP when allocating persistent IPs: %w", err)
}
ips = append(ips, ipnets...)
}
if len(ips) > 0 {
if err := ipAllocator.AllocateIPs(ips); err != nil && !ipam.IsErrAllocated(err) {
return fmt.Errorf("failed allocating persistent ips: %w", err)

if len(ipnets) != 0 {
if err := ipAllocator.AllocateIPs(ipnets); err != nil && !ipam.IsErrAllocated(err) {
return fmt.Errorf("failed syncing persistent ips: %w", err)
}
}
}
return nil
Expand Down
4 changes: 3 additions & 1 deletion go-controller/pkg/persistentips/allocator_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,9 @@ var _ = Describe("Persistent IP allocator operations", func() {
},
Entry("no objects to sync with"),
Entry("an IPAMClaim without persisted IPs", emptyDummyIPAMClaim(namespace, claimName, networkName)),
Entry("an IPAMClaim with persisted IPs", ipamClaimWithIPs(namespace, claimName, networkName, "192.168.200.2/24", "fd10::1/64")),
Entry("an IPAMClaim with persisted IPs",
ipamClaimWithIPs(namespace, claimName, networkName, "192.168.200.2/24", "fd10::1/64"),
ipamClaimWithIPs(namespace, claimName, networkName, "192.168.200.3/24", "fd10::2/64")),
)
})

Expand Down
Loading