BIND 10 #2062: bind10-showtech initial version

[BIND 10 Development]

#2062: bind10-showtech initial version
-------------------------------------+-------------------------------------
                   Reporter:  shane  |                 Owner:  vorner
                       Type:  task   |                Status:  reviewing
                   Priority:         |             Milestone:
  medium                             |  Sprint-20120703
                  Component:         |            Resolution:
  Unclassified                       |             Sensitive:  0
                   Keywords:         |           Sub-Project:  Core
            Defect Severity:  N/A    |  Estimated Difficulty:  5
Feature Depending on Ticket:         |           Total Hours:  0
  bind10-showtech                    |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by muks):

 * owner:  muks => vorner


Comment:

 Hi vorner

 Replying to [comment:11 vorner]:
 > First, I'd modify the changelog slightly. It looks like Bind10 has a
 support staff. Maybe say future support staff?

 How does this look:

 {{{
 XXX.    [func]*         muks
         b10-showtech: An initial implementation of the b10-showtech tool
         is now available. It gathers and outputs system information which
         can be used by future tech support staff.
         (Trac #2062, git ...)
 }}}

 >
 > '''Regarding functionality'''
 >
 > I'm not sure what if anything of this is in scope of this ticket, but I
 want to note it down.
 >
 > I'd like an option to put the output to a file. The output is really
 long and scrolls through the screen. Also, once we include things like
 config.log, our log files and our configuration, it could be handy to have
 them as separate files and pack them together with tar or something.

 This has been implemented now. `-o`/`--output` options have been added.
 Manpage has also been updated.

 > While I agree it is good to have network information, routing info, list
 of open connections with process names and list of processes (as suggested
 on the wiki page), these information may be considered sensitive by some
 admins and they would not like to share them. Unless we provide some kind
 of switch, like `--no-sensitive`, it could lead to them not providing the
 output of `b10-showtech` at all.

 I agree with the idea, but it needs more discussion. If we cut out the
 entire data when `--no-sensitive` is specified, then the `b10-showtech`
 output won't be very useful as these are the data that support staff would
 want to see. We may want to filter or anonymize data. The format of the
 data varies from system to system, but this should not be problematic as
 it can be done in the implementations of `SysInfo`. I think we should
 create a separate ticket for it to be done after #2085.

 > There's no `--help` option.

 This has been added now. Manpage has also been updated.

 > If the script is run as non-root (which is reasonable to assume it
 will), it silently says the network information is Unknown. It would be
 nice to detect the situation and warn about it (preferably at the end of
 the output, so it does not scroll far away and the user sees).

 It now prints a message to stderr at the end of its run, if run as non-
 root.

 > '''Technical problems'''
 >
 > A test fails:
 > {{{
 > ======================================================================
 > FAIL: test_calls (__main__.TestRename)
 > ----------------------------------------------------------------------
 > Traceback (most recent call last):
 >   File
 "/home/vorner/work/bind10/bind10-devel-20120405/_build/src/bin/tests/process_rename_test.py",
 line 62, in test_calls
 >     self.__scan(d, script, fun)
 >   File
 "/home/vorner/work/bind10/bind10-devel-20120405/_build/src/bin/tests/process_rename_test.py",
 line 32, in __scan
 >     "Didn't find a call to isc.util.process.rename in " + prettyname)
 > AssertionError: None is not true : Didn't find a call to
 isc.util.process.rename in src/src/bin/showtech/b10-showtech
 >
 > ----------------------------------------------------------------------
 > }}}

 Aieeeee. Fixed! :)

 > Should the generated man page be in git?

 This follows the convention of all other bin programs whose manpages are
 also checked in.

 > I don't think the current tests are good enough. They test only little
 bit of the code that is just setting of some default values. Everything
 should be tested, if only on Linux or by replacing the system calls with
 some mock functions.

 I strugged with how to test this properly too, as different
 implementations return different data. But I don't think we should do
 this. Unit tests are supposed to check at the interfaces, not into the
 implementation. While some things (such as to check if a lock is being
 acquired) are notable, I don't think we should check if system calls are
 being called. It would be similar to testing if each line of code were
 written in a specific way. There would also not be a benefit of doing
 this. Each system's output will be different, and the implementation of
 how we gather a particular piece of data may change too (exec `free`
 instead of reading `/proc/meminfo`, or exec `df` intead of reading inside
 `/proc/`). We'd have to rewrite the tests too then to match the code,
 which would defeat the purpose of a check. The tests should not test at
 this level.

 A testcase for the factory has been added.

 > A lot of the calls are not system specific. For example, according to
 python documentation, os.uname() should be available on most Unix systems.
 So it would be nice to put it into some try-catch block to the base class
 so other OSes (or the unknown ones) don't need to reimplement it again and
 again. Other things might be also available more generally than on linux
 only. The derived ones can always override the detection if it does not
 work for the system.

 We can have a class in the hierarchy between `SysInfo` and `SysInfoLinux`
 from which other UNIXes also derive, and this can implement the common
 parts. This is better done in #2085 when we know what is common. I'll
 update that bug about it.

-- 
Ticket URL: <http://bind10.isc.org/ticket/2062#comment:12>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development