Skip to content

Commit b306c39

Browse files
bschimke95bschimke95
authored
Move Dqlite bind address logic into utils (#51)
This cleans up the API code by moving implementation details into a separate function.
1 parent 7916334 commit b306c39

File tree

3 files changed

+115
-35
lines changed

3 files changed

+115
-35
lines changed

‎pkg/api/v2/join.go‎

Lines changed: 2 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -130,45 +130,12 @@ func (a *API) Join(ctx context.Context, req JoinRequest) (*JoinResponse, int, er
130130
// Handle datastore updates
131131
switch {
132132
case kubeAPIServerUsesDqlite:
133-
// FIXME(neoaggelos): move this logic into a snaputil.MaybeUpdateDqliteBindAddress() to cleanup the code a little bit
134-
135-
// Check node is not in cluster already.
136133
a.dqliteMu.Lock()
137-
dqliteCluster, err := snaputil.WaitForDqliteCluster(ctx, a.Snap, func(c snaputil.DqliteCluster) (bool, error) {
138-
return len(c) >= 1, nil
139-
})
134+
err := snaputil.MaybeUpdateDqliteBindAddress(ctx, a.Snap, req.HostPort, remoteIP, a.findMatchingBindAddress)
135+
a.dqliteMu.Unlock()
140136
if err != nil {
141-
a.dqliteMu.Unlock()
142137
return nil, http.StatusInternalServerError, fmt.Errorf("failed to retrieve dqlite cluster nodes: %w", err)
143138
}
144-
for _, node := range dqliteCluster {
145-
if strings.HasPrefix(node.Address, remoteIP+":") {
146-
a.dqliteMu.Unlock()
147-
return nil, http.StatusGatewayTimeout, fmt.Errorf("the joining node (%s) is already known to dqlite", remoteIP)
148-
}
149-
}
150-
// Update dqlite cluster if needed
151-
if len(dqliteCluster) == 1 && strings.HasPrefix(dqliteCluster[0].Address, "127.0.0.1:") {
152-
newDqliteBindAddress, err := a.findMatchingBindAddress(req.HostPort)
153-
if err != nil {
154-
a.dqliteMu.Unlock()
155-
return nil, http.StatusInternalServerError, fmt.Errorf("failed to find matching dqlite bind address for %v: %w", req.HostPort, err)
156-
}
157-
if err := snaputil.UpdateDqliteIP(ctx, a.Snap, newDqliteBindAddress); err != nil {
158-
a.dqliteMu.Unlock()
159-
return nil, http.StatusInternalServerError, fmt.Errorf("failed to update dqlite address to %q: %w", newDqliteBindAddress, err)
160-
}
161-
// Wait for dqlite cluster to come up with new address
162-
_, err = snaputil.WaitForDqliteCluster(ctx, a.Snap, func(c snaputil.DqliteCluster) (bool, error) {
163-
return len(c) >= 1 && !strings.HasPrefix(c[0].Address, "127.0.0.1:"), nil
164-
})
165-
if err != nil {
166-
a.dqliteMu.Unlock()
167-
return nil, http.StatusInternalServerError, fmt.Errorf("failed waiting for dqlite cluster to come up: %w", err)
168-
}
169-
}
170-
a.dqliteMu.Unlock()
171-
172139
case req.CanHandleCustomEtcd:
173140
// no-op
174141

‎pkg/snap/util/dqlite.go‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"fmt"
66
"net"
7+
"strings"
78
"time"
89

910
"github.com/canonical/microk8s-cluster-agent/pkg/snap"
@@ -95,3 +96,38 @@ func WaitForDqliteCluster(ctx context.Context, s snap.Snap, f func(DqliteCluster
9596
}
9697
}
9798
}
99+
100+
// MaybeUpdateDqliteBindAddress checks if the node is part of a dqlite cluster and updates it if necessary.
101+
// It ensures the node's hostPort is included in the cluster configuration.
102+
func MaybeUpdateDqliteBindAddress(ctx context.Context, snap snap.Snap, hostPort string, remoteIP string, findMatchingBindAddress func(string) (string, error)) error {
103+
// Check node is not in cluster already.
104+
dqliteCluster, err := WaitForDqliteCluster(ctx, snap, func(c DqliteCluster) (bool, error) {
105+
return len(c) >= 1, nil
106+
})
107+
if err != nil {
108+
return fmt.Errorf("failed to retrieve dqlite cluster nodes: %w", err)
109+
}
110+
for _, node := range dqliteCluster {
111+
if strings.HasPrefix(node.Address, remoteIP+":") {
112+
return fmt.Errorf("the joining node (%s) is already known to dqlite", remoteIP)
113+
}
114+
}
115+
// Update dqlite cluster if needed
116+
if len(dqliteCluster) == 1 && strings.HasPrefix(dqliteCluster[0].Address, "127.0.0.1:") {
117+
newDqliteBindAddress, err := findMatchingBindAddress(hostPort)
118+
if err != nil {
119+
return fmt.Errorf("failed to find matching dqlite bind address for %v: %w", hostPort, err)
120+
}
121+
if err := UpdateDqliteIP(ctx, snap, newDqliteBindAddress); err != nil {
122+
return fmt.Errorf("failed to update dqlite address to %q: %w", newDqliteBindAddress, err)
123+
}
124+
// Wait for dqlite cluster to come up with new address
125+
_, err = WaitForDqliteCluster(ctx, snap, func(c DqliteCluster) (bool, error) {
126+
return len(c) >= 1 && !strings.HasPrefix(c[0].Address, "127.0.0.1:"), nil
127+
})
128+
if err != nil {
129+
return fmt.Errorf("failed waiting for dqlite cluster to come up: %w", err)
130+
}
131+
}
132+
return nil
133+
}

‎pkg/snap/util/dqlite_test.go‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88

99
"github.com/canonical/microk8s-cluster-agent/pkg/snap/mock"
1010
snaputil "github.com/canonical/microk8s-cluster-agent/pkg/snap/util"
11+
. "github.com/onsi/gomega"
1112
)
1213

1314
func TestUpdateDqliteIP(t *testing.T) {
@@ -84,3 +85,79 @@ Role: 0`,
8485
})
8586

8687
}
88+
89+
func TestUpdateDqliteBindAddress(t *testing.T) {
90+
findMatchingBindAddressMock := func(addr string) (string, error) {
91+
return "10.10.10.10", nil
92+
}
93+
94+
t.Run("MustFailIfNodeAlreadyKnown", func(t *testing.T) {
95+
96+
s := &mock.Snap{
97+
DqliteClusterYaml: `
98+
- Address: 127.0.0.1:19001
99+
ID: 1236189235178654365
100+
Role: 0`,
101+
DqliteInfoYaml: `
102+
Address: 127.0.0.1:19001
103+
ID: 1236189235178654365
104+
Role: 0`,
105+
}
106+
107+
g := NewWithT(t)
108+
err := snaputil.MaybeUpdateDqliteBindAddress(context.Background(), s, "127.0.0.1", "127.0.0.1", findMatchingBindAddressMock)
109+
g.Expect(err).To(MatchError("the joining node (127.0.0.1) is already known to dqlite"))
110+
111+
})
112+
113+
t.Run("MustNotUpdateIfMultipleNodes", func(t *testing.T) {
114+
s := &mock.Snap{
115+
DqliteClusterYaml: `
116+
- Address: 127.0.0.1:19001
117+
ID: 1236189235178654365
118+
Role: 0
119+
- Address: 10.10.10.10:19001
120+
ID: 12345678987654321
121+
Role: 0`,
122+
DqliteInfoYaml: `
123+
Address: 127.0.0.1:19001
124+
ID: 1236189235178654365
125+
Role: 0`,
126+
}
127+
128+
g := NewWithT(t)
129+
err := snaputil.MaybeUpdateDqliteBindAddress(context.Background(), s, "127.0.0.1:19001", "8.8.8.8", findMatchingBindAddressMock)
130+
g.Expect(err).To(BeNil())
131+
g.Expect(s.WriteDqliteUpdateYamlCalledWith).To(BeEmpty())
132+
})
133+
134+
t.Run("MustUpdateIfOneNodeAndBindsLocal", func(t *testing.T) {
135+
s := &mock.Snap{
136+
DqliteClusterYaml: `
137+
- Address: 127.0.0.1:19001
138+
ID: 1236189235178654365
139+
Role: 0`,
140+
DqliteInfoYaml: `
141+
Address: 127.0.0.1:19001
142+
ID: 1236189235178654365
143+
Role: 0`,
144+
}
145+
// Update cluster yaml asynchronously
146+
// The mock snap implementation doesn't actually write the yaml but instead
147+
// sets `WriteDqliteUpdateYamlCalledWith`.
148+
// Thus, we need to manually change the config as otherwise `WaitForDqliteCluster`
149+
// would wait forever.
150+
go func() {
151+
<-time.After(500 * time.Millisecond)
152+
s.DqliteClusterYaml = `
153+
- Address: 10.10.10.10:19001
154+
ID: 1236189235178654365
155+
Role: 0`
156+
}()
157+
158+
g := NewWithT(t)
159+
err := snaputil.MaybeUpdateDqliteBindAddress(context.Background(), s, "127.0.0.1:19001", "10.10.10.10", findMatchingBindAddressMock)
160+
g.Expect(err).To(BeNil())
161+
g.Expect(s.WriteDqliteUpdateYamlCalledWith).To(ConsistOf("Address: 10.10.10.10:19001\n"))
162+
})
163+
}

0 commit comments

Comments
 (0)