Add the location LED state to the physical_servers table

[skovic]

Added the location LED state to the physical_servers table so that the state can be displayed on a future server summary page.

[skovic]

@miq-bot add_labels wip, sql_migration

[miq-bot]

@skovic Cannot apply the following label because they are not recognized: sql_migration

[miq-bot]

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

[miq-bot]

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

[rodneyhbrown7]

@miq_bot remove_label: wip

[rodneyhbrown7]

@miq_bot assign @juliancheal
@miq-bot remove_label wip

[miq-bot]

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

[juliancheal]

@Fryguy would we need raw_loc_led_state here too?

[rodneyhbrown7]

what would raw_loc_led_state look like? Would this be an integer value? Is there an example of this pattern somewhere?

[skovic]

@rodneyhbrown7 @juliancheal @Fryguy There is an example of it here and also the reasoning:
#14287
https://github.com/ManageIQ/manageiq/pull/14042/files#r105482412
Apparently, they have power_state and raw_power_state as separate entities in other tables. raw_power_state is the raw string provided by the provider, and power_state is a normalized value used for reporting. With what I have so far, I am just retrieving the LED state via the provider and displaying the raw value on the server summary page as we do with other properties, so I'm not sure if this change is necessary here.

[juliancheal]

@blomquisg says it should be fine without the raw_ in this instance, so my bad, but it would be better if we used the full name of loc so something like locator_led_state or location_led_state, etc :)

[miq-bot]

Checked commits skovic/manageiq@e5e1772~...2ed571c with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
1 file checked, 0 offenses detected
Everything looks good. 🍪

[Fryguy]

@simaishi I am ok with this coming back to fine, but we might run into issues because of the numbering cc @carbonin I might need your help on this...it might need to be renumbered (not sure how that will affect devs)

[carbonin]

@Fryguy @simaishi This is the current state of things:

16:14:04:~/Source/manageiq (master)$ ls db/migrate/ | tail
20170315082311_update_o_virt_api_path.rb
20170315095936_update_persistent_volumes_parent_type.rb
20170316200500_remove_agent_state_from_jobs.rb
20170317134007_remove_agent_class_from_jobs.rb
20170317153953_copy_agent_id_to_miq_server_id_in_jobs_table.rb
20170320195659_remove_oid_integer_args_from_miq_queue.rb
20170324124452_add_vendor_property_to_physical_server.rb
20170328110106_fix_expression_in_tenant_quota_report.rb
20170404213245_add_loc_led_state_to_physical_servers.rb
data

16:14:26:~/Source/manageiq (fine)$ ls db/migrate/ | tail
20170313142647_add_raw_power_state_to_physical_server.rb
20170313160354_remove_ancestry_from_alerts.rb
20170313170754_add_mas_assignee.rb
20170315082311_update_o_virt_api_path.rb
20170315095936_update_persistent_volumes_parent_type.rb
20170316200500_remove_agent_state_from_jobs.rb
20170317134007_remove_agent_class_from_jobs.rb
20170317153953_copy_agent_id_to_miq_server_id_in_jobs_table.rb
20170320195659_remove_oid_integer_args_from_miq_queue.rb
data

If you don't feel like parsing that (I don't blame you) it says that there are currently 3 (soon to be 4, but I'll get to that later 👇) migrations that are on master, but not fine.

These are:

20170324124452_add_vendor_property_to_physical_server.rb
20170328110106_fix_expression_in_tenant_quota_report.rb
20170404213245_add_loc_led_state_to_physical_servers.rb

This PR adds the latest and #14488 adds the earliest.
We can fix this by either backporting the middle one as well or doing some renaming.

To add to the fun, #14687 is adding a migration which will also be backported to fine. I caught that one this morning and put the timestamp just after the latest in fine as of now, so it shouldn't cause a problem, but we should just keep it in mind.

---

The middle migration was added in #14558
That fixes an issue which was introduced in #7159 which is clearly in fine so it probably wouldn't hurt to backport that as well, but I haven't looked too deeply into what it's actually doing. Would probably need @lpichler or @kbrock to speak to that and determine if other changes would also need to come back (there are a lot of linked PRs there).

[simaishi]

Marking as fine/conflict for now...

[juliancheal]

@carbonin what do I need to do?

[carbonin]

Hopefully nothing @juliancheal 😄 I think we (me + @Fryguy + @simaishi) just need to work out what direction we want to take.

[juliancheal]

@carbonin 🍻

[Fryguy]

@carbonin Alternately, we can see if 20170328110106_fix_expression_in_tenant_quota_report can be backported.

Also @blomquisg @isimluk , please don't merge sql migrations, particularly if they need backport.

[blomquisg]

Also @blomquisg @isimluk , please don't merge sql migrations, particularly if they need backport.

I'm genuinely looking forward to the schema repo split 😄

[isimluk]

Also @blomquisg @isimluk , please don't merge sql migrations, particularly if they need backport.

Ok.

Also, it seems, that we cannot use cherry-pick to backport db migrations.

[lpichler]

@carbonin @Fryguy backporting of 20170328110106_fix_expression_in_tenant_quota_report to fine will not affect anything - it just fixing a format of field MiqExpression to be more strict. The migration pertains just Tenant Quotas reports which have been copied. Using of more strict format(according to MiqExpression::[Field|Count|Tag]:REGEX) will be required after merging this #13692. (but problem was causing only Quota tenant report was fixed by the migration #14558 )

so should #14558 be marked with fine/yes ?

[carbonin]

Alternately, we can see if 20170328110106_fix_expression_in_tenant_quota_report can be backported.

Yeah, @Fryguy That's what I was saying here:

The middle migration was added in #14558
That fixes an issue which was introduced in #7159 which is clearly in fine so it probably wouldn't hurt to backport that as well

[Fryguy]

@simaishi Please note that #14558 was marked fine/yes, so you should have no problem backporting all 3 to fine in order.

[simaishi]

Fine backport details:

$ git log -1
commit b43d43304b53906cf30cbd08fa9a56e4a43da0b6
Author: Greg Blomquist <[email protected]>
Date:   Tue Apr 4 17:01:29 2017 -0400

    Merge pull request #14277 from skovic/add-loc-led-state-to-physical-servers-table
    
    Add the location LED state to the physical_servers table
    (cherry picked from commit 64e64b8b3571737fb2d7b6547d4c2d3877ff483c)