Add methods to extract v2v log from conversion host. #17333
Conversation
There was a problem hiding this comment.
@lfu This is looking pretty good. I think we should be using transformation instead of v2v to be consistent with the naming being used on the backend.
| Net::SCP::download!(host.ipaddress, userid, logfile, nil, :ssh => {:password => password}) | ||
| rescue => scp_err | ||
| _log.error("Download of v2v log for #{description} with ID [#{id}] failed with error: #{scp_err}") | ||
| raise MiqException::Error, scp_err.to_s, scp_err.backtrace |
There was a problem hiding this comment.
Do we need to create an MiqException::Error here or can we just re-raise the original error?
| raise MiqException::Error, msg | ||
| end | ||
|
|
||
| userid, password = host.auth_user_pwd(:default) |
There was a problem hiding this comment.
This should have :remote as the parameter which will fallback to :default if no "Remote" credentials are configured.
Example: https://github.com/ManageIQ/manageiq/blob/master/app/models/host.rb#L1024
| :state => MiqTask::STATE_FINISHED, | ||
| :status => MiqTask::STATUS_ERROR, | ||
| :message => msg | ||
| ).id |
There was a problem hiding this comment.
Would be nice to pull this into a separate method.
lfu
force-pushed
the
v2v_log_extraction
branch
2 times, most recently
from
d2e81b4
to
b0beff3
Compare
|
|
||
| # Intend to be called by UI to display transformation log. The log is stored in MiqTask#task_results | ||
| # Since the task_results may contain a large block of data, it is desired to remove the task upon receiving the data | ||
| def transformation_log_queue(userid) |
There was a problem hiding this comment.
Seems like we should figure out userid up front and then pass that into create_error_status_task or queue_options instead of making both do the || 'system' logic.
What do you think about userid being set set from User.current_user instead of being passed in? If we keep the parameter I suggest defaulting it since you are handling the 'nil' case.
| begin | ||
| require 'net/scp' | ||
| Net::SCP::download!(host.ipaddress, userid, logfile, nil, :ssh => {:password => password}) | ||
| rescue => scp_err |
There was a problem hiding this comment.
Per Rubocop I think we should be rescuing Net::SCP::Error error types here.
Also, is the Net::SCP::download! form of the call the only one that downloads into a local buffer or does Net::SCP.download! do it as well?
|
|
||
| userid, password = host.auth_user_pwd(:remote) | ||
| if userid.blank? || password.blank? | ||
| msg = "Host credential is required: userid [#{userid}], password [#{password}]. Download of transformation log aborted." |
There was a problem hiding this comment.
should not log userid/password. Something like Credential was not found for host #{host.name}".
|
Checked commit lfu@4389006 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 |
|
@bzwei Please review/approve. |
|
@miq-bot add_label blocker |
Add methods to extract v2v log from conversion host. (cherry picked from commit 3d0c76e) https://bugzilla.redhat.com/show_bug.cgi?id=1584695
|
Gaprindashvili backport details: |
To download v2v logs.
@miq-bot assign @gmcculloug
@miq-bot add_label enhancement, transformation
cc @bzwei
BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1564244