(2.10) Allow instances to obtain externally reserved IPs
authorDimitris Aragiorgis <dimara@grnet.gr>
Fri, 1 Nov 2013 14:24:38 +0000 (16:24 +0200)
committerDimitris Aragiorgis <dimara@grnet.gr>
Tue, 17 Dec 2013 10:46:42 +0000 (12:46 +0200)
The administrator should be able to assign an externally reserved IP
to a Ganeti instance manually, if desired. Currently this is not
supported. External reservations should act as holes in the pool and
not just as IPs already used by someone outside of Ganeti.
Automatic allocation with ip=pool will continue to exclude those IPs
as happens now.

To allow such functionality the administrator needs to pass explicitly
the desired IP along with the ``--no-conflicts-check`` option, or else
an error will be produced as happens now.

The aforementioned require the following changes:

 - Make IsReserved() to look either in reservations or external ones.
 - Make Reserve() and Release() to use IsReserved() with external
   argument True or False.
 - Pass extra option to ReserveIp() to bypass checking of external reservations
 - Update man pages and design doc for this change.

Furthermore, a side effect of this patch is that it fixes the
following problem:
Currently, one could not mark an IP as external if it was already
reserved (i.e. belonged to an instance). The code would produce a warning
and fail silently.

Fix config_mock.py so that if network and ip is given then reserve it in
the pool.

Signed-off-by: Dimitris Aragiorgis <dimara@grnet.gr>
Signed-off-by: Thomas Thrainer <thomasth@google.com>
Reviewed-by: Thomas Thrainer <thomasth@google.com>

Conflicts:
test/py/cmdlib/testsupport/config_mock.py

Signed-off-by: Dimitris Aragiorgis <dimara@grnet.gr>

doc/design-network.rst
lib/cmdlib/instance.py
lib/cmdlib/network.py
lib/config.py
lib/network.py
man/gnt-instance.rst

index 837924c..8b52f62 100644 (file)
@@ -88,21 +88,24 @@ give IP pool management capabilities. A network's pool is defined by two
 bitfields, the length of the network size each:
 
 ``reservations``
-  This field holds all IP addresses reserved by Ganeti instances, as
-  well as cluster IP addresses (node addresses + cluster master)
+  This field holds all IP addresses reserved by Ganeti instances.
 
 ``external reservations``
   This field holds all IP addresses that are manually reserved by the
-  administrator, because some other equipment is using them outside the
-  scope of Ganeti.
+  administrator (external gateway, IPs of external servers, etc) or
+  automatically by ganeti (the network/broadcast addresses,
+  Cluster IPs (node addresses + cluster master)). These IPs are excluded
+  from the IP pool and cannot be assigned automatically by ganeti to
+  instances (via ip=pool).
 
 The bitfields are implemented using the python-bitarray package for
 space efficiency and their binary value stored base64-encoded for JSON
 compatibility. This approach gives relatively compact representations
 even for large IPv4 networks (e.g. /20).
 
-Ganeti-owned IP addresses (node + master IPs) are reserved automatically
-if the cluster's data network itself is placed under pool management.
+Cluster IP addresses (node + master IPs) are reserved automatically
+as external if the cluster's data network itself is placed under
+pool management.
 
 Helper ConfigWriter methods provide free IP address generation and
 reservation, using a TemporaryReservationManager.
@@ -129,10 +132,14 @@ node-specific underlying infrastructure.
 
 We also introduce a new ``ip`` address value, ``constants.NIC_IP_POOL``,
 that specifies that a given NIC's IP address should be obtained using
-the IP address pool of the specified network. This value is only valid
+the first available IP address inside the pool of the specified network.
+(reservations OR external_reservations). This value is only valid
 for NICs belonging to a network. A NIC's IP address can also be
 specified manually, as long as it is contained in the network the NIC
-is connected to.
+is connected to. In case this IP is externally reserved, Ganeti will produce
+an error which the user can override if explicitly requested. Of course
+this IP will be reserved and will not be able to be assigned to another
+instance.
 
 
 Hooks
index 756ef59..ac88bf6 100644 (file)
@@ -1041,7 +1041,8 @@ class LUInstanceCreate(LogicalUnit):
             self.LogInfo("Chose IP %s from network %s", nic.ip, nobj.name)
           else:
             try:
