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

Return existing reservation if podRef matches #296

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Return existing reservation if podRef matches #296

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
11 changes: 7 additions & 4 deletions pkg/allocate/allocate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,9 +194,9 @@ func IterateForAssignment(ipnet net.IPNet, rangeStart net.IP, rangeEnd net.IP, r
}
logging.Debugf("IterateForAssignment input >> ip: %v | ipnet: %v | first IP: %v | last IP: %v", rangeStart, ipnet, firstip, lastip)

reserved := make(map[string]bool)
reserved := make(map[string]string)
for _, r := range reservelist {
reserved[r.IP.String()] = true
reserved[r.IP.String()] = r.PodRef
}

// excluded, "192.168.2.229/30", "192.168.1.229/30",
Expand All @@ -212,8 +212,11 @@ func IterateForAssignment(ipnet net.IPNet, rangeStart net.IP, rangeEnd net.IP, r
endip := IPAddOffset(lastip, uint64(1))
for i := firstip; !i.Equal(endip); i = IPAddOffset(i, uint64(1)) {
// if already reserved, skip it
if reserved[i.String()] {
continue
if reserved[i.String()] != "" {
if reserved[i.String()] != podRef {
continue
}
logging.Debugf("Found existing reservation %v with matching podRef %s", i.String(), podRef)
Comment on lines +215 to +219
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the case where the reserved map is nonempty but doesn't contain the podRef? In other words, when will the "continue" get hit in the code?

}

// Lastly, we need to check if this IP is within the range of excluded subnets
Expand Down
16 changes: 13 additions & 3 deletions pkg/storage/kubernetes/ipam.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,14 +175,14 @@ func (i *KubernetesIPAM) GetOverlappingRangeStore() (storage.OverlappingRangeSto
}

// IsAllocatedInOverlappingRange checks for IP addresses to see if they're allocated cluster wide, for overlapping ranges
func (c *KubernetesOverlappingRangeStore) IsAllocatedInOverlappingRange(ctx context.Context, ip net.IP) (bool, error) {
func (c *KubernetesOverlappingRangeStore) IsAllocatedInOverlappingRange(ctx context.Context, ip net.IP, podRef string) (bool, error) {

// IPv6 doesn't make for valid CR names, so normalize it.
normalizedip := strings.ReplaceAll(fmt.Sprint(ip), ":", "-")

logging.Debugf("OverlappingRangewide allocation check for IP: %v", normalizedip)

_, err := c.client.WhereaboutsV1alpha1().OverlappingRangeIPReservations(c.namespace).Get(ctx, normalizedip, metav1.GetOptions{})
clusteripres, err := c.client.WhereaboutsV1alpha1().OverlappingRangeIPReservations(c.namespace).Get(ctx, normalizedip, metav1.GetOptions{})
if err != nil && errors.IsNotFound(err) {
// cluster ip reservation does not exist, this appears to be good news.
// logging.Debugf("IP %v is not reserved cluster wide, allowing.", ip)
Expand All @@ -192,6 +192,11 @@ func (c *KubernetesOverlappingRangeStore) IsAllocatedInOverlappingRange(ctx cont
return false, fmt.Errorf("k8s get OverlappingRangeIPReservation error: %s", err)
}

if clusteripres.Spec.PodRef == podRef {
logging.Debugf("IP %v matches existing podRef %s", ip, podRef)
return false, nil
}

logging.Debugf("IP %v is reserved cluster wide.", ip)
return true, nil
}
Expand Down Expand Up @@ -220,6 +225,11 @@ func (c *KubernetesOverlappingRangeStore) UpdateOverlappingRangeAllocation(ctx c
_, err = c.client.WhereaboutsV1alpha1().OverlappingRangeIPReservations(c.namespace).Create(
ctx, clusteripres, metav1.CreateOptions{})

if errors.IsAlreadyExists(err) {
logging.Debugf("clusteripres already exists, updating with %v", clusteripres.Spec)
_, err = c.client.WhereaboutsV1alpha1().OverlappingRangeIPReservations(c.namespace).Update(ctx, clusteripres, metav1.UpdateOptions{})
}

case whereaboutstypes.Deallocate:
verb = "deallocate"
err = c.client.WhereaboutsV1alpha1().OverlappingRangeIPReservations(c.namespace).Delete(ctx, clusteripres.GetName(), metav1.DeleteOptions{})
Expand Down Expand Up @@ -480,7 +490,7 @@ func IPManagementKubernetesUpdate(ctx context.Context, mode int, ipam *Kubernete
// When it's allocated overlappingrange wide, we add it to a local reserved list
// And we try again.
if ipamConf.OverlappingRanges {
isallocated, err := overlappingrangestore.IsAllocatedInOverlappingRange(requestCtx, newip.IP)
isallocated, err := overlappingrangestore.IsAllocatedInOverlappingRange(requestCtx, newip.IP, podRef)
if err != nil {
logging.Errorf("Error checking overlappingrange allocation: %v", err)
return newips, err
Expand Down
2 changes: 1 addition & 1 deletion pkg/storage/storage.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ type Store interface {

// OverlappingRangeStore is an interface for wrapping overlappingrange storage options
type OverlappingRangeStore interface {
IsAllocatedInOverlappingRange(ctx context.Context, ip net.IP) (bool, error)
IsAllocatedInOverlappingRange(ctx context.Context, ip net.IP, podRef string) (bool, error)
UpdateOverlappingRangeAllocation(ctx context.Context, mode int, ip net.IP, containerID string, podRef string) error
}

Expand Down