Making RPC Upgrade mode reloadable. #11144
Conversation
|
I have a long reproducer script in this gist. |
angrycub
force-pushed
the
reload-rpc-upgrade-mode
branch
from
23a8638
to
4bf03f6
Compare
|
🧐 I can't explain the test failures, but they don't appear to relate to this code. |
They were flakes. I reran the build and it passed just fine. |
There was a problem hiding this comment.
Looks good. Turning off RPC Upgrade Mode will not kill plaintext connections as it ideally should, but this is still a nice progressive enhancement. We also need to fix the race conditions around config reloading. :( Filed #11186 to track this
command/agent/agent_test.go
Outdated
| @@ -1291,6 +1291,48 @@ func TestServer_ShouldReload_ShouldHandleMultipleChanges(t *testing.T) { | |||
| } | |||
| } | |||
|
|
|||
| func TestServer_ShouldReload_ReturnTrueForRPCUpgradeModeChanges(t *testing.T) { | |||
| t.Parallel() | |||
| assert := assert.New(t) | |||
There was a problem hiding this comment.
Just fyi we're trying to get away from this package-shadowing naming style. Not a blocker.
- remove spurious comment - switch to require(t,...) form for test.
|
I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
Almost all of the TLS settings are reloadable, but changes to RPC upgrade mode don't trigger TLS configuration reloads. This PR adds the value to the ShouldReload function.