Skip to content

Commit 469ff7a

Browse files
ethanndicksonethanndickson
committed
review
1 parent 1ac2bae commit 469ff7a

File tree

2 files changed

+37
-35
lines changed

2 files changed

+37
-35
lines changed

‎tailnet/controllers.go

+32-31
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"maps"
88
"math"
99
"net/netip"
10+
"slices"
1011
"strings"
1112
"sync"
1213
"time"
@@ -19,6 +20,7 @@ import (
1920
"tailscale.com/util/dnsname"
2021

2122
"cdr.dev/slog"
23+
"github.com/coder/coder/v2/coderd/util/ptr"
2224
"github.com/coder/coder/v2/codersdk"
2325
"github.com/coder/coder/v2/tailnet/proto"
2426
"github.com/coder/quartz"
@@ -918,6 +920,19 @@ type Agent struct {
918920
Hosts map[dnsname.FQDN][]netip.Addr
919921
}
920922

923+
func (a *Agent) Clone() Agent {
924+
hosts := make(map[dnsname.FQDN][]netip.Addr, len(a.Hosts))
925+
for k, v := range a.Hosts {
926+
hosts[k] = slices.Clone(v)
927+
}
928+
return Agent{
929+
ID: a.ID,
930+
Name: a.Name,
931+
WorkspaceID: a.WorkspaceID,
932+
Hosts: hosts,
933+
}
934+
}
935+
921936
func (t *TunnelAllWorkspaceUpdatesController) New(client WorkspaceUpdatesClient) CloserWaiter {
922937
t.mu.Lock()
923938
defer t.mu.Unlock()
@@ -958,12 +973,7 @@ func (t *TunnelAllWorkspaceUpdatesController) CurrentState() (WorkspaceUpdate, e
958973
Status: w.Status,
959974
})
960975
for _, a := range w.agents {
961-
out.UpsertedAgents = append(out.UpsertedAgents, &Agent{
962-
ID: a.ID,
963-
Name: a.Name,
964-
WorkspaceID: a.WorkspaceID,
965-
Hosts: maps.Clone(a.Hosts),
966-
})
976+
out.UpsertedAgents = append(out.UpsertedAgents, ptr.Ref(a.Clone()))
967977
}
968978
}
969979
return out, nil
@@ -1064,12 +1074,7 @@ func (w *WorkspaceUpdate) Clone() WorkspaceUpdate {
10641074
}
10651075
}
10661076
for i, a := range w.UpsertedAgents {
1067-
clone.UpsertedAgents[i] = &Agent{
1068-
ID: a.ID,
1069-
Name: a.Name,
1070-
WorkspaceID: a.WorkspaceID,
1071-
Hosts: maps.Clone(a.Hosts),
1072-
}
1077+
clone.UpsertedAgents[i] = ptr.Ref(a.Clone())
10731078
}
10741079
for i, ws := range w.DeletedWorkspaces {
10751080
clone.DeletedWorkspaces[i] = &Workspace{
@@ -1079,12 +1084,7 @@ func (w *WorkspaceUpdate) Clone() WorkspaceUpdate {
10791084
}
10801085
}
10811086
for i, a := range w.DeletedAgents {
1082-
clone.DeletedAgents[i] = &Agent{
1083-
ID: a.ID,
1084-
Name: a.Name,
1085-
WorkspaceID: a.WorkspaceID,
1086-
Hosts: maps.Clone(a.Hosts),
1087-
}
1087+
clone.DeletedAgents[i] = ptr.Ref(a.Clone())
10881088
}
10891089
return clone
10901090
}
@@ -1112,7 +1112,7 @@ func (t *tunnelUpdater) handleUpdate(update *proto.WorkspaceUpdate) error {
11121112
ownerUsername: t.ownerUsername,
11131113
agents: make(map[uuid.UUID]*Agent),
11141114
}
1115-
t.upsertWorkspace(w)
1115+
t.upsertWorkspaceLocked(w)
11161116
currentUpdate.UpsertedWorkspaces = append(currentUpdate.UpsertedWorkspaces, w)
11171117
}
11181118

