On Mon, Nov 18, 2013 at 2:00 PM, Guido Trotter <[email protected]> wrote:

> On Fri, Nov 15, 2013 at 3:55 PM, Dimitris Aragiorgis <[email protected]>
> wrote:
> > * Guido Trotter <[email protected]> [2013-11-15 14:13:01 +0100]:
> >
> >> On Fri, Nov 1, 2013 at 4:11 PM, Dimitris Aragiorgis <[email protected]>
> wrote:
> >> > * Guido Trotter <[email protected]> [2013-11-01 15:57:07 +0100]:
> >> >
> >> >> Shouldn't they be actually reserved, but removed on node
> >> >> removal/modification or cluster modify?
> >> >>
> >> >
> >> > Makes sense but this is a rather big patch that should probably go to
> >> > master. I can give it a try for master, but I think we should merge
> this
> >> > patch to stable-2.10 to fix the aforeamentioned problem and make the
> >> > use case manageable.
> >> >
> >>
> >> But then would this mean that in 2.10 the node ips are externally
> >> reserved "as long as you don't change them", but can become unreserved
> >> if you do? In that case I believe this to be a bug anyway.
> >>
> >
> > Well currently you get a kinda "inconsistent" network; you see X's that
> > don't respond neither to instances nor to external IPs. And the worst
> > is that once they get reserved are unmanageable. IMHO this small patch
> > makes the use case of cluster renumbering easier for the admin plus
> > it provides a more consistent overview of the IP pool. Either way
> > I don't think this patch is blocking the other one from merging, right?
> >
>
> Ack, but please file a bug for the other issue and let's target it for
> 2.11-ish.
> I'll leave to thomas as the 2.10 release manager the decision whether
> these two patches can get into 2.10 or should be pushed to master.
>
>
As far as I have understood, these patches strictly improve the current
situation and are pretty non-intrusive, so I'm fine with integrating them
in 2.10.
But as Guido mentioned, removing or adding nodes etc. is still an issue, so
it would be great if this can be addressed in 2.11.

So, LGTM (with an issue tracking the remaining issues),

Thanks a lot for the contribution,
Thomas


