k8s: CiliumNode: Move generic IPAM parmeters into Spec.IPAM

Signed-off-by: Thomas Graf <[email protected]>

Documentation/concepts/ipam/crd.rst

@@ -110,6 +110,8 @@ list of all IPs which are available to the node for allocation:
 
 .. code-block:: go
 
+        // AllocationMap is a map of allocated IPs indexed by IP
+        type AllocationMap map[string]AllocationIP
 
         // NodeSpec is the configuration specific to a node
         type NodeSpec struct {
@@ -130,7 +132,7 @@ list of all IPs which are available to the node for allocation:
                 // Status.IPAM.InUse
                 //
                 // +optional
-                Pool map[string]AllocationIP `json:"pool,omitempty"`
+                Pool AllocationMap `json:"pool,omitempty"`
 	}
 
 	// AllocationIP is an IP available for allocation or already allocated
@@ -178,5 +180,5 @@ all used addresses on that node:
 		// allocated and are in use.
 		//
 		// +optional
-		InUse map[string]AllocationIP `json:"used,omitempty"`
+		InUse AllocationMap `json:"used,omitempty"`
 	}

Documentation/concepts/ipam/eni.rst

@@ -103,22 +103,22 @@ allocation:
 
   *This field is automatically populated when using ``--auto-create-cilium-node-resource``*
 
-``spec.eni.min-allocate``
+``spec.ipam.min-allocate``
   The minimum number of IPs that must be allocated when the node is first
   bootstrapped. It defines the minimum base socket of addresses that must be
   available. After reaching this watermark, the PreAllocate and
   MaxAboveWatermark logic takes over to continue allocating IPs.
 
   If unspecified, no minimum number of IPs is required.
 
-``spec.eni.pre-allocate``
+``spec.ipam.pre-allocate``
   The number of IP addresses that must be available for allocation at all
   times.  It defines the buffer of addresses available immediately without
   requiring for the operator to get involved.
 
   If unspecified, this value defaults to 8.
 
-``spec.eni.max-above-watermark``
+``spec.ipam.max-above-watermark``
   The maximum number of addresses to allocate beyond the addresses needed to
   reach the PreAllocate watermark.  Going above the watermark can help reduce
   the number of API calls to allocate IPs, e.g. when a new ENI is allocated, as

Makefile

@@ -273,6 +273,7 @@ generate-health-api: api/v1/health/openapi.yaml
 generate-k8s-api:
 	$(call generate_k8s_api_all,github.com/cilium/cilium/pkg/k8s/apis,"cilium.io:v2")
 	$(call generate_k8s_api_deepcopy,github.com/cilium/cilium/pkg/aws,"eni:types")
+	$(call generate_k8s_api_deepcopy,github.com/cilium/cilium/pkg,"ipam:types")
 	$(call generate_k8s_api_deepcopy,github.com/cilium/cilium/pkg,"policy:api")
 	$(call generate_k8s_api_deepcopy,github.com/cilium/cilium,"pkg:loadbalancer")
 	$(call generate_k8s_api_deepcopy,github.com/cilium/cilium,"pkg:k8s")

pkg/aws/eni/node.go

