Bug 889317 - spacewalk-clone-by-date crashes instead of printing a usage error message when usage is incorrect
spacewalk-clone-by-date crashes instead of printing a usage error message whe...
Status: CLOSED CURRENTRELEASE
Product: Spacewalk
Classification: Community
Component: Clients (Show other bugs)
1.9
x86_64 Linux
medium Severity low
: ---
: ---
Assigned To: Stephen Herr
Red Hat Satellite QA List
:
Depends On: 810090
Blocks: space19
  Show dependency treegraph
 
Reported: 2012-12-20 14:23 EST by Stephen Herr
Modified: 2013-03-06 13:34 EST (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: 810090
Environment:
Last Closed: 2013-03-06 13:34:22 EST
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)

  None (edit)
Description Stephen Herr 2012-12-20 14:23:41 EST
+++ This bug was initially created as a clone of Bug #810090 +++

Created attachment 575295 [details]
sos report from the Satellite server

Description of problem:
spacewalk-clone-by-date crashes on RHEL 6 with exception when given -d option


Version-Release number of selected component (if applicable):
spacewalk-utils-1.2.1-16.el6sat

How reproducible:
Everytime

Steps to Reproduce:
1. Login to Satellite server as root
2. Run spacewalk-clone-by-date as:
spacewalk-clone-by-date -l rhel-x86_64-server-5 -d 2012-02-17 -u adminuser
  
Actual results:
[root@xxxxxxxx ~]# spacewalk-clone-by-date -l rhel-x86_64-server-5 -d 2012-02-17 -u adminuser
Traceback (most recent call last):
  File "/usr/bin/spacewalk-clone-by-date", line 205, in <module>
    sys.exit(abs(main() or 0))
  File "/usr/bin/spacewalk-clone-by-date", line 194, in main
    args = parse_args();
  File "/usr/bin/spacewalk-clone-by-date", line 162, in parse_args
    options.to_date = parse_time(options.to_date)
  File "/usr/bin/spacewalk-clone-by-date", line 175, in parse_time
    if re.match('[0-9]{4}-[0-9]{2}-[0-9]{2}',time_str):
  File "/usr/lib64/python2.6/re.py", line 137, in match
    return _compile(pattern, flags).match(string)
TypeError: expected string or buffer
[root@xxxxxxxx ~]#

Expected results:
Successfully cloned channel

Additional info:

--- Additional comment from Ken Sugawara on 2012-05-07 04:26:42 EDT ---

I realized that there was a missing argument (the name of the cloned channel) in the usage shown above, so I changed the title of this bug accordingly.
spacewalk-clone-by-date should print an error message if

--- Additional comment from Ken Sugawara on 2012-05-07 04:28:59 EDT ---

(continuing from the previous comment; sorry) the usage is incorrect (e.g. missing arguments), instead of crashing and dumping traceback.

--- Additional comment from Dimitar Yordanov on 2012-12-06 05:59:53 EST ---

On one hand the issue described in the "Descriprion" is actually 

 Bug 866930 - spacewalk-clone-by-date without --to_date option produces traceback

On the other hand seems that Python OptionParser parser fails to work reliable with options with more than one argument.
Option "-l" should receive two arguments and in this case it takes the next option string as an argument.

If we take as un example:

  #spacewalk-clone-by-date -l rhel-x86_64-server-5 -d 2012-02-17 -u adminuser

the argumetns for option "-l"  are  "rhel-x86_64-server-5"  and "-d".
In this way we run into but Bug 866930 because the "Date" is not defined.

In case we switch the places of options -u and -d we will not longer hit  Bug 866930 but the problem with Python OptionParser.

  #spacewalk-clone-by-date -l rhel-x86_64-server-5 -u admin -d 2012-01-01 
  Username not specified


Possible hack-fix: 

    if options.channels:
        for chans in options.channels:
          for chan in chans:
            if re.match('^-',chan):
                raise UserError("Some channel name is missing")

--- Additional comment from Stephen Herr on 2012-12-20 13:57:01 EST ---

Since leaving off a channel will lead either to

1) error: -l option requires 2 arguments

if the -l option is at the end of the command or

2) the behavior described above, where the rest of the options are stuck into the args variable because they can't be interpreted as options

a less hacky fix might be to error if len(args) != 0.

Or we could simply migrate from optparse to argparse, which is incredibly straightforward and works much better. 

I think I'll do the latter.

--- Additional comment from Stephen Herr on 2012-12-20 14:22:30 EST ---

I take back the first part of what I said. It seems there are some cases where optparse will not stick the extra words in the args list and will just consume things it doesn't understand. For example:

spacewalk-clone-by-date -l rhel-x86_64-server-6 -d 2012-02-17 -u admin -p <password>

will use "rhel-x86_64-server-6" and "-d" as the two channel labels, and the "2012-02-17" just disappears into thin air. The args list will be empty.

So the only good way to fix this is to migrate from optparse to argparse. Output after migrating:

# spacewalk-clone-by-date -l rhel-x86_64-server-6 -d 2012-02-17 -u admin -p <password>
usage: spacewalk-clone-by-date [-h] [-c CONFIG] [-u USERNAME] [-p PASSWORD]
                               [-s SERVER] [-l CHANNELS CHANNELS]
                               [-b BLACKLIST] [-r REMOVELIST] [-d TO_DATE]
                               [-y] [-m] [-k] [-v] [-g] [-o]
spacewalk-clone-by-date: error: argument -l/--channels: expected 2 argument(s)
Comment 1 Stephen Herr 2012-12-20 14:27:17 EST
Committed to Spacewalk master: 4d31003ed6ed328412d0ef28b989de182cc36c99
Comment 2 Stephen Herr 2012-12-20 15:08:12 EST
Sadly this will not work for us, because argparse wasn't introduced until python 2.7 and we have to support python 2.4 and 2.6. :(

It looks like the hacky solution is the only one available to us.

Reverting the commit in comment 1 and committing the new fix to Spacewalk master: d5742d1c807f3fd001f05d5b8db940ce700a8298
Comment 3 Stephen Herr 2012-12-20 15:17:16 EST
And ae72e3433fab82459b98cb09290545a765abd3a3
Comment 4 Stephen Herr 2013-01-09 13:46:01 EST
and 1d9b7146214e663c77b4209fa1cf8df97d871ce0

The username check needs to happen after we verify the channel args. I have also added a similar check for the --parents args.
Comment 5 Stephen Herr 2013-01-10 16:03:47 EST
and 4c21c742bb41d326284118fc0532fb330a24b04a

Adding more thorough checking so we can detect all instances where the user does specify two args for either --channels or --parents.
Comment 6 Stephen Herr 2013-01-11 11:03:34 EST
and c1e54e3ea1d085e1a1626dc978af8b2b27731cee

Removing redundant (and potentially incorrect) check
Comment 7 Stephen Herr 2013-03-01 12:06:58 EST
Marking bug as ON_QA since tonight's build of Spacewalk nightly is a release candidate for Spacewalk 1.9.
Comment 8 Stephen Herr 2013-03-06 13:34:22 EST
Spacewalk 1.9 has been released.

https://fedorahosted.org/spacewalk/wiki/ReleaseNotes19

Note You need to log in before you can comment on or make changes to this bug.