Enable loading and saving of question answers.

This lays the groundwork for interactive init, as well as being able
to specify control and compute nodes.

Added preliminary config lists for control and compute nodes. Added
appropriate default snapctl config settings in install script.

Also changed "binary" questions to "boolean" questions, as that's
better wording, and it means that my docstrings are not a confusing
mix of "boolean" and "binary" when I forget which term I used.

Snuck in a fix for the "basic" testing environment -- it was missing
the Python requirements, and was therefore failing!

Change-Id: I7f95ab68f924fa4d4280703c372b807cc7c77758

README.md

@@ -28,7 +28,7 @@ To quickly configure networks and launch a vm, run
 
 This will configure various Openstack databases. Then run:
 
-`microstack.launch`.
+`microstack.launch test`.
 
 This will launch an instance for you, and make it available to manage via the command line, or via the Horizon Dashboard.
 

snap-overlay/bin/launch.sh

@@ -72,4 +72,5 @@ while :; do
     fi
 done
 
+extgateway=`snapctl get questions.ext-gateway`
 echo "You can also visit the openstack dashboard at 'http://$extgateway/'"

snap-overlay/bin/setup-br-ex

@@ -9,7 +9,7 @@
 
 set -ex
 
-extcidr=$(snapctl get extcidr)
+extcidr=$(snapctl get questions.ext-cidr)
 
 # Create external integration bridge
 ovs-vsctl --retry --may-exist add-br br-ex

snap-overlay/snap-openstack.yaml

@@ -57,10 +57,10 @@ setup:
     "{snap_common}/instances": 0755
     "{snap_common}/etc/microstack.rc": 0644
   snap-config-keys:
-    ospassword: 'ospassword'
-    extgateway: 'extgateway'
-    extcidr: 'extcidr'
-    dns: 'dns'
+    ospassword: 'questions.os-password'
+    extgateway: 'questions.ext-gateway'
+    extcidr: 'questions.ext-cidr'
+    dns: 'questions.dns'
 entry_points:
   keystone-manage:
     binary: "{snap}/bin/keystone-manage"

snap/hooks/configure

@@ -1,17 +1,6 @@
 #!/bin/bash
 set -ex
 
-ospassword=$(snapctl get ospassword)
-extgateway=$(snapctl get extgateway)
-extcidr=$(snapctl get extcidr)
-dns=$(snapctl get dns)
-
-if [ -z "$ospassword" -o -z "$extgateway" -o -z "$dns" -o -z "$extcidr"]; then
-    echo "Missing required config value."
-    snapctl get microstack
-    exit 1
-fi
-
 snap-openstack setup # Write out templates
 
 source $SNAP_COMMON/etc/microstack.rc

snap/hooks/install

@@ -1,11 +1,21 @@
 #!/bin/bash
 set -ex
 
+# Config
+# Set default answers to the questions that microstack.init asks.
+# TODO: put this in a nice yaml format, and parse it.
 snapctl set \
-        ospassword=keystone \
-        extgateway=10.20.20.1 \
-        extcidr=10.20.20.1/24 \
-        dns=1.1.1.1
+        questions.ip-forwarding=true \
+        questions.dns=1.1.1.1 \
+        questions.ext-gateway=10.20.20.1 \
+        questions.ext-cidr=10.20.20.1/24 \
+        questions.os-password=keystone \
+        questions.rabbit-mq=true \
+        questions.database-setup=true \
+        questions.nova-setup=true \
+        questions.neutron-setup=true \
+        questions.glance-setup=true \
+        questions.post-setup=true \
 
 # MySQL snapshot for speedy install
 # snapshot is a mysql data dir with
@@ -22,6 +32,3 @@ done
 mkdir -p ${SNAP_COMMON}/etc/horizon/local_settings.d
 
 snap-openstack setup  # Sets up templates for the first time.
-
-
-

snapcraft.yaml

@@ -830,7 +830,6 @@ parts:
   init:
     plugin: python
     python-version: python3
-    python-packages:
-      - pymysql
-      - wget
+    requirements:
+      - requirements.txt  # Relative to source path, so tools/init/req...txt
     source: tools/init

tests/basic-test.sh

@@ -85,7 +85,7 @@ fi
 
 # Install the snap under test -- try again if the machine is not yet ready.
 $PREFIX sudo snap install --classic --dangerous microstack*.snap
