-
Notifications
You must be signed in to change notification settings - Fork 356
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Send oVirt metrics details to raw_connect
#2651
Send oVirt metrics details to raw_connect
#2651
Conversation
@jntullo @martinpovolny @agrare @pkliczewski @borod108 @masayag please review. |
:version => 4, | ||
:verify_ssl => params[:default_tls_verify] == 'on' ? OpenSSL::SSL::VERIFY_PEER : OpenSSL::SSL::VERIFY_NONE, | ||
:ca_certs => params[:default_tls_ca_certs], | ||
:metrics_userid => metrics_user, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens when the metrics credentials are empty?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When the metrics credentials are empty this will just send nil
or a blank strings.
Note that this is just the first step. Second step is to modify the raw_connect
method of the provider so that it takes these values and does something useful with them.
raw_connect
raw_connect
@miq-bot add_label gaprindashvili/yes |
@@ -116,6 +116,7 @@ def create | |||
|
|||
def get_task_args(ems) | |||
user, password = params[:default_userid], MiqPassword.encrypt(params[:default_password]) | |||
metrics_user, metrics_password = params[:metrics_userid], MiqPassword.encrypt(params[:metrics_password]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do any other providers use metrics_
auth type? We might want to move these provider specific ones down into the respective case statements otherwise this list can grow quickly (add hawkular and prometheus for containers at least)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really don't know if other providers use metrics_
. Anyhow, moving this to the case statement doesn't hurt, so I will do it.
A recent patch changed the UI worker in order to perform authentication validation of infrastructure providers using a queued task. This had the side effect of not sending to the oVirt provider the metrics connection details and. Without this these providers will not be validated. This patch partially solves the following bug: Failed validation when adding RHV provider https://bugzilla.redhat.com/1509432
Checked commit https://github.com/jhernand/manageiq-ui-classic/commit/7a676fc5bf66fab0c7db957b7f1e9098019b0b5c with ruby 2.3.3, rubocop 0.47.1, and haml-lint 0.20.0 |
@miq-bot add-label bug |
Send oVirt metrics details to `raw_connect` (cherry picked from commit 30cd822) https://bugzilla.redhat.com/show_bug.cgi?id=1511079
Gaprindashvili backport details:
|
A recent patch changed the UI worker in order to perform authentication
validation of infrastructure providers using a queued task. This had the
side effect of not sending to the oVirt provider the metrics connection
details and. Without this these providers will not be validated.
This patch partially solves the following bug:
RHV provider metrics validation is broken
https://bugzilla.redhat.com/1510533