Bug 755052 - aeolus-cli import should verify id passed
Summary: aeolus-cli import should verify id passed
Keywords:
Status: CLOSED DUPLICATE of bug 765714
Alias: None
Product: CloudForms Cloud Engine
Classification: Retired
Component: rubygem-aeolus-cli
Version: 1.0.0
Hardware: Unspecified
OS: Unspecified
unspecified
medium
Target Milestone: rc
Assignee: Jason Guiditta
QA Contact: wes hayutin
URL:
Whiteboard:
: 761533 (view as bug list)
Depends On:
Blocks: ce-sprint
TreeView+ depends on / blocked
 
Reported: 2011-11-18 16:44 UTC by Dave Johnson
Modified: 2012-01-26 12:19 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2012-01-05 21:41:23 UTC


Attachments (Terms of Use)

Description Dave Johnson 2011-11-18 16:44:53 UTC
Description of problem:
================================
Ran into a instance where I was passing the wrong value to aeolus-cli and it came back successful.  This command, at least for rhevm should check the existence of the value before we add it to warehouse by querying the rhevm api.

Comment 1 Mike Orazi 2011-11-30 19:38:26 UTC
Martyn -- any thoughts on who would be best to address this one?

Comment 2 Martyn Taylor 2011-12-08 10:51:20 UTC
This really should be addressed in Imagefactory.

I am going to assign this over to Ian Mcleod.

Comment 3 wes hayutin 2011-12-10 15:07:58 UTC
*** Bug 761533 has been marked as a duplicate of this bug. ***

Comment 4 Matt Wagner 2011-12-19 15:52:33 UTC
While Factory should probably do this, I should note that we're also handling this on our end (Conductor) in #765714 by checking with Deltacloud first.

Comment 5 wes hayutin 2012-01-03 17:42:54 UTC
adding ce-sprint-next bugs to ce-sprint

Comment 6 Ian McLeod 2012-01-05 15:24:43 UTC
Factory does "check" this and will eventually return an error via our status field when attempts to communicate with the provider fail due to bad credentials.  

We could in theory do some up-front check in the builder to produce a credentials error as soon as possible.  However, this would still result in a "successful" aeolus-cli run, as our only mechanism for error reporting is to set our builder status to "FAILED".

If you really need to get a failure at aeolus-cli runtime, I believe the check will need to be performed within aeolus-cli.

For the moment I am going to close this as NOTABUG.

If anyone strongly disagrees please reopen and state your case.

-Ian

Comment 7 wes hayutin 2012-01-05 16:21:23 UTC
Ya.. the bug should not be set to closed based on your comments.. we should move it to the aeolus cli guys and see if they have any way of helping here.

Comment 8 Jason Guiditta 2012-01-05 19:01:36 UTC
I have not really been working on aeolus-cli, so I have cc'ed martyn so he can add more insight, but I'll toss my 2 cents in here to be either shot down or agreed with.  I see this is set as a sprint blocker, and we are more or less already at code freeze for the current sprint, so that is a concern to me. I'd like to figure out if we really think this is a bug or a feature request, and a realistic time of when  it is needed, as well as the proper approach to correct any perceived issues.

The first thing we need to decide here is whether this is something that should be a 'runtime' error, as Ian mentions above, or merely something that is reported back as part of the status message from the factory.  The latter would be my inclination (Martyn, here is where you can object if you have a different idea).  The reason I suggest this is that the cli did not in fact fail, the import itself failed, and the reason is a non-existant matching image to import.  This, I believe, is the reason Martyn earlier said it should be addressed in image factory (and from what Ian says, it appears to be, at least partially- the missing piece being setting the status/message from the factory side, iiuic).  Remember that the cli is only a thin wrapper to talk to the rest APIs of various services, so the runtime errors it throws itself shoudl be pretty limited.  Most errors should merely be passing through whatever response the called API has returned, with perhaps a bit of niceness added in the form of formatting output messages.

All that said - I think there are already some default exception types that cli knows how to set default messages for, so if factory returned such an exception, an appropriate default message could probably be added pretty easily on the cli codebase.

Comment 9 Matt Wagner 2012-01-05 21:12:33 UTC
Per my earlier comment, this was fixed in Conductor via BZ 765714 (which is in "verified" state), but I had left this BZ open as it referenced having Image Factory add the same functionality. Now that this BZ has been changed to reference aeolus-cli, I think it should be closed as a dupe of 765714, no?

$ aeolus-image import --provider_account ec2 --id some-bogus-ami

ERROR:  ImageNotFound => Could not find Image some-bogus-ami on provider

Comment 10 Matt Wagner 2012-01-05 21:23:02 UTC
Believe this should be closed as a dupe of 765714; see above

Comment 11 wes hayutin 2012-01-05 21:41:23 UTC
ugh.. I hate to close a bug as dupe that was actually opened before the bug I'm duping it to.. but has the patch.. so it wins.. 

closing as dupe of 765714.
I'm going to move 765714 back to on_qa to be verified across all the supported providers.

*** This bug has been marked as a duplicate of bug 765714 ***


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