From 526a662afbe2ee4dba50146f98d726d527afe40d Mon Sep 17 00:00:00 2001
From: Michael Hanselmann <hansmi@google.com>
Date: Mon, 14 Feb 2011 16:14:06 +0100
Subject: [PATCH] RAPI: Clean up instance creation, use generated docs
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
- Use FillOpCode and unify parameter names between RAPI and opcode
- Generate parameter documentation
- Improve opcode parameter documentation
Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: RenΓ© Nussbaumer <rn@google.com>
---
NEWS | 2 +
doc/rapi.rst | 64 ++----------------
lib/opcodes.py | 30 +++++++--
lib/rapi/rlib2.py | 101 +++--------------------------
test/ganeti.rapi.rlib2_unittest.py | 75 ++++++++++++++++-----
5 files changed, 102 insertions(+), 170 deletions(-)
diff --git a/NEWS b/NEWS
index 8168cf7da..2668383f4 100644
--- a/NEWS
+++ b/NEWS
@@ -9,6 +9,8 @@ Version 2.5.0 beta1
- The default of the ``/2/instances/[instance_name]/rename`` RAPI
resource's ``ip_check`` parameter changed from ``True`` to ``False``
to match the underlying LUXI interface
+- When creating file-based instances via RAPI, the ``file_driver``
+ parameter no longer defaults to ``loop`` and must be specified
Version 2.4.0 beta1
diff --git a/doc/rapi.rst b/doc/rapi.rst
index f9088ebfa..316c8f1fb 100644
--- a/doc/rapi.rst
+++ b/doc/rapi.rst
@@ -567,63 +567,13 @@ Body parameters:
``__version__`` (int, required)
Must be ``1`` (older Ganeti versions used a different format for
instance creation requests, version ``0``, but that format is not
- documented).
-``mode`` (string, required)
- Instance creation mode.
-``name`` (string, required)
- Instance name.
-``disk_template`` (string, required)
- Disk template for instance.
-``disks`` (list, required)
- List of disk definitions. Example: ``[{"size": 100}, {"size": 5}]``.
- Each disk definition must contain a ``size`` value and can contain an
- optional ``mode`` value denoting the disk access mode (``ro`` or
- ``rw``).
-``nics`` (list, required)
- List of NIC (network interface) definitions. Example: ``[{}, {},
- {"ip": "198.51.100.4"}]``. Each NIC definition can contain the
- optional values ``ip``, ``mode``, ``link`` and ``bridge``.
-``os`` (string, required)
- Instance operating system.
-``osparams`` (dictionary)
- Dictionary with OS parameters. If not valid for the given OS, the job
- will fail.
-``force_variant`` (bool)
- Whether to force an unknown variant.
-``no_install`` (bool)
- Do not install the OS (will enable no-start)
-``pnode`` (string)
- Primary node.
-``snode`` (string)
- Secondary node.
-``src_node`` (string)
- Source node for import.
-``src_path`` (string)
- Source directory for import.
-``start`` (bool)
- Whether to start instance after creation.
-``ip_check`` (bool)
- Whether to ensure instance's IP address is inactive.
-``name_check`` (bool)
- Whether to ensure instance's name is resolvable.
-``file_storage_dir`` (string)
- File storage directory.
-``file_driver`` (string)
- File storage driver.
-``iallocator`` (string)
- Instance allocator name.
-``source_handshake`` (list)
- Signed handshake from source (remote import only).
-``source_x509_ca`` (string)
- Source X509 CA in PEM format (remote import only).
-``source_instance_name`` (string)
- Source instance name (remote import only).
-``hypervisor`` (string)
- Hypervisor name.
-``hvparams`` (dict)
- Hypervisor parameters, hypervisor-dependent.
-``beparams`` (dict)
- Backend parameters.
+ documented and should no longer be used).
+
+.. opcode_params:: OP_INSTANCE_CREATE
+
+Earlier versions used parameters named ``name`` and ``os``. These have
+been replaced by ``instance_name`` and ``os_type`` to match the
+underlying opcode. The old names can still be used.
``/2/instances/[instance_name]``
diff --git a/lib/opcodes.py b/lib/opcodes.py
index 113ce993f..5796e07cb 100644
--- a/lib/opcodes.py
+++ b/lib/opcodes.py
@@ -121,6 +121,12 @@ _TestClusterOsList = ht.TOr(ht.TNone,
ht.TElemOf(constants.DDMS_VALUES)))))
+# TODO: Generate check from constants.INIC_PARAMS_TYPES
+#: Utility function for testing NIC definitions
+_TestNicDef = ht.TDictOf(ht.TElemOf(constants.INIC_PARAMS),
+ ht.TOr(ht.TNone, ht.TNonEmptyString))
+
+
def _NameToId(name):
"""Convert an opcode class name to an OP_ID.
@@ -841,7 +847,17 @@ class OpInstanceCreate(OpCode):
_PWaitForSync,
_PNameCheck,
("beparams", ht.EmptyDict, ht.TDict, "Backend parameters for instance"),
- ("disks", ht.NoDefault, ht.TListOf(ht.TDict), "Disk descriptions"),
+ ("disks", ht.NoDefault,
+ # TODO: Generate check from constants.IDISK_PARAMS_TYPES
+ ht.TListOf(ht.TDictOf(ht.TElemOf(constants.IDISK_PARAMS),
+ ht.TOr(ht.TNonEmptyString, ht.TInt))),
+ "Disk descriptions, for example ``[{\"%s\": 100}, {\"%s\": 5}]``;"
+ " each disk definition must contain a ``%s`` value and"
+ " can contain an optional ``%s`` value denoting the disk access mode"
+ " (%s)" %
+ (constants.IDISK_SIZE, constants.IDISK_SIZE, constants.IDISK_SIZE,
+ constants.IDISK_MODE,
+ " or ".join("``%s``" % i for i in sorted(constants.DISK_ACCESS_SET)))),
("disk_template", ht.NoDefault, _CheckDiskTemplate, "Disk template"),
("file_driver", None, ht.TOr(ht.TNone, ht.TElemOf(constants.FILE_DRIVER)),
"Driver for file-backed disks"),
@@ -857,8 +873,12 @@ class OpInstanceCreate(OpCode):
("ip_check", True, ht.TBool, _PIpCheckDoc),
("mode", ht.NoDefault, ht.TElemOf(constants.INSTANCE_CREATE_MODES),
"Instance creation mode"),
- ("nics", ht.NoDefault, ht.TListOf(ht.TDict),
- "List of NIC (network interface) definitions"),
+ ("nics", ht.NoDefault, ht.TListOf(_TestNicDef),
+ "List of NIC (network interface) definitions, for example"
+ " ``[{}, {}, {\"%s\": \"198.51.100.4\"}]``; each NIC definition can"
+ " contain the optional values %s" %
+ (constants.INIC_IP,
+ ", ".join("``%s``" % i for i in sorted(constants.INIC_PARAMS)))),
("no_install", None, ht.TMaybeBool,
"Do not install the OS (will disable automatic start)"),
("osparams", ht.EmptyDict, ht.TDict, "OS parameters for instance"),
@@ -870,7 +890,8 @@ class OpInstanceCreate(OpCode):
("source_instance_name", None, ht.TMaybeString,
"Source instance name (remote import only)"),
("source_shutdown_timeout", constants.DEFAULT_SHUTDOWN_TIMEOUT,
- ht.TPositiveInt, "How long source instance was given to shut down"),
+ ht.TPositiveInt,
+ "How long source instance was given to shut down (remote import only)"),
("source_x509_ca", None, ht.TMaybeString,
"Source X509 CA in PEM format (remote import only)"),
("src_node", None, ht.TMaybeString, "Source node for import"),
@@ -1074,6 +1095,7 @@ class OpInstanceSetParams(OpCode):
_PInstanceName,
_PForce,
_PForceVariant,
+ # TODO: Use _TestNicDef
("nics", ht.EmptyList, ht.TList,
"List of NIC changes. Each item is of the form ``(op, settings)``."
" ``op`` can be ``%s`` to add a new NIC with the specified settings,"
diff --git a/lib/rapi/rlib2.py b/lib/rapi/rlib2.py
index e70044b91..c5b8c0035 100644
--- a/lib/rapi/rlib2.py
+++ b/lib/rapi/rlib2.py
@@ -45,7 +45,6 @@ from ganeti import opcodes
from ganeti import http
from ganeti import constants
from ganeti import cli
-from ganeti import utils
from ganeti import rapi
from ganeti import ht
from ganeti.rapi import baserlib
@@ -714,99 +713,17 @@ def _ParseInstanceCreateRequestVersion1(data, dry_run):
@return: Instance creation opcode
"""
- # Disks
- disks_input = baserlib.CheckParameter(data, "disks", exptype=list)
-
- disks = []
- for idx, i in enumerate(disks_input):
- baserlib.CheckType(i, dict, "Disk %d specification" % idx)
-
- # Size is mandatory
- try:
- size = i[constants.IDISK_SIZE]
- except KeyError:
- raise http.HttpBadRequest("Disk %d specification wrong: missing disk"
- " size" % idx)
-
- disk = {
- constants.IDISK_SIZE: size,
- }
-
- # Optional disk access mode
- try:
- disk_access = i[constants.IDISK_MODE]
- except KeyError:
- pass
- else:
- disk[constants.IDISK_MODE] = disk_access
-
- disks.append(disk)
-
- assert len(disks_input) == len(disks)
-
- # Network interfaces
- nics_input = baserlib.CheckParameter(data, "nics", exptype=list)
-
- nics = []
- for idx, i in enumerate(nics_input):
- baserlib.CheckType(i, dict, "NIC %d specification" % idx)
+ override = {
+ "dry_run": dry_run,
+ }
- nic = {}
+ rename = {
+ "os": "os_type",
+ "name": "instance_name",
+ }
- for field in constants.INIC_PARAMS:
- try:
- value = i[field]
- except KeyError:
- continue
-
- nic[field] = value
-
- nics.append(nic)
-
- assert len(nics_input) == len(nics)
-
- # HV/BE parameters
- hvparams = baserlib.CheckParameter(data, "hvparams", default={})
- utils.ForceDictType(hvparams, constants.HVS_PARAMETER_TYPES)
-
- beparams = baserlib.CheckParameter(data, "beparams", default={})
- utils.ForceDictType(beparams, constants.BES_PARAMETER_TYPES)
-
- return opcodes.OpInstanceCreate(
- mode=baserlib.CheckParameter(data, "mode"),
- instance_name=baserlib.CheckParameter(data, "name"),
- os_type=baserlib.CheckParameter(data, "os"),
- osparams=baserlib.CheckParameter(data, "osparams", default={}),
- force_variant=baserlib.CheckParameter(data, "force_variant",
- default=False),
- no_install=baserlib.CheckParameter(data, "no_install", default=False),
- pnode=baserlib.CheckParameter(data, "pnode", default=None),
- snode=baserlib.CheckParameter(data, "snode", default=None),
- disk_template=baserlib.CheckParameter(data, "disk_template"),
- disks=disks,
- nics=nics,
- src_node=baserlib.CheckParameter(data, "src_node", default=None),
- src_path=baserlib.CheckParameter(data, "src_path", default=None),
- start=baserlib.CheckParameter(data, "start", default=True),
- wait_for_sync=True,
- ip_check=baserlib.CheckParameter(data, "ip_check", default=True),
- name_check=baserlib.CheckParameter(data, "name_check", default=True),
- file_storage_dir=baserlib.CheckParameter(data, "file_storage_dir",
- default=None),
- file_driver=baserlib.CheckParameter(data, "file_driver",
- default=constants.FD_LOOP),
- source_handshake=baserlib.CheckParameter(data, "source_handshake",
- default=None),
- source_x509_ca=baserlib.CheckParameter(data, "source_x509_ca",
- default=None),
- source_instance_name=baserlib.CheckParameter(data, "source_instance_name",
- default=None),
- iallocator=baserlib.CheckParameter(data, "iallocator", default=None),
- hypervisor=baserlib.CheckParameter(data, "hypervisor", default=None),
- hvparams=hvparams,
- beparams=beparams,
- dry_run=dry_run,
- )
+ return baserlib.FillOpcode(opcodes.OpInstanceCreate, data, override,
+ rename=rename)
class R_2_instances(baserlib.R_Generic):
diff --git a/test/ganeti.rapi.rlib2_unittest.py b/test/ganeti.rapi.rlib2_unittest.py
index 6246f375c..ec97eacd9 100755
--- a/test/ganeti.rapi.rlib2_unittest.py
+++ b/test/ganeti.rapi.rlib2_unittest.py
@@ -53,9 +53,6 @@ class TestParseInstanceCreateRequestVersion1(testutils.GanetiTestCase):
# Disk with mode
[{"size": 123, "mode": constants.DISK_RDWR, }],
-
- # With unknown setting
- [{"size": 123, "unknown": 999 }],
]
nic_variants = [
@@ -72,9 +69,6 @@ class TestParseInstanceCreateRequestVersion1(testutils.GanetiTestCase):
},
{ "mode": constants.NIC_MODE_BRIDGED, "link": "n0", "bridge": "br1", },
],
-
- # Unknown settings
- [{ "unknown": 999, }, { "foobar": "Hello World", }],
]
beparam_variants = [
@@ -137,15 +131,69 @@ class TestParseInstanceCreateRequestVersion1(testutils.GanetiTestCase):
self.assertFalse("foobar" in opnic)
if beparams is None:
- self.assertEqualValues(op.beparams, {})
+ self.assertFalse(hasattr(op, "beparams"))
else:
self.assertEqualValues(op.beparams, beparams)
if hvparams is None:
- self.assertEqualValues(op.hvparams, {})
+ self.assertFalse(hasattr(op, "hvparams"))
else:
self.assertEqualValues(op.hvparams, hvparams)
+ def testLegacyName(self):
+ name = "inst29128.example.com"
+ data = {
+ "name": name,
+ "disks": [],
+ "nics": [],
+ "mode": constants.INSTANCE_CREATE,
+ "disk_template": constants.DT_PLAIN,
+ }
+ op = self.Parse(data, False)
+ self.assert_(isinstance(op, opcodes.OpInstanceCreate))
+ self.assertEqual(op.instance_name, name)
+ self.assertFalse(hasattr(op, "name"))
+
+ # Define both
+ data = {
+ "name": name,
+ "instance_name": "other.example.com",
+ "disks": [],
+ "nics": [],
+ "mode": constants.INSTANCE_CREATE,
+ "disk_template": constants.DT_PLAIN,
+ }
+ self.assertRaises(http.HttpBadRequest, self.Parse, data, False)
+
+ def testLegacyOs(self):
+ name = "inst4673.example.com"
+ os = "linux29206"
+ data = {
+ "name": name,
+ "os_type": os,
+ "disks": [],
+ "nics": [],
+ "mode": constants.INSTANCE_CREATE,
+ "disk_template": constants.DT_PLAIN,
+ }
+ op = self.Parse(data, False)
+ self.assert_(isinstance(op, opcodes.OpInstanceCreate))
+ self.assertEqual(op.instance_name, name)
+ self.assertEqual(op.os_type, os)
+ self.assertFalse(hasattr(op, "os"))
+
+ # Define both
+ data = {
+ "instance_name": name,
+ "os": os,
+ "os_type": "linux9584",
+ "disks": [],
+ "nics": [],
+ "mode": constants.INSTANCE_CREATE,
+ "disk_template": constants.DT_PLAIN,
+ }
+ self.assertRaises(http.HttpBadRequest, self.Parse, data, False)
+
def testErrors(self):
# Test all required fields
reqfields = {
@@ -154,7 +202,6 @@ class TestParseInstanceCreateRequestVersion1(testutils.GanetiTestCase):
"nics": [],
"mode": constants.INSTANCE_CREATE,
"disk_template": constants.DT_PLAIN,
- "os": "debootstrap",
}
for name in reqfields.keys():
@@ -164,14 +211,8 @@ class TestParseInstanceCreateRequestVersion1(testutils.GanetiTestCase):
# Invalid disks and nics
for field in ["disks", "nics"]:
- invalid_values = [None, 1, "", {}, [1, 2, 3], ["hda1", "hda2"]]
-
- if field == "disks":
- invalid_values.append([
- # Disks without size
- {},
- { "mode": constants.DISK_RDWR, },
- ])
+ invalid_values = [None, 1, "", {}, [1, 2, 3], ["hda1", "hda2"],
+ [{"_unknown_": 999, }]]
for invvalue in invalid_values:
data = reqfields.copy()
--
GitLab