signal not-connected southbound as codes.Unavailable

pkg/datastore/datastore_rpc.go

@@ -207,7 +207,7 @@ func (d *Datastore) Delete(ctx context.Context) error {
 
 func (d *Datastore) ConnectionState() *targettypes.TargetStatus {
 	if d.sbi == nil {
-		return targettypes.NewTargetStatus(targettypes.TargetStatusNotConnected)
+		return targettypes.NewTargetStatus(sdcpb.TargetStatus_NOT_CONNECTED)
 	}
 	return d.sbi.Status()
 }

pkg/datastore/intent_rpc.go

@@ -46,12 +46,12 @@ func (d *Datastore) applyIntent(ctx context.Context, source targettypes.TargetSo
 	}
 
 	if d.sbi == nil {
-		return nil, fmt.Errorf("%s is not connected", d.config.Name)
+		return nil, fmt.Errorf("%s: %w", d.config.Name, targettypes.ErrNotConnected)
 	}
 
 	rsp, err = d.sbi.Set(ctx, source)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("%s: %w", d.config.Name, err)
 	}
 	log.V(logf.VDebug).Info("got SetResponse from SBI", "raw-response", utils.FormatProtoJSON(rsp))
 

pkg/datastore/target/gnmi/gnmi.go

@@ -160,8 +160,8 @@ func (t *gnmiTarget) Set(ctx context.Context, source targetTypes.TargetSource) (
 	var deletes []*sdcpb.Path
 	var err error
 
-	if t == nil {
-		return nil, fmt.Errorf("%s", "not connected")
+	if err := t.Status().Err(); err != nil {
+		return nil, err
 	}
 
 	// deletes from protos
@@ -251,19 +251,19 @@ func (t *gnmiTarget) Set(ctx context.Context, source targetTypes.TargetSource) (
 }
 
 func (t *gnmiTarget) Status() *targetTypes.TargetStatus {
-	result := targetTypes.NewTargetStatus(targetTypes.TargetStatusNotConnected)
+	result := targetTypes.NewTargetStatus(sdcpb.TargetStatus_NOT_CONNECTED)
 
 	if t == nil || t.target == nil {
 		result.Details = "connection not initialized"
 		return result
 	}
-	switch t.target.ConnState() {
-	case connectivity.Ready.String(), connectivity.Idle.String():
-		result.Status = targetTypes.TargetStatusConnected
-		result.Details = t.target.ConnState()
-	case connectivity.Connecting.String(), connectivity.Shutdown.String(), connectivity.TransientFailure.String():
-		result.Status = targetTypes.TargetStatusNotConnected
-		result.Details = t.target.ConnState()
+	state := t.target.ConnectivityState()
+	result.Details = state.String()
+	switch state {
+	case connectivity.Ready, connectivity.Idle:
+		result.Status = sdcpb.TargetStatus_CONNECTED
+	default:
+		result.Status = sdcpb.TargetStatus_NOT_CONNECTED
 	}
 
 	return result

pkg/datastore/target/netconf/nc.go

@@ -98,8 +98,8 @@ func (t *ncTarget) internalGet(ctx context.Context, req *sdcpb.GetDataRequest) (
 	log := logf.FromContext(ctx).WithName("Get")
 	ctx = logf.IntoContext(ctx, log)
 
-	if !t.Status().IsConnected() {
-		return nil, fmt.Errorf("%s", types.TargetStatusNotConnected)
+	if err := t.Status().Err(); err != nil {
+		return nil, err
 	}
 	source := "running"
 
@@ -162,8 +162,8 @@ func (t *ncTarget) Get(ctx context.Context, req *sdcpb.GetDataRequest) (*sdcpb.G
 func (t *ncTarget) Set(ctx context.Context, source types.TargetSource) (*sdcpb.SetDataResponse, error) {
 	log := logf.FromContext(ctx).WithName("Set")
 	ctx = logf.IntoContext(ctx, log)
-	if !t.Status().IsConnected() {
-		return nil, fmt.Errorf("%s", types.TargetStatusNotConnected)
+	if err := t.Status().Err(); err != nil {
+		return nil, err
 	}
 
 	switch t.sbiConfig.NetconfOptions.CommitDatastore {
@@ -175,13 +175,13 @@ func (t *ncTarget) Set(ctx context.Context, source types.TargetSource) (*sdcpb.S
 }
 
 func (t *ncTarget) Status() *types.TargetStatus {
-	result := types.NewTargetStatus(types.TargetStatusNotConnected)
+	result := types.NewTargetStatus(sdcpb.TargetStatus_NOT_CONNECTED)
 	if t == nil || t.driver == nil {
 		result.Details = "connection not initialized"
 		return result
 	}
 	if t.driver.IsAlive() {
-		result.Status = types.TargetStatusConnected
+		result.Status = sdcpb.TargetStatus_CONNECTED
 	}
 	return result
 }

pkg/datastore/target/netconf/status_test.go

@@ -0,0 +1,53 @@
+// Copyright 2024 Nokia
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package netconf
+
+import (
+	"errors"
+	"strings"
+	"testing"
+
+	"github.com/sdcio/data-server/pkg/datastore/target/types"
+	sdcpb "github.com/sdcio/sdc-protos/sdcpb"
+)
+
+// Get and Set reject requests via Status().Err() before touching any other
+// field, so both must survive a nil receiver rather than panicking.
+func Test_ncTarget_Status_NilReceiver(t *testing.T) {
+	var target *ncTarget
+
+	st := target.Status()
+	if st.Status != sdcpb.TargetStatus_NOT_CONNECTED {
+		t.Errorf("expected NOT_CONNECTED, got %v", st.Status)
+	}
+	if st.IsConnected() {
+		t.Error("expected a nil target to report not connected")
+	}
+	if err := st.Err(); !errors.Is(err, types.ErrNotConnected) {
+		t.Errorf("expected ErrNotConnected, got %v", err)
+	}
+}
+
+func Test_ncTarget_Status_UninitializedDriver(t *testing.T) {
+	target := &ncTarget{name: "dev1"}
+
+	err := target.Status().Err()
+	if !errors.Is(err, types.ErrNotConnected) {
+		t.Fatalf("expected ErrNotConnected, got %v", err)
+	}
+	if !strings.Contains(err.Error(), "connection not initialized") {
+		t.Errorf("expected the status details to reach the error, got %q", err.Error())
+	}
+}

pkg/datastore/target/noop/noop.go

@@ -96,7 +96,7 @@ func (t *noopTarget) Set(ctx context.Context, source types.TargetSource) (*sdcpb
 
 func (t *noopTarget) Status() *types.TargetStatus {
 	return &types.TargetStatus{
-		Status: types.TargetStatusConnected,
+		Status: sdcpb.TargetStatus_CONNECTED,
 	}
 }
 

pkg/datastore/target/types/targetstatus.go

@@ -1,22 +1,42 @@
 package types
 
+import (
+	"errors"
+	"fmt"
+
+	sdcpb "github.com/sdcio/sdc-protos/sdcpb"
+)
+
+// ErrNotConnected indicates the southbound interface (device connection) of a
+// datastore is not established
+var ErrNotConnected = errors.New("not connected")
+
+// TargetStatus is the southbound connection state of a target. The zero value
+// is sdcpb.TargetStatus_UNKNOWN
 type TargetStatus struct {
-	Status  TargetConnectionStatus
+	Status  sdcpb.TargetStatus
 	Details string
 }
 
-func NewTargetStatus(status TargetConnectionStatus) *TargetStatus {
+func NewTargetStatus(status sdcpb.TargetStatus) *TargetStatus {
 	return &TargetStatus{
 		Status: status,
 	}
 }
 func (ts *TargetStatus) IsConnected() bool {
-	return ts.Status == TargetStatusConnected
+	return ts.Status == sdcpb.TargetStatus_CONNECTED
 }
 
-type TargetConnectionStatus string
-
-const (
-	TargetStatusConnected    TargetConnectionStatus = "connected"
-	TargetStatusNotConnected TargetConnectionStatus = "not connected"
-)
+// Err reports the connection state as an error, so callers can propagate it and
+// match it with errors.Is. It returns nil when the target is connected. This is
+// the single place where a connection state turns into ErrNotConnected, keeping
+// the Details a target collected attached to the error.
+func (ts *TargetStatus) Err() error {
+	if ts.IsConnected() {
+		return nil
+	}
+	if ts.Details != "" {
+		return fmt.Errorf("%w: %s", ErrNotConnected, ts.Details)
+	}
+	return ErrNotConnected
+}

pkg/datastore/target/types/targetstatus_test.go

@@ -0,0 +1,72 @@
+package types
+
+import (
+	"errors"
+	"strings"
+	"testing"
+
+	sdcpb "github.com/sdcio/sdc-protos/sdcpb"
+)
+
+func TestTargetStatusErr(t *testing.T) {
+	cases := map[string]struct {
+		status      *TargetStatus
+		wantNil     bool
+		wantDetails string
+	}{
+		"connected": {
+			status:  NewTargetStatus(sdcpb.TargetStatus_CONNECTED),
+			wantNil: true,
+		},
+		"connected with details": {
+			status:  &TargetStatus{Status: sdcpb.TargetStatus_CONNECTED, Details: "READY"},
+			wantNil: true,
+		},
+		"not connected": {
+			status: NewTargetStatus(sdcpb.TargetStatus_NOT_CONNECTED),
+		},
+		"not connected with details": {
+			status:      &TargetStatus{Status: sdcpb.TargetStatus_NOT_CONNECTED, Details: "connection not initialized"},
+			wantDetails: "connection not initialized",
+		},
+		"unknown": {
+			status: NewTargetStatus(sdcpb.TargetStatus_UNKNOWN),
+		},
+		"zero value defaults to unknown": {
+			status: &TargetStatus{},
+		},
+	}
+
+	for name, tc := range cases {
+		t.Run(name, func(t *testing.T) {
+			err := tc.status.Err()
+
+			if tc.wantNil {
+				if err != nil {
+					t.Fatalf("expected nil, got %v", err)
+				}
+				return
+			}
+
+			if !errors.Is(err, ErrNotConnected) {
+				t.Fatalf("expected error matching ErrNotConnected, got %v", err)
+			}
+			if tc.wantDetails != "" && !strings.Contains(err.Error(), tc.wantDetails) {
+				t.Fatalf("expected error to contain %q, got %q", tc.wantDetails, err.Error())
+			}
+		})
+	}
+}
+
+// The zero value must not read as connected, otherwise a status that was never
+// populated would let a write through to a device.
+func TestTargetStatusZeroValueIsNotConnected(t *testing.T) {
+	var ts TargetStatus
+
+	if ts.Status != sdcpb.TargetStatus_UNKNOWN {
+		t.Fatalf("expected zero value to be UNKNOWN, got %v", ts.Status)
+	}
+	if ts.IsConnected() {
+		t.Fatal("expected zero value to report not connected")
+	}
+}

pkg/server/datastore.go

@@ -23,7 +23,6 @@ import (
 
 	"github.com/sdcio/data-server/pkg/config"
 	"github.com/sdcio/data-server/pkg/datastore"
-	targettypes "github.com/sdcio/data-server/pkg/datastore/target/types"
 	"github.com/sdcio/data-server/pkg/utils"
 	logf "github.com/sdcio/logger"
 	sdcpb "github.com/sdcio/sdc-protos/sdcpb"
@@ -312,15 +311,9 @@ func (s *Server) datastoreToRsp(ctx context.Context, ds *datastore.Datastore) (*
 	if err != nil {
 		return nil, err
 	}
-	// map datastore sbi conn state to sdcpb.TargetStatus
-	switch ds.ConnectionState().Status {
-	case targettypes.TargetStatusConnected:
-		rsp.Target.Status = sdcpb.TargetStatus_CONNECTED
-	case targettypes.TargetStatusNotConnected:
-		rsp.Target.Status = sdcpb.TargetStatus_NOT_CONNECTED
-	default:
-		rsp.Target.Status = sdcpb.TargetStatus_UNKNOWN
-	}
+	connState := ds.ConnectionState()
+	rsp.Target.Status = connState.Status
+	rsp.Target.StatusDetails = connState.Details
 
 	rsp.Schema = ds.Config().Schema.GetSchema()
 	return rsp, nil

pkg/server/transaction.go

@@ -7,6 +7,7 @@ import (
 	"time"
 
 	"github.com/sdcio/data-server/pkg/datastore"
+	targettypes "github.com/sdcio/data-server/pkg/datastore/target/types"
 	"github.com/sdcio/data-server/pkg/datastore/types"
 	"github.com/sdcio/data-server/pkg/tree/consts"
 	"github.com/sdcio/data-server/pkg/utils"
@@ -153,8 +154,12 @@ func (s *Server) TransactionCancel(ctx context.Context, req *sdcpb.TransactionCa
 
 // translateInternalToGrpcError central function to map internal errors to grpc error codes
 func translateInternalToGrpcError(err error) error {
-	if errors.Is(err, datastore.ErrDatastoreLocked) {
+	switch {
+	case errors.Is(err, datastore.ErrDatastoreLocked):
 		return status.Error(codes.Aborted, err.Error())
+	case errors.Is(err, targettypes.ErrNotConnected):
+		return status.Error(codes.Unavailable, err.Error())
+	default:
+		return err
 	}
-	return err
 }