Skip to content

Fix Merge Shmoo #241

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jun 23, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 28 additions & 9 deletions pkg/lbmanager/lbmanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,23 @@ func (m *LBManager) CreateSvcLBIfNone(ctx context.Context, in *api.Postgres) err
return fmt.Errorf("failed to fetch Service of type LoadBalancer: %w", err)
}

nextFreePort, err := m.nextFreePort(ctx)
existingLBIP, nextFreePort, err := m.nextFreeSocket(ctx)
if err != nil {
return fmt.Errorf("failed to get the next free port: %w", err)
return fmt.Errorf("failed to get a free port for creating Service of type LoadBalancer: %w", err)
}
var lbIPToUse string
if m.LBIP != "" {
// a specific IP was configured in the config, so use that one
lbIPToUse = m.LBIP
} else if existingLBIP != "" {
// no ip was configured, but one is already in use, so use the existing one
lbIPToUse = existingLBIP
} else {
// nothing was configured, nothing exists yet, so use an empty address so a new loadbalancer will be created and assigned
lbIPToUse = ""
}

if err := m.Create(ctx, in.ToSvcLB(m.LBIP, nextFreePort)); err != nil {
if err := m.Create(ctx, in.ToSvcLB(lbIPToUse, nextFreePort)); err != nil {
return fmt.Errorf("failed to create Service of type LoadBalancer: %w", err)
}
return nil
Expand All @@ -64,27 +75,35 @@ func (m *LBManager) DeleteSvcLB(ctx context.Context, in *api.Postgres) error {
}

// nextFreeSocket finds any existing LoadBalancerIP and the next free port out of the configure port range.
func (m *LBManager) nextFreePort(ctx context.Context) (int32, error) {
func (m *LBManager) nextFreeSocket(ctx context.Context) (string, int32, error) {
// TODO prevent concurrency issues when calculating port / ip.

anyExistingLBIP := ""

// Fetch all services managed by this postgreslet
lbs := &corev1.ServiceList{}
if err := m.List(ctx, lbs, client.MatchingLabels(api.SvcLoadBalancerLabel)); err != nil {
return 0, fmt.Errorf("failed to fetch the list of services of type LoadBalancer: %w", err)
return anyExistingLBIP, 0, fmt.Errorf("failed to fetch the list of services of type LoadBalancer: %w", err)
}

// If there are none, this will be the first (managed) service we create, so start with PortRangeStart and return
if len(lbs.Items) == 0 {
return m.PortRangeStart, nil
return anyExistingLBIP, m.PortRangeStart, nil
}

// If there are already any managed services, store all the used ports in a slice.
portsInUse := []int32{}
// Also store the LoadBalancerIP.
portsInUse := make([]int32, 0, len(lbs.Items))
for i := range lbs.Items {
svc := lbs.Items[i]
if len(svc.Spec.Ports) > 0 {
portsInUse = append(portsInUse, svc.Spec.Ports[0].Port)
}
if svc.Spec.LoadBalancerIP != "" {
// Technically, we only store the IP of the last Service in this list.
// As there should only be one IP per postgreslet and one postgreslet per cluster, this is good enough.
anyExistingLBIP = svc.Spec.LoadBalancerIP
}
}

// Now try all ports in the configured port range to find a free one.
Expand All @@ -96,11 +115,11 @@ func (m *LBManager) nextFreePort(ctx context.Context) (int32, error) {
continue
}
// The postgreslet hasn't assigned this port yet, so use it.
return port, nil
return anyExistingLBIP, port, nil
}

// If we made it this far, no free port could be found.
return 0, errors.New("no free port in the configured port range found")
return anyExistingLBIP, 0, errors.New("no free port in the configured port range found")
}

func containsElem(s []int32, v int32) bool {
Expand Down
34 changes: 17 additions & 17 deletions pkg/lbmanager/lbmanager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@ func TestLBManager_nextFreePort(t *testing.T) {
portRangeSize := int32(5)

tests := []struct {
name string
lbMgr *LBManager
want int32
wantErr bool
name string
lbMgr *LBManager
portWant int32
wantErr bool
}{
{
name: "no svc in the cluster",
Expand All @@ -29,8 +29,8 @@ func TestLBManager_nextFreePort(t *testing.T) {
PortRangeStart: portRangeStart,
PortRangeSize: portRangeSize,
},
want: 0,
wantErr: false,
portWant: 0,
wantErr: false,
},
{
name: "one svc already in the cluster",
Expand All @@ -40,8 +40,8 @@ func TestLBManager_nextFreePort(t *testing.T) {
PortRangeStart: portRangeStart,
PortRangeSize: portRangeSize,
},
want: 1,
wantErr: false,
portWant: 1,
wantErr: false,
},
{
name: "last free port left",
Expand All @@ -51,8 +51,8 @@ func TestLBManager_nextFreePort(t *testing.T) {
PortRangeStart: portRangeStart,
PortRangeSize: portRangeSize,
},
want: 4,
wantErr: false,
portWant: 4,
wantErr: false,
},
{
name: "no free port",
Expand All @@ -62,8 +62,8 @@ func TestLBManager_nextFreePort(t *testing.T) {
PortRangeStart: portRangeStart,
PortRangeSize: portRangeSize,
},
want: 0,
wantErr: true,
portWant: 0,
wantErr: true,
},
{
name: "re-use releaased port",
Expand All @@ -73,21 +73,21 @@ func TestLBManager_nextFreePort(t *testing.T) {
PortRangeStart: portRangeStart,
PortRangeSize: portRangeSize,
},
want: 1,
wantErr: false,
portWant: 1,
wantErr: false,
},
}

for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
got, err := tt.lbMgr.nextFreePort(context.Background())
_, portGot, err := tt.lbMgr.nextFreeSocket(context.Background())
if (err != nil) != tt.wantErr {
t.Errorf("LBManager.nextFreePort() error = %v, wantErr %v", err, tt.wantErr)
return
}
if got != tt.want {
t.Errorf("LBManager.nextFreePort() = %v, want %v", got, tt.want)
if portGot != tt.portWant {
t.Errorf("LBManager.nextFreePort() = %v, want %v", portGot, tt.portWant)
}
})
}
Expand Down