network: pass network interface RawFlags to agent by zhabinecho · Pull Request #1393 · kata-containers/runtime

zhabinecho · 2019-03-20T03:07:52Z

In order to support NOARP in ipvlan interface, the runtime
will pass the rawflags to agent, which also apply to other
network interfaces, not just ipvlan.

Fixes: #1391

Signed-off-by: Zha Bin zhabin@linux.alibaba.com

grahamwhaley · 2019-03-20T09:44:51Z

Hi @zhabinecho - thanks for the PR! I've pulled in some of the network aware folks to review.
I will note for @sboeuf there looks to be a 'protobuf' change here as well, so we will need to scrutinize :-)

CI failed on 'no Fixes:' check, but I see one in the commit - so I have nudged, and if it still happens we'll have to figure out why.

bergwolf · 2019-03-20T10:01:48Z

@zhabinecho Please do not edit vendor directly. Instead, please vendor the agent change in a separate commit when kata-containers/agent#493 is merged.

Other than that, the PR looks good to me.

Adding dnm because of dependency on kata-containers/agent#493.

jodh-intel · 2019-03-20T11:07:04Z

See the following for information on how we handle vendoring:

zhabinecho · 2019-03-20T11:20:17Z

https://github.com/kata-containers/community/blob/master/VENDORING.md

OK, Thanks.

mcastelino

LGTM

This is a good catch. We have never really tested pure L3 interfaces properly.

amshinde

lgtm!

raravena80 · 2019-03-25T22:13:13Z

@zhabinecho ping, any updates? Thx!

zhabinecho · 2019-03-26T05:51:56Z

There are some failure. I can see the following error:

