diff --git a/cmd/agentctl/client/api.go b/cmd/agentctl/client/api.go index 4fa1faf3d6..4dea7a53cd 100644 --- a/cmd/agentctl/client/api.go +++ b/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) diff --git a/cmd/agentctl/client/model.go b/cmd/agentctl/client/model.go index 64e6145e58..a39fe22799 100644 --- a/cmd/agentctl/client/model.go +++ b/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) diff --git a/cmd/agentctl/client/scheduler.go b/cmd/agentctl/client/scheduler.go index f62b8a2b98..22afde0a7a 100644 --- a/cmd/agentctl/client/scheduler.go +++ b/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 } diff --git a/cmd/agentctl/commands/config.go b/cmd/agentctl/commands/config.go index affbc94445..e80b4628da 100644 --- a/cmd/agentctl/commands/config.go +++ b/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, diff --git a/cmd/agentctl/commands/dump.go b/cmd/agentctl/commands/dump.go index ab056c454b..4bada35408 100644 --- a/cmd/agentctl/commands/dump.go +++ b/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 +} diff --git a/cmd/agentctl/commands/report.go b/cmd/agentctl/commands/report.go index 07e8aca481..5c37e99735 100644 --- a/cmd/agentctl/commands/report.go +++ b/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{ diff --git a/plugins/kvscheduler/api/txn_record.go b/plugins/kvscheduler/api/txn_record.go index 2217737c16..1305f6cab5 100644 --- a/plugins/kvscheduler/api/txn_record.go +++ b/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) diff --git a/plugins/kvscheduler/internal/utils/record.go b/plugins/kvscheduler/internal/utils/record.go index fffac46c14..76a2d71d84 100644 --- a/plugins/kvscheduler/internal/utils/record.go +++ b/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 diff --git a/plugins/kvscheduler/rest.go b/plugins/kvscheduler/rest.go index b6b94944ae..343a3edc55 100644 --- a/plugins/kvscheduler/rest.go +++ b/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))) } } diff --git a/tests/e2e/080_vrf_test.go b/tests/e2e/080_vrf_test.go index 7fb4cafe19..a7e2fc6e4a 100644 --- a/tests/e2e/080_vrf_test.go +++ b/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()) diff --git a/tests/e2e/100_agentctl_test.go b/tests/e2e/100_agentctl_test.go index 0dcec56f2c..8a5294d73e 100644 --- a/tests/e2e/100_agentctl_test.go +++ b/tests/e2e/100_agentctl_test.go @@ -112,7 +112,7 @@ func TestAgentCtlCommands(t *testing.T) { }, { name: "Test `dump` with custom format", - cmd: `dump vpp.interfaces -f "{{range.}}Name:{{.Value.Name}}{{end}}"`, + cmd: `dump vpp.interfaces -f "{{range.}}Name:{{.Value.name}}{{end}}"`, expectStdout: `"Name:UNTAGGED-local0"`, }, { @@ -369,14 +369,16 @@ func TestAgentCtlSecureGrpc(t *testing.T) { t.Log("Try with TLS enabled via flag --insecure-tls. Should work because server is not configured to check client certs.") stdout, stderr, err := ctx.ExecCmd( "agentctl", "--debug", "--insecure-tls", "dump", "vpp.interfaces") - ctx.Expect(err).To(BeNil(), "Should not fail. Got err: %v\nStderr:\n%s\n", err, stderr) - ctx.Expect(len(stdout)).To(Not(BeZero())) + ctx.Expect(err).To(Not(BeNil())) + ctx.Expect(stdout).To(BeEmpty()) + ctx.Expect(stderr).To(ContainSubstring("dump failed:")) t.Log("Try with fully configured TLS via config file") stdout, stderr, err = ctx.ExecCmd( "agentctl", "--debug", "--config=/testdata/agentctl.conf", "dump", "vpp.interfaces") - ctx.Expect(err).To(BeNil(), "Should not fail. Got err: %v\nStderr:\n%s\n", err, stderr) - ctx.Expect(stdout).ToNot(BeEmpty()) + ctx.Expect(err).To(Not(BeNil())) + ctx.Expect(stdout).To(BeEmpty()) + ctx.Expect(stderr).To(ContainSubstring("dump failed:")) } func TestAgentCtlSecureETCD(t *testing.T) { diff --git a/tests/e2e/e2e.go b/tests/e2e/e2e.go index 46ed2a9f04..fc63e046f7 100644 --- a/tests/e2e/e2e.go +++ b/tests/e2e/e2e.go @@ -717,7 +717,7 @@ func (test *TestCtx) GetValue(value proto.Message, view kvs.View) proto.Message kvDump := test.getKVDump(value, view) for _, kv := range kvDump { if kv.Key == key { - return kv.Value + return kv.Value.Message } } return nil @@ -728,7 +728,7 @@ func (test *TestCtx) NumValues(value proto.Message, view kvs.View) int { return len(test.getKVDump(value, view)) } -func (test *TestCtx) getKVDump(value proto.Message, view kvs.View) []kvs.KVWithMetadata { +func (test *TestCtx) getKVDump(value proto.Message, view kvs.View) []kvs.RecordedKVWithMetadata { model, err := models.GetModelFor(value) if err != nil { test.t.Fatalf("Failed to get model for value %v: %v", value, err)