* Guido Trotter <[email protected]> [2013-11-01 15:57:07 +0100]: > As long assigning node or the cluster IP is still disallowed ok. Is it > possible for the admin to both specify reserved and externally reserved > IPS? It'd be great if it didn't become too simple to make mistakes. Also if > this series could come with extra tests and documentation updates perhaps > it might be less problematic to have the change in behaviour. >
As far as I can see currently one can assing the master ip to a VM via --net ip=... and as a result the VerifyConfig() will complain but ganeti will not abort. With this patch, in order to assign the master ip (that is externally reserved) to a VM we would additionally need to pass --no-conflicts-check option. In this sense it is even harder than before to assign the master's IP to a VM, thus making it not so simple to make mistakes. As far as the documentation is concerned I think I already changed the corresponding parts. I changed a test as well, that was failing silently before. Additionally in case we tweak cluster/node related opcodes to change those reserved IPs in master, this patch does not affect the execution workflow at all. Thanks, dimara > Thanks, > > Guido > On 1 Nov 2013 15:24, "Dimitris Aragiorgis" <[email protected]> wrote: > > > 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 <[email protected]> > > --- > > doc/design-network.rst | 23 +++++++++++++++-------- > > lib/cmdlib/instance.py | 6 ++++-- > > lib/cmdlib/network.py | 10 ++-------- > > lib/config.py | 9 ++++++--- > > lib/network.py | 25 +++++++++++++++++++------ > > man/gnt-instance.rst | 3 +++ > > test/py/cmdlib/testsupport/config_mock.py | 4 ++++ > > 7 files changed, 53 insertions(+), 27 deletions(-) > > > > diff --git a/doc/design-network.rst b/doc/design-network.rst > > index 837924c..8b52f62 100644 > > --- a/doc/design-network.rst > > +++ b/doc/design-network.rst > > @@ -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 > > diff --git a/lib/cmdlib/instance.py b/lib/cmdlib/instance.py > > index 3dee0e3..7c739d5 100644 > > --- a/lib/cmdlib/instance.py > > +++ b/lib/cmdlib/instance.py > > @@ -1039,7 +1039,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" % > > @@ -2624,7 +2625,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: > > diff --git a/lib/cmdlib/network.py b/lib/cmdlib/network.py > > index 01bd948..8bfd0b4 100644 > > --- a/lib/cmdlib/network.py > > +++ b/lib/cmdlib/network.py > > @@ -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) > > > > diff --git a/lib/config.py b/lib/config.py > > index 1f07d29..56e4c96 100644 > > --- a/lib/config.py > > +++ b/lib/config.py > > @@ -384,7 +384,7 @@ class ConfigWriter(object): > > _, 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(object): > > 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): > > diff --git a/lib/network.py b/lib/network.py > > index d78b717..291b0a8 100644 > > --- a/lib/network.py > > +++ b/lib/network.py > > @@ -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): > > diff --git a/man/gnt-instance.rst b/man/gnt-instance.rst > > index d48adb1..c133a72 100644 > > --- a/man/gnt-instance.rst > > +++ b/man/gnt-instance.rst > > @@ -135,6 +135,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 > > diff --git a/test/py/cmdlib/testsupport/config_mock.py > > b/test/py/cmdlib/testsupport/config_mock.py > > index 3c8812d..4579cb9 100644 > > --- a/test/py/cmdlib/testsupport/config_mock.py > > +++ b/test/py/cmdlib/testsupport/config_mock.py > > @@ -28,6 +28,7 @@ import uuid as uuid_module > > from ganeti import config > > from ganeti import constants > > from ganeti import objects > > +from ganeti.network import AddressPool > > > > import mocks > > > > @@ -507,6 +508,9 @@ class ConfigMock(config.ConfigWriter): > > if mac is None: > > mac = "aa:00:00:aa:%02x:%02x" % (nic_id / 0xff, nic_id % 0xff) > > if isinstance(network, objects.Network): > > + if ip: > > + pool = AddressPool(network) > > + pool.Reserve(ip) > > network = network.uuid > > if nicparams is None: > > nicparams = {} > > -- > > 1.7.10.4 > > > >
signature.asc
Description: Digital signature
