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

Add detection for Arista EOS #158

Merged
merged 3 commits into from
Nov 3, 2016
Merged

Conversation

jerearista
Copy link
Contributor

Note: EOS has 2 possible modes: the network CLI and a bash shell, typically only accessed in a test scenario such as with Vagrant.

Copy link
Contributor

@chris-rock chris-rock left a comment

Choose a reason for hiding this comment

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

Awesome, thanks @jerearista

unless output.empty?
eos_ver = JSON.parse(output)
@platform[:name] = @platform[:name] || 'arista_eos'
family ||= 'arista_eos'
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we not using fedora in this case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When you are in the bash shell, then you can do all the fedora things. However, when you login the way you would to a typical running switch, you are in a custom EOS CLI. You can still prefix linux commands with 'bash' from this mode but common commands are things like 'show running-config', 'show interfaces status', etc. Thus, things that expect to be in a regular linux shell will need special handling.

I'm definitely willing to go another route, if appropriate.

You can test yourself by registering at http://Arista.com/, then going to Software Downloads --> vEOS and downloading the files for your preferred hypervisor.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should use arista_eos as the family for both instead of fedora?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I assumed that more resources would adapt their behavior based on :family but since that's not the case, I'm fine with setting both to arista_eos if you think that's best.

Copy link
Contributor

@chris-rock chris-rock Nov 2, 2016

Choose a reason for hiding this comment

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

If we want to count EOS as linux, we should also add it to https://github.com/chef/train/blob/master/lib/train/extras/os_common.rb#L68 because most resources go with inspec.os.linux?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was thinking about this a bit. If we always mark EOS as family: arista_eos and include that as "linux", then resources will fail instead of skipping when we're in the usual CLI mode unless from the os detection I can specify to default to prefixing all linux commands with bash to drop to a linux shell (Ex: bash uname -a or bash sudo cat /etc/passwd).

Alternatively, I could just patch up the resources as we make use of them.

Copy link
Contributor

Choose a reason for hiding this comment

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

Or we make bash default and use the EOS CLI only withinEOS specific InSpec resources. Is bash available on all devices?

Copy link
Contributor

Choose a reason for hiding this comment

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

We could achieve that by implementing a specific command wrapper for eos https://github.com/chef/train/blob/master/lib/train/extras/command_wrapper.rb#L164-L175

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that will work. I'm not sure I'll get the wrapper in by Friday but I don't think that needs to hold up the initial platform support. So, to recap, I'll set the family to arista_eos for both cases and we can improve handling the differences with a command wrapper.

@chris-rock
Copy link
Contributor

Thanks @jerearista Lets cover the other improvements in another PR

@chris-rock chris-rock merged commit 0a40928 into inspec:master Nov 3, 2016
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.

2 participants