Skip to content

Commit 1f9c7bc

Browse files
kozlovickozlovic
committed
More updates
- Replaced DO_REMOTES goto with a for loop - Fixed typo in test - Use `lrc.name()` instead of `lrc.RemoteLeafOpts.name()` Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
1 parent ea7803b commit 1f9c7bc

File tree

2 files changed

+95
-89
lines changed

2 files changed

+95
-89
lines changed

‎server/reload.go‎

Lines changed: 94 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -914,103 +914,109 @@ func getLeafNodeOptionsChanges(s *Server, old, new *LeafNodeOpts) (*leafNodeOpti
914914
remoteErrFormat = "remote %s: %s"
915915
maxAttempts = 20
916916
)
917-
var failed int
918-
919-
DO_REMOTES:
920-
if failed > 0 {
921-
// If we failed once, we will wait a bit before trying again the remotes.
922-
// This could give enough time for connections that were in progress to complete.
923-
select {
924-
case <-time.After(50 * time.Millisecond):
925-
case <-s.quitCh:
926-
return nil, ErrServerNotRunning
927-
}
928-
}
929-
nlo := &leafNodeOption{
930-
tlsFirstChanged: (old.TLSHandshakeFirst != new.TLSHandshakeFirst || old.TLSHandshakeFirstFallback != new.TLSHandshakeFirstFallback),
931-
compressionChanged: !old.Compression.equals(&new.Compression),
932-
// Start with all, will update when processing existing ones.
933-
// Since the list will be modified, we need to clone it.
934-
added: slices.Clone(new.Remotes),
935-
}
917+
var (
918+
nlo *leafNodeOption
919+
// Track whether any existing remote was not found (i.e. removed).
920+
removed bool
921+
)
936922