-$PREFIX sudo /snap/bin/microstack.init
+$PREFIX sudo /snap/bin/microstack.init --auto
 
 # Comment out the above and uncomment below to install the version of
 # the snap from the store.

tools/init/init/main.py

@@ -33,16 +33,27 @@ import sys
 from init.config import log
 
 from init import questions
+from init.shell import check
+
+# Figure out whether to prompt for user input, and which type of node
+# we're running.
+# TODO drop in argparse and formalize this.
+COMPUTE = '--compute' in sys.argv
+CONTROL = '--control' in sys.argv
+AUTO = ('--auto' in sys.argv) or COMPUTE or CONTROL
 
 
 def main() -> None:
     question_list = [
-        questions.Setup(),
+        questions.Dns(),
+        questions.ExtGateway(),
+        questions.ExtCidr(),
+        questions.OsPassword(),  # TODO: turn this off if COMPUTE.
         questions.IpForwarding(),
         # The following are not yet implemented:
         # questions.VmSwappiness(),
         # questions.FileHandleLimits(),
-        questions.RabbitMQ(),
+        questions.RabbitMq(),
         questions.DatabaseSetup(),
         questions.NovaSetup(),
         questions.NeutronSetup(),
@@ -50,7 +61,27 @@ def main() -> None:
         questions.PostSetup(),
     ]
 
+    # If we are setting up a "control" or "compute" node, override
+    # some of the default yes/no questions.
+    # TODO: move this into a nice little yaml parsing lib, and
+    # allow people to pass in a config file from the command line.
+    if CONTROL:
+        check('snapctl', 'set', 'questions.nova-setup=false')
+
+    if COMPUTE:
+        check('snapctl', 'set', 'questions.rabbit-mq=false')
+        check('snapctl', 'set', 'questions.database-setup=false')
+        check('snapctl', 'set', 'questions.neutron-setup=false')
+        check('snapctl', 'set', 'questions.glance-setup=false')
+
     for question in question_list:
+        if AUTO:
+            # If we are automatically answering questions, replace the
+            # prompt for user input with a function that returns None,
+            # causing the question to fall back to the already set
+            # default
+            question._input_func = lambda prompt: None
+
         try:
             question.ask()
         except questions.ConfigError as e:

tools/init/init/question.py

@@ -24,6 +24,10 @@ limitations under the License.
 
 from typing import Tuple
 
+import inflection
+
+from init import shell
+
 
 class InvalidQuestion(Exception):
     """Exception to raies in the case where a Question subclass has not
@@ -56,20 +60,20 @@ class Question():
 
     """
     _valid_types = [
-        'binary',  # Yes or No, and variants thereof
+        'boolean',  # Yes or No, and variants thereof
         'string',  # Accept (and sanitize) any string
         'auto'  # Don't actually ask a question -- just execute self.yes(True)
     ]
 
     _question = '(required)'
-    _type = 'auto'  # can be binary, string or auto
+    _type = 'auto'  # can be boolean, string or auto
     _invalid_prompt = 'Please answer Yes or No.'
     _retries = 3
     _input_func = input
 
     def __init__(self):
 
-        if self._type not in ['binary', 'string', 'auto']:
+        if self._type not in ['boolean', 'string', 'auto']:
             raise InvalidQuestion(
                 'Invalid type {} specified'.format(self._type))
 
@@ -84,13 +88,11 @@ class Question():
         if self._type == 'auto':
             return True, True
 
-        answer = answer.decode('utf8')
-
         if self._type == 'string':
             # TODO Santize this!
             return answer, True
 
-        # self._type is binary
+        # self._type is boolean
         if answer.lower() in ['y', 'yes']:
             return True, True
 
@@ -99,31 +101,103 @@ class Question():
 
         return answer, False
 
