On Tue, Aug 27, 2013 at 2:52 PM, Thomas Thrainer <[email protected]>wrote:
> > > > On Tue, Aug 27, 2013 at 1:21 PM, Helga Velroyen <[email protected]> wrote: > >> >> >> >> On Mon, Aug 26, 2013 at 4:31 PM, Thomas Thrainer <[email protected]>wrote: >> >>> >>> >>> >>> On Thu, Aug 22, 2013 at 10:16 AM, Helga Velroyen <[email protected]>wrote: >>> >>>> This adjusts and extends the QA for 'gnt-cluster modify' >>>> with respect to the changes regarding the DRBD >>>> usermode helper. >>>> >>>> Signed-off-by: Helga Velroyen <[email protected]> >>>> --- >>>> qa/qa_cluster.py | 78 >>>> ++++++++++++++++++++++++++++++++++++++------------------ >>>> 1 file changed, 53 insertions(+), 25 deletions(-) >>>> >>>> diff --git a/qa/qa_cluster.py b/qa/qa_cluster.py >>>> index 0d051f9..500155d 100644 >>>> --- a/qa/qa_cluster.py >>>> +++ b/qa/qa_cluster.py >>>> @@ -553,8 +553,8 @@ def TestClusterModifyDiskTemplates(): >>>> enabled_disk_templates = qa_config.GetEnabledDiskTemplates() >>>> default_disk_template = qa_config.GetDefaultDiskTemplate() >>>> >>>> - _TestClusterModifyDiskTemplatesArguments(default_disk_template, >>>> - enabled_disk_templates) >>>> + _TestClusterModifyDiskTemplatesArguments(default_disk_template) >>>> + _TestClusterModifyDiskTemplatesDrbdHelper(enabled_disk_templates) >>>> _TestClusterModifyDiskTemplatesVgName(enabled_disk_templates) >>>> >>>> _RestoreEnabledDiskTemplates() >>>> @@ -591,8 +591,57 @@ def _RestoreEnabledDiskTemplates(): >>>> AssertCommand(cmd, fail=False) >>>> >>>> >>>> -def _TestClusterModifyDiskTemplatesArguments(default_disk_template, >>>> - enabled_disk_templates): >>>> +def _TestClusterModifyDiskTemplatesDrbdHelper(enabled_disk_templates): >>>> + """Tests argument handling of 'gnt-cluster modify' with respect to >>>> + the parameter '--drbd-usermode-helper'. This test is independent >>>> + of instances. >>>> + >>>> + """ >>>> + _RestoreEnabledDiskTemplates() >>>> + >>>> + if constants.DT_DRBD8 not in enabled_disk_templates: >>>> + return >>>> + if constants.DT_PLAIN not in enabled_disk_templates: >>>> + return >>>> + >>>> + drbd_usermode_helper = qa_config.get("drbd-usermode-helper", >>>> "/bin/true") >>>> + bogus_usermode_helper = "/tmp/pinkbunny" >>>> + for command, fail in \ >>>> + [(["gnt-cluster", "modify", >>>> + "--enabled-disk-templates=%s" % constants.DT_DRBD8, >>>> + "--ipolicy-disk-templates=%s" % constants.DT_DRBD8], False), >>>> + (["gnt-cluster", "modify", >>>> + "--drbd-usermode-helper=%s" % drbd_usermode_helper], False), >>>> + (["gnt-cluster", "modify", >>>> + "--drbd-usermode-helper=%s" % bogus_usermode_helper], True), >>>> + # unsetting helper when DRBD is enabled should not work >>>> + (["gnt-cluster", "modify", >>>> + "--drbd-usermode-helper="], True), >>>> + (["gnt-cluster", "modify", >>>> + "--enabled-disk-templates=%s" % constants.DT_PLAIN, >>>> + "--ipolicy-disk-templates=%s" % constants.DT_PLAIN], False), >>>> + (["gnt-cluster", "modify", >>>> + "--drbd-usermode-helper="], True), >>>> + (["gnt-cluster", "modify", >>>> + "--drbd-usermode-helper=%s" % drbd_usermode_helper], False), >>>> + (["gnt-cluster", "modify", >>>> + "--drbd-usermode-helper=%s" % drbd_usermode_helper, >>>> + "--enabled-disk-templates=%s" % constants.DT_DRBD8, >>>> + "--ipolicy-disk-templates=%s" % constants.DT_DRBD8], False), >>>> + (["gnt-cluster", "modify", >>>> + "--drbd-usermode-helper=", >>>> + "--enabled-disk-templates=%s" % constants.DT_PLAIN, >>>> + "--ipolicy-disk-templates=%s" % constants.DT_PLAIN], False), >>>> + (["gnt-cluster", "modify", >>>> + "--drbd-usermode-helper=%s" % drbd_usermode_helper, >>>> + "--enabled-disk-templates=%s" % constants.DT_DRBD8, >>>> + "--ipolicy-disk-templates=%s" % constants.DT_DRBD8], False), >>>> + ]: >>>> >>> >>> Could we skip some of those tests because there are already unit tests >>> for them? >>> >> >> Hm, actually I found a couple of bugs during development that weren't >> caught by the unit tests because they were due to the interface between >> gnt_cluster -> cmdlib and gnt_cluster -> bootstrap. My suggestion would be >> to make those tests configurable with a separate qa config key so that we >> can enable them only in the full QA. What do you think? >> > > If they make sense, I would keep them. At some point in time it would be > cool though to convert all those "throw invalid parameters at Ganeti and > see if exceptions are raised" tests to unit tests and remove them from QA. > > This would probably require unit tests for bootstrap, which might or might > not be difficult... > > OTOH, cluster init is a somewhat special case. I guess for a lot of other > cases, where no second flow (gnt_cluster -> bootstrap) exists, we could > already remove QA tests. > I agree. I suggest to assess the 'unecessary QA coverage' once the unit test coverage for bootstrap is high enough. > > >> >> >>> >>> >>>> + AssertCommand(command, fail=fail) >>>> + _RestoreEnabledDiskTemplates() >>>> + >>>> + >>>> +def _TestClusterModifyDiskTemplatesArguments(default_disk_template): >>>> """Tests argument handling of 'gnt-cluster modify' with respect to >>>> the parameter '--enabled-disk-templates'. This test is independent >>>> of instances. >>>> @@ -613,27 +662,6 @@ def >>>> _TestClusterModifyDiskTemplatesArguments(default_disk_template, >>>> "--ipolicy-disk-templates=%s" % default_disk_template], >>>> fail=False) >>>> >>>> - if constants.DT_DRBD8 in enabled_disk_templates: >>>> - # interaction with --drbd-usermode-helper option >>>> - drbd_usermode_helper = qa_config.get("drbd-usermode-helper", None) >>>> - if not drbd_usermode_helper: >>>> - drbd_usermode_helper = "/bin/true" >>>> - # specifying a helper when drbd gets disabled is ok. Note that >>>> drbd still >>>> - # has to be installed on the nodes in this case >>>> - AssertCommand(["gnt-cluster", "modify", >>>> - "--drbd-usermode-helper=%s" % drbd_usermode_helper, >>>> - "--enabled-disk-templates=%s" % >>>> constants.DT_DISKLESS, >>>> - "--ipolicy-disk-templates=%s" % >>>> constants.DT_DISKLESS], >>>> - fail=False) >>>> - # specifying a helper when drbd is re-enabled >>>> - AssertCommand(["gnt-cluster", "modify", >>>> - "--drbd-usermode-helper=%s" % drbd_usermode_helper, >>>> - "--enabled-disk-templates=%s" % >>>> - ",".join(enabled_disk_templates), >>>> - "--ipolicy-disk-templates=%s" % >>>> - ",".join(enabled_disk_templates)], >>>> - fail=False) >>>> - >>>> >>>> def _TestClusterModifyDiskTemplatesVgName(enabled_disk_templates): >>>> """Tests argument handling of 'gnt-cluster modify' with respect to >>>> -- >>>> 1.8.3 >>>> >>>> >>> Rest LGTM, thanks. >>> >> >> Cheers, >> Helga >> >> > > > -- > 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 >
