8000 refactored FreshState to UpdateKind to be a better indicator of the f… · coder/coder@5dec731 · GitHub
[go: up one dir, main page]
Skip to content

Commit 5dec731

Browse files
ibetitsmikeibetitsmike
committed
refactored FreshState to UpdateKind to be a better indicator of the field's usage
1 parent ca2e1bf commit 5dec731

File tree

4 files changed

+42
-29
lines changed

4 files changed

+42
-29
lines changed

‎tailnet/controllers.go

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1049,7 +1049,7 @@ func (t *tunnelUpdater) recvLoop() {
10491049
t.logger.Debug(context.Background(), "tunnel updater recvLoop started")
10501050
defer t.logger.Debug(context.Background(), "tunnel updater recvLoop done")
10511051
defer close(t.recvLoopDone)
1052-
freshState := true
1052+
updateKind := Snapshot
10531053
for {
10541054
update, err := t.client.Recv()
10551055
if err != nil {
@@ -1062,10 +1062,10 @@ func (t *tunnelUpdater) recvLoop() {
10621062
}
10631063
t.logger.Debug(context.Background(), "got workspace update",
10641064
slog.F("workspace_update", update),
1065-
slog.F("fresh_state", freshState),
1065+
slog.F("update_kind", updateKind),
10661066
)
1067-
err = t.handleUpdate(update, freshState)
1068-
freshState = false
1067+
err = t.handleUpdate(update, updateKind)
1068+
updateKind = Diff
10691069
if err != nil {
10701070
t.logger.Critical(context.Background(), "failed to handle workspace Update", slog.Error(err))
10711071
cErr := t.client.Close()
@@ -1086,16 +1086,23 @@ type WorkspaceUpdate struct {
10861086
UpsertedAgents []*Agent
10871087
DeletedWorkspaces []*Workspace
10881088
DeletedAgents []*Agent
1089-
FreshState bool
1089+
Kind UpdateKind
10901090
}
10911091

1092+
type UpdateKind int
1093+
1094+
const (
1095+
Diff UpdateKind = iota
1096+
Snapshot
1097+
)
1098+
10921099
func (w *WorkspaceUpdate) Clone() WorkspaceUpdate {
10931100
clone := WorkspaceUpdate{
10941101
UpsertedWorkspaces: make([]*Workspace, len(w.UpsertedWorkspaces)),
10951102
UpsertedAgents: make([]*Agent, len(w.UpsertedAgents)),
10961103
DeletedWorkspaces: make([]*Workspace, len(w.DeletedWorkspaces)),
10971104
DeletedAgents: make([]*Agent, len(w.DeletedAgents)),
1098-
FreshState: w.FreshState,
1105+
Kind: w.Kind,
10991106
}
11001107
for i, ws := range w.UpsertedWorkspaces {
11011108
clone.UpsertedWorkspaces[i D7AE ] = &Workspace{
@@ -1120,7 +1127,7 @@ func (w *WorkspaceUpdate) Clone() WorkspaceUpdate {
11201127
return clone
11211128
}
11221129

1123-
func (t *tunnelUpdater) handleUpdate(update *proto.WorkspaceUpdate, freshState bool) error {
1130+
func (t *tunnelUpdater) handleUpdate(update *proto.WorkspaceUpdate, updateKind UpdateKind) error {
11241131
t.Lock()
11251132
defer t.Unlock()
11261133

@@ -1129,7 +1136,7 @@ func (t *tunnelUpdater) handleUpdate(update *proto.WorkspaceUpdate, freshState b
11291136
UpsertedAgents: []*Agent{},
11301137
DeletedWorkspaces: []*Workspace{},
11311138
DeletedAgents: []*Agent{},
1132-
FreshState: freshState,
1139+
Kind: updateKind,
11331140
}
11341141

11351142
for _, uw := range update.UpsertedWorkspaces {

‎tailnet/controllers_test.go

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1611,15 +1611,14 @@ func TestTunnelAllWorkspaceUpdatesController_Initial(t *testing.T) {
16111611
},
16121612
DeletedWorkspaces: []*tailnet.Workspace{},
16131613
DeletedAgents: []*tailnet.Agent{},
1614-
FreshState: true,
1614+
Kind: tailnet.Snapshot,
16151615
}
16161616

16171617
// And the callback
16181618
cbUpdate := testutil.TryReceive(ctx, t, fUH.ch)
16191619
require.Equal(t, currentState, cbUpdate)
16201620

16211621
// Current recvState should match but shouldn't be a fresh state
1622-
currentState.FreshState = false
16231622
recvState, err := updateCtrl.CurrentState()
16241623
require.NoError(t, err)
16251624
slices.SortFunc(recvState.UpsertedWorkspaces, func(a, b *tailnet.Workspace) int {
@@ -1628,6 +1627,9 @@ func TestTunnelAllWorkspaceUpdatesController_Initial(t *testing.T) {
16281627
slices.SortFunc(recvState.UpsertedAgents, func(a, b *tailnet.Agent) int {
16291628
return strings.Compare(a.Name, b.Name)
16301629
})
1630+
// tunnel is still open, so it's a diff
1631+
currentState.Kind = tailnet.Diff
1632+
16311633
require.Equal(t, currentState, recvState)
16321634
}
16331635

@@ -1694,16 +1696,17 @@ func TestTunnelAllWorkspaceUpdatesController_DeleteAgent(t *testing.T) {
16941696
},
16951697
DeletedWorkspaces: []*tailnet.Workspace{},
16961698
DeletedAgents: []*tailnet.Agent{},
1697-
FreshState: true,
1699+
Kind: tailnet.Snapshot,
16981700
}
16991701

17001702
cbUpdate := testutil.TryReceive(ctx, t, fUH.ch)
17011703
require.Equal(t, initRecvUp, cbUpdate)
17021704

1703-
// Current state should match initial but shouldn't be a fresh state
1704-
initRecvUp.FreshState = false
17051705
state, err := updateCtrl.CurrentState()
17061706
require.NoError(t, err)
1707+
// tunnel is still open, so it's a diff
1708+
initRecvUp.Kind = tailnet.Diff
1709+
17071710
require.Equal(t, initRecvUp, state)
17081711

17091712
// Send update that removes w1a1 and adds w1a2
@@ -1757,7 +1760,6 @@ func TestTunnelAllWorkspaceUpdatesController_DeleteAgent(t *testing.T) {
17571760
"w1.coder.": {ws1a1IP},
17581761
}},
17591762
},
1760-
FreshState: false,
17611763
}
17621764
require.Equal(t, sndRecvUpdate, cbUpdate)
17631765

@@ -1776,7 +1778,6 @@ func TestTunnelAllWorkspaceUpdatesController_DeleteAgent(t *testing.T) {
17761778
},
17771779
DeletedWorkspaces: []*tailnet.Workspace{},
17781780
DeletedAgents: []*tailnet.Agent{},
1779-
FreshState: false,
17801781
}, state)
17811782
}
17821783

‎vpn/tunnel.go

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -397,9 +397,9 @@ func (u *updater) sendUpdateResponse(req *request[*TunnelMessage, *ManagerMessag
397397
// createPeerUpdateLocked creates a PeerUpdate message from a workspace update, populating
398398
// the network status of the agents.
399399
func (u *updater) createPeerUpdateLocked(update tailnet.WorkspaceUpdate) *PeerUpdate {
400-
// this flag is true on the first update after a reconnect
401-
if update.FreshState {
402-
processFreshState(&update, u.agents)
400+
// if the update is a snapshot, we need to process the full state
401+
if update.Kind == tailnet.Snapshot {
402+
processSnapshotUpdate(&update, u.agents)
403403
}
404404

405405
out := &PeerUpdate{
@@ -554,8 +554,12 @@ func (u *updater) netStatusLoop() {
554554
}
555555
}
556556

557-
// processFreshState handles the logic for when a fresh state update is received.
558-
func processFreshState(update *tailnet.WorkspaceUpdate, agents map[uuid.UUID]tailnet.Agent) {
557+
// processSnapshotUpdate handles the logic when a full state update is received.
558+
// When the tunnel is live, we only receive diffs, but the first packet on any given
559+
// reconnect to the tailnet API is a full state.
560+
// Without this logic we weren't processing deletes for any workspaces or agents deleted
561+
// while the client was disconnected while the computer was asleep.
562+
func processSnapshotUpdate(update *tailnet.WorkspaceUpdate, agents map[uuid.UUID]tailnet.Agent) {
559563
// ignoredWorkspaces is initially populated with the workspaces that are
560564
// in the current update. Later on we populate it with the deleted workspaces too
561565
// so that we don't send duplicate updates. Same applies to ignoredAgents.

‎vpn/tunnel_internal_test.go

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -567,7 +567,7 @@ func TestTunnel_sendAgentUpdateReconnect(t *testing.T) {
567567
},
568568
},
569569
},
570-
FreshState: true,
570+
Kind: tailnet.Snapshot,
571571
})
572572
require.NoError(t, err)
573573

@@ -579,6 +579,7 @@ func TestTunnel_sendAgentUpdateReconnect(t *testing.T) {
579579
require.NotNil(t, peerUpdate)
580580
require.Len(t, peerUpdate.UpsertedAgents, 1)
581581
require.Len(t, peerUpdate.DeletedAgents, 1)
582+
require.Len(t, peerUpdate.DeletedWorkspaces, 0)
582583

583584
require.Equal(t, aID2[:], peerUpdate.UpsertedAgents[0].Id)
584585
require.Equal(t, hsTime, peerUpdate.UpsertedAgents[0].LastHandshake.AsTime())
@@ -667,7 +668,7 @@ func TestTunnel_sendAgentUpdateWorkspaceReconnect(t *testing.T) {
667668
},
668669
},
669670
},
670-
FreshState: true,
671+
Kind: tailnet.Snapshot,
671672
})
672673
require.NoError(t, err)
673674

@@ -757,7 +758,7 @@ func TestProcessFreshState(t *testing.T) {
757758
name: "NoChange",
758759
initialAgents: initialAgents,
759760
update: &tailnet.WorkspaceUpdate{
760-
FreshState: true,
761+
Kind: tailnet.Snapshot,
761762
UpsertedWorkspaces: []*tailnet.Workspace{&ws1, &ws2},
762763
UpsertedAgents: []*tailnet.Agent{&agent1, &agent2, &agent4},
763764
DeletedWorkspaces: []*tailnet.Workspace{},
@@ -772,7 +773,7 @@ func TestProcessFreshState(t *testing.T) {
772773
name: "AgentAdded", // Agent 3 added in update
773774
initialAgents: initialAgents,
774775
update: &tailnet.WorkspaceUpdate{
775-
FreshState: true,
776+
Kind: tailnet.Snapshot,
776777
UpsertedWorkspaces: []*tailnet.Workspace{&ws1, &ws2, &ws3},
777778
UpsertedAgents: []*tailnet.Agent{&agent1, &agent2, &agent3, &agent4},
778779
DeletedWorkspaces: []*tailnet.Workspace{},
@@ -787,7 +788,7 @@ func TestProcessFreshState(t *testing.T) {
787788
name: "AgentRemovedWorkspaceAlsoRemoved", // Agent 2 removed, ws2 also removed
788789
initialAgents: initialAgents,
789790
update: &tailnet.WorkspaceUpdate{
790-
FreshState: true,
791+
Kind: tailnet.Snapshot,
791792
UpsertedWorkspaces: []*tailnet.Workspace{&ws1}, // ws2 not present
792793
UpsertedAgents: []*tailnet.Agent{&agent1, &agent4}, // agent2 not present
793794
DeletedWorkspaces: []*tailnet.Workspace{},
@@ -804,7 +805,7 @@ func TestProcessFreshState(t *testing.T) {
804805
name: "AgentRemovedWorkspaceStays", // Agent 4 removed, but ws1 stays (due to agent1)
805806
initialAgents: initialAgents,
806807
update: &tailnet.WorkspaceUpdate{
807-
FreshState: true,
808+
Kind: tailnet.Snapshot,
808809
UpsertedWorkspaces: []*tailnet.Workspace{&ws1, &ws2}, // ws1 still present
809810
UpsertedAgents: []*tailnet.Agent{&agent1, &agent2}, // agent4 not present
810811
DeletedWorkspaces: []*tailnet.Workspace{},
@@ -821,7 +822,7 @@ func TestProcessFreshState(t *testing.T) {
821822
name: "InitialAgentsEmpty",
822823
initialAgents: map[uuid.UUID]tailnet.Agent{}, // Start with no agents known
823824
update: &tailnet.WorkspaceUpdate{
824-
FreshState: true,
825+
Kind: tailnet.Snapshot,
825826
UpsertedWorkspaces: []*tailnet.Workspace{&ws1, &ws2},
826827
UpsertedAgents: []*tailnet.Agent{&agent1, &agent2},
827828
DeletedWorkspaces: []*tailnet.Workspace{},
@@ -836,7 +837,7 @@ func TestProcessFreshState(t *testing.T) {
836837
name: "UpdateEmpty", // Fresh state says nothing exists
837838
initialAgents: initialAgents,
838839
update: &tailnet.WorkspaceUpdate{
839-
FreshState: true,
840+
Kind: tailnet.Snapshot,
840841
UpsertedWorkspaces: []*tailnet.Workspace{},
841842
UpsertedAgents: []*tailnet.Agent{},
842843
DeletedWorkspaces: []*tailnet.Workspace{},
@@ -861,7 +862,7 @@ func TestProcessFreshState(t *testing.T) {
861862
agentsCopy := make(map[uuid.UUID]tailnet.Agent)
862863
maps.Copy(agentsCopy, tt.initialAgents)
863864

864-
processFreshState(tt.update, agentsCopy)
865+
processSnapshotUpdate(tt.update, agentsCopy)
865866

866867
require.ElementsMatch(t, tt.expectedDelete.DeletedAgents, tt.update.DeletedAgents, "DeletedAgents mismatch")
867868
require.ElementsMatch(t, tt.expectedDelete.DeletedWorkspaces, tt.update.DeletedWorkspaces, "DeletedWorkspaces mismatch")

0 commit comments

Comments
 (0)
0