From 8d3bcf972d9f6aa7e568e203f749cc878cd6fd34 Mon Sep 17 00:00:00 2001 From: Ivan Devat Date: Thu, 15 Jun 2017 11:46:12 +0200 Subject: [PATCH] make container type mandatory in "bundle create" --- pcs/cli/resource/parse_args.py | 4 +- pcs/cli/resource/test/test_parse_args.py | 32 +++++++------- pcs/pcs.8 | 4 +- pcs/test/cib_resource/test_bundle.py | 75 +++++++++++++++++++------------- pcs/test/cib_resource/test_create.py | 2 +- pcs/test/test_constraints.py | 2 +- pcs/test/test_resource.py | 2 +- pcs/usage.py | 3 +- 8 files changed, 67 insertions(+), 57 deletions(-) diff --git a/pcs/cli/resource/parse_args.py b/pcs/cli/resource/parse_args.py index 366acac..1bdcd5b 100644 --- a/pcs/cli/resource/parse_args.py +++ b/pcs/cli/resource/parse_args.py @@ -84,7 +84,7 @@ def _parse_bundle_groups(arg_list): def parse_bundle_create_options(arg_list): groups = _parse_bundle_groups(arg_list) container_options = groups.get("container", []) - container_type = None + container_type = "" if container_options and "=" not in container_options[0]: container_type = container_options.pop(0) parts = { @@ -101,8 +101,6 @@ def parse_bundle_create_options(arg_list): ], "meta": prepare_options(groups.get("meta", [])) } - if not parts["container_type"]: - parts["container_type"] = "docker" return parts def _split_bundle_map_update_op_and_options( diff --git a/pcs/cli/resource/test/test_parse_args.py b/pcs/cli/resource/test/test_parse_args.py index 0c936cc..791b60d 100644 --- a/pcs/cli/resource/test/test_parse_args.py +++ b/pcs/cli/resource/test/test_parse_args.py @@ -215,7 +215,7 @@ class ParseBundleCreateOptions(TestCase): self.assert_produce( [], { - "container_type": "docker", + "container_type": "", "container": {}, "network": {}, "port_map": [], @@ -229,9 +229,9 @@ class ParseBundleCreateOptions(TestCase): def test_container_type(self): self.assert_produce( - ["container", "lxc"], + ["container", "docker"], { - "container_type": "lxc", + "container_type": "docker", "container": {}, "network": {}, "port_map": [], @@ -244,7 +244,7 @@ class ParseBundleCreateOptions(TestCase): self.assert_produce( ["container", "a=b", "c=d"], { - "container_type": "docker", + "container_type": "", "container": {"a": "b", "c": "d"}, "network": {}, "port_map": [], @@ -255,9 +255,9 @@ class ParseBundleCreateOptions(TestCase): def test_container_type_and_options(self): self.assert_produce( - ["container", "lxc", "a=b", "c=d"], + ["container", "docker", "a=b", "c=d"], { - "container_type": "lxc", + "container_type": "docker", "container": {"a": "b", "c": "d"}, "network": {}, "port_map": [], @@ -279,7 +279,7 @@ class ParseBundleCreateOptions(TestCase): self.assert_produce( ["network", "a=b", "c=d"], { - "container_type": "docker", + "container_type": "", "container": {}, "network": {"a": "b", "c": "d"}, "port_map": [], @@ -309,7 +309,7 @@ class ParseBundleCreateOptions(TestCase): self.assert_produce( ["port-map", "a=b", "c=d"], { - "container_type": "docker", + "container_type": "", "container": {}, "network": {}, "port_map": [{"a": "b", "c": "d"}], @@ -322,7 +322,7 @@ class ParseBundleCreateOptions(TestCase): self.assert_produce( ["port-map", "a=b", "c=d", "port-map", "e=f"], { - "container_type": "docker", + "container_type": "", "container": {}, "network": {}, "port_map": [{"a": "b", "c": "d"}, {"e": "f"}], @@ -349,7 +349,7 @@ class ParseBundleCreateOptions(TestCase): self.assert_produce( ["storage-map", "a=b", "c=d"], { - "container_type": "docker", + "container_type": "", "container": {}, "network": {}, "port_map": [], @@ -362,7 +362,7 @@ class ParseBundleCreateOptions(TestCase): self.assert_produce( ["storage-map", "a=b", "c=d", "storage-map", "e=f"], { - "container_type": "docker", + "container_type": "", "container": {}, "network": {}, "port_map": [], @@ -381,7 +381,7 @@ class ParseBundleCreateOptions(TestCase): self.assert_produce( ["meta", "a=b", "c=d"], { - "container_type": "docker", + "container_type": "", "container": {}, "network": {}, "port_map": [], @@ -402,7 +402,7 @@ class ParseBundleCreateOptions(TestCase): def test_all(self): self.assert_produce( [ - "container", "lxc", "a=b", "c=d", + "container", "docker", "a=b", "c=d", "network", "e=f", "g=h", "port-map", "i=j", "k=l", "port-map", "m=n", "o=p", @@ -411,7 +411,7 @@ class ParseBundleCreateOptions(TestCase): "meta", "y=z", "A=B", ], { - "container_type": "lxc", + "container_type": "docker", "container": {"a": "b", "c": "d"}, "network": {"e": "f", "g": "h"}, "port_map": [{"i": "j", "k": "l"}, {"m": "n", "o": "p"}], @@ -427,7 +427,7 @@ class ParseBundleCreateOptions(TestCase): "meta", "y=z", "port-map", "i=j", "k=l", "network", "e=f", - "container", "lxc", "a=b", + "container", "docker", "a=b", "storage-map", "u=v", "w=x", "port-map", "m=n", "o=p", "meta", "A=B", @@ -435,7 +435,7 @@ class ParseBundleCreateOptions(TestCase): "container", "c=d", ], { - "container_type": "lxc", + "container_type": "docker", "container": {"a": "b", "c": "d"}, "network": {"e": "f", "g": "h"}, "port_map": [{"i": "j", "k": "l"}, {"m": "n", "o": "p"}], diff --git a/pcs/pcs.8 b/pcs/pcs.8 index 20b5c2e..27298a7 100644 --- a/pcs/pcs.8 +++ b/pcs/pcs.8 @@ -162,8 +162,8 @@ Remove the clone which contains the specified group or resource (the resource or master [] [options] [\fB\-\-wait\fR[=n]] Configure a resource or group as a multi\-state (master/slave) resource. If \fB\-\-wait\fR is specified, pcs will wait up to 'n' seconds for the operation to finish (including starting and promoting resource instances if appropriate) and then return 0 on success or 1 on error. If 'n' is not specified it defaults to 60 minutes. Note: to remove a master you must remove the resource/group it contains. .TP -bundle create [container [] ] [network ] [port\-map ]... [storage\-map ]... [meta ] [\fB\-\-disabled\fR] [\fB\-\-wait\fR[=n]] -Create a new bundle encapsulating no resources. The bundle can be used either as it is or a resource may be put into it at any time. If the container type is not specified, it defaults to 'docker'. If \fB\-\-disabled\fR is specified, the bundle is not started automatically. If \fB\-\-wait\fR is specified, pcs will wait up to 'n' seconds for the bundle to start and then return 0 on success or 1 on error. If 'n' is not specified it defaults to 60 minutes. +bundle create container [] [network ] [port\-map ]... [storage\-map ]... [meta ] [\fB\-\-disabled\fR] [\fB\-\-wait\fR[=n]] +Create a new bundle encapsulating no resources. The bundle can be used either as it is or a resource may be put into it at any time. If \fB\-\-disabled\fR is specified, the bundle is not started automatically. If \fB\-\-wait\fR is specified, pcs will wait up to 'n' seconds for the bundle to start and then return 0 on success or 1 on error. If 'n' is not specified it defaults to 60 minutes. .TP bundle update [container ] [network ] [port\-map (add ) | (remove ...)]... [storage\-map (add ) | (remove ...)]... [meta ] [\fB\-\-wait\fR[=n]] Add, remove or change options to specified bundle. If you wish to update a resource encapsulated in the bundle, use the 'pcs resource update' command instead and specify the resource id. If \fB\-\-wait\fR is specified, pcs will wait up to 'n' seconds for the operation to finish (including moving resources if appropriate) and then return 0 on success or 1 on error. If 'n' is not specified it defaults to 60 minutes. diff --git a/pcs/test/cib_resource/test_bundle.py b/pcs/test/cib_resource/test_bundle.py index 29e4339..50ea1df 100644 --- a/pcs/test/cib_resource/test_bundle.py +++ b/pcs/test/cib_resource/test_bundle.py @@ -41,7 +41,7 @@ class BundleCreateUpgradeCib(BundleCreateCommon): def test_success(self): self.assert_effect( - "resource bundle create B1 container image=pcs:test", + "resource bundle create B1 container docker image=pcs:test", """ @@ -59,7 +59,7 @@ class BundleCreate(BundleCreateCommon): def test_minimal(self): self.assert_effect( - "resource bundle create B1 container image=pcs:test", + "resource bundle create B1 container docker image=pcs:test", """ @@ -73,7 +73,8 @@ class BundleCreate(BundleCreateCommon): self.assert_effect( """ resource bundle create B1 - container replicas=4 replicas-per-host=2 run-command=/bin/true + container docker replicas=4 replicas-per-host=2 + run-command=/bin/true port-map port=1001 meta target-role=Stopped network control-port=12345 host-interface=eth0 host-netmask=24 @@ -171,15 +172,24 @@ class BundleCreate(BundleCreateCommon): stdout_start="\nUsage: pcs resource bundle create...\n" ) - def test_fail_when_missing_required(self): + def test_fail_when_missing_container_type(self): self.assert_pcs_fail_regardless_of_force( "resource bundle create B1", + "Error: '' is not a valid container type value, use docker\n" + ) + + def test_fail_when_missing_required(self): + self.assert_pcs_fail_regardless_of_force( + "resource bundle create B1 container docker", "Error: required container option 'image' is missing\n" ) def test_fail_on_unknown_option(self): self.assert_pcs_fail( - "resource bundle create B1 container image=pcs:test extra=option", + """ + resource bundle create B1 container docker image=pcs:test + extra=option + """, "Error: invalid container option 'extra', allowed options are: " "image, masters, network, options, replicas, replicas-per-host," " run-command, use --force to override\n" @@ -192,8 +202,8 @@ class BundleCreate(BundleCreateCommon): # supported by pacemaker and so the command fails. self.assert_pcs_fail( """ - resource bundle create B1 container image=pcs:test extra=option - --force + resource bundle create B1 container docker image=pcs:test + extra=option --force """ , stdout_start="Error: Unable to update cib\n" @@ -201,7 +211,7 @@ class BundleCreate(BundleCreateCommon): def test_more_errors(self): self.assert_pcs_fail_regardless_of_force( - "resource bundle create B#1 container replicas=x", + "resource bundle create B#1 container docker replicas=x", outdent( """\ Error: invalid bundle name 'B#1', '#' is not a valid character for a bundle name @@ -239,25 +249,25 @@ class BundleUpdate(BundleCreateCommon): def fixture_bundle(self, name): self.assert_pcs_success( - "resource bundle create {0} container image=pcs:test".format( + "resource bundle create {0} container docker image=pcs:test".format( name ) ) def fixture_bundle_complex(self, name): self.assert_pcs_success( - ( - "resource bundle create {0} " - "container image=pcs:test replicas=4 masters=2 " - "network control-port=12345 host-interface=eth0 host-netmask=24 " - "port-map internal-port=1000 port=2000 " - "port-map internal-port=1001 port=2001 " - "port-map internal-port=1002 port=2002 " - "storage-map source-dir=/tmp/docker1a target-dir=/tmp/docker1b " - "storage-map source-dir=/tmp/docker2a target-dir=/tmp/docker2b " - "storage-map source-dir=/tmp/docker3a target-dir=/tmp/docker3b " - "meta priority=15 resource-stickiness=100 is-managed=false " - ).format(name) + (""" + resource bundle create {0} + container docker image=pcs:test replicas=4 masters=2 + network control-port=12345 host-interface=eth0 host-netmask=24 + port-map internal-port=1000 port=2000 + port-map internal-port=1001 port=2001 + port-map internal-port=1002 port=2002 + storage-map source-dir=/tmp/docker1a target-dir=/tmp/docker1b + storage-map source-dir=/tmp/docker2a target-dir=/tmp/docker2b + storage-map source-dir=/tmp/docker3a target-dir=/tmp/docker3b + meta priority=15 resource-stickiness=100 is-managed=false + """).format(name) ) def test_fail_when_missing_args_1(self): @@ -415,7 +425,7 @@ class BundleShow(TestCase, AssertPcsMixin): def test_minimal(self): self.assert_pcs_success( - "resource bundle create B1 container image=pcs:test" + "resource bundle create B1 container docker image=pcs:test" ) self.assert_pcs_success("resource show B1", outdent( """\ @@ -428,7 +438,8 @@ class BundleShow(TestCase, AssertPcsMixin): self.assert_pcs_success( """ resource bundle create B1 - container image=pcs:test masters=2 replicas=4 options='a b c' + container docker image=pcs:test masters=2 replicas=4 + options='a b c' """ ) self.assert_pcs_success("resource show B1", outdent( @@ -442,7 +453,7 @@ class BundleShow(TestCase, AssertPcsMixin): self.assert_pcs_success( """ resource bundle create B1 - container image=pcs:test + container docker image=pcs:test network host-interface=eth0 host-netmask=24 control-port=12345 """ ) @@ -458,7 +469,7 @@ class BundleShow(TestCase, AssertPcsMixin): self.assert_pcs_success( """ resource bundle create B1 - container image=pcs:test + container docker image=pcs:test port-map id=B1-port-map-1001 internal-port=2002 port=2000 port-map range=3000-3300 """ @@ -477,7 +488,7 @@ class BundleShow(TestCase, AssertPcsMixin): self.assert_pcs_success( """ resource bundle create B1 - container image=pcs:test + container docker image=pcs:test storage-map source-dir=/tmp/docker1a target-dir=/tmp/docker1b storage-map id=my-storage-map source-dir=/tmp/docker2a target-dir=/tmp/docker2b @@ -494,9 +505,10 @@ class BundleShow(TestCase, AssertPcsMixin): )) def test_meta(self): - self.assert_pcs_success( - "resource bundle create B1 container image=pcs:test --disabled" - ) + self.assert_pcs_success(""" + resource bundle create B1 container docker image=pcs:test + --disabled + """) self.assert_pcs_success("resource show B1", outdent( # pylint:disable=trailing-whitespace """\ @@ -508,7 +520,7 @@ class BundleShow(TestCase, AssertPcsMixin): def test_resource(self): self.assert_pcs_success( - "resource bundle create B1 container image=pcs:test" + "resource bundle create B1 container docker image=pcs:test" ) self.assert_pcs_success( "resource create A ocf:pacemaker:Dummy bundle B1 --no-default-ops" @@ -526,7 +538,8 @@ class BundleShow(TestCase, AssertPcsMixin): self.assert_pcs_success( """ resource bundle create B1 - container image=pcs:test masters=2 replicas=4 options='a b c' + container docker image=pcs:test masters=2 replicas=4 + options='a b c' network host-interface=eth0 host-netmask=24 control-port=12345 port-map id=B1-port-map-1001 internal-port=2002 port=2000 port-map range=3000-3300 diff --git a/pcs/test/cib_resource/test_create.py b/pcs/test/cib_resource/test_create.py index 2adef5a..2492ba9 100644 --- a/pcs/test/cib_resource/test_create.py +++ b/pcs/test/cib_resource/test_create.py @@ -888,7 +888,7 @@ class Bundle(ResourceTest): def fixture_bundle(self, name): self.assert_pcs_success( - "resource bundle create {0} container image=pcs:test".format( + "resource bundle create {0} container docker image=pcs:test".format( name ) ) diff --git a/pcs/test/test_constraints.py b/pcs/test/test_constraints.py index 69d955d..4160b01 100644 --- a/pcs/test/test_constraints.py +++ b/pcs/test/test_constraints.py @@ -3246,7 +3246,7 @@ class Bundle(ConstraintEffect): def fixture_bundle(self, name): self.assert_pcs_success( - "resource bundle create {0} container image=pcs:test".format( + "resource bundle create {0} container docker image=pcs:test".format( name ) ) diff --git a/pcs/test/test_resource.py b/pcs/test/test_resource.py index 4bdc194..c015fa4 100644 --- a/pcs/test/test_resource.py +++ b/pcs/test/test_resource.py @@ -4710,7 +4710,7 @@ class BundleCommon( def fixture_bundle(self, name): self.assert_pcs_success( - "resource bundle create {0} container image=pcs:test".format( + "resource bundle create {0} container docker image=pcs:test".format( name ) ) diff --git a/pcs/usage.py b/pcs/usage.py index 75cb118..9cbf7de 100644 --- a/pcs/usage.py +++ b/pcs/usage.py @@ -428,13 +428,12 @@ Commands: If 'n' is not specified it defaults to 60 minutes. Note: to remove a master you must remove the resource/group it contains. - bundle create [container [] ] + bundle create container [] [network ] [port-map ]... [storage-map ]... [meta ] [--disabled] [--wait[=n]] Create a new bundle encapsulating no resources. The bundle can be used either as it is or a resource may be put into it at any time. - If the container type is not specified, it defaults to 'docker'. If --disabled is specified, the bundle is not started automatically. If --wait is specified, pcs will wait up to 'n' seconds for the bundle to start and then return 0 on success or 1 on error. If 'n' is not -- 1.8.3.1