Fix storage domain issue when attaching LUN

[sbernhard]

Thanks @Manisha15

[sbernhard]

Workaround for https://projects.theforeman.org/issues/30199

Review would be nice @orrabin / @ShimShtein

[sbernhard]

Thoughts on this @ShimShtein ?

[sbernhard]

ping @ShimShtein :-)

[ShimShtein]

I think we don't want to push 0 as the storage domain id, if storage_domains returned nil or and empty list. If we don't use the safe navigation operator here, we will end up in the rescue block (which is not desired, I think)

I can understand that in the case of an actual error, we can assume there is at least one domain, but we can't get the list, so we assume there is only one.

[sbernhard]

I think we don't want to push 0 as the storage domain id, if storage_domains returned nil or and empty list. If we don't use the safe navigation operator here, we will end up in the rescue block (which is not desired, I think)

I can understand that in the case of an actual error, we can assume there is at least one domain, but we can't get the list, so we assume there is only one.

This would then not work. The original issue was:

So, attachment_disk.storage_domains is nil and the access to "[0]" crashes. In this case, and only in this case it should return with "0" which would then work in case of direct attached LUN disks.

Its still better to return 0 instead of crashing.

[ShimShtein]

Didn't test it, but now the code looks good.

I see three cases here:

1. storage_domains returns one or more domains (the "happy" path), then the storage_domain property would be set to the first domain from the list
2. storage_domains returns an empty list, then the storage_domain property should be set to nil (we didn't find a good candidate).
3. storage_domains throws an exception, then we want to guess the storage domain id. We are setting the storage_domain property to 0, since it's our best guess.

[dosas]

@sbernhard There were quite some changes on the default branch. Could you rebase to enable the CI?

[sbernhard]

@ShimShtein I had a look at the code again.

1. in case of there is 'attachment_disk_storage_domains' first entry with an id => use this id
2. in case of there is 'attachment_disk_storage_domains' first entry but no id field is found => nil should be returned
3. in case of NO 'attachment_disk_storage_domains' first entry, fallback to 0

This is what https://bugzilla.redhat.com/show_bug.cgi?id=1882404#c9 suggestes and works for us.

[sbernhard]

@ShimShtein can you have a look at this again, please?

[dosas]

@ShimShtein Do you think you could merge this? We do not have the necessary permissions.