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 openvms detection #159

Merged
merged 8 commits into from
Nov 11, 2016
Merged

Conversation

briandoodyie
Copy link
Contributor

This has been tested with OpenVMS 8.3.

  • Added openvms detection
  • Removed the hardcoding of ssh_connection setting platform to unix.
  • In os_common.rb we can now more accurately determine the platform by using uname_s method. Openvms does not have understand uname_s and doesnt generate an error detectable by the command object. However you can check the message coming from that for the generic openvms error.

@@ -96,6 +98,8 @@ def detect_family
# TODO: extend base implementation for detecting the family type
# to Windows and others
case uname_s
when /unrecognized command verb/
Copy link
Contributor

Choose a reason for hiding this comment

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

not sure if we can expect that every system that has not uname_s is openvms

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 just don't set any family 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.

problem is that if we omit this part, it matches

when /./
@platform[:family] = 'unix'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Its a pain because the uname_s command doesnt returns an error that can be detected by the command object (old os) but a message that says : %DCL-W-IVVERB, unrecognized command verb - check validity and spelling

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should create a method for detecting non-uname operating systems. Is there a reliable command that only works on openvms to detect the family?

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay, lets use this for now until we find any other non-uname operating system

@@ -124,6 +129,7 @@ def detect_family_type # rubocop:disable Metrics/CyclomaticComplexity, Metrics/P
# unix based systems combine the above
return true if pf == 'unix' and detect_darwin
return true if pf == 'unix' and detect_esx
#This is assuming that pf is set to unix, this should be if pf == 'linux'
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can argue that arista should be in detect_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.

@backend.run_command('show system/noprocess') will always return OpenVMS Vxx e.g. "OpenVMS V8.2 on node VS3B2 3-NOV-2016 21:41:10.97 Uptime 171 04:49:02

"

@@ -96,6 +98,8 @@ def detect_family
# TODO: extend base implementation for detecting the family type
# to Windows and others
case uname_s
when /unrecognized command verb/
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should create a method for detecting non-uname operating systems. Is there a reliable command that only works on openvms to detect the family?

@chris-rock
Copy link
Contributor

chris-rock commented Nov 3, 2016

Could you please sign off your commits? Are you resolving the linting issues?

@briandoodyie briandoodyie force-pushed the add_openvms_detection branch from e5e1178 to 4c1071e Compare November 4, 2016 11:28
@briandoodyie
Copy link
Contributor Author

Looking at the lint testing part now

…ie/train into add_openvms_detection

Signed-off-by: Brian Doody <[email protected]>

Conflicts:
	lib/train/extras/os_detect_openvms.rb
@briandoodyie
Copy link
Contributor Author

@chris-rock not sure on this final error. All the lint testing should pass now.

@chris-rock
Copy link
Contributor

awesome @briandoodyie I think we are getting very close...

@@ -124,6 +129,7 @@ def detect_family_type # rubocop:disable Metrics/CyclomaticComplexity, Metrics/P
# unix based systems combine the above
return true if pf == 'unix' and detect_darwin
return true if pf == 'unix' and detect_esx
# This is assuming that pf is set to unix, this should be if pf == 'linux'
Copy link
Contributor

Choose a reason for hiding this comment

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

we should remove that command, since from trains perspective linux is a subset of a unix-like operating system.

@@ -96,6 +98,8 @@ def detect_family
# TODO: extend base implementation for detecting the family type
# to Windows and others
case uname_s
when /unrecognized command verb/
@platform[:family] = 'openvms'
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we cannot expect that every non-uname system is openvms I would prefer to set it to nil here?

@@ -230,7 +230,7 @@ def to_s

class OS < OSCommon
def initialize(backend)
super(backend, { family: 'unix' })
Copy link
Contributor

Choose a reason for hiding this comment

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

I just thinking if it is not easier to count openvms as a unix-like system as well? Not sure about that. This would allow us to just use

return true if pf =='unix' && detect_openvms 

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 was the original code and the problem there is that

@chris-rock
Copy link
Contributor

@briandoodyie Anything I can help with?

@briandoodyie
Copy link
Contributor Author

Hmmm the ssh access will work and was the original change that was made. It made everything a lot simpler but doesnt windows now also support ssh. Might be assumption that just doesnt work now. thoughts? @chris-rock

@chris-rock
Copy link
Contributor

chris-rock commented Nov 11, 2016

@briandoodyie I wanna clean this up as well, I just want to do this in two steps. I apologize for the confusion

@chris-rock
Copy link
Contributor

@briandoodyie I go with the current PR. Thanks @briandoodyie for putting all the effort into that

@chris-rock chris-rock merged commit 0404986 into inspec:master Nov 11, 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