Add status column to MiddlewareDomain

[xeviknal]

Related to issue #44

Adding status to MiddlewareDomain model.

[xeviknal]

@josejulio could you please check this out? Happy to have a code review :)

[lpichler]

Nice but we don't need to test structural changes because these things are tested in rails.

We need to test only when any data migration is happening.

so you can remove it and just leave only part of adding the column in migration.

thanks

[lpichler]

@miq-bot assign @Fryguy

[abonas]

cc @agrare

[xeviknal]

@lpichler good point. I'll remove it, I am testing a method which is already tested by rails.

Gotta say, that I wanted to check some of the tooling you have for testing migrations. My first time on a project with that :)

[miq-bot]

Checked commits xeviknal/manageiq-schema@8ae549f~...e32cc42 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
1 file checked, 0 offenses detected
Everything looks fine. 👍

[agrare]

I think you want to disable STI here
self.inheritance_column = :_type_disabled # disable STI

[xeviknal]

Could you please elaborate that a little bit?
I don't understand how the migration can change by disabling the STI

[agrare]

@Fryguy can correct me if I'm wrong but I believe it is so we don't end up loading the models during the migration if the record is subclassed with STI

[lpichler]

self.inheritance_column = :_type_disabled # disable STI
here is when the table has column type. For more details see:#4

Anyway, you don't need to define model here, as you are not using the model in migration either in spec.

[abonas]

@josejulio @israel-hdez I've seen a bit different handling/persisting of status in the middleware server case - isn't all status related info stored in a properties object and not in a separate field? (availability, state, etc) I just want to make sure we are not doing something inconsistent here, but if there is indeed some difference that is ok - then we can leave this as is.

[josejulio]

Deployments have their status as a separate field.
Not sure about Servers, but could not see any separate field.

[israel-hdez]

Servers don't have status in its own column. The status and the availability are both stored in the properties column/hash.

Note that MiddlewareDomain is storing the following data in the properties hash:

Suspend State: 
Running Mode: NORMAL
Home Directory: "/opt/jboss/wildfly"
Version: 10.1.0.Final
Server State: 
Product Name: WildFly Full
Host State: running
Is Domain Controller: 'true'
UUID: 2dc495bf-5c2c-4074-8487-023ca7427fae
Name: master

Note that there is a Host State property. Is this what you want?

Also, be warned that the Host State most likely is reported by inventory and may not be the current status. Usually, you'll want to check the availability metric of the domain host (like the case of the servers).

[xeviknal]

thanks for that @israel-hdez and @josejulio.

Regarding your question, I don't exactly know which attribute of the hash is the one that says the availability of the domain. I guess it is, yes, the Host State. Could you please confirm that?

Regarding to the last sentence, I guess you mean that I should add logic to AvailabilityUpdates parser/collector in order to have current domain availability. My approach, thanks to @josejulio, here
Is that what you meant?

[josejulio]

@xeviknal I think we are missing this part: https://github.com/ManageIQ/manageiq-providers-hawkular/blob/master/app/models/manageiq/providers/hawkular/inventory/parser/middleware_manager.rb#L177-L198

[xeviknal]

@josejulio don't you thing that properties for each domain are already stored before (MiddlewareManager#fetch_domains_with_servers), thus, the availability is already stored.
Here the stacktrace:
MiddlewareManager#L14: fetch domains with servers.
MiddlewareManager#L62: parsing middleware domain.
MiddlewareManager#L312: mapping hawkular item to inventory_object
MiddlewareManager#L359: map :ems_ref, :nativeid, :feed, :properties.

Then, probably the lines you sent, are not needed in this case. Anything to say @israel-hdez ?

[josejulio]

@josejulio don't you thing that properties for each domain are already stored before (MiddlewareManager#fetch_domains_with_servers), thus, the availability is already stored.
Here the stacktrace:
MiddlewareManager#L14: fetch domains with servers.
MiddlewareManager#L62: parsing middleware domain.
MiddlewareManager#L312: mapping hawkular item to inventory_object
MiddlewareManager#L359: map :ems_ref, :nativeid, :feed, :properties.

IIRC that fetches the data/properties from the Inventory. As @israel-hdez pointed out Host State is reported on a given refresher and could be outdated.
We fetch the availability metric for the given resource (server or deployment) and would need to do the same for the domain.

I have checked the default configuration, and I don't see any avail metric for the domain itself(only for the domain servers). I suppose we might need to add one.

@israel-hdez, Can you please correct me if I'm wrong?

[israel-hdez]

@josejulio @xeviknal Yes, Host State is reported by inventory and usually is not updated when the domain goes down.
Hmm... Yes, there is no avail metric in the config for the domain. Perhaps you could try querying Hawkular to see if, by any chances, it's there?

BTW, I checked the changes in @xeviknal branch. The availability_changes parser/collector/persister are there for use in the event catcher. So, you are missing some changes here: https://github.com/ManageIQ/manageiq-providers-hawkular/blob/master/app/models/manageiq/providers/hawkular/middleware_manager/event_catcher/stream.rb#L49

And also, the changes that @josejulio said are also required so that availabilities are collected when a refresh is done.

[abonas]

@miq-bot add_label providers/middleware

[xeviknal]

Okay, I am closing this PR since there is no need to add a new column to domain. Availability is going to be stored on MiddlewareDomain#properties.