| Summary: | aeolus-cli import should verify id passed | ||
|---|---|---|---|
| Product: | [Retired] CloudForms Cloud Engine | Reporter: | Dave Johnson <dajohnso> |
| Component: | rubygem-aeolus-cli | Assignee: | Jason Guiditta <jguiditt> |
| Status: | CLOSED DUPLICATE | QA Contact: | wes hayutin <whayutin> |
| Severity: | medium | Docs Contact: | |
| Priority: | unspecified | ||
| Version: | 1.0.0 | CC: | matt.wagner, morazi, mtaylor, sreichar |
| Target Milestone: | rc | Keywords: | Reopened |
| Target Release: | --- | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
| Whiteboard: | |||
| Fixed In Version: | Doc Type: | Bug Fix | |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2012-01-05 21:41:23 UTC | Type: | --- |
| Regression: | --- | Mount Type: | --- |
| Documentation: | --- | CRM: | |
| Verified Versions: | Category: | --- | |
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
| Cloudforms Team: | --- | Target Upstream Version: | |
| Bug Depends On: | |||
| Bug Blocks: | 744194 | ||
|
Description
Dave Johnson
2011-11-18 16:44:53 UTC
Martyn -- any thoughts on who would be best to address this one? This really should be addressed in Imagefactory. I am going to assign this over to Ian Mcleod. *** Bug 761533 has been marked as a duplicate of this bug. *** 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. adding ce-sprint-next bugs to ce-sprint 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 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. 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. 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 Believe this should be closed as a dupe of 765714; see above 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 *** |