-              self.cfg.ReserveIp(net_uuid, nic.ip, self.proc.GetECId())
+              self.cfg.ReserveIp(net_uuid, nic.ip, self.proc.GetECId(),
+                                 check=self.op.conflicts_check)
             except errors.ReservationError:
               raise errors.OpPrereqError("IP address %s already in use"
                                          " or does not belong to network %s" %
@@ -2586,7 +2587,8 @@ class LUInstanceSetParams(LogicalUnit):
         # Reserve new IP if in the new network if any
         elif new_net_uuid:
           try:
-            self.cfg.ReserveIp(new_net_uuid, new_ip, self.proc.GetECId())
+            self.cfg.ReserveIp(new_net_uuid, new_ip, self.proc.GetECId(),
+                               check=self.op.conflicts_check)
             self.LogInfo("Reserving IP %s in network %s",
                          new_ip, new_net_obj.name)
           except errors.ReservationError:
index 32e3ff7..605978b 100644 (file)
@@ -365,10 +365,7 @@ class LUNetworkSetParams(LogicalUnit):
     if self.op.add_reserved_ips:
       for ip in self.op.add_reserved_ips:
         try:
-          if self.pool.IsReserved(ip):
-            self.LogWarning("IP address %s is already reserved", ip)
-          else:
-            self.pool.Reserve(ip, external=True)
+          self.pool.Reserve(ip, external=True)
         except errors.AddressPoolError, err:
           self.LogWarning("Cannot reserve IP address %s: %s", ip, err)
 
@@ -378,10 +375,7 @@ class LUNetworkSetParams(LogicalUnit):
           self.LogWarning("Cannot unreserve Gateway's IP")
           continue
         try:
-          if not self.pool.IsReserved(ip):
-            self.LogWarning("IP address %s is already unreserved", ip)
-          else:
-            self.pool.Release(ip, external=True)
+          self.pool.Release(ip, external=True)
         except errors.AddressPoolError, err:
           self.LogWarning("Cannot release IP address %s: %s", ip, err)
 
index ad99684..a3cd9fd 100644 (file)
@@ -384,7 +384,7 @@ class ConfigWriter:
     _, address, _ = self._temporary_ips.Generate([], gen_one, ec_id)
     return address
 
-  def _UnlockedReserveIp(self, net_uuid, address, ec_id):
+  def _UnlockedReserveIp(self, net_uuid, address, ec_id, check=True):
     """Reserve a given IPv4 address for use by an instance.
 
     """
@@ -392,22 +392,25 @@ class ConfigWriter:
     pool = network.AddressPool(nobj)
     try:
       isreserved = pool.IsReserved(address)
+      isextreserved = pool.IsReserved(address, external=True)
     except errors.AddressPoolError:
       raise errors.ReservationError("IP address not in network")
     if isreserved:
       raise errors.ReservationError("IP address already in use")
+    if check and isextreserved:
+      raise errors.ReservationError("IP is externally reserved")
 
     return self._temporary_ips.Reserve(ec_id,
                                        (constants.RESERVE_ACTION,
                                         address, net_uuid))
 
   @locking.ssynchronized(_config_lock, shared=1)
-  def ReserveIp(self, net_uuid, address, ec_id):
+  def ReserveIp(self, net_uuid, address, ec_id, check=True):
     """Reserve a given IPv4 address for use by an instance.
 
     """
     if net_uuid:
-      return self._UnlockedReserveIp(net_uuid, address, ec_id)
+      return self._UnlockedReserveIp(net_uuid, address, ec_id, check)
 
   @locking.ssynchronized(_config_lock, shared=1)
   def ReserveLV(self, lv_name, ec_id):
index 0356476..8059e31 100644 (file)
@@ -153,8 +153,6 @@ class AddressPool(object):
   def Validate(self):
     assert len(self.reservations) == self._GetSize()
     assert len(self.ext_reservations) == self._GetSize()
-    all_res = self.reservations & self.ext_reservations
-    assert not all_res.any()
 
     if self.gateway is not None:
       assert self.gateway in self.network
@@ -188,25 +186,40 @@ class AddressPool(object):
     """
     return self.all_reservations.to01().replace("1", "X").replace("0", ".")
 
-  def IsReserved(self, address):
+  def IsReserved(self, address, external=False):
     """Checks if the given IP is reserved.
 
     """
     idx = self._GetAddrIndex(address)
-    return self.all_reservations[idx]
+    if external:
+      return self.ext_reservations[idx]
+    else:
+      return self.reservations[idx]
 
   def Reserve(self, address, external=False):
     """Mark an address as used.
 
     """
-    if self.IsReserved(address):
-      raise errors.AddressPoolError("%s is already reserved" % address)
+    if self.IsReserved(address, external):
+      if external:
+        msg = "IP %s is already externally reserved" % address
+      else:
+        msg = "IP %s is already used by an instance" % address
+      raise errors.AddressPoolError(msg)
+
     self._Mark(address, external=external)
 
   def Release(self, address, external=False):
     """Release a given address reservation.
 
     """
+    if not self.IsReserved(address, external):
+      if external:
+        msg = "IP %s is not externally reserved" % address
+      else:
+        msg = "IP %s is not used by an instance" % address
+      raise errors.AddressPoolError(msg)
+
     self._Mark(address, value=False, external=external)
 
   def GetFreeAddress(self):
index 401decc..4d61d35 100644 (file)
@@ -132,6 +132,9 @@ ip
     connected to the said network. ``--no-conflicts-check`` can be used
     to override this check. The special value **pool** causes Ganeti to
     select an IP from the the network the NIC is or will be connected to.
+    One can pick an externally reserved IP of a network along with
+    ``--no-conflict-check``. Note that this IP cannot be assigned to
+    any other instance until it gets released.
 
 mode
     specifies the connection mode for this NIC: routed, bridged or