Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .scratch/device-profile-parallel-stacks/INDEX.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ Delivery: **device-profile base** → parallel **SONiC** and **Cisco** NOS PRs (
| 02 | [Land generic correctness fixes on `main`](issues/02-land-generic-fixes-on-main.md) | 01 | done on branch → open PR [`ticket-02-generic-fixes`](issues/02-land-generic-fixes-on-main.md) |
| 03 | [Device-profile base PR](issues/03-device-profile-base-pr.md) | 01; 02 merges to `main` (inventory) | done on branch → `device-profile-base` |
| 04 | [config-server: `deviceProfile` on base stack](issues/04-config-server-device-profile-on-base.md) | 03 | done on branch → `device-profile-base` (config-server) |
| 05 | [SONiC NOS PR](issues/05-sonic-nos-pr.md) | 03 | ready-for-agent |
| 05 | [SONiC NOS PR](issues/05-sonic-nos-pr.md) | 03 | done on branch → `sonic-device-profile` |
| 06 | [Cisco IOS-XR NOS PR (draft)](issues/06-cisco-ios-xr-nos-pr-draft.md) | 03 | ready-for-agent |
| 07 | [Supersede legacy PRs #442 and #480](issues/07-supersede-legacy-prs-442-480.md) | 03, 05, 06 | ready-for-agent |

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@

**Blocked by:** 03 — Device-profile base PR

**Status:** ready-for-agent
**Status:** done on branch `sonic-device-profile` (worktree `/home/mava/projects/data-server-sonic`)

- [ ] Only `sonic` is enabled; `cisco-ios-xr` remains on the base stub
- [ ] Set and Get behavior is isolated to SONiC packages and enabled dispatch arms
- [ ] Config, server mapping, materialize routing, and encoder unit tests cover enablement and routing without live devices
- [x] Only `sonic` is enabled; `cisco-ios-xr` remains on the base stub
- [x] Set and Get behavior is isolated to SONiC packages and enabled dispatch arms
- [x] Config, server mapping, materialize routing, and encoder unit tests cover enablement and routing without live devices
- [ ] PR body lists precondition links to open generic-fix PRs on `main` if any remain
7 changes: 7 additions & 0 deletions pkg/config/datastore.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"fmt"
"net"
"os"
"strings"
"time"

"github.com/sdcio/data-server/pkg/utils"
Expand Down Expand Up @@ -216,6 +217,9 @@ func (s *SBI) validateSetDefaults() error {
case sbiNOOP:
return nil
case sbiNETCONF:
if s.DeviceProfile == DeviceProfileSonic {
return fmt.Errorf("device-profile %q is only supported with sbi type %q", DeviceProfileSonic, sbiGNMI)
}
switch s.NetconfOptions.CommitDatastore {
case "":
s.NetconfOptions.CommitDatastore = ncCommitDatastoreCandidate
Expand All @@ -229,6 +233,9 @@ func (s *SBI) validateSetDefaults() error {
if s.GnmiOptions.Encoding == "" {
return errors.New("no encoding defined")
}
if s.DeviceProfile == DeviceProfileSonic && !strings.EqualFold(s.GnmiOptions.Encoding, "JSON_IETF") {
return fmt.Errorf("device-profile %q requires gnmi encoding JSON_IETF, got %q", s.DeviceProfile, s.GnmiOptions.Encoding)
}
default:
return fmt.Errorf("unknown sbi type: %q", s.Type)
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/config/device_profile.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@ var ErrDeviceProfileNotEnabled = errors.New("device profile is not enabled")
// corresponding NOS stack PR enables them.
func ValidateDeviceProfileEnabled(profile DeviceProfile) error {
switch profile {
case DeviceProfileNone:
case DeviceProfileNone, DeviceProfileSonic:
return nil
case DeviceProfileSonic, DeviceProfileCiscoIOSXR:
case DeviceProfileCiscoIOSXR:
return fmt.Errorf("%w: %q", ErrDeviceProfileNotEnabled, profile)
default:
return fmt.Errorf("unknown device-profile: %q", profile)
Expand Down
35 changes: 29 additions & 6 deletions pkg/config/sbi_device_profile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,14 +50,37 @@ func TestSBI_validateSetDefaults_DeviceProfile_CiscoIOSXRIsNotEnabled(t *testing
}
}

func TestSBI_validateSetDefaults_DeviceProfile_SonicIsNotEnabled(t *testing.T) {
func TestSBI_validateSetDefaults_DeviceProfile_SonicGNMIJSONIETFIsAccepted(t *testing.T) {
sbi := validGNMISBI("JSON_IETF", DeviceProfileSonic)
err := sbi.validateSetDefaults()
if err == nil {
t.Fatal("expected error for sonic on base branch, got nil")
if err := sbi.validateSetDefaults(); err != nil {
t.Fatalf("unexpected error for sonic + gnmi + JSON_IETF: %v", err)
}
if !errors.Is(err, ErrDeviceProfileNotEnabled) {
t.Fatalf("expected ErrDeviceProfileNotEnabled, got %v", err)
}

func TestSBI_validateSetDefaults_DeviceProfile_SonicGNMIJSONIsRejected(t *testing.T) {
sbi := validGNMISBI("JSON", DeviceProfileSonic)
if err := sbi.validateSetDefaults(); err == nil {
t.Fatal("expected error for sonic + gnmi + JSON, got nil")
}
}

func TestSBI_validateSetDefaults_DeviceProfile_SonicGNMIProtoIsRejected(t *testing.T) {
sbi := validGNMISBI("PROTO", DeviceProfileSonic)
if err := sbi.validateSetDefaults(); err == nil {
t.Fatal("expected error for sonic + gnmi + PROTO, got nil")
}
}

func TestSBI_validateSetDefaults_DeviceProfile_SonicNetconfIsRejected(t *testing.T) {
sbi := &SBI{
Type: sbiNETCONF,
Address: "192.0.2.1",
Port: 830,
NetconfOptions: &SBINetconfOptions{},
DeviceProfile: DeviceProfileSonic,
}
if err := sbi.validateSetDefaults(); err == nil {
t.Fatal("expected error for sonic + netconf, got nil")
}
}

Expand Down
5 changes: 2 additions & 3 deletions pkg/datastore/datastore_rpc.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
"sync"
"time"

"github.com/sdcio/logger"
logf "github.com/sdcio/logger"
sdcpb "github.com/sdcio/sdc-protos/sdcpb"
"google.golang.org/grpc"
Expand Down Expand Up @@ -250,8 +249,8 @@ func (d *Datastore) Stop(ctx context.Context) error {
}

func (d *Datastore) BlameConfig(ctx context.Context, includeDefaults bool) (*sdcpb.BlameTreeElement, error) {
log := logger.FromContext(ctx).WithName("BlameConfig")
ctx = logger.IntoContext(ctx, log)
log := logf.FromContext(ctx).WithName("BlameConfig")
ctx = logf.IntoContext(ctx, log)

// create a new TreeRoot by copying the syncTree
d.syncTreeMutex.RLock()
Expand Down
5 changes: 2 additions & 3 deletions pkg/datastore/deviations.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"github.com/sdcio/data-server/pkg/config"
"github.com/sdcio/data-server/pkg/tree/ops"
treetypes "github.com/sdcio/data-server/pkg/tree/types"
"github.com/sdcio/logger"
logf "github.com/sdcio/logger"
sdcpb "github.com/sdcio/sdc-protos/sdcpb"
"google.golang.org/grpc/codes"
Expand Down Expand Up @@ -180,7 +179,7 @@ type DeviationEntry interface {

func (d *Datastore) calculateDeviations(ctx context.Context) (<-chan *treetypes.DeviationEntry, error) {

log := logger.FromContext(ctx)
log := logf.FromContext(ctx)

d.syncTreeMutex.RLock()
deviationTree, err := d.syncTree.DeepCopy(ctx)
Expand All @@ -199,7 +198,7 @@ func (d *Datastore) calculateDeviations(ctx context.Context) (<-chan *treetypes.
return nil, err
}

if log := log.V(logger.VTrace); log.Enabled() {
if log := log.V(logf.VTrace); log.Enabled() {
log.Info("deviation tree", "content", deviationTree.String())
log.Info("nonrevertive infos", "data", deviationTree.GetTreeContext().NonRevertiveInfo().String())
}
Expand Down
36 changes: 27 additions & 9 deletions pkg/datastore/sync_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,9 @@ func TestApplyToRunning(t *testing.T) {
}

var v any
json.Unmarshal([]byte(confStr), &v)
if err := json.Unmarshal([]byte(confStr), &v); err != nil {
t.Fatalf("unmarshal test config: %v", err)
}

vpf := pool.NewSharedTaskPool(ctx, runtime.GOMAXPROCS(0))
_, err = root.ImportConfig(ctx, &sdcpb.Path{}, jsonImporter.NewJsonTreeImporter(v, consts.RunningIntentName, consts.RunningValuesPrio, false), types.NewUpdateInsertFlags(), vpf)
Expand Down Expand Up @@ -109,7 +111,9 @@ func TestApplyToRunning(t *testing.T) {
}

var v any
json.Unmarshal([]byte(confStr), &v)
if err := json.Unmarshal([]byte(confStr), &v); err != nil {
t.Fatalf("unmarshal test config: %v", err)
}

return jsonImporter.NewJsonTreeImporter(v, consts.RunningIntentName, consts.RunningValuesPrio, false)
},
Expand All @@ -131,7 +135,9 @@ func TestApplyToRunning(t *testing.T) {
}

var v any
json.Unmarshal([]byte(confStr), &v)
if err := json.Unmarshal([]byte(confStr), &v); err != nil {
t.Fatalf("unmarshal test config: %v", err)
}
return v
},
wantErr: false,
Expand Down Expand Up @@ -186,7 +192,9 @@ func TestApplyToRunning(t *testing.T) {
}

var v any
json.Unmarshal([]byte(confStr), &v)
if err := json.Unmarshal([]byte(confStr), &v); err != nil {
t.Fatalf("unmarshal test config: %v", err)
}

vpf := pool.NewSharedTaskPool(ctx, runtime.GOMAXPROCS(0))
_, err = root.ImportConfig(ctx, &sdcpb.Path{}, jsonImporter.NewJsonTreeImporter(v, consts.RunningIntentName, consts.RunningValuesPrio, false), types.NewUpdateInsertFlags(), vpf)
Expand Down Expand Up @@ -214,7 +222,9 @@ func TestApplyToRunning(t *testing.T) {
}

var v any
json.Unmarshal([]byte(confStr), &v)
if err := json.Unmarshal([]byte(confStr), &v); err != nil {
t.Fatalf("unmarshal test config: %v", err)
}

return jsonImporter.NewJsonTreeImporter(v, consts.RunningIntentName, consts.RunningValuesPrio, false)
},
Expand All @@ -235,7 +245,9 @@ func TestApplyToRunning(t *testing.T) {
}

var v any
json.Unmarshal([]byte(confStr), &v)
if err := json.Unmarshal([]byte(confStr), &v); err != nil {
t.Fatalf("unmarshal test config: %v", err)
}
return v
},
wantErr: false,
Expand Down Expand Up @@ -290,7 +302,9 @@ func TestApplyToRunning(t *testing.T) {
}

var v any
json.Unmarshal([]byte(confStr), &v)
if err := json.Unmarshal([]byte(confStr), &v); err != nil {
t.Fatalf("unmarshal test config: %v", err)
}

vpf := pool.NewSharedTaskPool(ctx, runtime.GOMAXPROCS(0))
_, err = root.ImportConfig(ctx, &sdcpb.Path{}, jsonImporter.NewJsonTreeImporter(v, consts.RunningIntentName, consts.RunningValuesPrio, false), types.NewUpdateInsertFlags(), vpf)
Expand Down Expand Up @@ -319,7 +333,9 @@ func TestApplyToRunning(t *testing.T) {
}

var v any
json.Unmarshal([]byte(confStr), &v)
if err := json.Unmarshal([]byte(confStr), &v); err != nil {
t.Fatalf("unmarshal test config: %v", err)
}

return jsonImporter.NewJsonTreeImporter(v, consts.RunningIntentName, consts.RunningValuesPrio, false)
},
Expand All @@ -341,7 +357,9 @@ func TestApplyToRunning(t *testing.T) {
}

var v any
json.Unmarshal([]byte(confStr), &v)
if err := json.Unmarshal([]byte(confStr), &v); err != nil {
t.Fatalf("unmarshal test config: %v", err)
}
return v
},
wantErr: false,
Expand Down
2 changes: 1 addition & 1 deletion pkg/datastore/target/gnmi/get_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ func TestGetSync_syncConfig_UsesDataTypeConfig(t *testing.T) {
if err := gs.Start(); err != nil {
t.Fatalf("Start: %v", err)
}
defer gs.Stop()
defer func() { _ = gs.Stop() }()

select {
case req := <-captured:
Expand Down
9 changes: 4 additions & 5 deletions pkg/datastore/target/gnmi/gnmi.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,11 @@ import (
gtarget "github.com/openconfig/gnmic/pkg/api/target"
"github.com/openconfig/gnmic/pkg/api/types"
"github.com/sdcio/data-server/pkg/config"
"github.com/sdcio/data-server/pkg/datastore/target/gnmi/sonic"
gnmiutils "github.com/sdcio/data-server/pkg/datastore/target/gnmi/utils"
targetTypes "github.com/sdcio/data-server/pkg/datastore/target/types"
"github.com/sdcio/data-server/pkg/pool"
"github.com/sdcio/data-server/pkg/utils"
dsutils "github.com/sdcio/data-server/pkg/utils"
logf "github.com/sdcio/logger"
sdcpb "github.com/sdcio/sdc-protos/sdcpb"

Expand All @@ -46,15 +46,15 @@ type gnmiTarget struct {
cfg *config.SBI
syncs map[string]GnmiSync
runningStore targetTypes.RunningStore
schemaClient dsutils.SchemaClientBound
schemaClient utils.SchemaClientBound
taskpoolFactory pool.VirtualPoolFactory
// shapeGetRequest applies device-profile-specific adjustments to an
// outgoing gNMI GetRequest. Selected once in NewTarget from cfg.DeviceProfile;
// defaults to a no-op so Get never has to know profiles exist.
shapeGetRequest func(*gnmi.GetRequest)
}

func NewTarget(ctx context.Context, name string, cfg *config.SBI, runningStore targetTypes.RunningStore, schemaClient dsutils.SchemaClientBound, taskpoolFactory pool.VirtualPoolFactory, opts ...grpc.DialOption) (*gnmiTarget, error) {
func NewTarget(ctx context.Context, name string, cfg *config.SBI, runningStore targetTypes.RunningStore, schemaClient utils.SchemaClientBound, taskpoolFactory pool.VirtualPoolFactory, opts ...grpc.DialOption) (*gnmiTarget, error) {
tc := &types.TargetConfig{
Name: name,
Address: fmt.Sprintf("%s:%d", cfg.Address, cfg.Port),
Expand Down Expand Up @@ -116,8 +116,7 @@ func NewTarget(ctx context.Context, name string, cfg *config.SBI, runningStore t
func getRequestShaperFor(profile config.DeviceProfile) func(*gnmi.GetRequest) {
switch profile {
case config.DeviceProfileSonic:
// SONiC Get shaping is wired in the SONiC NOS PR; base keeps the seam as a no-op.
return func(*gnmi.GetRequest) {}
return sonic.ShapeGetRequest
default:
return func(*gnmi.GetRequest) {}
}
Expand Down
Loading