@@ -1126,7 +1126,7 @@ func (t *tunnelUpdater) handleUpdate(update *proto.WorkspaceUpdate) error {
11261126
if err != nil {
11271127
return xerrors.Errorf("failed to parse workspace ID: %w", err)
11281128
}
1129-
deletedAgent, err := t.deleteAgent(workspaceID, agentID)
1129+
deletedAgent, err := t.deleteAgentLocked(workspaceID, agentID)
11301130
if err != nil {
11311131
return xerrors.Errorf("failed to delete agent: %w", err)
11321132
}
@@ -1137,7 +1137,7 @@ func (t *tunnelUpdater) handleUpdate(update *proto.WorkspaceUpdate) error {
11371137
if err != nil {
11381138
return xerrors.Errorf("failed to parse workspace ID: %w", err)
11391139
}
1140-
deletedWorkspace, err := t.deleteWorkspace(workspaceID)
1140+
deletedWorkspace, err := t.deleteWorkspaceLocked(workspaceID)
11411141
if err != nil {
11421142
return xerrors.Errorf("failed to delete workspace: %w", err)
11431143
}
@@ -1156,15 +1156,15 @@ func (t *tunnelUpdater) handleUpdate(update *proto.WorkspaceUpdate) error {
11561156
return xerrors.Errorf("failed to parse workspace ID: %w", err)
11571157
}
11581158
a := &Agent{Name: ua.Name, ID: agentID, WorkspaceID: workspaceID}
1159-
err = t.upsertAgent(workspaceID, a)
1159+
err = t.upsertAgentLocked(workspaceID, a)
11601160
if err != nil {
11611161
return xerrors.Errorf("failed to upsert agent: %w", err)
11621162
}
11631163
currentUpdate.UpsertedAgents = append(currentUpdate.UpsertedAgents, a)
11641164
}
1165-
allAgents := t.allAgentIDs()
1165+
allAgents := t.allAgentIDsLocked()
11661166
t.coordCtrl.SyncDestinations(allAgents)
1167-
dnsNames := t.updateDNSNames()
1167+
dnsNames := t.updateDNSNamesLocked()
11681168
if t.dnsHostsSetter != nil {
11691169
t.logger.Debug(context.Background(), "updating dns hosts")
11701170
err := t.dnsHostsSetter.SetDNSHosts(dnsNames)
@@ -1184,7 +1184,7 @@ func (t *tunnelUpdater) handleUpdate(update *proto.WorkspaceUpdate) error {
11841184
return nil
11851185
}
11861186

1187-
func (t *tunnelUpdater) upsertWorkspace(w *Workspace) *Workspace {
1187+
func (t *tunnelUpdater) upsertWorkspaceLocked(w *Workspace) *Workspace {
11881188
old, ok := t.workspaces[w.ID]
11891189
if !ok {
11901190
t.workspaces[w.ID] = w
@@ -1196,7 +1196,7 @@ func (t *tunnelUpdater) upsertWorkspace(w *Workspace) *Workspace {
11961196
return w
11971197
}
11981198

1199-
func (t *tunnelUpdater) deleteWorkspace(id uuid.UUID) (*Workspace, error) {
1199+
func (t *tunnelUpdater) deleteWorkspaceLocked(id uuid.UUID) (*Workspace, error) {
12001200
w, ok := t.workspaces[id]
12011201
if !ok {
12021202
return nil, xerrors.Errorf("workspace %s not found", id)
@@ -1205,7 +1205,7 @@ func (t *tunnelUpdater) deleteWorkspace(id uuid.UUID) (*Workspace, error) {
12051205
return w, nil
12061206
}
12071207

1208-
func (t *tunnelUpdater) upsertAgent(workspaceID uuid.UUID, a *Agent) error {
1208+
func (t *tunnelUpdater) upsertAgentLocked(workspaceID uuid.UUID, a *Agent) error {
12091209
w, ok := t.workspaces[workspaceID]
12101210
if !ok {
12111211
return xerrors.Errorf("workspace %s not found", workspaceID)
@@ -1214,7 +1214,7 @@ func (t *tunnelUpdater) upsertAgent(workspaceID uuid.UUID, a *Agent) error {
12141214
return nil
12151215
}
12161216

1217-
func (t *tunnelUpdater) deleteAgent(workspaceID, id uuid.UUID) (*Agent, error) {
1217+
func (t *tunnelUpdater) deleteAgentLocked(workspaceID, id uuid.UUID) (*Agent, error) {
12181218
w, ok := t.workspaces[workspaceID]
12191219
if !ok {
12201220
return nil, xerrors.Errorf("workspace %s not found", workspaceID)
@@ -1227,7 +1227,7 @@ func (t *tunnelUpdater) deleteAgent(workspaceID, id uuid.UUID) (*Agent, error) {
12271227
return a, nil
12281228
}
12291229

1230-
func (t *tunnelUpdater) allAgentIDs() []uuid.UUID {
1230+
func (t *tunnelUpdater) allAgentIDsLocked() []uuid.UUID {
12311231
out := make([]uuid.UUID, 0, len(t.workspaces))
12321232
for _, w := range t.workspaces {
12331233
for id := range w.agents {
@@ -1237,8 +1237,9 @@ func (t *tunnelUpdater) allAgentIDs() []uuid.UUID {
12371237
return out
12381238
}
12391239

1240-
// updateDNSNames updates the DNS names for all workspaces in the tunnelUpdater.
1241-
func (t *tunnelUpdater) updateDNSNames() map[dnsname.FQDN][]netip.Addr {
1240+
// updateDNSNamesLocked updates the DNS names for all workspaces in the tunnelUpdater.
1241+
// t.Mutex must be held.
1242+
func (t *tunnelUpdater) updateDNSNamesLocked() map[dnsname.FQDN][]netip.Addr {
12421243
names := make(map[dnsname.FQDN][]netip.Addr)
12431244
for _, w := range t.workspaces {
12441245
err := w.updateDNSNames()

‎vpn/tunnel.go

+5-4
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,9 @@ type Tunnel struct {
3535
client Client
3636
conn Conn
3737

38-
// clientLogger is deliberately separate, to avoid the tunnel using itself
39-
// as a sink for it's own logs, which could lead to deadlocks
38+
// clientLogger is a separate logger than `logger` when the `UseAsLogger`
39+
// option is used, to avoid the tunnel using itself as a sink for it's own
40+
// logs, which could lead to deadlocks.
4041
clientLogger slog.Logger
4142
// router and dnsConfigurator may be nil
4243
router router.Router
@@ -63,7 +64,7 @@ func NewTunnel(
6364
speaker: *(s),
6465
ctx: ctx,
6566
logger: logger,
66-
clientLogger: slog.Make(),
67+
clientLogger: logger,
6768
requestLoopDone: make(chan struct{}),
6869
client: client,
6970
}
@@ -165,7 +166,7 @@ func UseAsRouter() TunnelOption {
165166

166167
func UseAsLogger() TunnelOption {
167168
return func(t *Tunnel) {
168-
t.clientLogger = t.clientLogger.AppendSinks(t)
169+
t.clientLogger = slog.Make(t)
169170
}
170171
}
171172

0 commit comments

Comments
 (0)