virtcontainers/kata_agent_test.go:194:63: AddInterfaceRequest not declared by package grpc (typecheck)
func (p *gRPCProxy) AddInterface(ctx context.Context, req *pb.AddInterfaceRequest) (*aTypes.Interface, error) {
^
virtcontainers/kata_agent_test.go:198:66: RemoveInterfaceRequest not declared by package grpc (typecheck)
func (p *gRPCProxy) RemoveInterface(ctx context.Context, req *pb.RemoveInterfaceRequest) (*aTypes.Interface, error) {

The AddInterface and RemoveInterface have already removed in agent.

sboeuf · 2019-03-26T13:15:21Z

@amshinde I think you removed the add/delete interface from the agent, right? Did you forget to update the vendoring on the runtime repo?

zhabinecho · 2019-03-28T01:06:34Z

@sboeuf Do I need to update the commit? Thanks!

devimc · 2019-03-28T13:49:42Z

 [[constraint]]
  name = "github.com/kata-containers/agent"
-  revision = "8e48125fa2a793706e5e660e95bdc9755f1cf474"
+  revision = "48dd1c031530fce9bf16b0f6a7305979cedd8fc9"


vendor/github.com/mdlayher/vsock has changed, did you forget to include it?

No that's fine, it changed because this is a subdependency. The runtime does not directly import the vsock package but the agent does. And by revendoring the agent package, we pull the updated package of vsock.

sboeuf

@zhabinecho The PR looks good, but your carried some changes from another PR, and I don't expect to see virtcontainers/kata_agent_test.go in here.

sboeuf · 2019-03-28T14:56:51Z

+func (p *gRPCProxy) MemHotplugByProbe(ctx context.Context, req *pb.MemHotplugByProbeRequest) (*gpb.Empty, error) {
+	return &gpb.Empty{}, nil
+}
+


@zhabinecho I think something went wrong with the rebase of master branch here. Those changes have not been introduced by your PR, right?

Sorry, I should add one comment to describe it. The agent also add MemHotplugByProbeRequest, StartTracingRequest and StopTracingRequest. The continuous-integration/travis-ci will failed:

virtcontainers/kata_agent_test.go:262:36: cannot use g (variable of type *gRPCProxy) as github.com/kata-containers/runtime/vendor/github.com/kata-containers/agent/protocols/grpc.AgentServiceServer value in argument to pb.RegisterAgentServiceServer: missing method MemHotplugByProbe (typecheck)

pb.RegisterAgentServiceServer(s, g)

Also the StartTracingRequest and StopTracingRequest are the same.

Oh I see, those have not been updated from a runtime perspective yet... Then that's fine, let's approve this PR :)

sboeuf · 2019-03-29T14:43:53Z

/test

zhabinecho · 2019-03-31T14:38:16Z

@sboeuf, I don't understand the relation of commit with the jenkins-ci failing. Can we merge it now? Thanks.

sboeuf · 2019-04-01T14:01:49Z

/test

sboeuf · 2019-04-01T14:03:29Z

@zhabinecho the CI is not very stable, I'm restarting it, hopefully it'll pass this time.

zhabinecho · 2019-04-03T12:13:37Z

@zhabinecho the CI is not very stable, I'm restarting it, hopefully it'll pass this time.

@sboeuf Please check the CI. Thanks.

sboeuf · 2019-04-03T14:14:54Z

@chavafg could you check the CI failures and decide if this is mergeable?

chavafg · 2019-04-03T14:51:19Z

Arm CI is failed, but it is already fixed with #1433.

Fedora jobs failed on the functional tests:

• Failure [21.735 seconds]
state
/tmp/jenkins/workspace/kata-containers-runtime-fedora-PR/go/src/github.com/kata-containers/tests/functional/state_test.go:26
  container
  /tmp/jenkins/workspace/kata-containers-runtime-fedora-PR/go/src/github.com/kata-containers/tests/vendor/github.com/onsi/ginkgo/extensions/table/table.go:92
    with workload [true], timeWait 5 [It]
    /tmp/jenkins/workspace/kata-containers-runtime-fedora-PR/go/src/github.com/kata-containers/tests/vendor/github.com/onsi/ginkgo/extensions/table/table_entry.go:46

    Expected
        <int>: 1
    to equal
        <int>: 0

The error I see is:

Running command '/usr/local/bin/kata-runtime [kata-runtime --log=/tmp/bundle024583885/log run --bundle=/tmp/bundle024583885 --console= --pid-file=/tmp/bundle024583885/pid --detach 0gwgKp887QQmi0Kbmm27]'
command failed error 'exit status 1'
[kata-runtime --log=/tmp/bundle024583885/log run --bundle=/tmp/bundle024583885 --console= --pid-file=/tmp/bundle024583885/pid --detach 0gwgKp887QQmi0Kbmm27]
Timeout: 120 seconds
Exit Code: 1
Stdout: 
Stderr: Failed to check if grpc server is working: rpc error: code = Unavailable desc = transport is closing

http://jenkins.katacontainers.io/job/kata-containers-runtime-fedora-PR/1776/console
http://jenkins.katacontainers.io/job/kata-containers-runtime-fedora-vsocks-PR/486/console

I don't recall having instabilities with this test. I have restarted both jobs to see how they behave.

chavafg · 2019-04-03T17:45:51Z

@sboeuf still getting failures on the same state container test

Stderr: Failed to check if grpc server is working: context deadline exceeded

and

Stderr: Failed to check if grpc server is working: rpc error: code = Unavailable desc = transport is closing

http://jenkins.katacontainers.io/job/kata-containers-runtime-fedora-PR/1804/
http://jenkins.katacontainers.io/job/kata-containers-runtime-fedora-vsocks-PR/515/

sboeuf · 2019-04-03T18:20:53Z

@zhabinecho please take a look as those failures are caused by the PR.

jcvenegas · 2019-04-10T12:55:53Z

@zhabinecho, this PR require rebase and check erros.

zhabinecho · 2019-04-17T01:30:20Z

@zhabinecho, this PR require rebase and check erros.

OK, thanks.

devimc · 2019-04-22T14:24:10Z

/test

In order to support NOARP in ipvlan interface, the runtime will pass the rawflags to agent, which also apply to other network interfaces, not just ipvlan. Fixes: kata-containers#1391 Signed-off-by: Zha Bin <zhabin@linux.alibaba.com>

zhabinecho · 2019-04-23T10:21:24Z

@devimc, please review the patch again. Thanks.

devimc · 2019-04-23T13:01:19Z

/test

grahamwhaley requested review from amshinde, bergwolf and mcastelino March 20, 2019 09:41

bergwolf added the do-not-merge label Mar 20, 2019

mcastelino approved these changes Mar 20, 2019

View reviewed changes

amshinde approved these changes Mar 20, 2019

View reviewed changes

zhabinecho force-pushed the noarp-feature branch from ee8ac7b to f4fc162 Compare March 26, 2019 05:39

zhabinecho force-pushed the noarp-feature branch 3 times, most recently from fa9e64c to 9021485 Compare March 26, 2019 06:57

devimc reviewed Mar 28, 2019

View reviewed changes

sboeuf suggested changes Mar 28, 2019

View reviewed changes

zhabinecho force-pushed the noarp-feature branch 2 times, most recently from 50bbd05 to f47d09e Compare March 29, 2019 02:27

sboeuf approved these changes Mar 29, 2019

View reviewed changes

sboeuf removed the do-not-merge label Mar 29, 2019

sboeuf added do-not-merge and removed do-not-merge labels Mar 29, 2019

zhabinecho force-pushed the noarp-feature branch from f47d09e to 2270a41 Compare April 22, 2019 11:59

network: pass network interface RawFlags to agent

dd0808a

In order to support NOARP in ipvlan interface, the runtime will pass the rawflags to agent, which also apply to other network interfaces, not just ipvlan. Fixes: kata-containers#1391 Signed-off-by: Zha Bin <zhabin@linux.alibaba.com>

zhabinecho force-pushed the noarp-feature branch from 2270a41 to dd0808a Compare April 23, 2019 01:42

devimc approved these changes Apr 23, 2019

View reviewed changes

devimc merged commit 63e1c44 into kata-containers:master Apr 23, 2019

Conversation

zhabinecho commented Mar 20, 2019

Uh oh!

grahamwhaley commented Mar 20, 2019

Uh oh!

bergwolf commented Mar 20, 2019

Uh oh!

jodh-intel commented Mar 20, 2019

Uh oh!

zhabinecho commented Mar 20, 2019

Uh oh!

mcastelino left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

amshinde left a comment

Choose a reason for hiding this comment

Uh oh!

raravena80 commented Mar 25, 2019

Uh oh!

zhabinecho commented Mar 26, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sboeuf commented Mar 26, 2019

Uh oh!

zhabinecho commented Mar 28, 2019

Uh oh!

devimc Mar 28, 2019

Choose a reason for hiding this comment

Uh oh!

sboeuf Mar 28, 2019

Choose a reason for hiding this comment

Uh oh!

sboeuf left a comment

Choose a reason for hiding this comment

Uh oh!

sboeuf Mar 28, 2019

Choose a reason for hiding this comment

Uh oh!

zhabinecho Mar 29, 2019

Choose a reason for hiding this comment

Uh oh!

sboeuf Mar 29, 2019

Choose a reason for hiding this comment

Uh oh!

sboeuf commented Mar 29, 2019

Uh oh!

zhabinecho commented Mar 31, 2019

Uh oh!

sboeuf commented Apr 1, 2019

Uh oh!

sboeuf commented Apr 1, 2019

Uh oh!

zhabinecho commented Apr 3, 2019

Uh oh!

sboeuf commented Apr 3, 2019

Uh oh!

chavafg commented Apr 3, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chavafg commented Apr 3, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sboeuf commented Apr 3, 2019

Uh oh!

jcvenegas commented Apr 10, 2019

Uh oh!

zhabinecho commented Apr 17, 2019

Uh oh!

devimc commented Apr 22, 2019

Uh oh!

zhabinecho commented Apr 23, 2019

Uh oh!

devimc commented Apr 23, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

mcastelino left a comment •

edited

Loading

zhabinecho commented Mar 26, 2019 •

edited

Loading

chavafg commented Apr 3, 2019 •

edited

Loading

chavafg commented Apr 3, 2019 •

edited

Loading