libnetwork/netavark: remove ipam bucket on network rm

This is good to prevent any leaks but more important here there is a
bug because we cache the last assigned ip. However when a network is
removed the recreated with a different LeaseRange that ip might be very
well outside the expected range and the logic seems to handle this
correctly. I could fix it there but deleting the full bucket seems best
as it avoid other issues and leaking the bucket forever.

Fixes containers/podman#22034

Signed-off-by: Paul Holzinger <[email protected]>

libnetwork/netavark/config.go

@@ -376,6 +376,11 @@ func (n *netavarkNetwork) NetworkRemove(nameOrID string) error {
 		return fmt.Errorf("default network %s cannot be removed", n.defaultNetwork)
 	}
 
+	// remove the ipam bucket for this network
+	if err := n.removeNetworkIPAMBucket(network); err != nil {
+		return err
+	}
+
 	file := filepath.Join(n.networkConfigDir, network.Name+".json")
 	// make sure to not error for ErrNotExist
 	if err := os.Remove(file); err != nil && !errors.Is(err, os.ErrNotExist) {

libnetwork/netavark/ipam.go

@@ -4,6 +4,7 @@ package netavark
 
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
 	"net"
 
@@ -357,6 +358,26 @@ func (n *netavarkNetwork) deallocIPs(opts *types.NetworkOptions) error {
 	return err
 }
 
+func (n *netavarkNetwork) removeNetworkIPAMBucket(network *types.Network) error {
+	if !requiresIPAMAlloc(network) {
+		return nil
+	}
+	db, err := n.openDB()
+	if err != nil {
+		return err
+	}
+	defer db.Close()
+
+	return db.Update(func(tx *bbolt.Tx) error {
+		// Ignore ErrBucketNotFound, can happen if the network never allocated any ips,
+		// i.e. because no container was started.
+		if err := tx.DeleteBucket([]byte(network.Name)); err != nil && !errors.Is(err, bbolt.ErrBucketNotFound) {
+			return err
+		}
+		return nil
+	})
+}
+
 // requiresIPAMAlloc return true when we have to allocate ips for this network
 // it checks the ipam driver and if subnets are set
 func requiresIPAMAlloc(network *types.Network) bool {

libnetwork/netavark/ipam_test.go

@@ -498,4 +498,72 @@ var _ = Describe("IPAM", func() {
 			}
 		})
 	}
+
+	It("ipam delete network then recreate with different LeaseRange", func() {
+		s, _ := types.ParseCIDR("10.0.0.0/26")
+		network, err := networkInterface.NetworkCreate(
+			types.Network{
+				Subnets: []types.Subnet{
+					{
+						Subnet: s,
+					},
+				},
+			},
+			nil,
+		)
+		Expect(err).ToNot(HaveOccurred())
+
+		netName := network.Name
+
+		opts := &types.NetworkOptions{
+			ContainerID: "someContainerID",
+			Networks: map[string]types.PerNetworkOptions{
+				netName: {},
+			},
+		}
+
+		err = networkInterface.allocIPs(opts)
+		Expect(err).ToNot(HaveOccurred())
+		Expect(opts.Networks).To(HaveKey(netName))
+		Expect(opts.Networks[netName].StaticIPs).To(HaveLen(1))
+		Expect(opts.Networks[netName].StaticIPs[0]).To(Equal(net.ParseIP("10.0.0.2").To4()))
+
+		// dealloc the ip
+		err = networkInterface.deallocIPs(opts)
+		Expect(err).ToNot(HaveOccurred())
+
+		// delete the network
+		err = networkInterface.NetworkRemove(netName)
+		Expect(err).ToNot(HaveOccurred())
+
+		network, err = networkInterface.NetworkCreate(
+			types.Network{
+				Name: netName,
+				Subnets: []types.Subnet{
+					{
+						Subnet: s,
+						LeaseRange: &types.LeaseRange{
+							StartIP: net.ParseIP("10.0.0.10"),
+							EndIP:   net.ParseIP("10.0.0.20"),
+						},
+					},
+				},
+			},
+			nil,
+		)
+		Expect(err).ToNot(HaveOccurred())
+
+		opts = &types.NetworkOptions{
+			ContainerID: "someContainerID",
+			Networks: map[string]types.PerNetworkOptions{
+				netName: {},
+			},
+		}
+
+		err = networkInterface.allocIPs(opts)
+		Expect(err).ToNot(HaveOccurred())
+		Expect(opts.Networks).To(HaveKey(netName))
+		Expect(opts.Networks[netName].StaticIPs).To(HaveLen(1))
+		Expect(opts.Networks[netName].StaticIPs[0]).To(Equal(net.ParseIP("10.0.0.10").To4()))
+	})
 })