Sbd dump gatherer

[nelsonkopliku]

Follows up #74 and enables porting of checks

• B089BE SBD watchdog timeout is set to expected value
• 68626E SBD msgwait timeout value is two times the watchdog timeout

A little sanitization of the keys has been applied to simplify usage of the result at expression level (ie Timeout (watchdog) becomes timeout_watchdog)

facts:
  - name: sbd_devices_dump
    gatherer: sbd_dump

expectations:
  # B089BE
  - name: sbd_watchdog_timeout_is_correct
    expect: facts.sbd_devices_dump.values().all(|device| device.timeout_watchdog == values.expected_watchdog_timeout)

  # 68626E
  - name: sbd_msgwait_timeout_is_double_of_watchdog_timeout
    expect: facts.sbd_devices_dump.values().all(|device| device.timeout_msgwait == 2 * device.timeout_watchdog)

Doc added here trento-project/wanda#137

[rtorrero]

Hey @nelsonkopliku ! thanks for this, after some thought I think you took the right path by changing the gatherer to use data also from the SBDconfig (instead of the mess that it would have been to implement the dependency as I originally suggested)

Btw, is the output right?

I did:

^^/W/t/agent >>> ./trento-agent facts gather --gatherer sbd_dump                                                                                                                                                                                   ^(sbd-dump-gatherer+154) 15:06:06 
INFO[2022-12-19 15:06:31] Using config file: /home/rtorrero/.config/trento/agent.yaml 
INFO[2022-12-19 15:06:31] loading plugins                              
INFO[2022-12-19 15:06:31] Starting sbd_dump facts gathering process    
INFO[2022-12-19 15:06:31] Starting SBD config Facts gathering          
INFO[2022-12-19 15:06:31] Requested sbd_dump facts gathered            
INFO[2022-12-19 15:06:31] Gathered fact for "sbd_dump" with argument "": 
INFO[2022-12-19 15:06:31] {
  "Name": "",
  "CheckID": "",
  "Value": {
    "Value": {
      "/dev/vdb": {
      ...

Why do we have a nested Value?

Apart from that, consider in a future PR adding support for retrieving specific keys using arguments (rather than the whole map). We have done this in other gatherers and I think it might be a good standard to adopt for gatherers in general.

[nelsonkopliku]

@rtorrero thanks for the review.

Why do we have a nested Value?

That's the internal representation of different types of values that can be returned as facts, and that's suppported by a bit of polymorphism here https://github.com/trento-project/agent/blob/main/pkg/factsengine/entities/fact_value.go

[arbulu89]

I find the usage of other gatherers dependency a big smell.
I would get the devices in a plain way, using the cluster.LoadSbdConfig function and getting the key from it.
Less dependency and leaner code.
Besides that, some minor nitpick

[rtorrero]

@rtorrero thanks for the review.

Why do we have a nested Value?

That's the internal representation of different types of values that can be returned as facts, and that's suppported by a bit of polymorphism here https://github.com/trento-project/agent/blob/main/pkg/factsengine/entities/fact_value.go

true, we already discussed this, vacations finished frying the last working part of my brain 😄

[arbulu89]

Hey @nelsonkopliku ,
Thank you for the changes. The code looks much better now in my opinion.
Have a look on the quoted thing in the SBD_DEVICE I commented. If that works, we are ready to go

[arbulu89]

@nelsonkopliku
I was not expecting the test this way. I would expect a new test in the gatherer itself, as using the LoadSbdConfig funtion is a internal tech detail.
Anyway, I see that it works, so feel free to merge if you want.