lxd icon indicating copy to clipboard operation
lxd copied to clipboard

Stopping instances during LXD shutdown when no database available doesn't work

Open tomponline opened this issue 3 years ago • 0 comments

When a LXD server is shutdown using lxd shutdown one of the steps in the shutdown process is to attempt to cleanly shutdown any of the instances running on that server.

In a clustered environment it is possible that the server has no DB access (perhaps this is the last member of the cluster to be shutdown and the cluster DB no longer has quorum).

In this situation there is functionality in LXD that attempts to discover the instances running on the local server without using the database so that they can still be shutdown cleanly.

Unfortunately there are numerous problems with the current implementation:

  • The instancesOnDisk function did not cater for VM instances until recently, unfortunately because this function only returns instance names and not instance.Instance records it is not possible to differentiate between containers and VMs, and thus it is not possible to actually stop VMs - they will be created as containers.
  • When the instance names from instancesOnDisk are used in instancesShutdown they are created as containers (https://github.com/lxc/lxd/blob/2a036b5dcae85022ac43171aa0cd604421d7703f/lxd/instances.go#L366-L385) but they will not have their instance.id property populated (because they didn't get loaded from the database). This means the instance operationlock functionality that creates locks using the instance.id property to control the shutdown of an instance will always ID 0 for the lock. This causes all the instances stopping to race each other using the same lock, which causes both unexpected delays and shutdown device cleanup stages running before the instance is actually shutdown.
  • The temporary instance.Instance record used to initiate shutdown doesn't container the device config, so the devices are not cleaned up.
  • The container and VM stop hooks rely on loading the instance from the database and so these will fail too.
  • During normal shutdown, the instancesShutdown function wipes the current power state for all members in a cluster, rather than just the server member shutting down.
  • In instancesShutdown the instance Shutdown and Stop functions are always called rather than inspecting the return value of Shutdown and only calling Stop if needed.
  • patchUpdateFromV11 and patchUpdateFromV15 rely on instancesOnDisk function, however they seem to expect this function to return snapshots, which it doesn't, so that needs reviewing to check correct behaviour of the patches.

Proposed approach to fixing this:

  • [x] Move operationlock to use project and instance name rather than instance ID - they are still unique per server (which is what is required) but can be extracted from disk backup.yaml file.
  • [x] Update instancesOnDisk to return a slice of instance.Instance correctly populated with the instance type and the parsed contents of the backup.yaml file (if the instance is running, which is the only ones we care about).
  • [x] Review and update container and VM driver start up routines to ensure that the backup.yaml file is written to disk just before the instance process is started, but after all devices have been initialised (so that the volatile keys are fresh).
  • [x] Update container and VM stop hooks to also load the instance from DB from but fallback to backup.yaml on error, that way if they are being stopped when the DB isn't available its 'normal' to get config from disk.
  • [x] Stop wiping all instance last power state across the cluster on a single server stopping.
  • [x] Review patches that rely on accessing instances on disk (patchUpdateFromV11 and patchUpdateFromV15) to make sure they work with the refactored instancesOnDisk function.
  • [ ] Update instance.Instance and storage layer to load the instance's root disk config and make accessing via instance.Instance so that when stopping without DB, the storage disk config can be re-populated from the backup.yaml. This will allow unmounting of volumes that required specific disk config to succeed.
  • [ ] Add tests and an internal API route for triggering offline instance shutdown to prevent future regressions of this functionality.

tomponline avatar Oct 04 '21 08:10 tomponline