fix(coderd): pass block endpoints into servertailnet #12149
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
cc569a7 to
26107fc
Compare
| @@ -163,6 +165,12 @@ func NewServerTailnet( | |||
| return tn, nil | |||
| } | |||
|
|
|||
| // Conn is used to access the underlying tailnet conn of the ServerTailnet. It | |||
| // should only be used for read-only purposes. | |||
There was a problem hiding this comment.
Adding this plus the multiple levels of Get methods seems like a lot just to test that we successfully set a flag.
Can we call the coordinator.Node() method and directly verify that endpoints are not set?
There was a problem hiding this comment.
Yeah, that's definitely true. What I was trying to get at was to only test the specific logic of the servertailnet, which is only responsible for setting the two fields on the configmap and nodeupdater. By checking the node I'm essentially testing logic in tailnet, but that might be a fair tradeoff.
There was a problem hiding this comment.
The more I think about this I'm not sure that checking the endpoints is a solid way to go, since it seems very racey. How do I know that the agent just hasn't finished the STUN dance before I check? Should I just wait x seconds and verify that we never get a STUN addr?
There was a problem hiding this comment.
Don't want racy tests! If this plumbing is the cost, then so be it, I guess.
b8bc41d to
52a7f86
Compare
|
@coadler we still want this one, right? |
52a7f86 to
fac774b
Compare
No description provided.