937-
s.mu.RLock()
938-
// Track whether any existing remote was not found (i.e. removed).
939-
var removed bool
940-
// Go through the list of existing remote configurations.
941-
for lrc := range s.leafRemoteCfgs {
942-
var rlo *RemoteLeafOpts
943-
// Look for the corresponding `*RemoteLeafOpts` in the `nlo.added`
944-
// list. If it is found, that function returns an updated list
945-
// with the element removed from it.
946-
lrc.RLock()
947-
rlo, nlo.added = getRemoteLeafOpts(lrc.RemoteLeafOpts.name(), nlo.added)
948-
if rlo == nil {
949-
// Not found, will be removed in leafNodeOption.Apply().
950-
removed = true
951-
lrc.RUnlock()
952-
continue
923+
forLoop:
924+
for failed := range maxAttempts {
925+
removed = false
926+
if failed > 0 {
927+
// If we failed once, we will wait a bit before trying again the remotes.
928+
// This could give enough time for connections that were in progress to complete.
929+
select {
930+
case <-time.After(50 * time.Millisecond):
931+
case <-s.quitCh:
932+
return nil, ErrServerNotRunning
933+
}
953934
}
954-
// Now we need to make sure that there are no changes that we don't
955-
// support for a RemoteLeafOpts.
956-
err := checkConfigsEqual(lrc.RemoteLeafOpts, rlo, []string{
957-
"Compression",
958-
"Disabled",
959-
"LocalAccount",
960-
"TLS",
961-
"TLSHandshakeFirst",
962-
"TLSConfig",
963-
})
964-
if err != nil {
965-
lrc.RUnlock()
966-
s.mu.RUnlock()
967-
return nil, fmt.Errorf(remoteErrFormat, rlo.safeName(), err)
968-
}
969-
disabledChanged := lrc.Disabled != rlo.Disabled
970-
// If this remote was disabled and is now enabled, we need to make sure
971-
// that there is no connect in progress. If that is the case, either
972-
// try again (if it is the first failure) or return an error.
973-
if disabledChanged && lrc.Disabled && lrc.connInProgress {
974-
lrc.RUnlock()
975-
s.mu.RUnlock()
976-
if failed++; failed < maxAttempts {
977-
goto DO_REMOTES
935+
nlo = &leafNodeOption{
936+
tlsFirstChanged: (old.TLSHandshakeFirst != new.TLSHandshakeFirst || old.TLSHandshakeFirstFallback != new.TLSHandshakeFirstFallback),
937+
compressionChanged: !old.Compression.equals(&new.Compression),
938+
// Start with all, will update when processing existing ones.
939+
// Since the list will be modified, we need to clone it.
940+
added: slices.Clone(new.Remotes),
941+
}
942+
943+
s.mu.RLock()
944+
// Go through the list of existing remote configurations.
945+
for lrc := range s.leafRemoteCfgs {
946+
var rlo *RemoteLeafOpts
947+
// Look for the corresponding `*RemoteLeafOpts` in the `nlo.added`
948+
// list. If it is found, that function returns an updated list
949+
// with the element removed from it.
950+
lrc.RLock()
951+
rlo, nlo.added = getRemoteLeafOpts(lrc.name(), nlo.added)
952+
if rlo == nil {
953+
// Not found, will be removed in leafNodeOption.Apply().
954+
removed = true
955+
lrc.RUnlock()
956+
continue
957+
}
958+
// Now we need to make sure that there are no changes that we don't
959+
// support for a RemoteLeafOpts.
960+
err := checkConfigsEqual(lrc.RemoteLeafOpts, rlo, []string{
961+
"Compression",
962+
"Disabled",
963+
"LocalAccount",
964+
"TLS",
965+
"TLSHandshakeFirst",
966+
"TLSConfig",
967+
})
968+
if err != nil {
969+
lrc.RUnlock()
970+
s.mu.RUnlock()
971+
return nil, fmt.Errorf(remoteErrFormat, rlo.safeName(), err)
978972
}
979-
return nil, fmt.Errorf(remoteErrFormat, rlo.safeName(),
980-
"cannot be enabled at the moment, try again")
981-
}
982-
// Since we will use the new `rlo.TLSConfig` later on, consider all
983-
// existing remote configs as "changed" and store them in the
984-
// `nlo.changed` map.
985-
if nlo.changed == nil {
986-
nlo.changed = make(map[*leafNodeCfg]*remoteLeafOption)
987-
}
988-
lnro := &remoteLeafOption{
989-
tlsFirstChanged: lrc.TLSHandshakeFirst != rlo.TLSHandshakeFirst,
990-
compressionChanged: !lrc.Compression.equals(&rlo.Compression),
991-
disabledChanged: disabledChanged,
992-
opts: rlo,
993-
}
994-
lrc.RUnlock()
995-
nlo.changed[lrc] = lnro
996-
}
997-
if len(nlo.added) > 0 {
998-
// Go through the added list and check if an added was recently removed and,
999-
// if that is the case, is it still in the `s.rmLeafRemoteCfgs` map, which
1000-
// may mean that there was a connect-in-progress that did not complete yet.
1001-
// Either try again (if it is the first failure) or return an error.
1002-
for _, rlo := range nlo.added {
1003-
if _, cip := s.rmLeafRemoteCfgs[rlo.name()]; cip {
973+
disabledChanged := lrc.Disabled != rlo.Disabled
974+
// If this remote was disabled and is now enabled, we need to make sure
975+
// that there is no connect in progress. If that is the case, either
976+
// try again (if it is the first failure) or return an error.
977+
if disabledChanged && lrc.Disabled && lrc.connInProgress {
978+
lrc.RUnlock()
1004979
s.mu.RUnlock()
1005-
if failed++; failed < maxAttempts {
1006-
goto DO_REMOTES
980+
if failed < maxAttempts-1 {
981+
continue forLoop
1007982
}
1008983
return nil, fmt.Errorf(remoteErrFormat, rlo.safeName(),
1009-
"cannot be added at the moment, try again")
984+
"cannot be enabled at the moment, try again")
985+
}
986+
// Since we will use the new `rlo.TLSConfig` later on, consider all
987+
// existing remote configs as "changed" and store them in the
988+
// `nlo.changed` map.
989+
if nlo.changed == nil {
990+
nlo.changed = make(map[*leafNodeCfg]*remoteLeafOption)
991+
}
992+
lnro := &remoteLeafOption{
993+
tlsFirstChanged: lrc.TLSHandshakeFirst != rlo.TLSHandshakeFirst,
994+
compressionChanged: !lrc.Compression.equals(&rlo.Compression),
995+
disabledChanged: disabledChanged,
996+
opts: rlo,
997+
}
998+
lrc.RUnlock()
999+
nlo.changed[lrc] = lnro
1000+
}
1001+
if len(nlo.added) > 0 {
1002+
// Go through the added list and check if an added was recently removed and,
1003+
// if that is the case, is it still in the `s.rmLeafRemoteCfgs` map, which
1004+
// may mean that there was a connect-in-progress that did not complete yet.
1005+
// Either try again (if it is the first failure) or return an error.
1006+
for _, rlo := range nlo.added {
1007+
if _, cip := s.rmLeafRemoteCfgs[rlo.name()]; cip {
1008+
s.mu.RUnlock()
1009+
if failed < maxAttempts-1 {
1010+
continue forLoop
1011+
}
1012+
return nil, fmt.Errorf(remoteErrFormat, rlo.safeName(),
1013+
"cannot be added at the moment, try again")
1014+
}
10101015
}
10111016
}
1017+
s.mu.RUnlock()
1018+
break
10121019
}
1013-
s.mu.RUnlock()
10141020

10151021
// Now we want to make sure that there were actual changes, so that we don't
10161022
// cause a reload of leafnodes for nothing. However, if one has (or all have)

‎server/reload_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7156,7 +7156,7 @@ func TestConfigReloadAddRemoveRemoteLeafNodes(t *testing.T) {
71567156
checkLeafNodeConnectedCount(t, s2, 3)
71577157
checkLeafs([]string{"A", "B", "C"})
71587158

7159-
// Remote remote with local account "A" and "C"
7159+
// Remove remote with local account "A" and "C"
71607160
reloadUpdateConfig(t, s2, conf2, fmt.Sprintf(tmpl2, _EMPTY_, accB, _EMPTY_))
71617161

71627162
checkLeafNodeConnectedCount(t, s2, 1)

0 commit comments

Comments
 (0)