Skip to content

Commit

Permalink
cnmallocator: make newPortAllocator() infallible
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
corhere committed Feb 20, 2024
1 parent dcda100 commit 196c38f
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 28 deletions.
8 changes: 2 additions & 6 deletions manager/allocator/cnmallocator/networkallocator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
}

Expand Down
19 changes: 7 additions & 12 deletions manager/allocator/cnmallocator/portallocator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 6 additions & 10 deletions manager/allocator/cnmallocator/portallocator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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",
Expand All @@ -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{
Expand Down

0 comments on commit 196c38f

Please sign in to comment.