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? > > 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? > + > 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 > + 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
