-
Notifications
You must be signed in to change notification settings - Fork 876
chore(vpn): upsert agents with their network status #15659
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
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
cf43bdd
to
594fbc6
Compare
594fbc6
to
1347916
Compare
vpn/tunnel.go
Outdated
resp.Msg = &TunnelMessage_PeerUpdate{ | ||
PeerUpdate: convertWorkspaceUpdate(state), | ||
PeerUpdate: update, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it OK to send a nil PeerUpdate
in the response?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, I don't think we should.
For pushed updates, if there's no active connection (for whatever reason, should be impossible once the tunnel has started), we'll just send a nil last handshake time.
For requested updates, I think we have to send a nil PeerUpdate
if we can't call CurentWorkspaceState
.
d150a25
to
a456281
Compare
@@ -175,8 +175,6 @@ linters-settings: | |||
- name: modifies-value-receiver | |||
- name: package-comments | |||
- name: range | |||
- name: range-val-address | |||
- name: range-val-in-closure |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -238,7 +241,6 @@ linters: | |||
- errname | |||
- errorlint | |||
- exhaustruct | |||
- exportloopref |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Outdated: https://github.com/kyoh86/exportloopref
default: | ||
t.logger.Warn(t.ctx, "unhandled manager request", slog.F("request", msg)) | ||
return resp | ||
} | ||
if err := req.sendReply(resp); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The original code also sent resp := &TunnelMessage{}
as a reply for the default:
branch, so I assume it's still fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
469ff7a
to
07dc614
Compare
e3f8508
to
561f65f
Compare
07dc614
to
359aaeb
Compare
561f65f
to
6cd9c76
Compare
359aaeb
to
ba48069
Compare
6cd9c76
to
b2fd151
Compare
b2fd151
to
acdd09d
Compare
Closes #14734.