neoave / mrack

Multicloud use-case based multihost async provisioner for CIs and testing during development
Apache License 2.0
12 stars 14 forks source link

fix: Creating inventory with None host (re-running traceback) #177

Closed dav-pascual closed 2 years ago

dav-pascual commented 2 years ago

The code in create_invetory method was looping hostnames from the DB, which some of them may have already been deleted, which causes a None return when trying to get their metadata (get_host_from_metadata).

This fixes traceback caused by rerunning mrack up with a different host name without previously deleting mrackdb.json

dav-pascual commented 2 years ago

The solution I came up with (the simplest) is checking None return from get_host_from_metadata function. This should prevent further traceback, when not deleting mrackdb.json between reruns.

An alternative to this solution would be to check if the status of the host is set to active, or using success_host list.

pvoborni commented 2 years ago

Hi, I didn't test it. But conceptually the approach looks OK to me - we can go with it.

Just some thoughts: it is more a band-aid then a fix of root cause. E.g., we can easily face it in the future again somewhere else when this fix is "forgotten". I like the idea of success_host list, mainly from the perspective that it can serve as a basis for new features. But that is not needed now and might be better to do with the new features.

dav-pascual commented 2 years ago

Thanks. Let's merge this quick patch for now, with a view on implementing a more robust solution regarding the cleaning of previous runs.