Skip to content

Commit

Permalink
Fix some system service imports going missing after reload (#4360)
Browse files Browse the repository at this point in the history
On reload some of the imports from the system account where going
missing on reload, this adds them back after a reload:

```
$SYS.REQ.SERVER.PING.CONNZ
$SYS.REQ.ACCOUNT.PING.STATZ
$SYS.REQ.ACCOUNT.PING.CONNZ
```
  • Loading branch information
derekcollison committed Aug 2, 2023
2 parents 7c9a91f + 23b5cb9 commit 5577d18
Show file tree
Hide file tree
Showing 2 changed files with 112 additions and 7 deletions.
116 changes: 109 additions & 7 deletions server/reload_test.go
Expand Up @@ -2603,6 +2603,25 @@ func TestConfigReloadAccountUsers(t *testing.T) {
t.Fatalf("Error on subscribe: %v", err)
}

// confirm subscriptions before and after reload.
var expectedSubs uint32 = 4
sAcc, err := s.LookupAccount("synadia")
require_NoError(t, err)
sAcc.mu.RLock()
n := sAcc.sl.Count()
sAcc.mu.RUnlock()
if n != expectedSubs {
t.Errorf("Synadia account should have %d sub, got %v", expectedSubs, n)
}
nAcc, err := s.LookupAccount("nats.io")
require_NoError(t, err)
nAcc.mu.RLock()
n = nAcc.sl.Count()
nAcc.mu.RUnlock()
if n != expectedSubs {
t.Errorf("Nats.io account should have %d sub, got %v", expectedSubs, n)
}

// Remove user from account and whole account
reloadUpdateConfig(t, s, conf, `
listen: "127.0.0.1:-1"
Expand Down Expand Up @@ -2654,7 +2673,8 @@ func TestConfigReloadAccountUsers(t *testing.T) {
// being reconnected does not mean that resent of subscriptions
// has already been processed.
checkFor(t, 2*time.Second, 100*time.Millisecond, func() error {
gAcc, _ := s.LookupAccount(globalAccountName)
gAcc, err := s.LookupAccount(globalAccountName)
require_NoError(t, err)
gAcc.mu.RLock()
n := gAcc.sl.Count()
fooMatch := gAcc.sl.Match("foo")
Expand All @@ -2673,25 +2693,28 @@ func TestConfigReloadAccountUsers(t *testing.T) {
return fmt.Errorf("Global account should have baz sub")
}

sAcc, _ := s.LookupAccount("synadia")
sAcc, err := s.LookupAccount("synadia")
require_NoError(t, err)
sAcc.mu.RLock()
n = sAcc.sl.Count()
barMatch := sAcc.sl.Match("bar")

sAcc.mu.RUnlock()
if n != 1 {
return fmt.Errorf("Synadia account should have 1 sub, got %v", n)
if n != expectedSubs {
return fmt.Errorf("Synadia account should have %d sub, got %v", expectedSubs, n)
}
if len(barMatch.psubs) != 1 {
return fmt.Errorf("Synadia account should have bar sub")
}

nAcc, _ := s.LookupAccount("nats.io")
nAcc, err := s.LookupAccount("nats.io")
require_NoError(t, err)
nAcc.mu.RLock()
n = nAcc.sl.Count()
batMatch := nAcc.sl.Match("bat")
nAcc.mu.RUnlock()
if n != 1 {
return fmt.Errorf("Nats.io account should have 1 sub, got %v", n)
if n != expectedSubs {
return fmt.Errorf("Nats.io account should have %d sub, got %v", expectedSubs, n)
}
if len(batMatch.psubs) != 1 {
return fmt.Errorf("Synadia account should have bar sub")
Expand All @@ -2700,6 +2723,85 @@ func TestConfigReloadAccountUsers(t *testing.T) {
})
}

func TestConfigReloadAccountWithNoChanges(t *testing.T) {
conf := createConfFile(t, []byte(`
listen: "127.0.0.1:-1"
system_account: sys
accounts {
A {
users = [{ user: a }]
}
B {
users = [{ user: b }]
}
C {
users = [{ user: c }]
}
sys {
users = [{ user: sys }]
}
}
`))
s, opts := RunServerWithConfig(conf)
defer s.Shutdown()

ncA, err := nats.Connect(fmt.Sprintf("nats://a:@%s:%d", opts.Host, opts.Port))
if err != nil {
t.Fatalf("Error on connect: %v", err)
}
defer ncA.Close()

// Confirm default service imports are ok.
checkSubs := func(t *testing.T) {
resp, err := ncA.Request("$SYS.REQ.ACCOUNT.PING.CONNZ", nil, time.Second)
if err != nil {
t.Error(err)
}
if resp == nil || !strings.Contains(string(resp.Data), `"num_connections":1`) {
t.Fatal("unexpected data in connz response")
}
resp, err = ncA.Request("$SYS.REQ.SERVER.PING.CONNZ", nil, time.Second)
if err != nil {
t.Error(err)
}
if resp == nil || !strings.Contains(string(resp.Data), `"num_connections":1`) {
t.Fatal("unexpected data in connz response")
}
resp, err = ncA.Request("$SYS.REQ.ACCOUNT.PING.STATZ", nil, time.Second)
if err != nil {
t.Error(err)
}
if resp == nil || !strings.Contains(string(resp.Data), `"conns":1`) {
t.Fatal("unexpected data in connz response")
}
}
checkSubs(t)
before := s.NumSubscriptions()
s.Reload()
after := s.NumSubscriptions()
if before != after {
t.Errorf("Number of subscriptions changed after reload: %d -> %d", before, after)
}

// Confirm this still works after a reload...
checkSubs(t)
before = s.NumSubscriptions()
s.Reload()
after = s.NumSubscriptions()
if before != after {
t.Errorf("Number of subscriptions changed after reload: %d -> %d", before, after)
}

// Do another extra reload just in case.
checkSubs(t)
before = s.NumSubscriptions()
s.Reload()
after = s.NumSubscriptions()
if before != after {
t.Errorf("Number of subscriptions changed after reload: %d -> %d", before, after)
}
}

func TestConfigReloadAccountNKeyUsers(t *testing.T) {
conf := createConfFile(t, []byte(`
listen: "127.0.0.1:-1"
Expand Down
3 changes: 3 additions & 0 deletions server/server.go
Expand Up @@ -900,6 +900,9 @@ func (s *Server) configureAccounts(reloading bool) (map[string]struct{}, error)
c.processUnsub(sid)
}
acc.addAllServiceImportSubs()
s.mu.Unlock()
s.registerSystemImports(acc)
s.mu.Lock()
}

// Set the system account if it was configured.
Expand Down

0 comments on commit 5577d18

Please sign in to comment.