Skip to content
This repository has been archived by the owner on Apr 3, 2018. It is now read-only.

Ignore route with ipv6 gateway #673

Merged
merged 2 commits into from
Mar 13, 2018

Conversation

amshinde
Copy link
Collaborator

@amshinde amshinde commented Mar 12, 2018

We ignore routes that have an ipv6 destination, since we do not have support for ipv6 currently. We should do the same for default route that has an ipv6 gateway address.

@amshinde amshinde force-pushed the ignore-ipv6-gateway branch from 281be5c to 66d2b8e Compare March 12, 2018 18:41
@amshinde
Copy link
Collaborator Author

@mcastelino @egernst PTAL

@mcastelino
Copy link
Collaborator

@chavafg can we enable IPv6 in our docker CI https://docs.docker.com/config/daemon/ipv6/

This will help catch issues easily. We should ignore IPv6 settings till we support them.

Copy link
Collaborator

@mcastelino mcastelino left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Collaborator

@egernst egernst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any reason not to squash here?

@amshinde
Copy link
Collaborator Author

Changes to two different agent implementations, makes sense to have them in a separate commit. We can revert one once we add ipv6 support.

@sboeuf
Copy link
Collaborator

sboeuf commented Mar 12, 2018

@amshinde I don't mind having one or two commits here, but please could you add a subsystem for each of your commit messages, identifying which component is modified:
kata_agent: ipv6: ...
hyperstart_agent: ipv6: ...

Or simply squash them. Up to you !

We ignore routes that have an ipv6 destination, since hyperstart agent
does not have support for ipv6 currently. We should do the same
for default route that has an ipv6 gateway address.

Signed-off-by: Archana Shinde <archana.m.shinde@intel.com>
We ignore routes that have an ipv6 destination, since kata agent
does not have support for ipv6 currently. We should do the same
for default route that has an ipv6 gateway address.

Fixes containers#672

Signed-off-by: Archana Shinde <archana.m.shinde@intel.com>
@amshinde amshinde force-pushed the ignore-ipv6-gateway branch from 66d2b8e to 9bb9779 Compare March 12, 2018 21:29
@amshinde
Copy link
Collaborator Author

@sboeuf I have modified the commit message.

@sboeuf
Copy link
Collaborator

sboeuf commented Mar 12, 2018

@amshinde LGTM

@amshinde
Copy link
Collaborator Author

@sboeuf I see the fedora CI failing this time, even though I just changed the commit message.
http://cc-jenkins-ci.westus2.cloudapp.azure.com/job/virtcontainers-fedora-26-PR/519/console

--- FAIL: TestRunPodHyperstartAgentSuccessful (0.04s)
21:48:14 	cc_proxy_mock.go:90: [CCProxyMock] listening on /tmp/cc-proxy-test162776106/cc-proxy-test.sock
21:48:14 	cc_proxy_mock.go:90: [CCProxyMock] Client connected
21:48:14 	cc_proxy_mock.go:90: [CCProxyMock] Register VM
21:48:14 	cc_proxy_mock.go:90: [CCProxyMock] Error serving client : EOF
21:48:14 	cc_proxy_mock.go:90: [CCProxyMock] Client closed connection
21:48:14 	cc_proxy_mock.go:90: [CCProxyMock] Client connected
21:48:14 	cc_proxy_mock.go:90: [CCProxyMock] Attach VM
21:48:14 	cc_proxy_mock.go:90: [CCProxyMock] Hyper command
21:48:14 	cc_proxy_mock.go:90: [CCProxyMock] Error serving client : EOF
21:48:14 	cc_proxy_mock.go:90: [CCProxyMock] Client closed connection
21:48:14 	cc_proxy_mock.go:90: [CCProxyMock] Client connected
21:48:14 	cc_proxy_mock.go:90: [CCProxyMock] Attach VM
21:48:14 	cc_proxy_mock.go:90: [CCProxyMock] Error serving client : EOF
21:48:14 	cc_proxy_mock.go:90: [CCProxyMock] Client closed connection
21:48:14 	api_test.go:778: Could not bind mount /tmp/virtcontainers-tmp-636974255/bundle to /run/hyper/shared/pods/7f49d00d-1995-4156-8c79-5f5ab24ce138/1/rootfs: no such file or directory

I am going to rerun to see if I get the same error, but maybe a potential bug/race somewhere else in our code.

@sboeuf
Copy link
Collaborator

sboeuf commented Mar 12, 2018

@amshinde some race conditions in our unit tests... I have re-triggered a build already.

@jodh-intel
Copy link
Collaborator

jodh-intel commented Mar 13, 2018

lgtm

Approved with PullApprove Approved with PullApprove

@sboeuf sboeuf merged commit 8e89616 into containers:master Mar 13, 2018
@sboeuf sboeuf removed the review label Mar 13, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants