yahoo-eng-team team mailing list archive

[OpenStack Infra <1549984@xxxxxxxxxxxxxxxxxx>]

Reviewed:  https://review.openstack.org/291847
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=66e79e1bbca66a6f0f7be14006a87426c4dda402
Submitter: Jenkins
Branch:    master

commit 66e79e1bbca66a6f0f7be14006a87426c4dda402
Author: Jay Pipes <jaypipes@xxxxxxxxx>
Date:   Fri Mar 11 13:16:40 2016 -0500

    pci - Claim devices outside of Claim constructor
    
    During the nova.compute.Claim.__init__() call, there are a bunch of
    _test_XXX() methods that are called. These methods should test to see
    whether the requested resources of various types can be satisfied by the
    inventory on the ComputeNode. However, that inventory *should not* be
    claimed for a particular request during the Claim object's constructor.
    
    The Claim._test_pci() method was *actually* claiming the PCI device for
    the requested instance. Unfortunately, this meant that if an instance
    launch request's demand for a resource like RAM was not able to be
    satisfied by the compute node but the launch request's demand for PCI
    devices *was* able to be satisfied by the compute node, those PCI
    devices were actually claimed for the instance even though the claim
    itself would end up being aborted. This resulted in a data
    corruption/inconsistency where a PCI device would be claimed for an
    instance that actually was not running on the node.
    
    This patch moves the claim of PCI devices out of the _test_pci() method
    and into the ResourceTracker.instance_claim() method. In the process of
    fixing this bug, it was discovered that the unit tests for the Claim
    object with regards to PCI devices were just plain broken. They were
    testing for nothing at all because of the way the Claim constructor
    works. These unit tests were reworked completely, along with the
    MoveClaim unit tests which similarly were not testing the PCI code paths
    at all. An additional unit test was added on the resource tracker to
    verify that nova.pci.manager.PciDevTracker.claim_instance() is called
    when PCI requests are included and satisfied by the Claim.
    
    Change-Id: Icf75439a552de84ec31c1a47faeee3caf8a5b0a7
    Closes-bug: #1549984


** Changed in: nova
       Status: In Progress => Fix Released

-- 
You received this bug notification because you are a member of Yahoo!
Engineering Team, which is subscribed to OpenStack Compute (nova).
https://bugs.launchpad.net/bugs/1549984

Title:
  PCI devices claimed on compute node during _claim_test()

Status in OpenStack Compute (nova):
  Fix Released

Bug description:
  The nova.compute.claims.Claim object is used to test whether a set of
  requested resources can be satisfied by the compute node. In the
  constructor of the Claim object, the Claim._claim_test() object is
  called:

  
      def __init__(self, context, instance, tracker, resources, overhead=None,
                   limits=None):
          super(Claim, self).__init__()
          <snip>
          # Check claim at constructor to avoid mess code
          # Raise exception ComputeResourcesUnavailable if claim failed
          self._claim_test(resources, limits)

  If we take a look at _claim_test(), we see pretty clearly that
  resources are NOT supposed to be actually claimed -- instead, the
  method should only *check* to see if the request can be fulfilled:

  
      def _claim_test(self, resources, limits=None):
          """Test if this claim can be satisfied given available resources and
          optional oversubscription limits

          This should be called before the compute node actually consumes the
          resources required to execute the claim.

          :param resources: available local compute node resources
          :returns: Return true if resources are available to claim.
          """
         <snip>
          reasons = [self._test_memory(resources, memory_mb_limit),
                     self._test_disk(resources, disk_gb_limit),
                     self._test_vcpus(resources, vcpus_limit),
                     self._test_numa_topology(resources, numa_topology_limit),
                     self._test_pci()]
          reasons = reasons + self._test_ext_resources(limits)
          reasons = [r for r in reasons if r is not None]
          if len(reasons) > 0:
              raise exception.ComputeResourcesUnavailable(reason=
                      "; ".join(reasons))

  Unfortunately, the PCI devices are *actually* claimed in the
  _test_pci() method:

      def _test_pci(self):
          pci_requests = objects.InstancePCIRequests.get_by_instance_uuid(
              self.context, self.instance.uuid)

          if pci_requests.requests:
              devs = self.tracker.pci_tracker.claim_instance(self.context,
                                                             self.instance)
              if not devs:
                  return _('Claim pci failed.')

  What this means is that if an instance is attempted to be launched on
  a compute node and that instance has PCI requests that can be
  satisfied by the compute host, but say, there isn't enough available
  RAM on the node, the Claim will raise ComputeResourcesUnavailable
  which will trigger a Retry operation to the scheduler, but the PCI
  devices will have already been marked as claimed by that instance in
  the PCI device tracker:

              devs = self.tracker.pci_tracker.claim_instance(self.context,
                                                             self.instance)

  The above code actually marks one or more PCI devices on the compute
  host as claimed for the instance. This introduces inconsistent state
  into the system. Making things worse is the fact that the
  nova.pci.manager.PciDevTracker object uses the
  nova.pci.stats.PciDevStats object for tracking consumed quantities of
  "pools" of the PCI device types and both the stats aggregation AND the
  PciDevTracker.pci_devs PciDeviceList object have their state changed
  improperly.

To manage notifications about this bug go to:
https://bugs.launchpad.net/nova/+bug/1549984/+subscriptions