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

Platform framework and detect DSL #204

Merged
merged 24 commits into from
Nov 13, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
dd0b848
Initial building block for platforms
jquick Oct 13, 2017
b4aac00
Refactor platform classes and deprecate os
jquick Oct 13, 2017
62b9d11
Add basic detect logic
jquick Oct 13, 2017
922a716
Move in linux detect logic
jquick Oct 17, 2017
f7567a7
Add in local detect and more unix checks
jquick Oct 17, 2017
d296abc
Refactor os specifications and some cleaning
jquick Oct 19, 2017
c7ae1a2
Added windows and finalize detect
jquick Oct 20, 2017
ab3a04c
Add dynamic family and platform methods and refactor os specification
jquick Oct 24, 2017
83d4aad
add unit tests and finalize detect logic
jquick Oct 26, 2017
efdab1d
fix lint issues
jquick Oct 26, 2017
6c27336
add unknown fallback incase a platform option is not detected
jquick Oct 26, 2017
aff0d80
Merge branch 'master' into jq/platform_objects
jquick Oct 26, 2017
83e01ab
Merge branch 'master' into jq/platform_objects
jquick Oct 26, 2017
810b3d6
Merge branch 'master' into jq/platform_objects
jquick Oct 30, 2017
7fd444d
add skip empty line on os_release detect
jquick Oct 30, 2017
021e6fd
removed unneeded platform type
jquick Oct 30, 2017
1c50340
fix style issues and clear up logic
jquick Oct 31, 2017
7cc0267
style fixes and wrapped os specifications in a class
jquick Oct 31, 2017
453a51d
move around platform files into more logical spots
jquick Nov 1, 2017
98c10dd
move redhat check to be at the end of the family as a catchall
jquick Nov 1, 2017
64a0ccd
update raspbian detect to also check in os_release
jquick Nov 1, 2017
42e6dff
update suse detect to be case insensitive
jquick Nov 1, 2017
0425afb
add helper comments and refactor transport test
jquick Nov 1, 2017
babc875
update detect logic for solaris and arista
jquick Nov 9, 2017
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions lib/train/platform.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
module Train
class Platform
include Train::Platforms::Common
attr_accessor :condition, :families, :backend, :platform, :family_hierarchy
attr_accessor :backend, :condition, :families, :family_hierarchy, :platform

def initialize(name, condition = {})
@name = name
Expand All @@ -12,7 +12,7 @@ def initialize(name, condition = {})
@family_hierarchy = []
@platform = {}
@detect = nil
@title = name =~ /^[[:alpha:]]+$/ ? name.capitalize : name
@title = name.to_s.capitalize

# add itself to the platform list
Train::Platforms.list[name] = self
Expand Down Expand Up @@ -48,33 +48,33 @@ def to_hash
@platform
end

# Add genaric family? and platform methods to an existing platform
# Add generic family? and platform methods to an existing platform
#
# This is done later to add any custom
# families/properties that were created
def add_platform_methods
family_list = Train::Platforms.families
family_list.each_value do |k|
next if respond_to?(k.name + '?')
define_singleton_method(k.name + '?') {
define_singleton_method(k.name + '?') do
family_hierarchy.include?(k.name)
}
end
end

# Helper methods for direct platform info
@platform.each_key do |m|
next if respond_to?(m)
define_singleton_method(m) {
define_singleton_method(m) do
@platform[m]
}
end
end

# Create method for name if its not already true
plat_name = name.downcase.tr(' ', '_') + '?'
return if respond_to?(plat_name)
define_singleton_method(plat_name) {
define_singleton_method(plat_name) do
true
}
end
end
end
end
2 changes: 1 addition & 1 deletion lib/train/platforms.rb
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ def self.print_children(parent, pad = 2)
parent.children.each do |key, value|
obj = key
puts "#{' ' * pad}-> #{obj.title}#{value unless value.empty?}"
print_children(obj, pad + 2) unless !defined?(obj.children) || obj.children.nil?
print_children(obj, pad + 2) if defined?(obj.children) && !obj.children.nil?
end
end
end
29 changes: 15 additions & 14 deletions lib/train/platforms/detect/os_common.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,36 +20,37 @@ def winrm?

def unix_file_contents(path)
# keep a log of files incase multiple checks call the same one
return @files[path] unless @files[path].nil?
return @files[path] if @files.key?(path)

res = @backend.run_command("test -f #{path} && cat #{path}")
@files[path] = res.stdout
# ignore files that can't be read
return nil if res.exit_status != 0
res.stdout
@files[path] = res.exit_status.zero? ? res.stdout : nil
@files[path]
end

def unix_file_exist?(path)
@backend.run_command("test -f #{path}").exit_status.zero?
end

def uname_call(cmd)
Copy link
Contributor

Choose a reason for hiding this comment

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

This is really just run_command now, rather than uname-specific... maybe we should change the method name and that way it can be used for other things too?

res = @backend.run_command(cmd).stdout
res.strip! unless res.nil?
res
end

def unix_uname_s
return @uname[:s] unless @uname[:s].nil?
@uname[:s] = @backend.run_command('uname -s').stdout
return @uname[:s] if @uname.key?(:s)
@uname[:s] = uname_call('uname -s')
end

