Refactor away the flags.DEFINE_* helpers

The next obvious step in porting to cfg is to define all options using
cfg schemas directly rather than using the flags.DEFINE_* helpers.

This is a large change, but it is almost entirely pure refactoring and
does not result in any functional changes.

The only change to note is that the default values for glance_host,
glance_api_servers and default_publisher_id options are now using opt
value interpolation i.e.

 -glance_host=_get_my_ip()
 +glance_host='$my_ip'

 -glance_api_servers=['%s:%d' % (FLAGS.glance_host, FLAGS.glance_port)]
 +glance_api_servers=['$glance_host:$glance_port']

 -default_publisher_id=FLAGS.host
 +default_publisher_id='$host'

Also note that the lower_bound check on the {report,periodic}_interval
options are no more, but this has been true since cfg was first added.

Change-Id: Ia58c8f0aaf61628bb55b1b8485118a2a9852ed17

5 files changed, 57 insertions, 39 deletions

diff --git a/nova/tests/api/ec2/test_cloud.py b/nova/tests/api/ec2/test_cloud.py
index ffdb74c8c..d5fccffb9 100644
--- a/nova/tests/api/ec2/test_cloud.py
+++ b/nova/tests/api/ec2/test_cloud.py
@@ -28,6 +28,7 @@ from M2Crypto import RSA
 from nova.api.ec2 import cloud
 from nova.api.ec2 import ec2utils
 from nova.api.ec2 import inst_state
+from nova.common import cfg
 from nova.compute import power_state
 from nova.compute import vm_states
 from nova import context
@@ -43,12 +44,15 @@ from nova import test
 from nova import utils
 
 
-FLAGS = flags.FLAGS
 LOG = logging.getLogger('nova.tests.cloud')
 
+ajax_proxy_manager_opt = \
+    cfg.StrOpt('ajax_proxy_manager',
+               default='nova.tests.api.ec2.test_cloud.AjaxProxyManager',
+               help='')
 
-flags.DEFINE_string('ajax_proxy_manager',
-        'nova.tests.api.ec2.test_cloud.AjaxProxyManager', '')
+FLAGS = flags.FLAGS
+FLAGS.add_option(ajax_proxy_manager_opt)
 
 
 class AjaxProxyManager(manager.SchedulerDependentManager):
diff --git a/nova/tests/declare_flags.py b/nova/tests/declare_flags.py
index 51a55ec72..9dc578867 100644
--- a/nova/tests/declare_flags.py
+++ b/nova/tests/declare_flags.py
@@ -16,8 +16,8 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+from nova.common import cfg
 from nova import flags
 
 FLAGS = flags.FLAGS
-
-flags.DEFINE_integer('answer', 42, 'test flag')
+FLAGS.add_option(cfg.IntOpt('answer', default=42, help='test flag'))
diff --git a/nova/tests/runtime_flags.py b/nova/tests/runtime_flags.py
index 1eb501406..1a28f4fbd 100644
--- a/nova/tests/runtime_flags.py
+++ b/nova/tests/runtime_flags.py
@@ -16,8 +16,8 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+from nova.common import cfg
 from nova import flags
 
 FLAGS = flags.FLAGS
-
-flags.DEFINE_integer('runtime_answer', 54, 'test flag')
+FLAGS.add_option(cfg.IntOpt('runtime_answer', default=54, help='test flag'))
diff --git a/nova/tests/test_flags.py b/nova/tests/test_flags.py
index b5fe11983..02ee49ef0 100644
--- a/nova/tests/test_flags.py
+++ b/nova/tests/test_flags.py
@@ -21,11 +21,14 @@ import exceptions
 import os
 import tempfile
 
+from nova.common import cfg
 from nova import flags
 from nova import test
 
 FLAGS = flags.FLAGS
-flags.DEFINE_string('flags_unittest', 'foo', 'for testing purposes only')
+FLAGS.add_option(cfg.StrOpt('flags_unittest',
+                            default='foo',
+                            help='for testing purposes only'))
 
 
 class FlagsTestCase(test.TestCase):
