Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolve "Felix dies if interface missing" on Alpine #902

Merged
merged 1 commit into from
Nov 19, 2015

Conversation

alexwlchan
Copy link
Contributor

Alpine and Ubuntu have slightly different error messages when you run ip link show <non-existent interface>; respectively

ip: can't find device 'thisdoesnotexist'
Device "thisdoesnotexist" does not exist.

We run Felix inside an Alpine container for calico/node, but the different wording meant Felix didn't recognise the missing interface as a known error, so raised an exception. Now we catch both wordings and handle them correctly.

Fixes #899.

@caseydavenport
Copy link
Member

LGTM.

Worth checking our other supported platforms to make sure they produce one of those two outputs?

@alexwlchan
Copy link
Contributor Author

@caseydavenport We only support Ubuntu and RHEL/CentOS on the OpenStack side, and everything with containers runs inside the calico/node Alpine-based image.

RHEL/CentOS have the same message as Ubuntu:

$ docker run -t centos ip link show notarealinterface
Device "notarealinterface" does not exist

so this is probably fine to merge, but I’d like to find out why the FV run failed first.

@fasaxc
Copy link
Member

fasaxc commented Nov 19, 2015

Please add a UT for the new case and a comment explaining which message belongs to which.

Alpine and Ubuntu have slightly different error messages when you
run 'ip link show <non-existent interface>'; respectively

    ip: can't find device 'thisdoesnotexist'
    Device "thisdoesnotexist" does not exist.

We run Felix inside an Alpine container for calico/node, but the
different wording meant Felix didn't recognise the missing
interface as a known error, so raised an exception.  Now we catch
both wordings and handle them correctly.

Fixes #899.
@alexwlchan alexwlchan force-pushed the awlc/alpine-missing-iface-fix branch from c186d50 to fdfc50d Compare November 19, 2015 10:08
@alexwlchan
Copy link
Contributor Author

@fasaxc UT and comment added.

@alexwlchan alexwlchan assigned fasaxc and unassigned alexwlchan Nov 19, 2015
for stderr in error_messages:
err = futils.FailedSystemCall("From test", args, retcode, stdout, stderr)

with mock.patch('calico.felix.futils.check_call', side_effect=err):
Copy link
Member

Choose a reason for hiding this comment

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

I prefer getting the mock by doing as m_check_call; it's a bit more explicit

@fasaxc
Copy link
Member

fasaxc commented Nov 19, 2015

FV failure looks like the intermittent error log issue, which is unrelated. Happy for @alexwlchan to merge when ready.

alexwlchan added a commit that referenced this pull request Nov 19, 2015
Resolve "Felix dies if interface missing" on Alpine
@alexwlchan alexwlchan merged commit 2a36f87 into master Nov 19, 2015
@alexwlchan alexwlchan deleted the awlc/alpine-missing-iface-fix branch November 19, 2015 16:00
@caseydavenport
Copy link
Member

Hey folks - any idea when this fix will make its way into a release? Waiting on it to fix projectcalico/k8s-exec-plugin#92

@alexwlchan

@Smana
Copy link

Smana commented Dec 9, 2015

Any news please ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants