From be47bcab3ba135cefd9b2bd58a29d2271a8766cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20Moti=C4=8D=C3=A1k?= Date: Mon, 8 Aug 2022 16:31:44 +0200 Subject: [PATCH 1/8] Fix agentctl dump not working with remote models MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Peter Motičák --- cmd/agentctl/client/scheduler.go | 40 +++++---------------------- cmd/agentctl/commands/dump.go | 20 ++++++++++++++ plugins/kvscheduler/api/txn_record.go | 5 ++++ plugins/kvscheduler/rest.go | 16 ++++++----- 4 files changed, 41 insertions(+), 40 deletions(-) diff --git a/cmd/agentctl/client/scheduler.go b/cmd/agentctl/client/scheduler.go index f62b8a2b98..36e05a25e4 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 - } query := url.Values{} query.Set("key-prefix", opts.KeyPrefix) query.Set("view", opts.View) @@ -33,31 +20,18 @@ func (c *Client) SchedulerDump(ctx context.Context, opts types.SchedulerDumpOpti if err != nil { return nil, err } - var kvdump []KVWithMetadata + var kvdump []api.RecordedKVWithMetadata if err := json.NewDecoder(resp.body).Decode(&kvdump); 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) + dump = append(dump, api.KVWithMetadata{ + Key: kvd.Key, + Value: kvd.Value.Message, + Metadata: kvd.Metadata, + Origin: kvd.Origin, + }) } return dump, nil } diff --git a/cmd/agentctl/commands/dump.go b/cmd/agentctl/commands/dump.go index ab056c454b..2769185c23 100644 --- a/cmd/agentctl/commands/dump.go +++ b/cmd/agentctl/commands/dump.go @@ -122,6 +122,23 @@ func runDump(cli agentcli.Cli, opts DumpOptions) error { ctx, cancel := context.WithCancel(context.Background()) defer cancel() + gc, err := cli.Client().GenericClient() + if err != nil { + return err + } + knownModels, err := gc.KnownModels("config") + if err != nil { + return fmt.Errorf("getting registered models: %w", 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 fmt.Errorf("registering model failed: %w", err) + } + } + } + allModels, err := cli.Client().ModelList(ctx, types.ModelListOptions{ Class: "config", }) @@ -153,6 +170,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 { diff --git a/plugins/kvscheduler/api/txn_record.go b/plugins/kvscheduler/api/txn_record.go index 2217737c16..35bfaa908c 100644 --- a/plugins/kvscheduler/api/txn_record.go +++ b/plugins/kvscheduler/api/txn_record.go @@ -172,6 +172,11 @@ type RecordedTxnOps []*RecordedTxnOp // RecordedTxns is a list of recorded transactions. type RecordedTxns []*RecordedTxn +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/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))) } } From f4fea0920802b26dc6f31423da0b6649223a5565 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20Moti=C4=8D=C3=A1k?= Date: Tue, 9 Aug 2022 12:02:17 +0200 Subject: [PATCH 2/8] Refactor the scheduler client API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use the RecordedKVWithMetadata type that can be properly un/marshalled from/to JSON. Signed-off-by: Peter Motičák --- cmd/agentctl/client/api.go | 2 +- cmd/agentctl/client/scheduler.go | 15 +++------------ cmd/agentctl/commands/dump.go | 8 ++++---- cmd/agentctl/commands/report.go | 2 +- 4 files changed, 9 insertions(+), 18 deletions(-) diff --git a/cmd/agentctl/client/api.go b/cmd/agentctl/client/api.go index 3796c70c9c..4562e9b28d 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/scheduler.go b/cmd/agentctl/client/scheduler.go index 36e05a25e4..22afde0a7a 100644 --- a/cmd/agentctl/client/scheduler.go +++ b/cmd/agentctl/client/scheduler.go @@ -11,7 +11,7 @@ import ( "go.ligato.io/vpp-agent/v3/proto/ligato/kvscheduler" ) -func (c *Client) SchedulerDump(ctx context.Context, opts types.SchedulerDumpOptions) ([]api.KVWithMetadata, error) { +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) @@ -20,19 +20,10 @@ func (c *Client) SchedulerDump(ctx context.Context, opts types.SchedulerDumpOpti if err != nil { return nil, err } - var kvdump []api.RecordedKVWithMetadata - 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 { - dump = append(dump, api.KVWithMetadata{ - Key: kvd.Key, - Value: kvd.Value.Message, - Metadata: kvd.Metadata, - Origin: kvd.Origin, - }) - } return dump, nil } diff --git a/cmd/agentctl/commands/dump.go b/cmd/agentctl/commands/dump.go index 2769185c23..b10f8736b2 100644 --- a/cmd/agentctl/commands/dump.go +++ b/cmd/agentctl/commands/dump.go @@ -154,7 +154,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{ @@ -190,11 +190,11 @@ func runDump(cli agentcli.Cli, opts DumpOptions) error { 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 @@ -204,7 +204,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", diff --git a/cmd/agentctl/commands/report.go b/cmd/agentctl/commands/report.go index aa35de88ea..f9dc5a0eea 100644 --- a/cmd/agentctl/commands/report.go +++ b/cmd/agentctl/commands/report.go @@ -423,7 +423,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{ From 611d25a8995b1a549e3e4aef4212f7375343a961 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20Moti=C4=8D=C3=A1k?= Date: Tue, 9 Aug 2022 12:49:54 +0200 Subject: [PATCH 3/8] Register fetched remote models in function ModelList MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Peter Motičák --- cmd/agentctl/client/model.go | 12 ++++++++++-- cmd/agentctl/commands/config.go | 21 ++++----------------- cmd/agentctl/commands/dump.go | 19 +------------------ 3 files changed, 15 insertions(+), 37 deletions(-) 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/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 b10f8736b2..1f8188cc12 100644 --- a/cmd/agentctl/commands/dump.go +++ b/cmd/agentctl/commands/dump.go @@ -122,23 +122,6 @@ func runDump(cli agentcli.Cli, opts DumpOptions) error { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - gc, err := cli.Client().GenericClient() - if err != nil { - return err - } - knownModels, err := gc.KnownModels("config") - if err != nil { - return fmt.Errorf("getting registered models: %w", 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 fmt.Errorf("registering model failed: %w", err) - } - } - } - allModels, err := cli.Client().ModelList(ctx, types.ModelListOptions{ Class: "config", }) @@ -171,7 +154,7 @@ func runDump(cli agentcli.Cli, opts DumpOptions) error { 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) + return fmt.Errorf("dump failed:\n%v", errs) } dumps = filterDumpByOrigin(dumps, opts.Origin) From 896b717009007cf18523417e834fa59413bde391 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20Moti=C4=8D=C3=A1k?= Date: Tue, 9 Aug 2022 15:55:27 +0200 Subject: [PATCH 4/8] Fix VRF e2e test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Peter Motičák --- tests/e2e/080_vrf_test.go | 14 ++++++++++---- tests/e2e/e2e.go | 4 ++-- 2 files changed, 12 insertions(+), 6 deletions(-) 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/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) From 932c32bab96abb8483bc64184fd720142f652a02 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20Moti=C4=8D=C3=A1k?= Date: Thu, 11 Aug 2022 15:54:40 +0200 Subject: [PATCH 5/8] Fix agentctl e2e test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * (Un)marshal RecordedProtoMessage as JSON instead of text * Add ability to create custom dump formats using templates for remote models * Fix the remaining agentctl e2e tests Signed-off-by: Peter Motičák --- cmd/agentctl/commands/dump.go | 42 +++++++++++++++++++- plugins/kvscheduler/internal/utils/record.go | 27 ++++--------- tests/e2e/100_agentctl_test.go | 12 +++--- 3 files changed, 56 insertions(+), 25 deletions(-) diff --git a/cmd/agentctl/commands/dump.go b/cmd/agentctl/commands/dump.go index 1f8188cc12..1a8e025917 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" @@ -166,7 +167,11 @@ 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 } } @@ -227,3 +232,38 @@ func printDumpTable(out io.Writer, dump []api.RecordedKVWithMetadata) { } 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 +} \ No newline at end of file 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/tests/e2e/100_agentctl_test.go b/tests/e2e/100_agentctl_test.go index a9e58fe9dc..9e92e18345 100644 --- a/tests/e2e/100_agentctl_test.go +++ b/tests/e2e/100_agentctl_test.go @@ -107,7 +107,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"`, }, { @@ -364,14 +364,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(stderr).To(ContainSubstring("dump failed:")) + ctx.Expect(err).To(Not(BeNil())) 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(stdout).To(BeEmpty()) + ctx.Expect(stderr).To(ContainSubstring("dump failed:")) + ctx.Expect(stdout).To(BeEmpty()) } func TestAgentCtlSecureETCD(t *testing.T) { From 665d360c0f66be585203462e85cd321da1d5fe25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20Moti=C4=8D=C3=A1k?= Date: Wed, 17 Aug 2022 12:29:57 +0200 Subject: [PATCH 6/8] go fmt MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Peter Motičák --- cmd/agentctl/commands/dump.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/agentctl/commands/dump.go b/cmd/agentctl/commands/dump.go index 1a8e025917..4bada35408 100644 --- a/cmd/agentctl/commands/dump.go +++ b/cmd/agentctl/commands/dump.go @@ -266,4 +266,4 @@ func convertDumps(in []api.RecordedKVWithMetadata) (out []formatDump, err error) } } return out, nil -} \ No newline at end of file +} From 9d7d135ac7ef076f79d558d2ab8adb9a19ea65cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20Moti=C4=8D=C3=A1k?= Date: Fri, 9 Sep 2022 14:41:33 +0200 Subject: [PATCH 7/8] e2e test correction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Peter Motičák --- tests/e2e/100_agentctl_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/e2e/100_agentctl_test.go b/tests/e2e/100_agentctl_test.go index 9e92e18345..07a1463e1d 100644 --- a/tests/e2e/100_agentctl_test.go +++ b/tests/e2e/100_agentctl_test.go @@ -365,15 +365,15 @@ func TestAgentCtlSecureGrpc(t *testing.T) { stdout, stderr, err := ctx.ExecCmd( "agentctl", "--debug", "--insecure-tls", "dump", "vpp.interfaces") ctx.Expect(err).To(Not(BeNil())) + ctx.Expect(stdout).To(BeEmpty()) ctx.Expect(stderr).To(ContainSubstring("dump failed:")) - ctx.Expect(err).To(Not(BeNil())) 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(Not(BeNil())) ctx.Expect(stdout).To(BeEmpty()) ctx.Expect(stderr).To(ContainSubstring("dump failed:")) - ctx.Expect(stdout).To(BeEmpty()) } func TestAgentCtlSecureETCD(t *testing.T) { From 3f719a73e30b5bf536ee204b4a768ea60cea2e60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20Moti=C4=8D=C3=A1k?= Date: Mon, 12 Sep 2022 16:10:23 +0200 Subject: [PATCH 8/8] Add comment for RecordedKVWithMetadata type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Peter Motičák --- plugins/kvscheduler/api/txn_record.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/plugins/kvscheduler/api/txn_record.go b/plugins/kvscheduler/api/txn_record.go index 35bfaa908c..1305f6cab5 100644 --- a/plugins/kvscheduler/api/txn_record.go +++ b/plugins/kvscheduler/api/txn_record.go @@ -172,6 +172,10 @@ 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