Fix config reload that failed because of Gateways

Although Gateways reload is not supported at the moment, I had
to add the trap in the switch statement because it would find
a difference. The reason is the TLSConfig object that is likely
to not pass the reflect.DeepEqual test. So for now, I exclude this
from the deep equal test and fail the reload only if the user
has explicitly changed the configuration.

Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
This commit is contained in:
Ivan Kozlovic
2018-12-04 19:25:59 -07:00
parent 522d4bc591
commit 4f8100ebc8
2 changed files with 36 additions and 4 deletions

View File

@@ -637,6 +637,22 @@ func (s *Server) diffOptions(newOpts *Options) ([]option, error) {
diffOpts = append(diffOpts, &clientAdvertiseOption{newValue: cliAdv})
case "accounts":
diffOpts = append(diffOpts, &accountsOption{})
case "gateway":
// Not supported for now, but report warning if configuration of gateway
// is actually changed so that user knows that it won't take effect.
// Any deep-equal is likely to fail for when there is a TLSConfig. so
// remove for the test.
tmpOld := oldValue.(GatewayOpts)
tmpNew := newValue.(GatewayOpts)
tmpOld.TLSConfig = nil
tmpNew.TLSConfig = nil
// If there is really a change prevents reload.
if !reflect.DeepEqual(tmpOld, tmpNew) {
// See TODO(ik) note below about printing old/new values.
return nil, fmt.Errorf("Config reload not supported for %s: old=%v, new=%v",
field.Name, oldValue, newValue)
}
case "nolog", "nosigs":
// Ignore NoLog and NoSigs options since they are not parsed and only used in
// testing.
@@ -649,6 +665,11 @@ func (s *Server) diffOptions(newOpts *Options) ([]option, error) {
}
fallthrough
default:
// TODO(ik): Implement String() on those options to have a nice print.
// %v is difficult to figure what's what, %+v print private fields and
// would print passwords. Tried json.Marshal but it is too verbose for
// the URL array.
// Bail out if attempting to reload any unsupported options.
return nil, fmt.Errorf("Config reload not supported for %s: old=%v, new=%v",
field.Name, oldValue, newValue)

View File

@@ -3186,8 +3186,6 @@ func TestConfigReloadAccountServicesImportExport(t *testing.T) {
// As of now, config reload does not support changes for gateways.
// However, ensure that if a gateway is defined, one can still
// do reload as long as we don't change the gateway spec.
// There was an issue with the initialization of default TLS timeout
// that caused the reload to fail.
func TestConfigReloadNotPreventedByGateways(t *testing.T) {
confTemplate := `
listen: "127.0.0.1:-1"
@@ -3198,14 +3196,27 @@ func TestConfigReloadNotPreventedByGateways(t *testing.T) {
tls {
cert_file: "configs/certs/server.pem"
key_file: "configs/certs/key.pem"
timeout: %s
}
gateways [
{
name: "B"
url: "nats://localhost:8888"
}
]
}
`
conf := createConfFile(t, []byte(fmt.Sprintf(confTemplate, "")))
conf := createConfFile(t, []byte(fmt.Sprintf(confTemplate, "", "5")))
defer os.Remove(conf)
s, _ := RunServerWithConfig(conf)
defer s.Shutdown()
// Cause reload with adding a param that is supported
reloadUpdateConfig(t, s, conf, fmt.Sprintf(confTemplate, "max_payload: 100000"))
reloadUpdateConfig(t, s, conf, fmt.Sprintf(confTemplate, "max_payload: 100000", "5"))
// Now update gateway, should fail to reload.
changeCurrentConfigContentWithNewContent(t, conf, []byte(fmt.Sprintf(confTemplate, "max_payload: 100000", "3")))
if err := s.Reload(); err == nil || !strings.Contains(err.Error(), "not supported for Gateway") {
t.Fatalf("Expected Reload to return a not supported error, got %v", err)
}
}