@@ -41,11 +44,11 @@ class FlagsTestCase(test.TestCase):
         self.assert_('false' not in self.FLAGS)
         self.assert_('true' not in self.FLAGS)
 
-        flags.DEFINE_string('string', 'default', 'desc',
-                            flag_values=self.FLAGS)
-        flags.DEFINE_integer('int', 1, 'desc', flag_values=self.FLAGS)
-        flags.DEFINE_bool('false', False, 'desc', flag_values=self.FLAGS)
-        flags.DEFINE_bool('true', True, 'desc', flag_values=self.FLAGS)
+        self.FLAGS.add_option(cfg.StrOpt('string',
+                                         default='default', help='desc'))
+        self.FLAGS.add_option(cfg.IntOpt('int', default=1, help='desc'))
+        self.FLAGS.add_option(cfg.BoolOpt('false', default=False, help='desc'))
+        self.FLAGS.add_option(cfg.BoolOpt('true', default=True, help='desc'))
 
         self.assert_(self.FLAGS['string'])
         self.assert_(self.FLAGS['int'])
@@ -69,12 +72,12 @@ class FlagsTestCase(test.TestCase):
         self.assertEqual(self.FLAGS.true, False)
 
     def test_define_float(self):
-        flags.DEFINE_float('float', 6.66, 'desc', flag_values=self.FLAGS)
+        self.FLAGS.add_option(cfg.FloatOpt('float', default=6.66, help='desc'))
         self.assertEqual(self.FLAGS.float, 6.66)
 
     def test_define_multistring(self):
-        flags.DEFINE_multistring('multi', ['blaa'], 'desc',
-                                 flag_values=self.FLAGS)
+        self.FLAGS.add_option(cfg.MultiStrOpt('multi',
+                                              default=['blaa'], help='desc'))
 
         self.assert_(self.FLAGS['multi'])
         self.assertEqual(self.FLAGS.multi, ['blaa'])
@@ -89,7 +92,8 @@ class FlagsTestCase(test.TestCase):
         self.assertEqual(self.FLAGS.multi, ['foo', 'bar'])
 
     def test_define_list(self):
-        flags.DEFINE_list('list', ['foo'], 'desc', flag_values=self.FLAGS)
+        self.FLAGS.add_option(cfg.ListOpt('list',
+                                          default=['foo'], help='desc'))
 
         self.assert_(self.FLAGS['list'])
         self.assertEqual(self.FLAGS.list, ['foo'])
@@ -100,7 +104,7 @@ class FlagsTestCase(test.TestCase):
         self.assertEqual(self.FLAGS.list, ['a', 'b', 'c', 'd'])
 
     def test_error(self):
-        flags.DEFINE_integer('error', 1, 'desc', flag_values=self.FLAGS)
+        self.FLAGS.add_option(cfg.IntOpt('error', default=1, help='desc'))
 
         self.assertEqual(self.FLAGS.error, 1)
 
@@ -143,16 +147,16 @@ class FlagsTestCase(test.TestCase):
         self.assertEqual(self.global_FLAGS.runtime_answer, 60)
 
     def test_long_vs_short_flags(self):
-        flags.DEFINE_string('duplicate_answer_long', 'val', 'desc',
-                            flag_values=self.global_FLAGS)
+        self.global_FLAGS.add_option(cfg.StrOpt('duplicate_answer_long',
+                                                default='val', help='desc'))
         argv = ['flags_test', '--duplicate_answer=60', 'extra_arg']
         args = self.global_FLAGS(argv)
 
         self.assert_('duplicate_answer' not in self.global_FLAGS)
         self.assert_(self.global_FLAGS.duplicate_answer_long, 60)
 
-        flags.DEFINE_integer('duplicate_answer', 60, 'desc',
-                             flag_values=self.global_FLAGS)
+        self.global_FLAGS.add_option(cfg.IntOpt('duplicate_answer',
+                                                default=60, help='desc'))
         self.assertEqual(self.global_FLAGS.duplicate_answer, 60)
         self.assertEqual(self.global_FLAGS.duplicate_answer_long, 'val')
 
