On May 30 13:06, Hrvoje Ribicic wrote:
> On Wed, May 28, 2014 at 12:37 PM, 'Jose A. Lopes' via ganeti-devel <
> [email protected]> wrote:
>
> > Fix instance queries to report USER_down when the instance is not
> > running but the admin state is ADMINST_down and the admin state source
> > is the user.
> >
> > * In instance info, fix 'hvparams' to get the dict filled with the
> > cluster defaults and fix the reported user down state.
> >
> > * In instance list (in Python and Haskell),
> >
> > Fix instance list (in Python and Haskell) to report USER_down in the same
> >
>
> Stray or incomplete sentence?
Fix instance queries to correctly report USER_down
* Fix instance queries (gnt-instance list and info) in Python and
Haskell to report 'USER_down' when the instance is not running but
the admin state is 'ADMINST_down' and the admin state source is the
user.
* Fix 'hvparams' to get the dict filled with the cluster defaults.
>
>
> >
> > Signed-off-by: Jose A. Lopes <[email protected]>
> > ---
> > lib/cmdlib/instance_query.py | 22 ++++++++++-----
> > lib/query.py | 64
> > +++++++++++++++++++++++++++++---------------
> > src/Ganeti/Query/Instance.hs | 33 +++++++++++++++--------
> > 3 files changed, 79 insertions(+), 40 deletions(-)
> >
> > diff --git a/lib/cmdlib/instance_query.py b/lib/cmdlib/instance_query.py
> > index 36ae190..a5fab51 100644
> > --- a/lib/cmdlib/instance_query.py
> > +++ b/lib/cmdlib/instance_query.py
> > @@ -28,7 +28,7 @@ from ganeti import constants
> > from ganeti import locking
> > from ganeti.cmdlib.base import NoHooksLU
> > from ganeti.cmdlib.common import ShareAll, GetWantedInstances, \
> > - CheckInstancesNodeGroups, AnnotateDiskParams, GetUpdatedParams
> > + CheckInstancesNodeGroups, AnnotateDiskParams
> > from ganeti.cmdlib.instance_utils import NICListToTuple
> > from ganeti.hypervisor import hv_base
> >
> > @@ -215,6 +215,7 @@ class LUInstanceQueryData(NoHooksLU):
> >
> > for instance in self.wanted_instances:
> > pnode = nodes[instance.primary_node]
> > + hvparams = cluster.FillHV(instance, skip_globals=True)
> >
> > if self.op.static or pnode.offline:
> > remote_state = None
> > @@ -228,12 +229,14 @@ class LUInstanceQueryData(NoHooksLU):
> > cluster.hvparams[instance.hypervisor])
> > remote_info.Raise("Error checking node %s" % pnode.name)
> > remote_info = remote_info.payload
> > +
> > + allow_userdown = \
> > + instance.hypervisor != constants.HT_KVM or \
> > + hvparams[constants.HV_KVM_USER_SHUTDOWN]
> >
>
> Only Xen and KVM support this at the moment - could the if reflect that?
As discussed offline, whether the hypevisor supports userdown does not
change this code.
>
> > +
> > if remote_info and "state" in remote_info:
> > - hvparams = GetUpdatedParams(instance.hvparams,
> > -
> > cluster.hvparams[instance.hypervisor])
> > if hv_base.HvInstanceState.IsShutdown(remote_info["state"]):
> > - if (instance.hypervisor != constants.HT_KVM
> > - or hvparams[constants.HV_KVM_USER_SHUTDOWN]):
> > + if allow_userdown:
> > remote_state = "user down"
> > else:
> > remote_state = "down"
> > @@ -242,8 +245,13 @@ class LUInstanceQueryData(NoHooksLU):
> > else:
> > if instance.admin_state == constants.ADMINST_UP:
> > remote_state = "down"
> > + elif instance.admin_state == constants.ADMINST_DOWN:
> > + if instance.admin_state_source == constants.USER_SOURCE:
> > + remote_state = "user down"
> > + else:
> > + remote_state = "down"
> > else:
> > - remote_state = instance.admin_state
> > + remote_state = "offline"
> >
> > group2name_fn = lambda uuid: groups[uuid].name
> > node_uuid2name_fn = lambda uuid: nodes[uuid].name
> > @@ -273,7 +281,7 @@ class LUInstanceQueryData(NoHooksLU):
> > "hypervisor": instance.hypervisor,
> > "network_port": instance.network_port,
> > "hv_instance": instance.hvparams,
> > - "hv_actual": cluster.FillHV(instance, skip_globals=True),
> > + "hv_actual": hvparams,
> > "be_instance": instance.beparams,
> > "be_actual": cluster.FillBE(instance),
> > "os_instance": instance.osparams,
> > diff --git a/lib/query.py b/lib/query.py
> > index f3fc1f0..e42a862 100644
> > --- a/lib/query.py
> > +++ b/lib/query.py
> > @@ -1524,6 +1524,42 @@ def _GetInstLiveData(name):
> > return fn
> >
> >
> > +def _GetLiveInstStatus(ctx, instance, instance_state):
> > + hvparams = ctx.cluster.FillHV(instance, skip_globals=True)
> > +
> > + allow_userdown = \
> > + instance.hypervisor != constants.HT_KVM or \
> > + hvparams[constants.HV_KVM_USER_SHUTDOWN]
> > +
> > + if instance.uuid in ctx.wrongnode_inst:
> > + return constants.INSTST_WRONGNODE
> > + else:
> > + if hv_base.HvInstanceState.IsShutdown(instance_state):
> > + if instance.admin_state == constants.ADMINST_UP and allow_userdown:
> > + return constants.INSTST_USERDOWN
> > + elif instance.admin_state == constants.ADMINST_UP:
> > + return constants.INSTST_ERRORDOWN
> > + else:
> > + return constants.INSTST_ADMINDOWN
> > + else:
> > + if instance.admin_state == constants.ADMINST_UP:
> > + return constants.INSTST_RUNNING
> > + else:
> > + return constants.INSTST_ERRORUP
> > +
> > +
> > +def _GetDeadInstStatus(inst):
> > + if inst.admin_state == constants.ADMINST_UP:
> > + return constants.INSTST_ERRORDOWN
> > + elif inst.admin_state == constants.ADMINST_DOWN:
> > + if inst.admin_state_source == constants.USER_SOURCE:
> > + return constants.INSTST_USERDOWN
> > + else:
> > + return constants.INSTST_ADMINDOWN
> > + else:
> > + return constants.INSTST_ADMINOFFLINE
> > +
> > +
> > def _GetInstStatus(ctx, inst):
> > """Get instance status.
> >
> > @@ -1541,28 +1577,9 @@ def _GetInstStatus(ctx, inst):
> > instance_live_data = ctx.live_data.get(inst.uuid)
> >
> > if bool(instance_live_data):
> > - instance_state = instance_live_data["state"]
> > -
> > - if inst.uuid in ctx.wrongnode_inst:
> > - return constants.INSTST_WRONGNODE
> > - else:
> > - if hv_base.HvInstanceState.IsShutdown(instance_state):
> > - if inst.admin_state == constants.ADMINST_UP:
> > - return constants.INSTST_USERDOWN
> > - else:
> > - return constants.INSTST_ADMINDOWN
> > - else:
> > - if inst.admin_state == constants.ADMINST_UP:
> > - return constants.INSTST_RUNNING
> > - else:
> > - return constants.INSTST_ERRORUP
> > -
> > - if inst.admin_state == constants.ADMINST_UP:
> > - return constants.INSTST_ERRORDOWN
> > - elif inst.admin_state == constants.ADMINST_DOWN:
> > - return constants.INSTST_ADMINDOWN
> > -
> > - return constants.INSTST_ADMINOFFLINE
> > + return _GetLiveInstStatus(ctx, inst, instance_live_data["state"])
> > + else:
> > + return _GetDeadInstStatus(inst)
> >
> >
> > def _GetInstDisk(index, cb):
> > @@ -2183,6 +2200,9 @@ def _BuildInstanceFields():
> > (_MakeField("admin_state", "InstanceState", QFT_TEXT,
> > "Desired state of instance"),
> > IQ_CONFIG, 0, _GetItemAttr("admin_state")),
> > + (_MakeField("admin_state_source", "InstanceStateSource", QFT_TEXT,
> > + "Who last changed the desired state of instance"),
> >
>
> Nitpick: "of the instance" below, should be the same here
diff --git a/lib/query.py b/lib/query.py
index e42a862..515e979 100644
--- a/lib/query.py
+++ b/lib/query.py
@@ -2198,16 +2198,16 @@ def _BuildInstanceFields():
lambda ctx, inst: map(compat.partial(_GetInstNodeGroup, ctx, None),
inst.secondary_nodes)),
(_MakeField("admin_state", "InstanceState", QFT_TEXT,
- "Desired state of instance"),
+ "Desired state of the instance"),
IQ_CONFIG, 0, _GetItemAttr("admin_state")),
- (_MakeField("admin_state_source", "InstanceStateSource", QFT_TEXT,
- "Who last changed the desired state of instance"),
- IQ_CONFIG, 0, _GetItemAttr("admin_state_source")),
(_MakeField("admin_up", "Autostart", QFT_BOOL,
- "Desired state of instance"),
+ "Desired state of the instance"),
IQ_CONFIG, 0, lambda ctx, inst: inst.admin_state == constants.ADMINST_UP),
+ (_MakeField("admin_state_source", "InstanceStateSource", QFT_TEXT,
+ "Who last changed the desired state of the instance"),
+ IQ_CONFIG, 0, _GetItemAttr("admin_state_source")),
(_MakeField("disks_active", "DisksActive", QFT_BOOL,
- "Desired state of instance disks"),
+ "Desired state of the instance disks"),
IQ_CONFIG, 0, _GetItemAttr("disks_active")),
(_MakeField("tags", "Tags", QFT_OTHER, "Tags"), IQ_CONFIG, 0,
lambda ctx, inst: list(inst.GetTags())),
>
> > + IQ_CONFIG, 0, _GetItemAttr("admin_state_source")),
> > (_MakeField("admin_up", "Autostart", QFT_BOOL,
> > "Desired state of instance"),
> > IQ_CONFIG, 0, lambda ctx, inst: inst.admin_state ==
> > constants.ADMINST_UP),
> > diff --git a/src/Ganeti/Query/Instance.hs b/src/Ganeti/Query/Instance.hs
> > index 856bbca..e40b81f 100644
> > --- a/src/Ganeti/Query/Instance.hs
> > +++ b/src/Ganeti/Query/Instance.hs
> > @@ -100,6 +100,10 @@ instanceFields =
> > [ (FieldDefinition "admin_state" "InstanceState" QFTText
> > "Desired state of instance",
> > FieldSimple (rsNormal . adminStateToRaw . instAdminState), QffNormal)
> > + , (FieldDefinition "admin_state_source" "InstanceStateSource" QFTText
> > + "Who last changed the desired state of the instance",
> > + FieldSimple (rsNormal . adminStateSourceToRaw .
> > instAdminStateSource),
> > + QffNormal)
> > , (FieldDefinition "admin_up" "Autostart" QFTBool
> > "Desired state of instance",
> > FieldSimple (rsNormal . (== AdminUp) . instAdminState), QffNormal)
> > @@ -621,20 +625,26 @@ isPrimaryOffline cfg inst =
> > Bad _ -> error "Programmer error - result assumed to be OK is
> > Bad!"
> >
> > -- | Determines the status of a live instance
> > -liveInstanceStatus :: (InstanceInfo, Bool) -> Instance -> InstanceStatus
> > -liveInstanceStatus (instInfo, foundOnPrimary) inst
> > +liveInstanceStatus :: ConfigData
> > + -> (InstanceInfo, Bool)
> > + -> Instance
> > + -> InstanceStatus
> > +liveInstanceStatus cfg (instInfo, foundOnPrimary) inst
> > | not foundOnPrimary = WrongNode
> > | otherwise =
> > case instanceState of
> > - InstanceStateRunning | adminState == AdminUp -> Running
> > - | otherwise -> ErrorUp
> > - InstanceStateShutdown | adminState == AdminUp && allowDown ->
> > UserDown
> > - | adminState == AdminUp -> ErrorDown
> > - | otherwise -> StatusDown
> > + InstanceStateRunning
> > + | adminState == AdminUp -> Running
> > + | otherwise -> ErrorUp
> > + InstanceStateShutdown
> > + | adminState == AdminUp && allowDown -> UserDown
> > + | adminState == AdminUp -> ErrorDown
> > + | otherwise -> StatusDown
> > where adminState = instAdminState inst
> > instanceState = instInfoState instInfo
> >
> > - hvparams = fromContainer $ instHvparams inst
> > + hvparams =
> > + fromContainer $ getFilledInstHvParams (C.toList C.hvcGlobals)
> > cfg inst
> >
> > allowDown =
> > instHypervisor inst /= Kvm ||
> > @@ -645,8 +655,9 @@ liveInstanceStatus (instInfo, foundOnPrimary) inst
> > deadInstanceStatus :: Instance -> InstanceStatus
> > deadInstanceStatus inst =
> > case instAdminState inst of
> > - AdminUp -> ErrorDown
> > - AdminDown -> StatusDown
> > + AdminUp -> ErrorDown
> > + AdminDown | instAdminStateSource inst == UserSource -> UserDown
> > + | otherwise -> StatusDown
> > AdminOffline -> StatusOffline
> >
> > -- | Determines the status of the instance, depending on whether it is
> > possible
> > @@ -660,7 +671,7 @@ determineInstanceStatus cfg res inst
> > | isPrimaryOffline cfg inst = NodeOffline
> > | otherwise = case res of
> > Left _ -> NodeDown
> > - Right (Just liveData, _) -> liveInstanceStatus liveData inst
> > + Right (Just liveData, _) -> liveInstanceStatus cfg liveData inst
> > Right (Nothing, _) -> deadInstanceStatus inst
> >
> > -- | Extracts the instance status, retrieving it using the functions
> > above and
> > --
> > 1.9.1.423.g4596e3a
> >
> >
> Barring the nitpicks, LGTM, thanks
--
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
