Commit 3636400f authored by Iustin Pop's avatar Iustin Pop
Browse files

Introduce a micro type system for opcodes



Currently, we have one structual validation for opcode attributes: the
_OP_REQP, which checks that a given attribute is not 'None', and the
rest of the checks are done at runtime. This means our type system has
two types: None versus Not-None.

We have been hit many times by small, trivial bugs in this area, and
only a huge amount of unittest and/or hand-written checks would ensure
that we cover all possibilities. This patch attempts to redress the
needs for manual checks by introducing a micro-type system for the
validation of the opcode attributes. What we lose, from the start, are
the custom error messages (e.g. "Invalid reboot mode, choose one of …",
or "The disk index must be a positive integer"). What we gain is the
ability to express easily things as:

- this parameter must be None or an int
- this parameter must be a non-empty list
- this parameter must be either none or a list of dictionaries with keys
  from the list of valid hypervisors and the values dictionaries with
  keys strings and values either None or strings; furthermore, the list
  must be non-empty

These examples show that we have a composable (as opposed to just a few
static types) system, and that we can nest it a few times (just for
sanity; we could nest it up to stack depth).

We also gain lots of ))))))), which is not that nice :)

The current patch moves the existing _OP_REQP to the new framework, but
if accepted, a lot more validations should move to it. In the end, we
definitely should declare a type for all the opcode parameters
(eventually moving _OP_REQP directly to opcodes.py and validating in the
load/init case, and build __slots__ from it).
Signed-off-by: default avatarIustin Pop <iustin@google.com>
Reviewed-by: default avatarGuido Trotter <ultrotter@google.com>
parent 3494b9f6
This diff is collapsed.
......@@ -353,6 +353,12 @@ REPLACE_DISK_PRI = "replace_on_primary" # replace disks on primary
REPLACE_DISK_SEC = "replace_on_secondary" # replace disks on secondary
REPLACE_DISK_CHG = "replace_new_secondary" # change secondary node
REPLACE_DISK_AUTO = "replace_auto"
REPLACE_MODES = frozenset([
REPLACE_DISK_PRI,
REPLACE_DISK_SEC,
REPLACE_DISK_CHG,
REPLACE_DISK_AUTO,
])
# Instance export mode
EXPORT_MODE_LOCAL = "local"
......@@ -414,6 +420,11 @@ EXIT_CONFIRMATION = 13 # need user confirmation
TAG_CLUSTER = "cluster"
TAG_NODE = "node"
TAG_INSTANCE = "instance"
VALID_TAG_TYPES = frozenset([
TAG_CLUSTER,
TAG_NODE,
TAG_INSTANCE,
])
MAX_TAG_LEN = 128
MAX_TAGS_PER_OBJ = 4096
......@@ -718,9 +729,18 @@ SSL_CERT_EXPIRATION_ERROR = 7
IALLOCATOR_VERSION = 2
IALLOCATOR_DIR_IN = "in"
IALLOCATOR_DIR_OUT = "out"
VALID_IALLOCATOR_DIRECTIONS = frozenset([
IALLOCATOR_DIR_IN,
IALLOCATOR_DIR_OUT,
])
IALLOCATOR_MODE_ALLOC = "allocate"
IALLOCATOR_MODE_RELOC = "relocate"
IALLOCATOR_MODE_MEVAC = "multi-evacuate"
VALID_IALLOCATOR_MODES = frozenset([
IALLOCATOR_MODE_ALLOC,
IALLOCATOR_MODE_RELOC,
IALLOCATOR_MODE_MEVAC,
])
IALLOCATOR_SEARCH_PATH = _autoconf.IALLOCATOR_SEARCH_PATH
# Job queue
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment