Added support for RHEL based systems where /etc/issue is modified

[dapatil]

Modified issue() so that it looks at /etc/redhat-release first.
This was tested on RHEL, Oracle Linux and Ubuntu.

[tmatilai]

@dapatil Thanks for the contribution! Do you have an AMI or some more detailed example that demonstrates the issue that this patch resolves? Pardon my ignorance but I don't normally use EL based distros.

[dapatil]

The following example should be sufficient.
Let me know if you need further clarification.

[root@nyci-claedwa01 ~]# cat /etc/issue
############################################################################
#                                                                          #
# Company Name System Login (C) 2013 Company Name                          #
#                                                                          #
# This is a Company Name proprietary system. No use is allowed without     #
# proper authorization. Unauthorized use of this computer or network       #
# resources may constitute a breach of Company Name policy and be          #
# liable to prosecution under relevant legislation.  Your use of Company   #
# Name electronic communication facilities is permitted only in            #
# accordance with company policies, is not private and is subject to       #
# auditing / monitoring at any time.                                       #
#                                                                          #
############################################################################
[root@nyci-claedwa01 ~]# cat /etc/redhat-release
Red Hat Enterprise Linux Server release 6.3 (Santiago)

[tmatilai]

Yes, you're right. /etc/issue is often displayed when a user logs in and thus customized by many service providers. We can not rely on it having the original distro information. This is second case in short period of time (see #223).

Maybe we should default to lsb-release and fallback to /etc/redhat-release, /etc/debian_version, etc. and finally to /etc/issue? I would also like to reduce the amount of separate ssh calls until we (hopefully) have ssh multiplexing. (As a long distance remote worker I'm oversensitive to latency for opening ssh/tcp connections.)

[matschaffer]

Definitely seen a fair number of systems that do this. Sounds like a good
plan to me.

On Thursday, April 25, 2013, Darshan Patil wrote:

The following example should be sufficient.
Let me know if you need further clarification.

[root@nyci-claedwa01 ~]# cat /etc/issue
############################################################################

Company Name System Login (C) 2013 Company Name

This is a Company Name proprietary system. No use is allowed without

proper authorization. Unauthorized use of this computer or network

resources may constitute a breach of Company Name policy and be

liable to prosecution under relevant legislation. Your use of Company

Name electronic communication facilities is permitted only in

accordance with company policies, is not private and is subject to

auditing / monitoring at any time.

############################################################################

[root@nyci-claedwa01 ~]# cat /etc/redhat-release
Red Hat Enterprise Linux Server release 6.3 (Santiago)

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/234#issuecomment-17016869
.

-Mat

about.me/matschaffer

[dapatil]

If you're worried about too many ssh connections, we could change it to.

def issue
    cmds = [
          "lsb_release -d -s",
          "cat /etc/redhat-release",
          "cat /etc/debian_version",
          "cat /etc/issue"
    ]
    prepare.run_command(cmds.join(" || ")).stdout.strip
end

[tmatilai]

Yep, except /etc/debian_version only includes the version number without the "Debian..." prefix so a bit more logic is needed. But if you want to update the PR according to your suggestion (dropping the debian_version for now if you prefer), would be great. We should at least test that the current integration test suite passes.

[dapatil]

done

[matschaffer]

I'd also rather if we reused the existing ssh connection than trying to be
conservative about the number of commands we run.

I have (yet another) long lived branch that has most of that refactor
working.

On Thursday, April 25, 2013, Teemu Matilainen wrote:

Yep, except /etc/debian_version only includes the version number
without the "Debian..." prefix so a bit more logic is needed. But if you
want to update the PR according to your suggestion (dropping the
debian_version for now if you prefer), would be great. We should at least
test that the current integration test suite passes.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/234#issuecomment-17032101
.

-Mat

about.me/matschaffer

[tmatilai]

Mat, so do you want the commands to be separated?
I didn't find any problems with integration tests with this change but not all have lsb_release installed. Would be good to verify that all distros report the same thing that we expected to be in /etc/issue. I can test manually at least some.

[matschaffer]

I prefer what's in 9de6492 rather than the latest. I'd rather not
prematurely optimize for a shortcoming that's already being worked on.

-Mat

about.me/matschaffer

On Fri, May 3, 2013 at 2:46 PM, Teemu Matilainen
[email protected]:

Mat, so do you want the commands to be separated?
I didn't find any problems with integration tests with this change but not
all have lsb_release installed. Would be good to verify that all distros
report the same thing that we expected to be in /etc/issue. I can test
manually at least some.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/234#issuecomment-17420036
.

[tmatilai]

Ack. I would still like to prefer lsb_release over the files. But we have to check the exit status of the commands as for example "command not found" gets printed to stdout.

My proposal is here. Feeling lazy for making another PR. =)

[matschaffer]

Nice. I think we could use a helper function though. Maybe something like run_with_fallbacks? e.g.,

run_with_fallbacks('lsb_release -d -s', 'cat /etc/redhat-release', 'cat /etc/issue').stdout.strip

And wrap the success checks in there?

[tmatilai]

Yeah, I was thinking something like ExecResult#stdout_if_successful. But I'm just trying to avoid writing tests. :p
I think I like your way more. But we need to check the exit status of the result to avoid false detections. Let's see...

[tmatilai]

Thanks Darshan! This is now fixed in master.
For the record, I abandoned the idea of using /etc/debian_version as also derivative distros might include it. For example Ubuntu 12.04 has one stating "wheezy/sid" or something as it is taken from Debian testing at the time. So we basically can't trust it.

[dapatil]

Thanks.