LGTM, thanks

On Fri, May 30, 2014 at 1:11 PM, Jose A. Lopes <[email protected]> wrote:

> On May 30 11:43, Hrvoje Ribicic wrote:
> > On Wed, May 28, 2014 at 12:37 PM, 'Jose A. Lopes' via ganeti-devel <
> > [email protected]> wrote:
> >
> > > ... which keeps track of who last changed the instance field
> > > 'admin_state', namely, the admin or the user.
> > >
> > > Signed-off-by: Jose A. Lopes <[email protected]>
> > > ---
> > >  lib/cmdlib/instance.py         |  1 +
> > >  lib/objects.py                 |  3 +++
> > >  src/Ganeti/Objects.hs          | 25 +++++++++++++------------
> > >  test/hs/Test/Ganeti/OpCodes.hs |  5 +++--
> > >  tools/cfgupgrade               | 10 ++++++++++
> > >  5 files changed, 30 insertions(+), 14 deletions(-)
> > >
> > > diff --git a/lib/cmdlib/instance.py b/lib/cmdlib/instance.py
> > > index fbb5939..91330d2 100644
> > > --- a/lib/cmdlib/instance.py
> > > +++ b/lib/cmdlib/instance.py
> > > @@ -1326,6 +1326,7 @@ class LUInstanceCreate(LogicalUnit):
> > >                              disk_template=self.op.disk_template,
> > >                              disks_active=False,
> > >                              admin_state=constants.ADMINST_DOWN,
> > > +                            admin_state_source=constants.ADMIN_SOURCE,
> > >                              network_port=network_port,
> > >                              beparams=self.op.beparams,
> > >                              hvparams=self.op.hvparams,
> > > diff --git a/lib/objects.py b/lib/objects.py
> > > index 00a9467..125d1e5 100644
> > > --- a/lib/objects.py
> > > +++ b/lib/objects.py
> > > @@ -1074,6 +1074,7 @@ class Instance(TaggableObject):
> > >      "beparams",
> > >      "osparams",
> > >      "admin_state",
> > > +    "admin_state_source",
> > >      "nics",
> > >      "disks",
> > >      "disk_template",
> > > @@ -1237,6 +1238,8 @@ class Instance(TaggableObject):
> > >      """Fill defaults for missing configuration values.
> > >
> > >      """
> > > +    if self.admin_state_source is None:
> > > +      self.admin_state_source = constants.ADMIN_SOURCE
> > >      for nic in self.nics:
> > >        nic.UpgradeConfig()
> > >      for disk in self.disks:
> > > diff --git a/src/Ganeti/Objects.hs b/src/Ganeti/Objects.hs
> > > index 5d9aebc..18ca1f5 100644
> > > --- a/src/Ganeti/Objects.hs
> > > +++ b/src/Ganeti/Objects.hs
> > > @@ -439,18 +439,19 @@ $(buildParam "Be" "bep"
> > >    ])
> > >
> > >  $(buildObject "Instance" "inst" $
> > > -  [ simpleField "name"           [t| String             |]
> > > -  , simpleField "primary_node"   [t| String             |]
> > > -  , simpleField "os"             [t| String             |]
> > > -  , simpleField "hypervisor"     [t| Hypervisor         |]
> > > -  , simpleField "hvparams"       [t| HvParams           |]
> > > -  , simpleField "beparams"       [t| PartialBeParams    |]
> > > -  , simpleField "osparams"       [t| OsParams           |]
> > > -  , simpleField "admin_state"    [t| AdminState         |]
> > > -  , simpleField "nics"           [t| [PartialNic]       |]
> > > -  , simpleField "disks"          [t| [Disk]             |]
> > > -  , simpleField "disk_template"  [t| DiskTemplate       |]
> > > -  , simpleField "disks_active"   [t| Bool               |]
> > > +  [ simpleField "name"               [t| String             |]
> > > +  , simpleField "primary_node"       [t| String             |]
> > > +  , simpleField "os"                 [t| String             |]
> > > +  , simpleField "hypervisor"         [t| Hypervisor         |]
> > > +  , simpleField "hvparams"           [t| HvParams           |]
> > > +  , simpleField "beparams"           [t| PartialBeParams    |]
> > > +  , simpleField "osparams"           [t| OsParams           |]
> > > +  , simpleField "admin_state"        [t| AdminState         |]
> > > +  , simpleField "admin_state_source" [t| AdminStateSource   |]
> > > +  , simpleField "nics"               [t| [PartialNic]       |]
> > > +  , simpleField "disks"              [t| [Disk]             |]
> > > +  , simpleField "disk_template"      [t| DiskTemplate       |]
> > > +  , simpleField "disks_active"       [t| Bool               |]
> > >    , optionalField $ simpleField "network_port" [t| Int  |]
> > >    ]
> > >    ++ timeStampFields
> > > diff --git a/test/hs/Test/Ganeti/OpCodes.hs
> > > b/test/hs/Test/Ganeti/OpCodes.hs
> > > index 5129f7f..84c60f3 100644
> > > --- a/test/hs/Test/Ganeti/OpCodes.hs
> > > +++ b/test/hs/Test/Ganeti/OpCodes.hs
> > > @@ -42,10 +42,11 @@ import qualified Data.Map as Map
> > >  import qualified Text.JSON as J
> > >  import Text.Printf (printf)
> > >
> > > +import Test.Ganeti.Objects ()
> > > +import Test.Ganeti.Query.Language ()
> > >  import Test.Ganeti.TestHelper
> > >  import Test.Ganeti.TestCommon
> > >  import Test.Ganeti.Types ()
> > > -import Test.Ganeti.Query.Language ()
> > >
> > >  import Ganeti.BasicTypes
> > >  import qualified Ganeti.Constants as C
> > > @@ -260,7 +261,7 @@ instance Arbitrary OpCodes.OpCode where
> > >            pure emptyJSObject <*> arbitrary <*> arbitrary
> > >        "OP_INSTANCE_SHUTDOWN" ->
> > >          OpCodes.OpInstanceShutdown <$> genFQDN <*> return Nothing <*>
> > > -          arbitrary <*> arbitrary <*> arbitrary <*> arbitrary
> > > +          arbitrary <*> arbitrary <*> arbitrary <*> arbitrary <*>
> > > arbitrary
> > >        "OP_INSTANCE_REBOOT" ->
> > >          OpCodes.OpInstanceReboot <$> genFQDN <*> return Nothing <*>
> > >            arbitrary <*> arbitrary <*> arbitrary
> > > diff --git a/tools/cfgupgrade b/tools/cfgupgrade
> > > index e045515..2862f1d 100755
> > > --- a/tools/cfgupgrade
> > > +++ b/tools/cfgupgrade
> > > @@ -250,6 +250,9 @@ def UpgradeInstances(config_data):
> > >        if not "spindles" in dobj:
> > >          missing_spindles = True
> > >
> > > +    if "admin_state_source" not in iobj:
> > > +      iobj["admin_state_source"] = "admin"
> > >
> >
> > Why not use the constant here?
>
> diff --git a/tools/cfgupgrade b/tools/cfgupgrade
> index 2862f1d..654d193 100755
> --- a/tools/cfgupgrade
> +++ b/tools/cfgupgrade
> @@ -251,7 +251,7 @@ def UpgradeInstances(config_data):
>          missing_spindles = True
>
>      if "admin_state_source" not in iobj:
> -      iobj["admin_state_source"] = "admin"
> +      iobj["admin_state_source"] = constants.ADMIN_SOURCE
>
>    if GetExclusiveStorageValue(config_data) and missing_spindles:
>      # We cannot be sure that the instances that are missing spindles have
>
> >
> > > +
> > >    if GetExclusiveStorageValue(config_data) and missing_spindles:
> > >      # We cannot be sure that the instances that are missing spindles
> have
> > >      # exclusive storage enabled (the check would be more
> complicated), so
> > > we
> > > @@ -401,6 +404,11 @@ def UpgradeAll(config_data):
> > >
> > >  # DOWNGRADE
> ------------------------------------------------------------
> > >
> > > +def DowngradeInstances(config_data):
> > > +  for _, iobj in config_data["instances"].items():
> > > +    if "admin_state_source" in iobj:
> > > +      del iobj["admin_state_source"]
> > > +
> > >
> > >  def DowngradeCluster(config_data):
> > >    cluster = config_data.get("cluster", None)
> > > @@ -417,6 +425,8 @@ def DowngradeCluster(config_data):
> > >    if "max_running_jobs" in cluster:
> > >      del cluster["max_running_jobs"]
> > >
> > > +  DowngradeInstances(config_data)
> > > +
> > >
> > >  def DowngradeGroups(config_data):
> > >    for group in config_data["nodegroups"].values():
> > > --
> > > 1.9.1.423.g4596e3a
> > >
> > >
>
> --
> Jose Antonio Lopes
> 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
>

Reply via email to