+    def _load(self):
+        """Get the current value of the answer to this question.
+
+        Useful for loading defaults during init, and for loading
+        operator specified settings during updates.
+
+        """
+        # Translate the CamelCase name of this class to the dash
+        # seperated name of a key in the snapctl config.
+        key = inflection.dasherize(
+            inflection.underscore(self.__class__.__name__))
+
+        answer = shell.check_output(
+            'snapctl', 'get', 'questions.{key}'.format(key=key)
+        )
+        # Convert boolean values in to human friendly "yes" or "no"
+        # values.
+        if answer.strip().lower() == 'true':
+            answer = 'yes'
+        if answer.strip().lower() == 'false':
+            answer = 'no'
+
+        return answer
+
+    def _save(self, answer):
+        """Save off our answer, for later retrieval.
+
+        Store the value of this question's answer in the questions
+        namespace in the snap config.
+
+        """
+        key = inflection.dasherize(
+            inflection.underscore(self.__class__.__name__))
+
+        # By this time in the process 'yes' or 'no' answers will have
+        # been converted to booleans. Convert them to a lowercase
+        # 'true' or 'false' string for storage in the snapctl config.
+        if self._type == 'boolean':
+            answer = str(answer).lower()
+
+        shell.check('snapctl', 'set', 'questions.{key}={val}'.format(
+            key=key, val=answer))
+
+        return answer
+
     def yes(self, answer: str) -> None:
-        """Routine to run if the user answers 'yes' or with a value."""
-        raise AnswerNotImplemented('You must override this method.')
+        """Routine to run if the user answers 'yes' or with a value.
+
+        Can be a noop.
+
+        """
+        pass
 
     def no(self, answer: str) -> None:
-        """Routine to run if the user answers 'no'"""
-        raise AnswerNotImplemented('You must override this method.')
+        """Routine to run if the user answers 'no'
+
+        Can be a noop.
+
+        """
+        pass
+
+    def after(self, answer: str) -> None:
+        """Routine to run after the answer has been saved to snapctl config.
+
+        Can be a noop.
+
+        """
+        pass
 
     def ask(self) -> None:
-        """
-        Ask the user a question.
+        """Ask the user a question.
 
         Run self.yes or self.no as appropriate. Raise an error if the
         user cannot specify a valid answer after self._retries tries.
 
+        Save off the answer for later retrieval, and run any cleanup
+        routines.
+
         """
-        prompt = self._question
+        default = self._load()
+
+        prompt = "{question}{choice}[default={default}] > ".format(
+            question=self._question,
+            choice=' (yes/no) ' if self._type == 'boolean' else ' ',
+            default=default)
 
         for i in range(0, self._retries):
             awr, valid = self._validate(
-                self._type == 'auto' or self._input_func(prompt))
+                self._type == 'auto' or self._input_func(prompt) or default)
             if valid:
                 if awr:
-                    return self.yes(awr)
-                return self.no(awr)
-            prompt = '{} is not valid. {}'.format(awr, self._invalid_prompt)
+                    self.yes(awr)
+                else:
+                    self.no(awr)
+                self._save(awr)
+                self.after(awr)
+                return awr
+            prompt = '{} is not valid. {} > '.format(awr, self._invalid_prompt)
 
         raise InvalidAnswer('Too many invalid answers.')

tools/init/init/questions.py

@@ -43,31 +43,27 @@ class ConfigError(Exception):
     """
 
 
-class Setup(Question):
-    """Prepare our environment.
+class ConfigQuestion(Question):
+    """Question class that simply asks for and sets a config value.
 
-    Check to make sure that everything is in place, and populate our
-    config object and os.environ with the correct values.
+    All we need to do is run 'snap-openstack setup' after we have saved
+    off the value. The value to be set is specified by the name of the
+    question class.
 
     """
-    def yes(self, answer: str) -> None:
-        """Since this is an auto question, we always execute yes."""
-        log.info('Loading config and writing templates ...')
+    def after(self, answer):
+        """Our value has been saved.
 
-        log.info('Validating config ...')
-        for key in ['ospassword', 'extgateway', 'extcidr', 'dns']:
-            val = check_output('snapctl', 'get', key)
-            if not val:
-                raise ConfigError(
-                    'Expected config value {} is not set.'.format(key))
-            _env[key] = val
+        Run 'snap-openstack setup' to write it out, and load any changes to
+        microstack.rc.
 
