From 196c38fd52ce6374a9a7a9aace79866d63fd3c0a Mon Sep 17 00:00:00 2001
From: Cory Snider <csnider@mirantis.com>
Date: Tue, 20 Feb 2024 17:19:15 -0500
Subject: [PATCH] cnmallocator: make newPortAllocator() infallible

The portAllocator constructor function newPortAllocator() takes no
arguments; all its initial state is taken from hardcoded constants in
the source code. Any failure to construct a port allocator must
therefore be the result of a bug in the code, which can only be fixed
with a code change. As there is nothing the caller can do at runtime to
cause the failure, there is little value in returning the error to the
caller. Change the constructor to panic on failure instead.

Signed-off-by: Cory Snider <csnider@mirantis.com>
---
 .../cnmallocator/networkallocator.go          |  8 ++------
 .../allocator/cnmallocator/portallocator.go   | 19 +++++++------------
 .../cnmallocator/portallocator_test.go        | 16 ++++++----------
 3 files changed, 15 insertions(+), 28 deletions(-)

diff --git a/manager/allocator/cnmallocator/networkallocator.go b/manager/allocator/cnmallocator/networkallocator.go
index 7ca55f8dce..3c9d0c4617 100644
--- a/manager/allocator/cnmallocator/networkallocator.go
+++ b/manager/allocator/cnmallocator/networkallocator.go
@@ -108,6 +108,8 @@ func New(pg plugingetter.PluginGetter, netConfig *NetworkConfig) (networkallocat
 		tasks:    make(map[string]struct{}),
 		nodes:    make(map[string]map[string]struct{}),
 		pg:       pg,
+
+		portAllocator: newPortAllocator(),
 	}
 
 	for ntype, i := range initializers {
@@ -126,12 +128,6 @@ func New(pg plugingetter.PluginGetter, netConfig *NetworkConfig) (networkallocat
 		return nil, fmt.Errorf("failed to initialize IPAM driver plugins: %w", err)
 	}
 
-	pa, err := newPortAllocator()
-	if err != nil {
-		return nil, err
-	}
-
-	na.portAllocator = pa
 	return na, nil
 }
 
diff --git a/manager/allocator/cnmallocator/portallocator.go b/manager/allocator/cnmallocator/portallocator.go
index 818613fe81..2ebb35b33e 100644
--- a/manager/allocator/cnmallocator/portallocator.go
+++ b/manager/allocator/cnmallocator/portallocator.go
@@ -101,36 +101,31 @@ func (ps allocatedPorts) delState(p *api.PortConfig) *api.PortConfig {
 	return nil
 }
 
-func newPortAllocator() (*portAllocator, error) {
+func newPortAllocator() *portAllocator {
 	portSpaces := make(map[api.PortConfig_Protocol]*portSpace)
 	for _, protocol := range []api.PortConfig_Protocol{api.ProtocolTCP, api.ProtocolUDP, api.ProtocolSCTP} {
-		ps, err := newPortSpace(protocol)
-		if err != nil {
-			return nil, err
-		}
-
-		portSpaces[protocol] = ps
+		portSpaces[protocol] = newPortSpace(protocol)
 	}
 
-	return &portAllocator{portSpaces: portSpaces}, nil
+	return &portAllocator{portSpaces: portSpaces}
 }
 
-func newPortSpace(protocol api.PortConfig_Protocol) (*portSpace, error) {
+func newPortSpace(protocol api.PortConfig_Protocol) *portSpace {
 	master, err := idm.New(masterPortStart, masterPortEnd)
 	if err != nil {
-		return nil, err
+		panic(err)
 	}
 
 	dynamic, err := idm.New(dynamicPortStart, dynamicPortEnd)
 	if err != nil {
-		return nil, err
+		panic(err)
 	}
 
 	return &portSpace{
 		protocol:         protocol,
 		masterPortSpace:  master,
 		dynamicPortSpace: dynamic,
-	}, nil
+	}
 }
 
 // getPortConfigKey returns a map key for doing set operations with
diff --git a/manager/allocator/cnmallocator/portallocator_test.go b/manager/allocator/cnmallocator/portallocator_test.go
index b514f40077..0970b37931 100644
--- a/manager/allocator/cnmallocator/portallocator_test.go
+++ b/manager/allocator/cnmallocator/portallocator_test.go
@@ -180,8 +180,7 @@ func TestReconcilePortConfigs(t *testing.T) {
 }
 
 func TestAllocateServicePorts(t *testing.T) {
-	pa, err := newPortAllocator()
-	assert.NoError(t, err)
+	pa := newPortAllocator()
 
 	// Service has no endpoint in ServiceSpec
 	s := &api.Service{
@@ -200,7 +199,7 @@ func TestAllocateServicePorts(t *testing.T) {
 		},
 	}
 
-	err = pa.serviceAllocatePorts(s)
+	err := pa.serviceAllocatePorts(s)
 	assert.NoError(t, err)
 
 	// Service has a published port 10001 in ServiceSpec
@@ -265,8 +264,7 @@ func TestAllocateServicePorts(t *testing.T) {
 }
 
 func TestHostPublishPortsNeedUpdate(t *testing.T) {
-	pa, err := newPortAllocator()
-	assert.NoError(t, err)
+	pa := newPortAllocator()
 
 	type Data struct {
 		name   string
@@ -494,8 +492,7 @@ func TestHostPublishPortsNeedUpdate(t *testing.T) {
 }
 
 func TestIsPortsAllocated(t *testing.T) {
-	pa, err := newPortAllocator()
-	assert.NoError(t, err)
+	pa := newPortAllocator()
 
 	type Data struct {
 		name   string
@@ -886,8 +883,7 @@ func TestIsPortsAllocated(t *testing.T) {
 }
 
 func TestAllocate(t *testing.T) {
-	pSpace, err := newPortSpace(api.ProtocolTCP)
-	assert.NoError(t, err)
+	pSpace := newPortSpace(api.ProtocolTCP)
 
 	pConfig := &api.PortConfig{
 		Name:          "test1",
@@ -897,7 +893,7 @@ func TestAllocate(t *testing.T) {
 	}
 
 	// first consume 30000 in dynamicPortSpace
-	err = pSpace.allocate(pConfig)
+	err := pSpace.allocate(pConfig)
 	assert.NoError(t, err)
 
 	pConfig = &api.PortConfig{