ipam: Move node manager to operator/pkg/ipam/nodemanager

The `Node` and `NodeManager` types in `pkg/ipam` (`node.go` and
`node_manager.go`) drive IP orchestration on the operator side: they
import `operator/k8s` and `operator/watchers`, and their only callers
are the cloud-provider allocators (now under
`operator/pkg/ipam/allocator`).

This commit continues work started in afbcb75 and moves those
types under `operator/pkg/ipam/nodemanager` so the agent/operator
boundary becomes more clear.

Signed-off-by: Hadrien Patte <hadrien.patte@datadoghq.com>

Author: HadrienPatte

operator/pkg/ipam/allocator/alibabacloud/alibabacloud.go

@@ -13,11 +13,11 @@ import (
 
 	operatorOption "github.com/cilium/cilium/operator/option"
 	"github.com/cilium/cilium/operator/pkg/ipam/allocator"
+	"github.com/cilium/cilium/operator/pkg/ipam/nodemanager"
 	alibabacloudAPI "github.com/cilium/cilium/pkg/alibabacloud/api"
 	"github.com/cilium/cilium/pkg/alibabacloud/eni"
 	"github.com/cilium/cilium/pkg/alibabacloud/eni/limits"
 	"github.com/cilium/cilium/pkg/alibabacloud/metadata"
-	"github.com/cilium/cilium/pkg/ipam"
 	"github.com/cilium/cilium/pkg/logging/logfields"
 )
 
@@ -89,11 +89,11 @@ func (a *AllocatorAlibabaCloud) Init(ctx context.Context, logger *slog.Logger, a
 // Start kicks off ENI allocation, the initial connection to AlibabaCloud
 // APIs is done in a blocking manner. Provided this is successful, a controller is
 // started to manage allocation based on CiliumNode custom resources
-func (a *AllocatorAlibabaCloud) Start(ctx context.Context, getterUpdater ipam.CiliumNodeGetterUpdater, iMetrics ipam.MetricsAPI) (allocator.NodeEventHandler, error) {
+func (a *AllocatorAlibabaCloud) Start(ctx context.Context, getterUpdater allocator.CiliumNodeGetterUpdater, iMetrics nodemanager.MetricsAPI) (allocator.NodeEventHandler, error) {
 	a.logger.Info("Starting AlibabaCloud ENI allocator...")
 
 	instances := eni.NewInstancesManager(a.rootLogger, a.client)
-	nodeManager, err := ipam.NewNodeManager(a.logger, instances, getterUpdater, iMetrics,
+	nodeManager, err := nodemanager.NewNodeManager(a.logger, instances, getterUpdater, iMetrics,
 		a.ParallelAllocWorkers, a.AlibabaCloudReleaseExcessIPs, 0, false)
 	if err != nil {
 		return nil, fmt.Errorf("unable to initialize AlibabaCloud node manager: %w", err)

operator/pkg/ipam/allocator/aws/aws.go

@@ -13,11 +13,11 @@ import (
 
 	operatorOption "github.com/cilium/cilium/operator/option"
 	"github.com/cilium/cilium/operator/pkg/ipam/allocator"
+	"github.com/cilium/cilium/operator/pkg/ipam/nodemanager"
 	ec2shim "github.com/cilium/cilium/pkg/aws/ec2"
 	"github.com/cilium/cilium/pkg/aws/eni"
 	"github.com/cilium/cilium/pkg/aws/metadata"
 	"github.com/cilium/cilium/pkg/defaults"
-	"github.com/cilium/cilium/pkg/ipam"
 	"github.com/cilium/cilium/pkg/logging/logfields"
 	"github.com/cilium/cilium/pkg/option"
 	"github.com/cilium/cilium/pkg/time"
@@ -125,7 +125,7 @@ func (a *AllocatorAWS) Init(ctx context.Context, logger *slog.Logger, aMetrics e
 // Start kicks of ENI allocation, the initial connection to AWS
 // APIs is done in a blocking manner, given that is successful, a controller is
 // started to manage allocation based on CiliumNode custom resources
-func (a *AllocatorAWS) Start(ctx context.Context, getterUpdater ipam.CiliumNodeGetterUpdater, iMetrics ipam.MetricsAPI) (allocator.NodeEventHandler, error) {
+func (a *AllocatorAWS) Start(ctx context.Context, getterUpdater allocator.CiliumNodeGetterUpdater, iMetrics nodemanager.MetricsAPI) (allocator.NodeEventHandler, error) {
 	a.logger.Info("Starting ENI allocator...")
 
 	imds, err := metadata.NewClient(ctx)
@@ -136,7 +136,7 @@ func (a *AllocatorAWS) Start(ctx context.Context, getterUpdater ipam.CiliumNodeG
 	if err != nil {
 		return nil, fmt.Errorf("unable to initialize ENI instances manager: %w", err)
 	}
-	nodeManager, err := ipam.NewNodeManager(a.logger, instances, getterUpdater, iMetrics,
+	nodeManager, err := nodemanager.NewNodeManager(a.logger, instances, getterUpdater, iMetrics,
 		a.ParallelAllocWorkers, a.AWSReleaseExcessIPs, a.ExcessIPReleaseDelay,
 		a.AWSEnablePrefixDelegation)
 	if err != nil {

operator/pkg/ipam/allocator/azure/azure.go

@@ -9,9 +9,9 @@ import (
 	"log/slog"
 
 	"github.com/cilium/cilium/operator/pkg/ipam/allocator"
+	"github.com/cilium/cilium/operator/pkg/ipam/nodemanager"
 	azureAPI "github.com/cilium/cilium/pkg/azure/api"
 	azureIPAM "github.com/cilium/cilium/pkg/azure/ipam"
-	"github.com/cilium/cilium/pkg/ipam"
 	"github.com/cilium/cilium/pkg/logging/logfields"
 )
 
@@ -37,7 +37,7 @@ func (a *AllocatorAzure) Init(ctx context.Context, logger *slog.Logger) error {
 }
 
 // Start kicks of the Azure IP allocation
-func (a *AllocatorAzure) Start(ctx context.Context, getterUpdater ipam.CiliumNodeGetterUpdater, azMetrics azureAPI.MetricsAPI, iMetrics ipam.MetricsAPI) (allocator.NodeEventHandler, error) {
+func (a *AllocatorAzure) Start(ctx context.Context, getterUpdater allocator.CiliumNodeGetterUpdater, azMetrics azureAPI.MetricsAPI, iMetrics nodemanager.MetricsAPI) (allocator.NodeEventHandler, error) {
 	a.logger.Info("Starting Azure IP allocator...")
 
 	a.logger.Debug("Retrieving Azure cloud name via Azure IMS")
@@ -73,7 +73,7 @@ func (a *AllocatorAzure) Start(ctx context.Context, getterUpdater ipam.CiliumNod
 		return nil, fmt.Errorf("unable to create Azure client: %w", err)
 	}
 	instances := azureIPAM.NewInstancesManager(a.rootLogger, azureClient)
-	nodeManager, err := ipam.NewNodeManager(a.logger, instances, getterUpdater, iMetrics, a.ParallelAllocWorkers, false, 0, false)
+	nodeManager, err := nodemanager.NewNodeManager(a.logger, instances, getterUpdater, iMetrics, a.ParallelAllocWorkers, false, 0, false)
 	if err != nil {
 		return nil, fmt.Errorf("unable to initialize Azure node manager: %w", err)
 	}

operator/pkg/ipam/allocator/clusterpool/clusterpool.go

@@ -11,7 +11,6 @@ import (
 	"github.com/cilium/cilium/operator/pkg/ipam/allocator"
 	"github.com/cilium/cilium/operator/pkg/ipam/allocator/clusterpool/cidralloc"
 	"github.com/cilium/cilium/operator/pkg/ipam/allocator/podcidr"
-	"github.com/cilium/cilium/pkg/ipam"
 	"github.com/cilium/cilium/pkg/logging/logfields"
 	"github.com/cilium/cilium/pkg/option"
 	"github.com/cilium/cilium/pkg/trigger"
@@ -77,7 +76,7 @@ func (a *AllocatorOperator) Init(ctx context.Context, logger *slog.Logger) error
 }
 
 // Start kicks of Operator allocation.
-func (a *AllocatorOperator) Start(ctx context.Context, updater ipam.CiliumNodeGetterUpdater, iMetrics trigger.MetricsObserver) (allocator.NodeEventHandler, error) {
+func (a *AllocatorOperator) Start(ctx context.Context, updater allocator.CiliumNodeGetterUpdater, iMetrics trigger.MetricsObserver) (allocator.NodeEventHandler, error) {
 	a.logger.Info(
 		"Starting ClusterPool IP allocator",
 		logfields.IPv4CIDRs, a.ClusterPoolIPv4CIDR,

operator/pkg/ipam/allocator/podcidr/podcidr.go

@@ -14,9 +14,9 @@ import (
 
 	k8sErrors "k8s.io/apimachinery/pkg/api/errors"
 
+	"github.com/cilium/cilium/operator/pkg/ipam/allocator"
 	"github.com/cilium/cilium/operator/pkg/ipam/allocator/clusterpool/cidralloc"
 	"github.com/cilium/cilium/pkg/controller"
-	"github.com/cilium/cilium/pkg/ipam"
 	v2 "github.com/cilium/cilium/pkg/k8s/apis/cilium.io/v2"
 	"github.com/cilium/cilium/pkg/lock"
 	"github.com/cilium/cilium/pkg/logging/logfields"
@@ -191,7 +191,7 @@ type NodesPodCIDRManager struct {
 func NewNodesPodCIDRManager(
 	logger *slog.Logger,
 	v4Allocators, v6Allocators []cidralloc.CIDRAllocator,
-	nodeGetter ipam.CiliumNodeGetterUpdater,
+	nodeGetter allocator.CiliumNodeGetterUpdater,
 	triggerMetrics trigger.MetricsObserver) *NodesPodCIDRManager {
 
 	n := &NodesPodCIDRManager{
@@ -242,7 +242,7 @@ func NewNodesPodCIDRManager(
 // In case any of the nodes failed to be synced with kubernetes the returned
 // error is for one of those nodes. Remaining nodes will still be synced with
 // kubernetes.
-func syncToK8s(logger *slog.Logger, nodeGetterUpdater ipam.CiliumNodeGetterUpdater, ciliumNodesToK8s map[string]*ciliumNodeK8sOp) (retErr error) {
+func syncToK8s(logger *slog.Logger, nodeGetterUpdater allocator.CiliumNodeGetterUpdater, ciliumNodesToK8s map[string]*ciliumNodeK8sOp) (retErr error) {
 	for nodeName, nodeToK8s := range ciliumNodesToK8s {
 		var (
 			err, err2     error

operator/pkg/ipam/allocator/types.go

@@ -16,3 +16,12 @@ type NodeEventHandler interface {
 	Delete(resource *v2.CiliumNode)
 	Resync(context.Context, time.Time)
 }
+
+// CiliumNodeGetterUpdater defines the interface used to interact with the k8s
+// apiserver to retrieve and update the CiliumNode custom resource
+type CiliumNodeGetterUpdater interface {
+	Create(node *v2.CiliumNode) (*v2.CiliumNode, error)
+	Update(origResource, newResource *v2.CiliumNode) (*v2.CiliumNode, error)
+	UpdateStatus(origResource, newResource *v2.CiliumNode) (*v2.CiliumNode, error)
+	Get(name string) (*v2.CiliumNode, error)
+}

operator/pkg/ipam/metrics/metrics.go

@@ -196,7 +196,7 @@ func NewMetrics() *Metrics {
 	}
 }
 
-// ipam.MetricsAPI implementation
+// nodemanager.MetricsAPI implementation
 
 func (m *Metrics) SetIPAvailable(node string, cap int) {
 	m.AvailableIPs.WithLabelValues(node).Set(float64(cap))

pkg/ipam/node.go operator/pkg/ipam/nodemanager/node.gopkg/ipam/node.go renamed to operator/pkg/ipam/nodemanager/node.go

@@ -3,7 +3,7 @@
 
 // Copyright 2017 Lyft, Inc.
 
-package ipam
+package nodemanager
 
 import (
 	"context"
@@ -47,6 +47,8 @@ const (
 	// operator status
 	success = "success"
 	failed  = "failed"
+
+	fieldName = "name"
 )
 
 func (n *Node) SetOpts(ops NodeOperations) {
@@ -478,9 +480,7 @@ func (n *Node) recalculate(ctx context.Context) {
 	defer n.mutex.Unlock()
 
 	if err != nil {
-		var limitsNotFound LimitsNotFound
-		ok := errors.As(err, &limitsNotFound)
-		if ok {
+		if errors.Is(err, ErrLimitsNotFound) {
 			scopedLog.Warn("Instance limits not found.", logfields.Error, err)
 		} else {
 			scopedLog.Warn("Instance not found! Please delete corresponding ciliumnode if instance has already been deleted.", logfields.Error, err)
@@ -765,6 +765,9 @@ type ReleaseAction struct {
 	IPPrefixesToRelease []string
 }
 
+// ErrLimitsNotFound signals lack of limits for given instance type.
+var ErrLimitsNotFound = errors.New("Limits not found")
+
 // maintenanceAction represents the resources available for allocation for a
 // particular ciliumNode. If an existing interface has IP allocation capacity
 // left, that capacity is used up first. If not, an available index is found to
@@ -1283,7 +1286,7 @@ func (n *Node) syncToAPIServer() error {
 		// The PreAllocate value is added here rather than where the CiliumNode
 		// resource is created ((*NodeDiscovery).mutateNodeResource() inside
 		// pkg/nodediscovery), because mutateNodeResource() does not have
-		// access to the ipam.Node object. Since we are in the CiliumNode
+		// access to the nodemanager.Node object. Since we are in the CiliumNode
 		// update sync loop, we can compute the value.
 		if node.Spec.IPAM.PreAllocate == 0 {
 			node.Spec.IPAM.PreAllocate = n.ops.GetMinimumAllocatableIPv4()

pkg/ipam/node_manager.go …tor/pkg/ipam/nodemanager/node_manager.gopkg/ipam/node_manager.go renamed to operator/pkg/ipam/nodemanager/node_manager.go pkg/ipam/node_manager.go renamed to operator/pkg/ipam/nodemanager/node_manager.go

@@ -3,7 +3,7 @@
 
 // Copyright 2017 Lyft, Inc.
 
-package ipam
+package nodemanager
 
 import (
 	"context"
@@ -13,6 +13,7 @@ import (
 
 	"golang.org/x/sync/semaphore"
 
+	"github.com/cilium/cilium/operator/pkg/ipam/allocator"
 	ipamStats "github.com/cilium/cilium/operator/pkg/ipam/stats"
 	"github.com/cilium/cilium/pkg/backoff"
 	"github.com/cilium/cilium/pkg/controller"
@@ -27,15 +28,6 @@ import (
 
 var ipamNodeIntervalControllerGroup = controller.NewGroup("ipam-node-interval-refresh")
 
-// CiliumNodeGetterUpdater defines the interface used to interact with the k8s
-// apiserver to retrieve and update the CiliumNode custom resource
-type CiliumNodeGetterUpdater interface {
-	Create(node *v2.CiliumNode) (*v2.CiliumNode, error)
-	Update(origResource, newResource *v2.CiliumNode) (*v2.CiliumNode, error)
-	UpdateStatus(origResource, newResource *v2.CiliumNode) (*v2.CiliumNode, error)
-	Get(name string) (*v2.CiliumNode, error)
-}
-
 // NodeOperations is the interface an IPAM implementation must provide in order
 // to provide IP allocation for a node. The structure implementing this API
 // *must* be aware of the node connected to this implementation. This is
@@ -171,7 +163,7 @@ type NodeManager struct {
 	mutex                lock.RWMutex
 	nodes                nodeMap
 	instancesAPI         AllocationImplementation
-	k8sAPI               CiliumNodeGetterUpdater
+	k8sAPI               allocator.CiliumNodeGetterUpdater
 	metricsAPI           MetricsAPI
 	parallelWorkers      int64
 	releaseExcessIPs     bool
@@ -189,7 +181,7 @@ func (n *NodeManager) ClusterSizeDependantInterval(baseInterval time.Duration) t
 }
 
 // NewNodeManager returns a new NodeManager
-func NewNodeManager(logger *slog.Logger, instancesAPI AllocationImplementation, k8sAPI CiliumNodeGetterUpdater, metrics MetricsAPI,
+func NewNodeManager(logger *slog.Logger, instancesAPI AllocationImplementation, k8sAPI allocator.CiliumNodeGetterUpdater, metrics MetricsAPI,
 	parallelWorkers int64, releaseExcessIPs bool, excessIPReleaseDelay int, prefixDelegation bool) (*NodeManager, error) {
 	if parallelWorkers < 1 {
 		parallelWorkers = 1

pkg/ipam/node_manager_test.go …kg/ipam/nodemanager/node_manager_test.gopkg/ipam/node_manager_test.go renamed to operator/pkg/ipam/nodemanager/node_manager_test.go pkg/ipam/node_manager_test.go renamed to operator/pkg/ipam/nodemanager/node_manager_test.go

@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: Apache-2.0
 // Copyright Authors of Cilium
 
-package ipam
+package nodemanager
 
 import (
 	"context"