fix: agentctl config dump not working with remote models

cmd/agentctl/client/api.go

@@ -54,7 +54,7 @@ type ModelAPIClient interface {
 
 // SchedulerAPIClient defines API client methods for the scheduler
 type SchedulerAPIClient interface {
-	SchedulerDump(ctx context.Context, opts types.SchedulerDumpOptions) ([]api.KVWithMetadata, error)
+	SchedulerDump(ctx context.Context, opts types.SchedulerDumpOptions) ([]api.RecordedKVWithMetadata, error)
 	SchedulerValues(ctx context.Context, opts types.SchedulerValuesOptions) ([]*kvscheduler.BaseValueStatus, error)
 	SchedulerResync(ctx context.Context, opts types.SchedulerResyncOptions) (*api.RecordedTxn, error)
 	SchedulerHistory(ctx context.Context, opts types.SchedulerHistoryOptions) (api.RecordedTxns, error)

cmd/agentctl/client/model.go

@@ -21,10 +21,18 @@ func (c *Client) ModelList(ctx context.Context, opts types.ModelListOptions) ([]
 	if err != nil {
 		return nil, err
 	}
+	for _, km := range knownModels {
+		kmSpec := models.ToSpec(km.GetSpec())
+		if _, err = models.DefaultRegistry.GetModel(kmSpec.ModelName()); err != nil {
+			if _, err = models.DefaultRegistry.Register(km, kmSpec); err != nil {
+				return nil, err
+			}
+		}
+	}
 	logrus.Debugf("retrieved %d known models", len(knownModels))
 	if debug.IsEnabledFor("models") {
-		for _, m := range knownModels {
-			logrus.Trace(" - ", prototext.Format(m))
+		for _, km := range knownModels {
+			logrus.Trace(" - ", prototext.Format(km))
 		}
 	}
 	allModels := convertModels(knownModels)

cmd/agentctl/client/scheduler.go

@@ -6,25 +6,12 @@ import (
 	"fmt"
 	"net/url"
 
-	"google.golang.org/protobuf/encoding/protojson"
-	"google.golang.org/protobuf/encoding/prototext"
-	"google.golang.org/protobuf/reflect/protoreflect"
-	"google.golang.org/protobuf/reflect/protoregistry"
-
 	"go.ligato.io/vpp-agent/v3/cmd/agentctl/api/types"
 	"go.ligato.io/vpp-agent/v3/plugins/kvscheduler/api"
 	"go.ligato.io/vpp-agent/v3/proto/ligato/kvscheduler"
 )
 
-func (c *Client) SchedulerDump(ctx context.Context, opts types.SchedulerDumpOptions) ([]api.KVWithMetadata, error) {
-	type ProtoWithName struct {
-		ProtoMsgName string
-		ProtoMsgData string
-	}
-	type KVWithMetadata struct {
-		api.KVWithMetadata
-		Value ProtoWithName
-	}
+func (c *Client) SchedulerDump(ctx context.Context, opts types.SchedulerDumpOptions) ([]api.RecordedKVWithMetadata, error) {
 	query := url.Values{}
 	query.Set("key-prefix", opts.KeyPrefix)
 	query.Set("view", opts.View)
@@ -33,32 +20,10 @@ func (c *Client) SchedulerDump(ctx context.Context, opts types.SchedulerDumpOpti
 	if err != nil {
 		return nil, err
 	}
-	var kvdump []KVWithMetadata
-	if err := json.NewDecoder(resp.body).Decode(&kvdump); err != nil {
+	var dump []api.RecordedKVWithMetadata
+	if err := json.NewDecoder(resp.body).Decode(&dump); err != nil {
 		return nil, fmt.Errorf("decoding reply failed: %v", err)
 	}
-	var dump []api.KVWithMetadata
-	for _, kvd := range kvdump {
-		d := kvd.KVWithMetadata
-		if kvd.Value.ProtoMsgName == "" {
-			return nil, fmt.Errorf("empty proto message name for key %s", d.Key)
-		}
-		valueType, err := protoregistry.GlobalTypes.FindMessageByName(protoreflect.FullName(kvd.Value.ProtoMsgName))
-		if err != nil {
-			return nil, fmt.Errorf("proto message defined for key %s error: %v", d.Key, err)
-		}
-		d.Value = valueType.New().Interface()
-
-		if len(kvd.Value.ProtoMsgData) > 0 && kvd.Value.ProtoMsgData[0] == '{' {
-			err = protojson.Unmarshal([]byte(kvd.Value.ProtoMsgData), d.Value)
-		} else {
-			err = prototext.Unmarshal([]byte(kvd.Value.ProtoMsgData), d.Value)
-		}
-		if err != nil {
-			return nil, fmt.Errorf("decoding dump reply for %v failed: %v", valueType, err)
-		}
-		dump = append(dump, d)
-	}
 	return dump, nil
 }
 

cmd/agentctl/commands/config.go

@@ -35,7 +35,6 @@ import (
 	"go.ligato.io/vpp-agent/v3/client"
 	"go.ligato.io/vpp-agent/v3/cmd/agentctl/api/types"
 	agentcli "go.ligato.io/vpp-agent/v3/cmd/agentctl/cli"
-	"go.ligato.io/vpp-agent/v3/pkg/models"
 	kvs "go.ligato.io/vpp-agent/v3/plugins/kvscheduler/api"
 	"go.ligato.io/vpp-agent/v3/proto/ligato/configurator"
 	"go.ligato.io/vpp-agent/v3/proto/ligato/kvscheduler"
@@ -585,25 +584,13 @@ func runConfigHistory(cli agentcli.Cli, opts ConfigHistoryOptions) (err error) {
 		}
 	}
 
-	// get remote config models from generic client
-	gc, err := cli.Client().GenericClient()
+	// register remote models into the default registry
+	_, err = cli.Client().ModelList(ctx, types.ModelListOptions{
+		Class: "config",
+	})
 	if err != nil {
 		return err
 	}
-	knownModels, err := gc.KnownModels("config")
-	if err != nil {
-		return fmt.Errorf("getting registered models: %w", err)
-	}
-
-	// register the remote config models into the global default registry
-	for _, km := range knownModels {
-		kmSpec := models.ToSpec(km.GetSpec())
-		if _, err = models.DefaultRegistry.GetModel(kmSpec.ModelName()); err != nil {
-			if _, err = models.DefaultRegistry.Register(km, kmSpec); err != nil {
-				return fmt.Errorf("registering model failed: %w", err)
-			}
-		}
-	}
 
 	txns, err := cli.Client().SchedulerHistory(ctx, types.SchedulerHistoryOptions{
 		SeqNum: ref,

cmd/agentctl/commands/dump.go

@@ -16,6 +16,7 @@ package commands
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
 	"io"
 	"sort"
@@ -137,7 +138,7 @@ func runDump(cli agentcli.Cli, opts DumpOptions) error {
 	}
 	var (
 		errs  Errors
-		dumps []api.KVWithMetadata
+		dumps []api.RecordedKVWithMetadata
 	)
 	for _, keyPrefix := range keyPrefixes {
 		dump, err := cli.Client().SchedulerDump(ctx, types.SchedulerDumpOptions{
@@ -153,6 +154,9 @@ func runDump(cli agentcli.Cli, opts DumpOptions) error {
 	if errs != nil {
 		logging.Debugf("dump finished with %d errors\n%v", len(errs), errs)
 	}
+	if len(errs) == len(keyPrefixes) {
+		return fmt.Errorf("dump failed:\n%v", errs)
+	}
 
 	dumps = filterDumpByOrigin(dumps, opts.Origin)
 	sort.Slice(dumps, func(i, j int) bool {
@@ -163,18 +167,22 @@ func runDump(cli agentcli.Cli, opts DumpOptions) error {
 	if len(format) == 0 {
 		printDumpTable(cli.Out(), dumps)
 	} else {
-		if err := formatAsTemplate(cli.Out(), format, dumps); err != nil {
+		fdumps, err := convertDumps(dumps)
+		if err != nil {
+			return err
+		}
+		if err := formatAsTemplate(cli.Out(), format, fdumps); err != nil {
 			return err
 		}
 	}
 	return nil
 }
 
-func filterDumpByOrigin(dumps []api.KVWithMetadata, origin string) []api.KVWithMetadata {
+func filterDumpByOrigin(dumps []api.RecordedKVWithMetadata, origin string) []api.RecordedKVWithMetadata {
 	if origin == "" {
 		return dumps
 	}
-	var filtered []api.KVWithMetadata
+	var filtered []api.RecordedKVWithMetadata
 	for _, d := range dumps {
 		if !strings.EqualFold(d.Origin.String(), origin) {
 			continue
@@ -184,7 +192,7 @@ func filterDumpByOrigin(dumps []api.KVWithMetadata, origin string) []api.KVWithM
 	return filtered
 }
 
-func printDumpTable(out io.Writer, dump []api.KVWithMetadata) {
+func printDumpTable(out io.Writer, dump []api.RecordedKVWithMetadata) {
 	table := tablewriter.NewWriter(out)
 	table.SetHeader([]string{
 		"Model", "Origin", "Value", "Metadata", "Key",
@@ -224,3 +232,38 @@ func printDumpTable(out io.Writer, dump []api.KVWithMetadata) {
 	}
 	table.Render()
 }
+
+// formatDump is a helper type that can be used with user defined custom dump formats
+type formatDump struct {
+	Key      string
+	Value    map[string]interface{}
+	Metadata api.Metadata
+	Origin   api.ValueOrigin
+}
+
+func convertDumps(in []api.RecordedKVWithMetadata) (out []formatDump, err error) {
+	for _, d := range in {
+		b, err := d.Value.MarshalJSON()
+		if err != nil {
+			return nil, err
+		}
+		var values map[string]interface{}
+		if err = json.Unmarshal(b, &values); err != nil {
+			return nil, err
+		}
+		// TODO: this "ProtoMsgData" string key has to be the same as the field name of
+		// the ProtoWithName struct that contains the message data. ProtoWithName struct
+		// is a part of kvschedulers internal utils package. Perhaps we could make this
+		// field name a part of the public kvscheduler API so we do not have to rely
+		// on string key here.
+		if val, ok := values["ProtoMsgData"]; ok {
+			out = append(out, formatDump{
+				Key:      d.Key,
+				Value:    val.(map[string]interface{}),
+				Metadata: d.Metadata,
+				Origin:   d.Origin,
+			})
+		}
+	}
+	return out, nil
+}

cmd/agentctl/commands/report.go

@@ -424,7 +424,7 @@ func writeKVschedulerReport(subTaskActionName string, view string, ignoreModels
 	// retrieve KVScheduler data
 	var (
 		errs  Errors
-		dumps []api.KVWithMetadata
+		dumps []api.RecordedKVWithMetadata
 	)
 	for _, keyPrefix := range keyPrefixes {
 		dump, err := cli.Client().SchedulerDump(ctx, types.SchedulerDumpOptions{

plugins/kvscheduler/api/txn_record.go

@@ -172,6 +172,15 @@ type RecordedTxnOps []*RecordedTxnOp
 // RecordedTxns is a list of recorded transactions.
 type RecordedTxns []*RecordedTxn
 
+// RecordedKVWithMetadata is the same as KVWithMetadata but with the field Value
+// of type utils.RecordedProtoMessage instead of proto.Message. This allows for
+// proper JSON marshalling and unmarshalling. Values of this type are used in
+// KVScheduler's REST API.
+type RecordedKVWithMetadata struct {
+	RecordedKVPair
+	Metadata Metadata
+}
+
 // String returns a *multi-line* human-readable string representation of recorded transaction.
 func (txn *RecordedTxn) String() string {
 	return txn.StringWithOpts(false, false, 0)

plugins/kvscheduler/internal/utils/record.go

@@ -19,7 +19,6 @@ import (
 
 	"go.ligato.io/cn-infra/v2/logging"
 	"google.golang.org/protobuf/encoding/protojson"
-	"google.golang.org/protobuf/encoding/prototext"
 	"google.golang.org/protobuf/proto"
 	"google.golang.org/protobuf/reflect/protoreflect"
 	"google.golang.org/protobuf/reflect/protoregistry"
@@ -38,7 +37,7 @@ type RecordedProtoMessage struct {
 // message name.
 type ProtoWithName struct {
 	ProtoMsgName string
-	ProtoMsgData string
+	ProtoMsgData json.RawMessage
 }
 
 // MarshalJSON marshalls proto message using the marshaller from protojson.
@@ -47,25 +46,20 @@ type ProtoWithName struct {
 func (p *RecordedProtoMessage) MarshalJSON() ([]byte, error) {
 	var (
 		msgName string
-		msgData string
+		msgData []byte
 		err     error
 	)
 	if p != nil {
-		msgName = string(proto.MessageName(p.Message))
-		b, err := prototext.Marshal(p.Message)
+		msgName = p.ProtoMsgName
+		msgData, err = protojson.Marshal(p.Message)
 		if err != nil {
 			return nil, err
 		}
-		msgData = string(b)
 	}
-	pwn, err := json.Marshal(ProtoWithName{
+	return json.Marshal(&ProtoWithName{
 		ProtoMsgName: msgName,
-		ProtoMsgData: msgData,
+		ProtoMsgData: json.RawMessage(msgData),
 	})
-	if err != nil {
-		return nil, err
-	}
-	return pwn, nil
 }
 
 // UnmarshalJSON un-marshalls proto message using the marshaller from protojson.
@@ -83,7 +77,7 @@ func (p *RecordedProtoMessage) UnmarshalJSON(data []byte) error {
 
 	// try to find the message type in the default registry
 	typeRegistry := models.DefaultRegistry.MessageTypeRegistry()
-	fullMsgName := protoreflect.FullName(pwn.ProtoMsgName)
+	fullMsgName := protoreflect.FullName(p.ProtoMsgName)
 	msgType, err := typeRegistry.FindMessageByName(fullMsgName)
 	if err != nil {
 		// if not found use the proto global types registry as a fallback
@@ -95,12 +89,7 @@ func (p *RecordedProtoMessage) UnmarshalJSON(data []byte) error {
 	}
 
 	msg := msgType.New().Interface()
-	if len(pwn.ProtoMsgData) > 0 && pwn.ProtoMsgData[0] == '{' {
-		err = protojson.Unmarshal([]byte(pwn.ProtoMsgData), msg)
-	} else {
-		err = prototext.Unmarshal([]byte(pwn.ProtoMsgData), msg)
-	}
-	if err != nil {
+	if err = protojson.Unmarshal(pwn.ProtoMsgData, msg); err != nil {
 		return err
 	}
 	p.Message = msg

plugins/kvscheduler/rest.go

@@ -133,15 +133,17 @@ type dumpIndex struct {
 	Views       []string
 }
 
-// kvsWithMetaForREST converts a list of key-value pairs with metadata
+// recordKVsWithMetadata converts a list of key-value pairs with metadata
 // into an equivalent list with proto.Message recorded for proper marshalling.
-func kvsWithMetaForREST(in []kvs.KVWithMetadata) (out []kvs.KVWithMetadata) {
+func recordKVsWithMetadata(in []kvs.KVWithMetadata) (out []kvs.RecordedKVWithMetadata) {
 	for _, kv := range in {
-		out = append(out, kvs.KVWithMetadata{
-			Key:      kv.Key,
-			Value:    utils.RecordProtoMessage(kv.Value),
+		out = append(out, kvs.RecordedKVWithMetadata{
+			RecordedKVPair: kvs.RecordedKVPair{
+				Key:    kv.Key,
+				Value:  utils.RecordProtoMessage(kv.Value),
+				Origin: kv.Origin,
+			},
 			Metadata: kv.Metadata,
-			Origin:   kv.Origin,
 		})
 	}
 	return out
@@ -463,7 +465,7 @@ func (s *Scheduler) dumpGetHandler(formatter *render.Render) http.HandlerFunc {
 				return
 			}
 		}
-		s.logError(formatter.JSON(w, http.StatusOK, kvsWithMetaForREST(dump)))
+		s.logError(formatter.JSON(w, http.StatusOK, recordKVsWithMetadata(dump)))
 	}
 }
 

tests/e2e/080_vrf_test.go

@@ -208,9 +208,13 @@ func TestVRFsWithSameSubnets(t *testing.T) {
 	ctx.Expect(ctx.GetValueState(vppVrf2)).To(Equal(kvscheduler.ValueState_CONFIGURED))
 
 	// vrf mtu check
-	linuxVrf1Mtu := ctx.GetValue(linuxVrf1, kvs.SBView).(*linux_interfaces.Interface).Mtu
+	linuxVrf1Msg := ctx.GetValue(linuxVrf1, kvs.SBView).ProtoReflect()
+	linuxVrf1Desc := linuxVrf1Msg.Descriptor().Fields().ByTextName("mtu")
+	linuxVrf1Mtu := linuxVrf1Msg.Get(linuxVrf1Desc).Uint()
 	ctx.Expect(int(linuxVrf1Mtu)).To(SatisfyAny(Equal(vrf.DefaultVrfDevMTU), Equal(vrf.DefaultVrfDevLegacyMTU)))
-	linuxVrf2Mtu := ctx.GetValue(linuxVrf2, kvs.SBView).(*linux_interfaces.Interface).Mtu
+	linuxVrf2Msg := ctx.GetValue(linuxVrf2, kvs.SBView).ProtoReflect()
+	linuxVrf2Desc := linuxVrf2Msg.Descriptor().Fields().ByTextName("mtu")
+	linuxVrf2Mtu := linuxVrf2Msg.Get(linuxVrf2Desc).Uint()
 	ctx.Expect(int(linuxVrf2Mtu)).To(SatisfyAny(Equal(vrf.DefaultVrfDevMTU), Equal(vrf.DefaultVrfDevLegacyMTU)))
 
 	// try to ping in both VRFs
@@ -244,8 +248,10 @@ func TestVRFsWithSameSubnets(t *testing.T) {
 	ctx.Expect(err).ToNot(HaveOccurred())
 
 	// vrf 1 mtu re-check
-	linuxVrf1Mtu = ctx.GetValue(linuxVrf1, kvs.SBView).(*linux_interfaces.Interface).Mtu
-	ctx.Expect(linuxVrf1Mtu).To(Equal(vrf1Mtu))
+	linuxVrf1Msg = ctx.GetValue(linuxVrf1, kvs.SBView).ProtoReflect()
+	linuxVrf1Desc = linuxVrf1Msg.Descriptor().Fields().ByTextName("mtu")
+	linuxVrf1Mtu = linuxVrf1Msg.Get(linuxVrf1Desc).Uint()
+	ctx.Expect(uint32(linuxVrf1Mtu)).To(Equal(vrf1Mtu))
 
 	ctx.Eventually(ctx.PingFromMsClb(msName, vrfVppIP, PingWithSourceInterface(vrf1Label+tapNameSuffix))).Should(Succeed())
 	ctx.Expect(ctx.PingFromMs(msName, vrfVppIP, PingWithSourceInterface(vrf2Label+tapNameSuffix))).To(Succeed())