@@ -178,13 +182,13 @@ class FlagsTestCase(test.TestCase):
         self.assertEqual(FLAGS.FlagValuesDict()['flags_unittest'], 'foo')
 
     def test_flagfile(self):
-        flags.DEFINE_string('string', 'default', 'desc',
-                            flag_values=self.FLAGS)
-        flags.DEFINE_integer('int', 1, 'desc', flag_values=self.FLAGS)
-        flags.DEFINE_bool('false', False, 'desc', flag_values=self.FLAGS)
-        flags.DEFINE_bool('true', True, 'desc', flag_values=self.FLAGS)
-        flags.DEFINE_multistring('multi', ['blaa'], 'desc',
-                                 flag_values=self.FLAGS)
+        self.FLAGS.add_option(cfg.StrOpt('string',
+                                         default='default', help='desc'))
+        self.FLAGS.add_option(cfg.IntOpt('int', default=1, help='desc'))
+        self.FLAGS.add_option(cfg.BoolOpt('false', default=False, help='desc'))
+        self.FLAGS.add_option(cfg.BoolOpt('true', default=True, help='desc'))
+        self.FLAGS.add_option(cfg.MultiStrOpt('multi',
+                                              default=['blaa'], help='desc'))
 
         (fd, path) = tempfile.mkstemp(prefix='nova', suffix='.flags')
 
@@ -208,14 +212,15 @@ class FlagsTestCase(test.TestCase):
             os.remove(path)
 
     def test_defaults(self):
-        flags.DEFINE_string('foo', 'bar', 'help', flag_values=self.FLAGS)
+        self.FLAGS.add_option(cfg.StrOpt('foo', default='bar', help='desc'))
         self.assertEqual(self.FLAGS.foo, 'bar')
 
         self.FLAGS['foo'].SetDefault('blaa')
         self.assertEqual(self.FLAGS.foo, 'blaa')
 
     def test_templated_values(self):
-        flags.DEFINE_string('foo', 'foo', 'help', flag_values=self.FLAGS)
-        flags.DEFINE_string('bar', 'bar', 'help', flag_values=self.FLAGS)
-        flags.DEFINE_string('blaa', '$foo$bar', 'help', flag_values=self.FLAGS)
+        self.FLAGS.add_option(cfg.StrOpt('foo', default='foo', help='desc'))
+        self.FLAGS.add_option(cfg.StrOpt('bar', default='bar', help='desc'))
+        self.FLAGS.add_option(cfg.StrOpt('blaa',
+                                         default='$foo$bar', help='desc'))
         self.assertEqual(self.FLAGS.blaa, 'foobar')
diff --git a/nova/tests/test_service.py b/nova/tests/test_service.py
index 99f091b22..c8a18f7d8 100644
--- a/nova/tests/test_service.py
+++ b/nova/tests/test_service.py
@@ -22,6 +22,7 @@ Unit Tests for remote procedure calls using queue
 
 import mox
 
+from nova.common import cfg
 from nova import context
 from nova import db
 from nova import exception
@@ -31,12 +32,20 @@ from nova import service
 from nova import manager
 from nova import wsgi
 
-flags.DEFINE_string("fake_manager", "nova.tests.test_service.FakeManager",
-                    "Manager for testing")
-flags.DEFINE_string("test_service_listen", None,
-                    "Host to bind test service to")
-flags.DEFINE_integer("test_service_listen_port", 0,
-                     "Port number to bind test service to")
+
+test_service_opts = [
+    cfg.StrOpt("fake_manager",
+               default="nova.tests.test_service.FakeManager",
+               help="Manager for testing"),
+    cfg.StrOpt("test_service_listen",
+               default=None,
+               help="Host to bind test service to"),
+    cfg.IntOpt("test_service_listen_port",
+               default=0,
+               help="Port number to bind test service to"),
+    ]
+
+flags.FLAGS.add_options(test_service_opts)
 
 
 class FakeManager(manager.Manager):