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