* Dimitris Aragiorgis <[email protected]> [2013-11-07 10:20:02 +0200]: > Hi! > > * Dimitris Aragiorgis <[email protected]> [2013-11-01 18:01:11 +0200]: > > > * 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. > > > > Any news on that? >
Sorry for asking again but is there any blocking reason for these two patches not to get merged? Thanks, dimara > > 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