> Thanks,
>
> Guido
>
>
> > Thanks,
> > dimara
> >
> >> Thanks,
> >>
> >> Guido
> >>
> >>
> >>
> >> > Thanks,
> >> > dimara
> >> >
> >> >> Thanks,
> >> >>
> >> >> Guido
> >> >> On 1 Nov 2013 15:24, "Dimitris Aragiorgis" <[email protected]> wrote:
> >> >>
> >> >> > Currently, upon network creation, nodes' and master's IPs
> >> >> > are reserved in the pool.
> >> >> >
> >> >> > This leads to pool reservations ('X' in map) that cannot be changed
> >> >> > afterwards, although they may need to (e.g. in case of node
> >> >> > removal/modification). Also, these IPs are not actually handled by
> >> >> > Ganeti in the sense that they are not assigned to the cluster's
> VMs.
> >> >> >
> >> >> > Thus, we should mark the as externally reserved and not just
> reserved.
> >> >> >
> >> >> > Signed-off-by: Dimitris Aragiorgis <[email protected]>
> >> >> > ---
> >> >> >  lib/cmdlib/network.py |    4 ++--
> >> >> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >> >> >
> >> >> > diff --git a/lib/cmdlib/network.py b/lib/cmdlib/network.py
> >> >> > index 4f172a4..01bd948 100644
> >> >> > --- a/lib/cmdlib/network.py
> >> >> > +++ b/lib/cmdlib/network.py
> >> >> > @@ -169,7 +169,7 @@ class LUNetworkAdd(LogicalUnit):
> >> >> >          for ip in [node.primary_ip, node.secondary_ip]:
> >> >> >            try:
> >> >> >              if pool.Contains(ip):
> >> >> > -              pool.Reserve(ip)
> >> >> > +              pool.Reserve(ip, external=True)
> >> >> >                self.LogInfo("Reserved IP address of node '%s'
> (%s)",
> >> >> >                             node.name, ip)
> >> >> >            except errors.AddressPoolError, err:
> >> >> > @@ -179,7 +179,7 @@ class LUNetworkAdd(LogicalUnit):
> >> >> >        master_ip = self.cfg.GetClusterInfo().master_ip
> >> >> >        try:
> >> >> >          if pool.Contains(master_ip):
> >> >> > -          pool.Reserve(master_ip)
> >> >> > +          pool.Reserve(master_ip, external=True)
> >> >> >            self.LogInfo("Reserved cluster master IP address (%s)",
> >> >> > master_ip)
> >> >> >        except errors.AddressPoolError, err:
> >> >> >          self.LogWarning("Cannot reserve cluster master IP address
> (%s):
> >> >> > %s",
> >> >> > --
> >> >> > 1.7.10.4
> >> >> >
> >> >> >
> >> >
> >> > -----BEGIN PGP SIGNATURE-----
> >> > Version: GnuPG v1.4.10 (GNU/Linux)
> >> >
> >> > iQEcBAEBAgAGBQJSc8SaAAoJEHFDHex6CBG9w7QH/R7UEaVnaVShi505rpYPllFX
> >> > 4btFfeWwtXN31WBB371/aaKD58w7F6Sf7MxTzfUF1D/VlSpasOOboLhVtFUxlUEl
> >> > 77wRochfepfkanCcym1dParjJNGCSKSPIXVlG2AcF+32FDl5Jhz0jmAYk7OPDDXO
> >> > KFPth2QyxE7dsmub/9laivm2tQ/fK/Uq4qD51ct8jwmCqJRkSo38fYwCE9UKemft
> >> > 1D3mc3X5E+HqwZM/Yssmga7KRm3aS+dinPiknDTxRsIFIP4j8Va9u/XEeMmbRQSq
> >> > EqU06fdFuHMxfofMCrpv1E44pROMkX+KvSZ3BMlejpOPoPdCu/zYH8qot8W0HWQ=
> >> > =5XYj
> >> > -----END PGP SIGNATURE-----
> >> >
> >>
> >>
> >>
> >> --
> >> Guido Trotter
> >> Ganeti Engineering
> >> Google Germany GmbH
> >> Dienerstr. 12, 80331, München
> >>
> >> Registergericht und -nummer: Hamburg, HRB 86891
> >> Sitz der Gesellschaft: Hamburg
> >> Geschäftsführer: Graham Law, Christine Elizabeth Flores
> >> Steuernummer: 48/725/00206
> >> Umsatzsteueridentifikationsnummer: DE813741370
> >
> > -----BEGIN PGP SIGNATURE-----
> > Version: GnuPG v1.4.10 (GNU/Linux)
> >
> > iQEcBAEBAgAGBQJShjXpAAoJEHFDHex6CBG9d/cH/R7dm98oYGo7Ud/xTUMN7Q7X
> > KJ5WgPkXBF03zavyS5tV4e8ike5xskZTSidJ1RtP3UIWKmqE0v/ZZ3qir46evTnK
> > cg58ofaJrsLDGYGxJ5chKuXPUtdI85E3Rj9aMD7QjcjtUTlGA2+JkzG06tWIT7/I
> > JqdLm45egwNUt51I428hZVzXw94MDzm6pMzrdRRS3vQ/l9vBB9mHEJPOdSBz4ToI
> > wU7KMvdSZZs4Pbtt9V4QtCwDn2ekIzUhx1y9Ik53zxSMFTtszCss56K3VRiHMWbw
> > 8rzcVHzJ18Mg39MttmHNklZuF7i+daIV8/qyetvMqo+G+uwByPj97G+oCZafxaY=
> > =CSRJ
> > -----END PGP SIGNATURE-----
> >
>
>
>
> --
> Guido Trotter
> Ganeti Engineering
> Google Germany GmbH
> Dienerstr. 12, 80331, München
>
> Registergericht und -nummer: Hamburg, HRB 86891
> Sitz der Gesellschaft: Hamburg
> Geschäftsführer: Graham Law, Christine Elizabeth Flores
> Steuernummer: 48/725/00206
> Umsatzsteueridentifikationsnummer: DE813741370
>



-- 
Thomas Thrainer | Software Engineer | [email protected] |

Google Germany GmbH
Dienerstr. 12
80331 München

Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Graham Law, Christine Elizabeth Flores

Reply via email to