Repository: nats-io/nats-server (go)

Issue:
Fix `/healthz` check for `js-enabled=true`
`js-enabled=true` is not a well defined health check request parameter currently.  It currently has the same effect that looking at the NATS Config to see if JS Should be enabled:

https://github.com/nats-io/nats-server/blob/d6f243f39bac2e0eaa577cd5e2bfa2d848a86389/server/monitor.go#L3012-L3021

We should instead just check the NATS Config to determine if JetStream should be enabled, and compare the current state to the config for the health check.

The `js-enabled=true` parameter should be changed to `js-enabled-only=true` and should terminate the Healthz endpoint after checking the Desired state (from the config) of JetStream against the Current state of JetStream

- `js-enabled-only=true` -> exit after checking if JetStream is enabled
- `js-server-only=true` -> exit after checking if Server is Current with Meta Leader
- no paramaters -> check everything


Pull request description:
Fix /healthz?js-enabled=true behavior
When js-enabled is set to true, the condition was only checked if
the `getJetStream()` call returned `nil`. However, if it non-nil,
all remaining checks were executed, including assessing the health
of the assets (streams and consumers).

This change addresses two issues:

- Switch to use `js.isEnabled()` which will check whether the value
  is nil OR `js.disabled = true` which can occur if the subsystem
  is temporarily disabled (insufficient resources).
- Correctly exit the check after the assertion and before meta and
  asset checks are performed.

In addition, the option has been renamed to `js-enabled-only` to align
with the `js-server-only` naming. The previous `js-enabled` name still
works, but is mapped to this new option. A warning is emitted noting
the previous option is deprecated.

Fix https://github.com/nats-io/nats-server/issues/3703

Signed-off-by: Byron Ruth <b@devel.io>

/cc @nats-io/core 

Patch:
diff --git a/server/monitor.go b/server/monitor.go
index c78afe93..5c5a5b7f 100644
--- a/server/monitor.go
+++ b/server/monitor.go
@@ -2631,8 +2631,8 @@ type JSzOptions struct {
 
 // HealthzOptions are options passed to Healthz
 type HealthzOptions struct {
-	JSEnabled    bool `json:"js-enabled,omitempty"`
-	JSServerOnly bool `json:"js-server-only,omitempty"`
+	JSEnabledOnly bool `json:"js-enabled-only,omitempty"`
+	JSServerOnly  bool `json:"js-server-only,omitempty"`
 }
 
 type StreamDetail struct {
@@ -2972,14 +2972,21 @@ func (s *Server) HandleHealthz(w http.ResponseWriter, r *http.Request) {
 	if err != nil {
 		return
 	}
+	if jsEnabled {
+		s.Warnf("Healthcheck: js-enabled deprecated, use js-enabled-only instead")
+	}
+	jsEnabledOnly, err := decodeBool(w, r, "js-enabled-only")
+	if err != nil {
+		return
+	}
 	jsServerOnly, err := decodeBool(w, r, "js-server-only")
 	if err != nil {
 		return
 	}
 
 	hs := s.healthz(&HealthzOptions{
-		JSEnabled:    jsEnabled,
-		JSServerOnly: jsServerOnly,
+		JSEnabledOnly: jsEnabledOnly || jsEnabled,
+		JSServerOnly:  jsServerOnly,
 	})
 	if hs.Error != _EMPTY_ {
 		s.Warnf("Healthcheck failed: %q", hs.Error)
@@ -3008,15 +3015,23 @@ func (s *Server) healthz(opts *HealthzOptions) *HealthStatus {
 		return health
 	}
 
-	// Check JetStream
+	sopts := s.getOpts()
+
+	// If JS is not enabled in the config, we stop.
+	if !sopts.JetStream {
+		return health
+	}
+
+	// Access the Jetstream state to perform additional checks.
 	js := s.getJetStream()
-	if js == nil {
-		// If JetStream should be enabled then return error status.
-		if opts.JSEnabled {
-			health.Status = "unavailable"
-			health.Error = NewJSNotEnabledError().Error()
-			return health
-		}
+
+	if !js.isEnabled() {
+		health.Status = "unavailable"
+		health.Error = NewJSNotEnabledError().Error()
+		return health
+	}
+	// Only check if JS is enabled, skip meta and asset check.
+	if opts.JSEnabledOnly {
 		return health
 	}
 


Test patch:
diff --git a/server/events_test.go b/server/events_test.go
index 605e687f..f78cdaa3 100644
--- a/server/events_test.go
+++ b/server/events_test.go
@@ -2178,7 +2178,7 @@ func TestServerEventsPingMonitorz(t *testing.T) {
 		{"JSZ", nil, &JSzOptions{}, []string{"now", "disabled"}},
 
 		{"HEALTHZ", nil, &JSzOptions{}, []string{"status"}},
-		{"HEALTHZ", &HealthzOptions{JSEnabled: true}, &JSzOptions{}, []string{"status"}},
+		{"HEALTHZ", &HealthzOptions{JSEnabledOnly: true}, &JSzOptions{}, []string{"status"}},
 		{"HEALTHZ", &HealthzOptions{JSServerOnly: true}, &JSzOptions{}, []string{"status"}},
 	}
 
diff --git a/server/jetstream_cluster_3_test.go b/server/jetstream_cluster_3_test.go
index fb53caf1..a8161039 100644
--- a/server/jetstream_cluster_3_test.go
+++ b/server/jetstream_cluster_3_test.go
@@ -406,8 +406,8 @@ func TestJetStreamClusterDeleteConsumerWhileServerDown(t *testing.T) {
 	s = c.restartServer(s)
 	checkFor(t, time.Second, 200*time.Millisecond, func() error {
 		hs := s.healthz(&HealthzOptions{
-			JSEnabled:    true,
-			JSServerOnly: false,
+			JSEnabledOnly: false,
+			JSServerOnly:  false,
 		})
 		if hs.Error != _EMPTY_ {
 			return errors.New(hs.Error)
@@ -448,8 +448,8 @@ func TestJetStreamClusterDeleteConsumerWhileServerDown(t *testing.T) {
 	s = c.restartServer(s)
 	checkFor(t, time.Second, 200*time.Millisecond, func() error {
 		hs := s.healthz(&HealthzOptions{
-			JSEnabled:    true,
-			JSServerOnly: false,
+			JSEnabledOnly: false,
+			JSServerOnly:  false,
 		})
 		if hs.Error != _EMPTY_ {
 			return errors.New(hs.Error)
diff --git a/server/jetstream_test.go b/server/jetstream_test.go
index f9cbba83..b66978c3 100644
--- a/server/jetstream_test.go
+++ b/server/jetstream_test.go
@@ -17365,14 +17365,19 @@ func TestJetStreamWorkQueueSourceNamingRestart(t *testing.T) {
 }
 
 func TestJetStreamDisabledHealthz(t *testing.T) {
-	s := RunRandClientPortServer()
+	s := RunBasicJetStreamServer()
+	if config := s.JetStreamConfig(); config != nil {
+		defer removeDir(t, config.StoreDir)
+	}
 	defer s.Shutdown()
 
-	if s.JetStreamEnabled() {
-		t.Fatalf("Expected JetStream to be disabled")
+	if !s.JetStreamEnabled() {
+		t.Fatalf("Expected JetStream to be enabled")
 	}
 
-	hs := s.healthz(&HealthzOptions{JSEnabled: true})
+	s.DisableJetStream()
+
+	hs := s.healthz(&HealthzOptions{JSEnabledOnly: true})
 	if hs.Status == "unavailable" && hs.Error == NewJSNotEnabledError().Error() {
 		return
 	}