Bug 1093224

Summary: bkr machine-test: Force test a system irrespective of its status
Product: [Retired] Beaker Reporter: Amit Saha <asaha>
Component: command lineAssignee: Amit Saha <asaha>
Status: CLOSED CURRENTRELEASE QA Contact: tools-bugs <tools-bugs>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: developCC: aigao, asaha, dcallagh, ebaak, rmancy, xma
Target Milestone: 0.17   
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2014-06-10 23:28:23 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On: 851354    
Bug Blocks:    

Description Amit Saha 2014-05-01 02:10:29 UTC
Description of problem:


If a system is broken (for example), job submitted via `bkr machine-test` will be aborted. This can be a limiting feature for some of the same reasons described in https://bugzilla.redhat.com/show_bug.cgi?id=851354#c9 .



Version-Release number of selected component (if applicable):


How reproducible:


Steps to Reproduce:
1.
2.
3.

Actual results:


Expected results:

Two possible solutions:

1. A new switch --force be added to force the system provisioning
2. Make this the default behaviour


Additional info:

Comment 1 Amit Saha 2014-05-01 02:12:09 UTC
(In reply to Amit Saha from comment #0)
> Two possible solutions:
> 
> 1. A new switch --force be added to force the system provisioning

This patch implements this solution: http://gerrit.beaker-project.org/#/c/3048/

Comment 3 Dan Callaghan 2014-05-06 00:34:02 UTC
(In reply to Amit Saha from comment #0)
> Two possible solutions:
> 1. A new switch --force be added to force the system provisioning
> 2. Make this the default behaviour

After discussing this with Nick, we think number 2 is the way to go. If you are naming a specific system then there is really no reason you would ever want the scheduler to abort your job. So we can use force="" unconditionally in bkr machine-test.

It means we are effectively treating this as a bug, rather than an RFE (and the release note should be worded to reflect that): Previously if you ran machine-test against a system that was Broken, Beaker aborted your recipe saying "No matching systems". Now, Beaker will do what you asked.

Flipping this back to ASSIGNED since the patch will need tweaking.

Comment 4 Amit Saha 2014-05-06 00:42:05 UTC
(In reply to Dan Callaghan from comment #3)
> (In reply to Amit Saha from comment #0)
> > Two possible solutions:
> > 1. A new switch --force be added to force the system provisioning
> > 2. Make this the default behaviour
> 
> After discussing this with Nick, we think number 2 is the way to go. If you
> are naming a specific system then there is really no reason you would ever
> want the scheduler to abort your job. So we can use force="" unconditionally
> in bkr machine-test.

Not sure I am clear about the reasoning. When you submit a job using <hostRequires> (without force) for a system which is broken/manual, the job will be aborted. This is the "default" behavior. If we implement `bkr machine-test` as per 2, this means it will have a different "default" behavior. That seems inconsistent to me. Of course, if the reasoning is that `bkr machine-test` has a different use case and is thus expected to work differently, that is reasonable and hence this is a bug and 2 is the correct solution.

Comment 5 Amit Saha 2014-05-06 06:20:53 UTC
bkr machine-test (like other workflow commands) have a --machine and a --hostrequire switch. Now, if you specify both these, the job gets the following in <hostRequires> (for example):

<hostRequires>
<hostlabcontroller op="=" value="testlc.labcontroller"/>
<hostname op="=" value="test.system"/>
</hostRequires>

So, if we *do not* want to have an extra force switch, here is what I can think of:

1. If only --machine is specified, we force provision irrespective of the status
2. If both --machine and --hostrequire are specified, we ignore --hostrequire completely
3. If only --hostrequire is specified, we do the usual (no forced provisioning)

Comment 6 Dan Callaghan 2014-05-06 22:42:39 UTC
(In reply to Amit Saha from comment #5)
> 2. If both --machine and --hostrequire are specified, we ignore
> --hostrequire completely

This seems like an existing flaw in the option handling. If --machine is given it should be mutually exclusive with the other system selection options (--hostrequire, --type, ...) and it should trigger a usage error. It doesn't make sense to select a particular hostname and then also apply additional filtering.

Comment 7 Amit Saha 2014-05-07 01:54:31 UTC
(In reply to Dan Callaghan from comment #6)
> (In reply to Amit Saha from comment #5)
> > 2. If both --machine and --hostrequire are specified, we ignore
> > --hostrequire completely
> 
> This seems like an existing flaw in the option handling. If --machine is
> given it should be mutually exclusive with the other system selection
> options (--hostrequire, --type, ...) and it should trigger a usage error. It
> doesn't make sense to select a particular hostname and then also apply
> additional filtering.

Yeah, seems like. Filed bug 1095026 (with patch).

Comment 8 Amit Saha 2014-05-16 06:00:09 UTC
Bug 1095026 resolves the issue of both "machine" and "hostrequire" (and others) being specified by always considering only "machine" into account. 

For the purpose of this RFE, "force" will now always be enforced as per comment 3.

Comment 9 Amit Saha 2014-05-19 00:05:28 UTC
(In reply to Amit Saha from comment #8)

> 
> For the purpose of this RFE, "force" will now always be enforced as per
> comment 3.


As per further discussion at https://lists.fedorahosted.org/pipermail/beaker-devel/2014-May/001015.html, there will be a separate switch for bkr machine-test which is 'ignore-system-status'. The default behavior remains unchanged.

http://gerrit.beaker-project.org/#/c/3048/

Comment 13 Dan Callaghan 2014-06-10 23:28:23 UTC
Beaker 0.17.0 has been released.