ligato / cn-infra Goto Github PK

We recently moved to using Dependabot [1] for Network Service Mesh, and it's pretty slick. I think using it for cn-infra (and vpp-agent, sfc-controller, etc.) makes a lot of sense. It's easy to add, keeps your dependencies up to date, and is a smooth workflow.

[1] https://dependabot.com/

With the new log redaction feature, we are seeing crashes in the logger when one of the values to be logged is nil

Example:

log.Debugf("RetrieveInterfaceStatusFromRamCache: %+v", ifStatus)

This panics if ifStatus is a pointer and is nil. It's completely valid in some cases to have a "nil" printed to the log.

Currently I've found solution to create wrapper for Logger such as:

type ProcessLogger struct {
	logging.Logger
}

func (pl *ProcessLogger) Write(p []byte) (n int, err error) {
	pl.Debug(string(p))
	return len(p), nil
}

and use this option when creating a new Process to set it for both out and err:

It would be nice if this template code could be avoided.

I'm not sure if this requires changes in processmanager or logging packages.

1. Setup https://github.com/ligato/vpp-agent to watch Redis "config/vpp/v2/interfaces" events. Set up Redis to notify all events CONFIG SET notify-keyspace-events AKE
2. Create many Put watch events by writing some values into Redis /vnf-agent/$mslbl/config/vpp/v2/interfaces/. E.g. 4000 GRE tunnels.
3. Immediately (<1sec) after Redis provisioning do launch Lua script that deletes these keys EVAL "return redis.call('del', unpack(redis.call('keys', ARGV[1])))" 0 /vnf-agent/*/config/vpp/v2/interfaces/*.

According to redis-cli watch CN-Infra has queue of keys to notify VPP-Agent with Put events. On each event it separately reads value for given key. But since Lua script blocks Redis and wipes all keys these values becomes nil. As soon as keys are deleted, Redis gets unblocked and plugin continues to read data that is already deleted.

[1 [::1]:35468] "get" "/vnf-agent/vpp1589/config/vpp/v2/interfaces/<GRE tunnel #1899>
[1 [::1]:35468] "get" "/vnf-agent/vpp1589/config/vpp/v2/interfaces/<GRE tunnel #1900>
[1 [::1]:35468] "get" "/vnf-agent/vpp1589/config/vpp/v2/interfaces/<GRE tunnel #1901>
[1 [::1]:35468] "get" "/vnf-agent/vpp1589/config/vpp/v2/interfaces/<GRE tunnel #1902>
[1 lua] "del" "/vnf-agent/vpp1589/config/vpp/v2/interfaces/<GRE tunnel #1>" "/vnf-agent/vpp1589/config/vpp/v2/interfaces/<GRE tunnel #2>" <...>
[1 [::1]:35468] "get" "/vnf-agent/vpp1589/config/vpp/v2/interfaces/<GRE tunnel #1903> <-- nil value, SIGSEGV

Expected results:
On nil value watch event CN-Infra protoWatchResp.GetValue() should return error. VPP agent's orchestrator plugin Plugin.watchEvents() should get an error, log it, and continue operating.
Actual results:
proto_watcher_impl.go:GetValue() tries to unmarshall protoWatchResp.BytesWatchResp.value that is nil. VPP-agent crashes with SIGSEGV

The docs in db/deyval/etcd refer to a function which allow use to load a config using a filepath
https://github.com/ligato/cn-infra/blob/master/db/keyval/etcd/doc.go#L29

The function in question takes another input however
https://github.com/ligato/cn-infra/blob/master/db/keyval/etcd/bytes_broker_impl.go#L57

Here is a question：

- Different plugins depend on the same plugin, Will the common plugin Init repeatedly？

I try it with a new test case like this:

func TestMultiPluginsDepsSamePlugin(t *testing.T) {
	RegisterTestingT(t)
	CommonPlugin := TestPlugin{
		PluginName:         "CommonPlugin",
	}
	plugin1 := PluginOneDep{
		PluginName:         "Plugin1",
		Plugin1: MissignCloseMethod{},
		Plugin2: CommonPlugin,
	}
	plugin2 := PluginTwoLevelDeps{
		PluginName:         "Plugin2",
		PluginTwoLevelDep1: plugin1,
		PluginTwoLevelDep2: CommonPlugin,
	}
	a := agent.NewAgent(agent.AllPlugins(&plugin1, &plugin2))
	Expect(a).ToNot(BeNil())
	Expect(a.Options()).ToNot(BeNil())
	Expect(a.Options().Plugins).ToNot(BeNil())
	logrus.DefaultLogger().Info(len(a.Options().Plugins))
	for _, plugin := range a.Options().Plugins {
		logrus.DefaultLogger().Info(plugin.String())
	}
}

The result show the agent.opts.Plugins has 10 plugins and has repeat CommonPlugin

Then I review the code of Start(), there isn't any code to avoid calling plugin.Init() repeatly. Is it necessary to avoid repeat
initialization?

When I use the DeclareHTTPPortFlag() function under the rest package to declare a flag of an http port, the defaultPort I pass in is directly converted with String(), resulting in the converted defaultPort being unavailable and the port being converted into a keyboard icon. The problem lies in the DeclareHTTPPortFlag() function , variables of type uint are converted directly using String() instead of strconv.Itoa().

FATA[0000] listen tcp: lookup tcp/⌨: Servname not supported for ai_socktype logger=defaultLogger

Make it possible to define duration as mydur: 5s instead of mydur: 5000000000 for fields with type time.Duration.

Hello,

Here is the config I am using:

# GRPC endpoint defines IP address and port (if tcp type) or unix domain socket file (if unix type).
endpoint: /var/lib/networkservicemesh/dataplane.sock

# If unix domain socket file is used for GRPC communication, permissions to the file can be set here.
# Permission value uses standard three-or-four number linux binary reference.
permission: 000

# If socket file exists in defined path, it is not removed by default, GRPC plugin tries to use it.
# Set the force removal flag to 'true' ensures that the socket file will be always re-created
force-socket-removal: false

# Available socket types: tcp, tcp4, tcp6, unix, unixpacket. If not set, defaults to tcp.
network: unix

# Maximum message size in bytes for inbound mesages. If not set, GRPC uses the default 4MB.
max-msg-size: 4096

# Limit of server streams to each server transport.
max-concurrent-streams: 0

It seems it does not get parsed correctly as grpc plugin still looks for TCP type and not Unix type as requested by the config.

time="2018-07-18 17:05:24.54274" level=error msg="Error loading core: plugin GRPC: Init error 'listen tcp: address /var/lib/networkservicemesh/dataplane.sock: missing port in address', took 256.069µs" loc="core/event_loop.go(29)" logger=agentcore
2018-07-18 17:05:24,547 DEBG fd 15 closed, stopped monitoring <POutputDispatcher at 140375511230656 for <Subprocess at 140375511229432 with name agent in state STARTING> (stdout)>
2018-07-18 17:05:24,547 INFO exited: agent (exit status 0; not expected)
2018-07-18 17:05:24,547 DEBG received SIGCLD indicating a child quit
2018-07-18 17:05:25,548 INFO gave up: agent entered FATAL state, too many start retries too quickly

All of the config samples (*.conf files) that we currently have were written mostly by hand. And it's very likely that most of them are probably quite out of date. It is definitely little insane to continue updating config samples manually.

This issue was created to track progress of solving this by automating and unifying generation of config sample files. Any problems related to config samples that have occurred in the past should be first resolved. For example, there has been few cases, when field name in the config sample had a typo (- vs. _), thus being unrecognized and ignored during parsing.

Following points should be addressed:

• field names, default values and comments should be extracted from the config struct
• structs used for config definitions should be taken as the only source of information and should be used as input for generation
• go:generate directives could be used together with marking Config struct so it can be recognized by generation tool (similarily to Stringer tool)
• ?

Open for discusssion..

If so, can this be documented? This would help developers do the right thing with their dependencies. If not, I'll open a new bug report requesting semver be used ;)

Is there a preferred way to stop the application built on cn-infra platform? ctrl+c seems to sometimes work, but many times results in a go backtrace- which appears to abruptly halt the application.

The reason I ask this is because I'm trying to generate coverage report from the binary itself, and if the binary abruptly stops, the report is not generated.

Here's an example of when it doesn't shutdown properly:

^CINFO[0063] Signal interrupt received, stopping.          loc="agent/agent.go(185)" logger=agent
INFO[0063] Stopping agent                                loc="agent/agent.go(246)" logger=agent
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0xe56793]

goroutine 15 [running]:
github.com/project/vendor/github.com/vishvananda/netlink.(*LinkUpdate).Attrs(0xc4203199b0, 0x30)
    <autogenerated>:1 +0x33
github.com/project/vendor/github.com/ligato/vpp-agent/plugins/linux/ifplugin.(*LinuxInterfaceStateUpdater).processLinkNotification(0xc4203e5c70, 0x13b71c0, 0xc4203199b0)
    /go/src/github.com/project/vendor/github.com/ligato/vpp-agent/plugins/linux/ifplugin/interface_state.go:127 +0x4d
github.com/project/vendor/github.com/ligato/vpp-agent/plugins/linux/ifplugin.(*LinuxInterfaceStateUpdater).watchLinuxInterfaces(0xc4203e5c70, 0x13c9f40, 0xc4204138c0)
    /go/src/github.com/project/vendor/github.com/ligato/vpp-agent/plugins/linux/ifplugin/interface_state.go:115 +0x13c
created by github.com/project/vendor/github.com/ligato/vpp-agent/plugins/linux/ifplugin.(*LinuxInterfaceStateUpdater).Init
    /go/src/github.com/project/vendor/github.com/ligato/vpp-agent/plugins/linux/ifplugin/interface_state.go:64 +0x125

I can see their is only GetValue(key) API, whether this support a list type ?