Skip to content

Commit

Permalink
Resilient network free. (#476)
Browse files Browse the repository at this point in the history
  • Loading branch information
Gerrit91 authored Oct 31, 2023
1 parent 6ea0044 commit d61abc7
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 18 deletions.
14 changes: 11 additions & 3 deletions cmd/metal-api/internal/ipam/ipam.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package ipam

import (
"fmt"
"net/netip"

"github.com/metal-stack/metal-api/cmd/metal-api/internal/metal"

Expand Down Expand Up @@ -42,13 +43,20 @@ func (i *Ipam) AllocateChildPrefix(parentPrefix metal.Prefix, childLength uint8)

// ReleaseChildPrefix release a child prefix from a parent prefix in the IPAM.
func (i *Ipam) ReleaseChildPrefix(childPrefix metal.Prefix) error {
ipamChildPrefix := i.ip.PrefixFrom(childPrefix.String())
_, err := netip.ParsePrefix(childPrefix.String())
if err != nil {
return fmt.Errorf("invalid child prefix: %w", err)
}

ipamChildPrefix := i.ip.PrefixFrom(childPrefix.String())
if ipamChildPrefix == nil {
return fmt.Errorf("error finding child prefix in ipam: %s", childPrefix.String())
// FIXME: unfortunately, go-ipam does not return a proper error here so we cannot deduce if the prefix
// was already deleted or not, so if the database is down or something we continue with network deletion
// even though there could be remainings in the go-ipam db
return nil
}

err := i.ip.ReleaseChildPrefix(ipamChildPrefix)
err = i.ip.ReleaseChildPrefix(ipamChildPrefix)
if err != nil {
return fmt.Errorf("error releasing child prefix in ipam: %w", err)
}
Expand Down
16 changes: 1 addition & 15 deletions cmd/metal-api/internal/service/network-service.go
Original file line number Diff line number Diff line change
Expand Up @@ -550,21 +550,6 @@ func (r *networkResource) freeNetwork(request *restful.Request, response *restfu
return
}

for _, prefix := range nw.Prefixes {
usage, err := r.ipamer.PrefixUsage(prefix.String())
if err != nil {
r.sendError(request, response, defaultError(err))
return
}

if usage.UsedIPs > 2 {
if err != nil {
r.sendError(request, response, defaultError(fmt.Errorf("cannot release child prefix %s because IPs in the prefix are still in use: %v", prefix.String(), usage.UsedIPs-2)))
return
}
}
}

for _, prefix := range nw.Prefixes {
err = r.ipamer.ReleaseChildPrefix(prefix)
if err != nil {
Expand Down Expand Up @@ -767,6 +752,7 @@ func getNetworkUsage(nw *metal.Network, ipamer ipam.IPAMer) *metal.NetworkUsage
for _, prefix := range nw.Prefixes {
u, err := ipamer.PrefixUsage(prefix.String())
if err != nil {
// FIXME: the error should not be swallowed here as this can return wrong usage information to the clients
continue
}
usage.AvailableIPs = usage.AvailableIPs + u.AvailableIPs
Expand Down

0 comments on commit d61abc7

Please sign in to comment.