def unix_uname_r
return @uname[:r] unless @uname[:r].nil?
@uname[:r] = begin
res = @backend.run_command('uname -r').stdout
res.strip! unless res.nil?
res
end
return @uname[:r] if @uname.key?(:r)
@uname[:r] = uname_call('uname -r')
end

def unix_uname_m
return @uname[:m] unless @uname[:m].nil?
@uname[:m] = @backend.run_command('uname -m').stdout.chomp
return @uname[:m] if @uname.key?(:m)
@uname[:m] = uname_call('uname -m')
end
end
end
8 changes: 3 additions & 5 deletions lib/train/platforms/detect/scanner.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,7 @@ def scan
top = Train::Platforms.top_platforms
top.each do |_name, plat|
next unless plat.detect
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we implement a method detect in family.rb? This would abstract the issue for us here. Also I am not sure if we want to use lambda for detect, so that we do not need to call instance_eval

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 have added the false lambda instead of the check for detect. This way everything can be evaluated accordingly.
As far as moving away from instance_eval/exec. The reason we are doing this is so the detect logic will have access to all the helper methods (os_common, os_linux, etc...) in the detect context. We may be able to require all this logic in each instance but that seems overkill for the memory and really doing the same thing under the hood. This also makes it easy to create custom platforms on the fly via inspec or other instances. After talking it over and due to its controlled nature we are voting to keep it for now. I can comment it up to explain a bit more in code.

result = instance_eval(&plat.detect)
next unless result == true
next unless instance_eval(&plat.detect) == true

# if we have a match start looking at the children
plat_result = scan_children(plat)
Expand All @@ -44,8 +43,7 @@ def scan
def scan_children(parent)
parent.children.each do |plat, condition|
next if plat.detect.nil?
result = instance_eval(&plat.detect)
next unless result == true
next unless instance_eval(&plat.detect) == true

if plat.class == Train::Platform
@platform[:family] = parent.name
Expand All @@ -60,7 +58,7 @@ def scan_children(parent)
end

def scan_family_children(plat)
child_result = scan_children(plat) if !plat.children.nil?
child_result = scan_children(plat) unless plat.children.nil?
return if child_result.nil?
@family_hierarchy << plat.name
child_result
Expand Down
4 changes: 2 additions & 2 deletions lib/train/platforms/family.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,15 @@
module Train::Platforms
class Family
include Train::Platforms::Common
attr_accessor :name, :condition, :families, :children
attr_accessor :children, :condition, :families, :name

def initialize(name, condition)
@name = name
@condition = condition
@families = {}
@children = {}
@detect = nil
@title = name =~ /^[[:alpha:]]+$/ ? "#{name.capitalize} Family" : name
@title = "#{name.to_s.capitalize} Family"

# add itself to the families list
Train::Platforms.families[@name.to_s] = self
Expand Down
7 changes: 3 additions & 4 deletions lib/train/transports/docker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -69,14 +69,13 @@ def close
# nothing to do at the moment
end

def os
platform
end

def platform
@platform ||= Train::Platforms::Detect.scan(self)
end

# we need to keep os as a method for backwards compatibility with inspec
alias os platform
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we do this in our base connection? This ensures that it is consistent across all connections and the implementations can focus on using platform

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 need a os_transport then?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This would be fine, if we are not going to be using BaseConnection for API.

Copy link
Contributor

Choose a reason for hiding this comment

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

We can also move it to base for now and add a os connection, once we add api


def file(path)
@files[path] ||=\
if os.aix?
Expand Down
7 changes: 3 additions & 4 deletions lib/train/transports/local.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,13 @@ def local?
true
end

def os
platform
end

def platform
@platform ||= Train::Platforms::Detect.scan(self)
end

# we need to keep os as a method for backwards compatibility with inspec
alias os platform

def file(path)
@files[path] ||= \
if os.windows?
Expand Down
7 changes: 3 additions & 4 deletions lib/train/transports/ssh_connection.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,14 +55,13 @@ def close
@session = nil
end

def os
platform
end

def platform
Copy link
Contributor

Choose a reason for hiding this comment

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

Since the method calls are more similar now, we could move that to base connection too?

@platform ||= Train::Platforms::Detect.scan(self)
end

# we need to keep os as a method for backwards compatibility with inspec
alias os platform

def file(path)
@files[path] ||= \
if os.aix?
Expand Down
7 changes: 3 additions & 4 deletions lib/train/transports/winrm_connection.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,13 @@ def close
@session = nil
end

def os
platform
end

def platform
@platform ||= Train::Platforms::Detect.scan(self)
end

# we need to keep os as a method for backwards compatibility with inspec
alias os platform

def file(path)
@files[path] ||= Train::File::Remote::Windows.new(self, path)
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ def initialize
it 'return new file contents' do
be = mock('Backend')
output = mock('Output', exit_status: 0)
output.expects(:stdout).twice.returns('test')
output.expects(:stdout).returns('test')
be.stubs(:run_command).with('test -f /etc/fstab && cat /etc/fstab').returns(output)
detector.instance_variable_set(:@backend, be)
detector.instance_variable_set(:@files, {})
Expand Down