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

Rescue from failure when neither ip nor ifconfig exists #39

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

hanazuki
Copy link

Hi, @sporkmonger!

I'm now working with some chroot-ed Linux environment where neither ip nor ifconfig command is accessible. In such an environment I've found that UUIDTools::UUID.ifconfig fails in trying to invoke an incomplete command such as " addr list" (path of ip is missing!).

This patch makes the method ignore such a failure and return an empty string.

UUIDTools::UUID.ip_command = "nonexistent"
UUIDTools::UUID.ip_path_default = "/bin/nonexistent"

expect(UUIDTools::UUID.ifconfig).to eq('')
Copy link
Collaborator

Choose a reason for hiding this comment

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

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

`UUIDTools::UUID.ifconfig` fails when neither `ip` nor `ifconfig` command exists on a UNIX-like system.
In such a case, just ignore the exception.
@t0ffel
Copy link

t0ffel commented Jun 7, 2016

@hanazuki @sporkmonger

Hi, I have same issue in containers where there is no ip or ifconfig. Is there a chance of getting this PR merged?

if os_class == :windows
begin
begin
if os_class == :windows
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use the return of the conditional for variable assignment and comparison.

@@ -625,14 +627,16 @@ def self.mac_address

os_class = UUID.os_class

if os_class == :windows
@@mac_address =
Copy link
Collaborator

Choose a reason for hiding this comment

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

Replace class var @@mac_address with a class instance var.

@sporkmonger
Copy link
Owner

I need to dedicate some time to testing this before I merge, and unfortunately that time is something I just don't have at the moment. I will try to get to it as soon as I can, but it could be awhile before I can give this the attention it needs.

@sporkmonger
Copy link
Owner

I'll probably try to update to a Docker-based CI setup since that's really what this kind of project desperately needs in order to effectively automate testing of weird environments.

@repeatedly
Copy link

@sporkmonger Any progress on this?
This is critical on fluentd and docker environment.

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.

5 participants