-        log.info('Writing out templates ...')
+        # TODO this is a bit messy and redundant. Come up with a clean
+        way of loading and writing config after the run of
+        ConfigQuestions have been asked.
+
+        """
         check('snap-openstack', 'setup')
 
-        # Parse microstack.rc, and load into _env
-        # TODO: write something more robust (this breaks on comments
-        # at end of line.)
         mstackrc = '{SNAP_COMMON}/etc/microstack.rc'.format(**_env)
         with open(mstackrc, 'r') as rc_file:
             for line in rc_file.readlines():
@@ -77,10 +73,73 @@ class Setup(Question):
                 _env[key.strip()] = val.strip()
 
 
+class Dns(Question):
+    """Possibly override default dns."""
+
+    _type = 'string'
+    _question = 'DNS to use'
+
+    def yes(self, answer: str):
+        """Override the default dhcp_agent.ini file."""
+
+        file_path = '{SNAP_COMMON}/etc/neutron/dhcp_agent.ini'.format(**_env)
+
+        with open(file_path, 'w') as f:
+            f.write("""\
+[DEFAULT]
+interface_driver = openvswitch
+dhcp_driver = neutron.agent.linux.dhcp.Dnsmasq
+enable_isolated_metadata = True
+dnsmasq_dns_servers = {answer}
+""".format(answer=answer))
+
+        # Neutron is not actually started at this point, so we don't
+        # need to restart.
+        # TODO: This isn't idempotent, because it will behave
+        # differently if we re-run this script when neutron *is*
+        # started. Need to figure that out.
+
+
+class ExtGateway(ConfigQuestion):
+    """Possibly override default ext gateway."""
+
+    _type = 'string'
+    _question = 'External Gateway'
+
+    def yes(self, answer):
+        # Preserve old behavior.
+        # TODO: update this
+        _env['extgateway'] = answer
+
+
+class ExtCidr(ConfigQuestion):
+    """Possibly override the cidr."""
+
+    _type = 'string'
+    _question = 'External Ip Range'
+
+    def yes(self, answer):
+        # Preserve old behavior.
+        # TODO: update this
+        _env['extcidr'] = answer
+
+
+class OsPassword(ConfigQuestion):
+    _type = 'string'
+    _question = 'Openstack Admin Password'
+
+    def yes(self, answer):
+        # Preserve old behavior.
+        # TODO: update this
+        _env['ospassword'] = answer
+
+    # TODO obfuscate the password!
+
+
 class IpForwarding(Question):
     """Possibly setup IP forwarding."""
 
-    _type = 'auto'  # Auto for now, to maintain old behavior.
+    _type = 'boolean'  # Auto for now, to maintain old behavior.
     _question = 'Do you wish to setup ip forwarding? (recommended)'
 
     def yes(self, answer: str) -> None:
@@ -92,7 +151,7 @@ class IpForwarding(Question):
 
 class VmSwappiness(Question):
 
-    _type = 'binary'
+    _type = 'boolean'
     _question = 'Do you wish to set vm swappiness to 1? (recommended)'
 
     def yes(self, answer: str) -> None:
@@ -102,7 +161,7 @@ class VmSwappiness(Question):
 
 class FileHandleLimits(Question):
 
-    _type = 'binary'
+    _type = 'boolean'
     _question = 'Do you wish to increase file handle limits? (recommended)'
 
     def yes(self, answer: str) -> None:
@@ -110,9 +169,11 @@ class FileHandleLimits(Question):
         pass
 
 
-class RabbitMQ(Question):
+class RabbitMq(Question):
     """Wait for Rabbit to start, then setup permissions."""
 
+    _type = 'boolean'
+
     def _wait(self) -> None:
         nc_wait(_env['extgateway'], '5672')
         log_file = '{SNAP_COMMON}/log/rabbitmq/startup_log'.format(**_env)
@@ -142,6 +203,8 @@ class RabbitMQ(Question):
 class DatabaseSetup(Question):
     """Setup keystone permissions, then setup all databases."""
 
+    _type = 'boolean'
+
     def _wait(self) -> None:
         nc_wait(_env['extgateway'], '3306')
         log_wait('{SNAP_COMMON}/log/mysql/error.log'.format(**_env),
@@ -210,6 +273,8 @@ class DatabaseSetup(Question):
 class NovaSetup(Question):
     """Create all relevant nova users and services."""
 
+    _type = 'boolean'
+
     def _flavors(self) -> None:
         """Create default flavors."""
 
@@ -311,6 +376,8 @@ class NovaSetup(Question):
 class NeutronSetup(Question):
     """Create all relevant neutron services and users."""
 
+    _type = 'boolean'
+
     def yes(self, answer: str) -> None:
         log.info('Configuring Neutron')
 
@@ -373,6 +440,8 @@ class NeutronSetup(Question):
 class GlanceSetup(Question):
     """Setup glance, and download an initial Cirros image."""
 
+    _type = 'boolean'
+
     def _fetch_cirros(self) -> None:
 
         if call('openstack', 'image', 'show', 'cirros'):

tools/init/requirements.txt

@@ -1,2 +1,3 @@
 pymysql
 wget
+inflection

tools/init/test-requirements.txt

@@ -1,4 +1,5 @@
+flake8
+mock
 pylint
 pep8
 stestr
-flake8

tools/init/tests/test_question.py

@@ -2,6 +2,8 @@ import sys
 import os
 import unittest
 
+import mock
+
 # TODO: drop in test runner and get rid of this line.
 sys.path.append(os.getcwd())  # noqa
 
@@ -20,10 +22,6 @@ class InvalidTypeQuestion(Question):
     _type = 'foo'
 
 
-class IncompleteQuestion(Question):
-    _type = 'auto'
-
-
 class GoodAutoQuestion(Question):
     _type = 'auto'
 
@@ -31,8 +29,8 @@ class GoodAutoQuestion(Question):
         return 'I am a good question!'
 
 
-class GoodBinaryQuestion(Question):
-    _type = 'binary'
+class GoodBooleanQuestion(Question):
+    _type = 'boolean'
 
     def yes(self, answer):
         return True
@@ -77,16 +75,14 @@ class TestQuestionClass(unittest.TestCase):
 
     def test_valid_type(self):
 
-        self.assertTrue(GoodBinaryQuestion())
+        self.assertTrue(GoodBooleanQuestion())
 
-    def test_not_implemented(self):
+    @mock.patch('init.question.shell.check_output')
+    @mock.patch('init.question.shell.check')
+    def test_auto_question(self, mock_check, mock_check_output):
+        mock_check_output.return_value = ''
 
-        with self.assertRaises(AnswerNotImplemented):
-            IncompleteQuestion().ask()
-
-    def test_auto_question(self):
-
-        self.assertEqual(GoodAutoQuestion().ask(), 'I am a good question!')
+        self.assertEqual(GoodAutoQuestion().ask(), True)
 
 
 class TestInput(unittest.TestCase):
@@ -97,29 +93,40 @@ class TestInput(unittest.TestCase):
     class's input handler.
 
     """
-    def test_binary_question(self):
+    @mock.patch('init.question.shell.check_output')
+    @mock.patch('init.question.shell.check')
+    def test_boolean_question(self, mock_check, mock_check_output):
+        mock_check_output.return_value = 'true'
 
-        q = GoodBinaryQuestion()
+        q = GoodBooleanQuestion()
 
         for answer in ['yes', 'Yes', 'y']:
-            q._input_func = lambda x: answer.encode('utf8')
+            q._input_func = lambda x: answer
             self.assertTrue(q.ask())
 
         for answer in ['No', 'n', 'no']:
-            q._input_func = lambda x: answer.encode('utf8')
+            q._input_func = lambda x: answer
             self.assertFalse(q.ask())
 
         with self.assertRaises(InvalidAnswer):
-            q._input_func = lambda x: 'foo'.encode('utf8')
+            q._input_func = lambda x: 'foo'
             q.ask()
 
-    def test_string_question(self):
+    @mock.patch('init.question.shell.check_output')
+    @mock.patch('init.question.shell.check')
+    def test_string_question(self, mock_check, mock_check_output):
+        mock_check_output.return_value = 'somedefault'
+
         q = GoodStringQuestion()
 
-        for answer in ['foo', 'bar', 'baz', '', 'yadayadayada']:
-            q._input_func = lambda x: answer.encode('utf8')
+        for answer in ['foo', 'bar', 'baz', 'yadayadayada']:
+            q._input_func = lambda x: answer
             self.assertEqual(answer, q.ask())
 
+        # Verify that a blank answer defaults properly
+        q._input_func = lambda x: ''
+        self.assertEqual('somedefault', q.ask())
+
 
 if __name__ == '__main__':
     unittest.main()

tox.ini

@@ -37,6 +37,7 @@ commands =
 
 [testenv:basic]
 # Just run basic_test.sh, with multipass support.
+deps = -r{toxinidir}/test-requirements.txt
 commands =
     {toxinidir}/tools/basic_setup.sh
     {toxinidir}/tests/basic-test.sh -m