Add support for monitoring selection in UI

[nimrodshn]

Add support for "monitoring selection" in the UI as well as "Prometheus" time series db when adding a new provider.

cc: @simon3z @moolitayer @yaacov

[nimrodshn]

@miq-bot add_label providers/containers

[miq-bot]

@nimrodshn Cannot apply the following label because they are not recognized: providers/containers

[moolitayer]

@nimrodshn 🎉 cool!

Please add a print of the endpoint you get from this
(connection_configurations is better actually)

[miq-bot]

This pull request is not mergeable. Please rebase and repush.

[nimrodshn]

@moolitayer Here you go :

[yaacov]

@simon3z @moolitayer @nimrodshn

Nice, IMHO wrongly implemented.

Kubernetes and Openshift providers should have properties that set the preferred endpoint for each monitoring task, e.g. charts/reports, alerts and logging,

For example if we have a cluster with Prometheus for volatile metrics and alerts, InfoDB for metrics (hourly / dailiy rollups reports and charting) and ElasticSearch for logging, we should have properties in the provider that set the preferred endpoint for each task ( reports, alerts and logging )

This implementation adds this properties to the endpoint, IMHO this is wrong, this new property belongs to the provider, the UI should reflect that, the new drop down should move up, to the provider properties area.

[nimrodshn]

Nice, IMHO wrongly implemented.

I tend to agree with @yaacov on this. (Although you mean 'wrong design' and not implemented, I think..)
@moolitayer @simon3z opinions?

[moolitayer]

The selection should be under monitoring and not in the provider section.

1. other providers

2. If the argument is "we can have a complex environment so the selection needs to be in the top section"

• the environment might become simpler (e.g we will probably end up with one option for our monitoring manager)
• lets imagine what you are describing "in the future". assume we have you have lets say 2 options for logging and 2 for monitoring how is that simpler to have 4 options in the top section then 2 and 2 under the relevant tabs?

[nimrodshn]

@moolitayer - so in you're view we would add more "tabs" alongside 'monitoring' instead of making more drop downs at the provider section / adding options to an existing drop down in the provider section? Okay this makes sense to me.

The only thing is - if the selection is at the top we can "eliminate" irrelevant tabs when selection is marked "disabled" which could be helpful when we have a complex environment. But, I get what your'e saying.

[simon3z]

Few comments based on the gif image:

1. <Choose> and Disabled are redundant, please just have Disabled
2. I see that the default port for Prometheus is 80, what about SSL?

[moolitayer]

I would argue removing tabs is more confusing but we can avoid that since we have a precedence for exactly that in ManageIQ: the vCloud screenshot I attached above.

[yaacov]

@simon3z @moolitayer @nimrodshn

a. we can add the property :monitoring_endpoint => prometheus/hawkular/none to the provider
b. we can add to each new endpoint the property :default_monitor_endpoint => true/false

@simon3z please advice, @moolitayer @nimrodshn and me discussed this, and we need a new look at this.

[simon3z]

a. we can add the property :monitoring_endpoint => prometheus/hawkular/none to the provider
b. we can add to each new endpoint the property :default_monitor_endpoint => true/false

@nimrodshn @moolitayer @yaacov I don't think either is needed. An OpenShift provider would have just one prometheus or hawkular endpoint so no need to mark anything as default.

[nimrodshn]

@simon3z fixed -
I dropped the 'Choose' on the drop down and added SSL as a default for security protocol - same as in Hawkular.

[simon3z]

@nimrodshn @moolitayer @yaacov I don't think either is needed. An OpenShift provider would have just one prometheus or hawkular endpoint so no need to mark anything as default.

@nimrodshn @moolitayer now that I spoke to @yaacov and I understood what the question was about. I have no strong feelings. I guess we'll follow other providers for consistency on this one. Otherwise we'll let the UI/UX maintainers decide.

[nimrodshn]

@simon3z I agree with you, could go both ways - but we should be consistent like @moolitayer stated.

[yaacov]

but we should be consistent like @moolitayer stated

@nimrodshn In all providers, properties that belong to the provider ( e.g. the endpoint used for ... ) are on the top area.

Since the type of metrics collection used is part of the provider properties, it should be on the top area like in all other providers.

If it was a question of endpoint exist or not is should be on the endpoint area, like in the screenshoots @moolitayer provided

[yaacov]

In cloud infrastructure, the "type" property of the provider, changes the possible endpoints, just like here, the "monitoring" property of the provider change the possible endpoint from default, hawkular to default, prometheus or just default.

[nimrodshn]

@simon3z @yaacov @moolitayer FYI

[yaacov]

FYI
@nimrodshn @simon3z @moolitayer , Thanks, looks much cleaner now.

[yaacov]

@nimrodshn you can squash ... I asked you not to squash, because I thought the second commit is only the js function that use the ugly element.hide thing, I see that it include lots of unrelated stuff, no need to keep separate.

[yaacov]

LGTM

[miq-bot]

Checked commits nimrodshn/manageiq-ui-classic@39f61a0~...91e1e99 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
5 files checked, 0 offenses detected
Everything looks fine. 🍰

[yaacov]

@h-kataria @himdel please review

[simon3z]

@miq-bot assign himdel

[himdel]

Code looks good, it can even correctly show prometheus conf when editing a provider which uses it..

Merging 👍