From: Sebastian Wagner <sebastian.wagner@suse.com>
Date: Wed, 15 Apr 2020 13:23:05 +0000 (+0200)
Subject: mgr/orch: upgrade: better input validation
X-Git-Tag: v15.2.2~59^2~2
X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=f38a4873fc067d5efe89f16c54cfd2144e463cdb;p=ceph.git

mgr/orch: upgrade: better input validation

Signed-off-by: Sebastian Wagner <sebastian.wagner@suse.com>
(cherry picked from commit 67a29846f80fe8903f2dbce5dca0f5a0134d55b8)
---

diff --git a/src/pybind/mgr/orchestrator/_interface.py b/src/pybind/mgr/orchestrator/_interface.py
index 4622fa6be2879..d6d94930de603 100644
--- a/src/pybind/mgr/orchestrator/_interface.py
+++ b/src/pybind/mgr/orchestrator/_interface.py
@@ -437,7 +437,8 @@ class Completion(_Promise):
     call one completion from another completion. I.e. making them re-usable
     using Promises E.g.::
 
-        >>> return Orchestrator().get_hosts().then(self._create_osd)
+        >>> #doctest: +SKIP
+        ... return Orchestrator().get_hosts().then(self._create_osd)
 
     where ``get_hosts`` returns a Completion of list of hosts and
     ``_create_osd`` takes a list of hosts.
@@ -445,7 +446,8 @@ class Completion(_Promise):
     The concept behind this is to store the computation steps
     explicit and then explicitly evaluate the chain:
 
-        >>> p = Completion(on_complete=lambda x: x*2).then(on_complete=lambda x: str(x))
+        >>> #doctest: +SKIP
+        ... p = Completion(on_complete=lambda x: x*2).then(on_complete=lambda x: str(x))
         ... p.finalize(2)
         ... assert p.result = "4"
 
@@ -695,7 +697,8 @@ class Orchestrator(object):
             is actually available in the orchestrator. I.e. this
             won't work as expected::
 
-                >>> if OrchestratorClientMixin().available()[0]:  # wrong.
+                >>> #doctest: +SKIP
+                ... if OrchestratorClientMixin().available()[0]:  # wrong.
                 ...     OrchestratorClientMixin().get_hosts()
 
         :return: two-tuple of boolean, string
@@ -727,13 +730,15 @@ class Orchestrator(object):
             is actually possible in the orchestrator. I.e. this
             won't work as expected::
 
-                >>> api = OrchestratorClientMixin()
+                >>> #doctest: +SKIP
+                ... api = OrchestratorClientMixin()
                 ... if api.get_feature_set()['get_hosts']['available']:  # wrong.
                 ...     api.get_hosts()
 
             It's better to ask for forgiveness instead::
 
-                >>> try:
+                >>> #doctest: +SKIP
+                ... try:
                 ...     OrchestratorClientMixin().get_hosts()
                 ... except (OrchestratorError, NotImplementedError):
                 ...     ...
@@ -1565,7 +1570,8 @@ class OrchestratorClientMixin(Orchestrator):
 
 
     >>> import mgr_module
-    >>> class MyImplentation(mgr_module.MgrModule, Orchestrator):
+    >>> #doctest: +SKIP
+    ... class MyImplentation(mgr_module.MgrModule, Orchestrator):
     ...     def __init__(self, ...):
     ...         self.orch_client = OrchestratorClientMixin()
     ...         self.orch_client.set_mgr(self.mgr))
diff --git a/src/pybind/mgr/orchestrator/module.py b/src/pybind/mgr/orchestrator/module.py
index f4580a2fe2356..7df8c847cecc7 100644
--- a/src/pybind/mgr/orchestrator/module.py
+++ b/src/pybind/mgr/orchestrator/module.py
@@ -2,6 +2,7 @@ import datetime
 import errno
 import json
 from typing import List, Set, Optional, Iterator
+import re
 
 import yaml
 import six
@@ -1041,12 +1042,28 @@ Usage:
         c = TrivialReadCompletion(result=True)
         assert c.has_result
 
+    @staticmethod
+    def _upgrade_check_image_name(image, ceph_version):
+        """
+        >>> OrchestratorCli._upgrade_check_image_name('v15.2.0', None)
+        Traceback (most recent call last):
+        orchestrator._interface.OrchestratorValidationError: Error: unable to pull image name `v15.2.0`.
+          Maybe you meant `--ceph-version 15.2.0`?
+
+        """
+        if image and re.match(r'^v?\d+\.\d+\.\d+$', image) and ceph_version is None:
+            ver = image[1:] if image.startswith('v') else image
+            s =  f"Error: unable to pull image name `{image}`.\n" \
+                 f"  Maybe you meant `--ceph-version {ver}`?"
+            raise OrchestratorValidationError(s)
+
     @_cli_write_command(
         'orch upgrade check',
         'name=image,type=CephString,req=false '
         'name=ceph_version,type=CephString,req=false',
         desc='Check service versions vs available and target containers')
     def _upgrade_check(self, image=None, ceph_version=None):
+        self._upgrade_check_image_name(image, ceph_version)
         completion = self.upgrade_check(image=image, version=ceph_version)
         self._orchestrator_wait([completion])
         raise_if_exception(completion)
@@ -1074,6 +1091,7 @@ Usage:
         'name=ceph_version,type=CephString,req=false',
         desc='Initiate upgrade')
     def _upgrade_start(self, image=None, ceph_version=None):
+        self._upgrade_check_image_name(image, ceph_version)
         completion = self.upgrade_start(image, ceph_version)
         self._orchestrator_wait([completion])
         raise_if_exception(completion)
diff --git a/src/pybind/mgr/tox.ini b/src/pybind/mgr/tox.ini
index d04c889db0a5e..e6a902a74763f 100644
--- a/src/pybind/mgr/tox.ini
+++ b/src/pybind/mgr/tox.ini
@@ -11,7 +11,14 @@ setenv =
 deps =
     cython
     -rrequirements.txt
-commands = pytest -v --cov --cov-append --cov-report= --doctest-modules {posargs:mgr_util.py tests/ cephadm/ pg_autoscaler/ progress/}
+commands =
+    pytest -v --cov --cov-append --cov-report= --doctest-modules {posargs: \
+        mgr_util.py \
+        tests/ \
+        cephadm/ \
+        orchestrator/ \
+        pg_autoscaler/ \
+        progress/}
 
 [testenv:mypy]
 basepython = python3