@@ -82,7 +82,7 @@ type Node struct {
 
 	enis map[string]eniTypes.ENI
 
-	available map[string]v2.AllocationIP
+	available ipamTypes.AllocationMap
 
 	manager *NodeManager
 
@@ -120,6 +120,33 @@ type nodeStatistics struct {
 	remainingInterfaces int
 }
 
+// GetMaxAboveWatermark returns the max-above-watermark setting for an AWS node
+func (n *Node) GetMaxAboveWatermark() int {
+	if n.resource.Spec.IPAM.MaxAboveWatermark != 0 {
+		return n.resource.Spec.IPAM.MaxAboveWatermark
+	}
+	return n.resource.Spec.ENI.MaxAboveWatermark
+}
+
+// GetPreAllocate returns the pre-allocation setting for an AWS node
+func (n *Node) GetPreAllocate() int {
+	if n.resource.Spec.IPAM.PreAllocate != 0 {
+		return n.resource.Spec.IPAM.PreAllocate
+	}
+	if n.resource.Spec.ENI.PreAllocate != 0 {
+		return n.resource.Spec.ENI.PreAllocate
+	}
+	return defaults.ENIPreAllocation
+}
+
+// GetMinAllocate returns the minimum-allocation setting of an AWS node
+func (n *Node) GetMinAllocate() int {
+	if n.resource.Spec.IPAM.MinAllocate != 0 {
+		return n.resource.Spec.IPAM.MinAllocate
+	}
+	return n.resource.Spec.ENI.MinAllocate
+}
+
 func (n *Node) logger() *logrus.Entry {
 	if n == nil {
 		return log
@@ -161,10 +188,6 @@ func (n *Node) getNeededAddresses() int {
 }
 
 func calculateNeededIPs(availableIPs, usedIPs, preAllocate, minAllocate int) (neededIPs int) {
-	if preAllocate == 0 {
-		preAllocate = defaults.ENIPreAllocation
-	}
-
 	neededIPs = preAllocate - (availableIPs - usedIPs)
 	if neededIPs < 0 {
 		neededIPs = 0
@@ -235,7 +258,7 @@ func (n *Node) updatedResource(resource *v2.CiliumNode) bool {
 
 func (n *Node) recalculateLocked() {
 	n.enis = map[string]eniTypes.ENI{}
-	n.available = map[string]v2.AllocationIP{}
+	n.available = ipamTypes.AllocationMap{}
 	enis := n.manager.instancesAPI.GetENIs(n.resource.Spec.ENI.InstanceID)
 	// An ec2 instance has at least one ENI attached, no ENI found implies instance not found.
 	if len(enis) == 0 {
@@ -253,13 +276,13 @@ func (n *Node) recalculateLocked() {
 		}
 
 		for _, ip := range e.Addresses {
-			n.available[ip] = v2.AllocationIP{Resource: e.ID}
+			n.available[ip] = ipamTypes.AllocationIP{Resource: e.ID}
 		}
 	}
 	n.stats.usedIPs = len(n.resource.Status.IPAM.Used)
 	n.stats.availableIPs = len(n.available)
-	n.stats.neededIPs = calculateNeededIPs(n.stats.availableIPs, n.stats.usedIPs, n.resource.Spec.ENI.PreAllocate, n.resource.Spec.ENI.MinAllocate)
-	n.stats.excessIPs = calculateExcessIPs(n.stats.availableIPs, n.stats.usedIPs, n.resource.Spec.ENI.PreAllocate, n.resource.Spec.ENI.MinAllocate, n.resource.Spec.ENI.MaxAboveWatermark)
+	n.stats.neededIPs = calculateNeededIPs(n.stats.availableIPs, n.stats.usedIPs, n.GetPreAllocate(), n.GetMinAllocate())
+	n.stats.excessIPs = calculateExcessIPs(n.stats.availableIPs, n.stats.usedIPs, n.GetPreAllocate(), n.GetMinAllocate(), n.GetMaxAboveWatermark())
 
 	n.loggerLocked().WithFields(logrus.Fields{
 		"available":                 n.stats.availableIPs,
@@ -293,8 +316,8 @@ func (n *Node) ENIs() (enis map[string]eniTypes.ENI) {
 }
 
 // Pool returns the IP allocation pool available to the node
-func (n *Node) Pool() (pool map[string]v2.AllocationIP) {
-	pool = map[string]v2.AllocationIP{}
+func (n *Node) Pool() (pool ipamTypes.AllocationMap) {
+	pool = ipamTypes.AllocationMap{}
 	n.mutex.RLock()
 	for k, allocationIP := range n.available {
 		pool[k] = allocationIP
@@ -401,7 +424,7 @@ func (n *Node) allocateENI(ctx context.Context, s *types.Subnet, a *allocatableR
 	neededAddresses := n.stats.neededIPs
 	desc := "Cilium-CNI (" + n.resource.Spec.ENI.InstanceID + ")"
 	// Must allocate secondary ENI IPs as needed, up to ENI instance limit - 1 (reserve 1 for primary IP)
-	toAllocate := int64(math.IntMin(neededAddresses+nodeResource.Spec.ENI.MaxAboveWatermark, a.limits.IPv4-1))
+	toAllocate := int64(math.IntMin(neededAddresses+n.GetMaxAboveWatermark(), a.limits.IPv4-1))
 	// Validate whether request has already been fulfilled in the meantime
 	if toAllocate == 0 {
 		n.mutex.RUnlock()
@@ -603,7 +626,7 @@ func (n *Node) determineMaintenanceAction() (*allocatableResources, error) {
 
 	// Validate that the node still requires addresses to be allocated, the
 	// request may have been resolved in the meantime.
-	maxAllocate := n.stats.neededIPs + n.resource.Spec.ENI.MaxAboveWatermark
+	maxAllocate := n.stats.neededIPs + n.GetMaxAboveWatermark()
 	if n.stats.neededIPs == 0 {
 		return nil, nil
 	}
@@ -803,7 +826,7 @@ func (n *Node) SyncToAPIServer() (err error) {
 	// fall back to the controller based background interval to retry.
 	for retry := 0; retry < 2; retry++ {
 		if node.Status.IPAM.Used == nil {
-			node.Status.IPAM.Used = map[string]v2.AllocationIP{}
+			node.Status.IPAM.Used = ipamTypes.AllocationMap{}
 		}
 
 		node.Status.ENI.ENIs = n.ENIs()
@@ -838,11 +861,11 @@ func (n *Node) SyncToAPIServer() (err error) {
 
 	for retry := 0; retry < 2; retry++ {
 		if node.Spec.IPAM.Pool == nil {
-			node.Spec.IPAM.Pool = map[string]v2.AllocationIP{}
+			node.Spec.IPAM.Pool = ipamTypes.AllocationMap{}
 		}
 
-		if node.Spec.ENI.PreAllocate == 0 {
-			node.Spec.ENI.PreAllocate = defaults.ENIPreAllocation
+		if node.Spec.IPAM.PreAllocate == 0 {
+			node.Spec.IPAM.PreAllocate = defaults.ENIPreAllocation
 		}
 
 		node.Spec.IPAM.Pool = n.Pool()

pkg/aws/eni/node_manager_test.go

@@ -27,6 +27,7 @@ import (
 	"github.com/cilium/cilium/pkg/aws/types"
 	"github.com/cilium/cilium/pkg/checker"
 	metricsmock "github.com/cilium/cilium/pkg/ipam/metrics/mock"
+	ipamTypes "github.com/cilium/cilium/pkg/ipam/types"
 	v2 "github.com/cilium/cilium/pkg/k8s/apis/cilium.io/v2"
 	"github.com/cilium/cilium/pkg/option"
 	"github.com/cilium/cilium/pkg/testutils"
@@ -146,18 +147,18 @@ func newCiliumNode(node, instanceID, instanceType, az, vpcID string, firstInterf
 				InstanceID:          instanceID,
 				InstanceType:        instanceType,
 				FirstInterfaceIndex: &firstInterfaceIndex,
-				PreAllocate:         preAllocate,
-				MinAllocate:         minAllocate,
 				AvailabilityZone:    az,
 				VpcID:               vpcID,
 			},
-			IPAM: v2.IPAMSpec{
-				Pool: map[string]v2.AllocationIP{},
+			IPAM: ipamTypes.IPAMSpec{
+				Pool:        ipamTypes.AllocationMap{},
+				PreAllocate: preAllocate,
+				MinAllocate: minAllocate,
 			},
 		},
 		Status: v2.NodeStatus{
-			IPAM: v2.IPAMStatus{
-				Used: map[string]v2.AllocationIP{},
+			IPAM: ipamTypes.IPAMStatus{
+				Used: ipamTypes.AllocationMap{},
 			},
 		},
 	}
@@ -173,19 +174,19 @@ func newCiliumNodeWithSGTags(node, instanceID, instanceType, az, vpcID string, s
 				InstanceID:          instanceID,
 				InstanceType:        instanceType,
 				FirstInterfaceIndex: &firstInterfaceIndex,
-				PreAllocate:         preAllocate,
-				MinAllocate:         minAllocate,
 				AvailabilityZone:    az,
 				VpcID:               vpcID,
 				SecurityGroupTags:   sgTags,
 			},
-			IPAM: v2.IPAMSpec{
-				Pool: map[string]v2.AllocationIP{},
+			IPAM: ipamTypes.IPAMSpec{
+				Pool:        ipamTypes.AllocationMap{},
+				PreAllocate: preAllocate,
+				MinAllocate: minAllocate,
 			},
 		},
 		Status: v2.NodeStatus{
-			IPAM: v2.IPAMStatus{
-				Used: map[string]v2.AllocationIP{},
+			IPAM: ipamTypes.IPAMStatus{
+				Used: ipamTypes.AllocationMap{},
 			},
 		},
 	}
@@ -194,12 +195,12 @@ func newCiliumNodeWithSGTags(node, instanceID, instanceType, az, vpcID string, s
 }
 
 func updateCiliumNode(cn *v2.CiliumNode, available, used int) *v2.CiliumNode {
-	cn.Spec.IPAM.Pool = map[string]v2.AllocationIP{}
+	cn.Spec.IPAM.Pool = ipamTypes.AllocationMap{}
 	for i := 0; i < used; i++ {
-		cn.Spec.IPAM.Pool[fmt.Sprintf("1.1.1.%d", i)] = v2.AllocationIP{Resource: "foo"}
+		cn.Spec.IPAM.Pool[fmt.Sprintf("1.1.1.%d", i)] = ipamTypes.AllocationIP{Resource: "foo"}
 	}
 
-	cn.Status.IPAM.Used = map[string]v2.AllocationIP{}
+	cn.Status.IPAM.Used = ipamTypes.AllocationMap{}
 	for ip, ipAllocation := range cn.Spec.IPAM.Pool {
 		if used > 0 {
 			delete(cn.Spec.IPAM.Pool, ip)
@@ -435,7 +436,7 @@ func (e *ENISuite) TestNodeManagerReleaseAddress(c *check.C) {
 
 	// Announce node, wait for IPs to become available
 	cn := newCiliumNode("node3", "i-testNodeManagerReleaseAddress-1", "m4.xlarge", "us-west-1", "vpc-1", 1, 4, 10)
-	cn.Spec.ENI.MaxAboveWatermark = 4
+	cn.Spec.IPAM.MaxAboveWatermark = 4
 	mngr.Update(cn)
 	c.Assert(testutils.WaitUntil(func() bool { return reachedAddressesNeeded(mngr, "node3", 0) }, 5*time.Second), check.IsNil)
 

pkg/aws/eni/types/types.go

@@ -36,6 +36,8 @@ type ENISpec struct {
 	// the PreAllocate and MaxAboveWatermark logic takes over to continue
 	// allocating IPs.
 	//
+	// OBSOLETE: This field is obsolete, please use Spec.IPAM.MinAllocate
+	//
 	// +optional
 	MinAllocate int `json:"min-allocate,omitempty"`
 
@@ -44,6 +46,8 @@ type ENISpec struct {
 	// addresses available immediately without requiring cilium-operator to
 	// get involved.
 	//
+	// OBSOLETE: This field is obsolete, please use Spec.IPAM.PreAllocate
+	//
 	// +optional
 	PreAllocate int `json:"pre-allocate,omitempty"`
 
@@ -54,6 +58,8 @@ type ENISpec struct {
 	// IPs as possible are allocated. Limiting the amount can help reduce
 	// waste of IPs.
 	//
+	// OBSOLETE: This field is obsolete, please use Spec.IPAM.MaxAboveWatermark
+	//
 	// +optional
 	MaxAboveWatermark int `json:"max-above-watermark,omitempty"`
 

pkg/ipam/crd.go

@@ -23,6 +23,7 @@ import (
 
 	eniTypes "github.com/cilium/cilium/pkg/aws/eni/types"
 	"github.com/cilium/cilium/pkg/cidr"
+	ipamTypes "github.com/cilium/cilium/pkg/ipam/types"
 	"github.com/cilium/cilium/pkg/k8s"
 	ciliumv2 "github.com/cilium/cilium/pkg/k8s/apis/cilium.io/v2"
 	"github.com/cilium/cilium/pkg/k8s/informer"
@@ -203,8 +204,12 @@ func (n *nodeStore) hasMinimumIPsInPool() (minimumReached bool, required, numAva
 	}
 
 	switch {
+	case n.ownNode.Spec.IPAM.MinAllocate != 0:
+		required = n.ownNode.Spec.IPAM.MinAllocate
 	case n.ownNode.Spec.ENI.MinAllocate != 0:
 		required = n.ownNode.Spec.ENI.MinAllocate
+	case n.ownNode.Spec.IPAM.PreAllocate != 0:
+		required = n.ownNode.Spec.IPAM.PreAllocate
 	case n.ownNode.Spec.ENI.PreAllocate != 0:
 		required = n.ownNode.Spec.ENI.PreAllocate
 	case option.Config.EnableHealthChecking:
@@ -289,7 +294,7 @@ func (n *nodeStore) refreshNode() error {
 	copy(staleCopyOfAllocators, n.allocators)
 	n.mutex.RUnlock()
 
-	node.Status.IPAM.Used = map[string]ciliumv2.AllocationIP{}
+	node.Status.IPAM.Used = ipamTypes.AllocationMap{}
 
 	for _, a := range staleCopyOfAllocators {
 		a.mutex.RLock()
@@ -320,7 +325,7 @@ func (n *nodeStore) addAllocator(allocator *crdAllocator) {
 }
 
 // allocate checks if a particular IP can be allocated or return an error
-func (n *nodeStore) allocate(ip net.IP) (*ciliumv2.AllocationIP, error) {
+func (n *nodeStore) allocate(ip net.IP) (*ipamTypes.AllocationIP, error) {
 	n.mutex.RLock()
 	defer n.mutex.RUnlock()
 
@@ -341,7 +346,7 @@ func (n *nodeStore) allocate(ip net.IP) (*ciliumv2.AllocationIP, error) {
 }
 
 // allocateNext allocates the next available IP or returns an error
-func (n *nodeStore) allocateNext(allocated map[string]ciliumv2.AllocationIP, family Family) (net.IP, *ciliumv2.AllocationIP, error) {
+func (n *nodeStore) allocateNext(allocated ipamTypes.AllocationMap, family Family) (net.IP, *ipamTypes.AllocationIP, error) {
 	n.mutex.RLock()
 	defer n.mutex.RUnlock()
 
@@ -383,7 +388,7 @@ type crdAllocator struct {
 
 	// allocated is a map of all allocated IPs indexed by the allocated IP
 	// represented as string
-	allocated map[string]ciliumv2.AllocationIP
+	allocated ipamTypes.AllocationMap
 
 	// family is the address family this allocator is allocator for
 	family Family
@@ -396,7 +401,7 @@ func newCRDAllocator(family Family, owner Owner, k8sEventReg K8sEventRegister) A
 	})
 
 	allocator := &crdAllocator{
-		allocated: map[string]ciliumv2.AllocationIP{},
+		allocated: ipamTypes.AllocationMap{},
 		family:    family,
 		store:     sharedNodeStore,
 	}
@@ -420,7 +425,7 @@ func deriveGatewayIP(eni eniTypes.ENI) string {
 	return net.IPv4(addr[0], addr[1], addr[2], addr[3]+1).String()
 }
 
-func (a *crdAllocator) buildAllocationResult(ip net.IP, ipInfo *ciliumv2.AllocationIP) (result *AllocationResult, err error) {
+func (a *crdAllocator) buildAllocationResult(ip net.IP, ipInfo *ipamTypes.AllocationIP) (result *AllocationResult, err error) {
 	result = &AllocationResult{IP: ip}
 
 	// In ENI mode, the Resource points to the ENI so we can derive the
@@ -494,7 +499,7 @@ func (a *crdAllocator) Release(ip net.IP) error {
 
 // markAllocated marks a particular IP as allocated and triggers the custom
 // resource update
-func (a *crdAllocator) markAllocated(ip net.IP, owner string, ipInfo ciliumv2.AllocationIP) {
+func (a *crdAllocator) markAllocated(ip net.IP, owner string, ipInfo ipamTypes.AllocationIP) {
 	ipInfo.Owner = owner
 	a.allocated[ip.String()] = ipInfo
 	a.store.refreshTrigger.TriggerWithReason(fmt.Sprintf("allocation of IP %s", ip.String()))