- Config reload tests have been modified to not rely on symlink.
- Close logger on shutdown (for Windows tests cleanup)
Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
When changing something in the cluster, such as Timeout and doing
a config reload, the route could be closed with an `Authorization
Error` report. Moreover, the route would not try to reconnect,
even if specified as an explicit route.
There were 2 issues:
- When checking if a solicited route is still valid, we need to
check the Routes' URL against the URL that we try to connect
to but not compare the pointers, but either do a reflect
deep equal, or compare their String representation (this is
what I do in the PR).
- We should check route authorization only if this is an accepted
route, not an explicit one. The reason is that we a server
explicitly connect to another server, it does not get the remote
server's username and password. So the check would always fail.
Note: It is possible that a config reload even without any change
in the cluster triggers the code checking if routes are properly
authorized, and that happens if there is TLS specified. When
the reload code checks if config has changed, the TLSConfig
between the old and new seem to indicate a change, eventhough there
is apparently none. Another reload does not detect a change. I
suspect some internal state in TLSConfig that causes the
reflect.DeepEqual() to report a difference.
Note2: This commit also contains fixes to regex that staticcheck
would otherwise complain about (they did not have any special
character), and I have removed printing the usage on startup when
getting an error. The usage is still correctly printed if passing
a parameter that is unknown.
Resolves#719
Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
I noticed that when running the test suite, there would be a file
server/log1.txt left. This file is created by one of the config
reload test. Running this test individually was doing the proper
cleanup. I noticed that the Signal test that was checking
that files could be rotated was causing this side effect.
It turns out that none of the config reload tests were disabling
the signal handler (NoSigs=true), and since the go routine would
be left running, running the TestSignalToReOpenLogFile() test
would interact with an already finished test.
I put a thread dump in handleSignals() to track all tests that
were causing this function to start the go routine because NoSigs
was not set to true. I fixed all those tests. At this time, there
are only 2 tests that need to start the signal handler.
I have also fixed the code so that the signal handler routine select
on a server quitCh that is closed on shutdown so that this go routine
exit and is waiting on using the grWG wait group.
Until now, a server would only notify clients of servers that join
the cluster. More than that, a server would send ot its clients only
information if new servers were added.
This PR changes this by sending to clients that support async INFO
the list of URLs for all servers in the cluster any time that there
is a change (joining or leaving the cluster).
As of now, clients will not be affected by the change (and will not
take benefit of this: removing servers from their server pool). This
will be addressed in each supported client once this is merged.
- Move the kill of a server in a cluster test to ensure that
list of routes to remove is not empty.
- Change write_deadline reload value to 3s to make it different
from default value
- Add test for option that does not support hot-swapping
It's hard to implement a bulletproof solution for cleaning up the
symlinks created by config reload tests on failure since, for example,
there is nothing we can do when log.Fatal is called. Instead, prevent
the existence of a symlink from failing the tests if there is one
hanging around. Generally, these symlinks will not be left unless
os.Exit was called (as is the case with log.Fatal).
Use include so that we can have logfile and remote sys log tested
on platforms other than Windows.
Added some missing defer server.Shutdown() statements.
Fixing various tests that were failing locally when running in
parallel mode (without -p=1).
In reload_test.go, lots of nats.Conn.Close() were missing which
would require too much memory when running with `-race` mode.
