Merge lp:~justin-fathomdb/nova/justinsb-hpsan into lp:~hudson-openstack/nova/trunk

[justinsb]

Support HP/LeftHand SANs. We control the SAN by SSHing and issuing CLIQ commands. Also improved the way iSCSI volumes are mounted: try to store the iSCSI connection info in the volume entity, in preference to doing discovery. Also CHAP authentication support.

CHAP support is necessary to avoid the attach-volume command require an export on the SAN device. If we had to do that, the attach command would have to hit both the volume controller and the compute controller, and that would be complex.

[lp:~justin-fathomdb/nova/justinsb-hpsan]

658. By justinsb on 2011-02-09

Fixed debug strings for i18n

[Jay Pipes (jaypipes)]

Hi! As I said on IRC, not much I can review as far as the volume stuff goes, so here is some general feedback :)

1 === added file 'nova/db/sqlalchemy/migrate_repo/versions/003_cactus.py'

As you mentioned on IRC, I think it would be best to name this something like 003_add_provider_columns.py instead of 003_cactus.py, to make the actual migration file more descriptive of its contents...

74 + # Create tables
75 + for table in (
76 + #certificates, consoles, console_pools, instance_actions
77 + ):
78 + try:
79 + table.create()
80 + except Exception:
81 + logging.info(repr(table))
82 + logging.exception('Exception while creating table')
83 + raise
84 +
85 + # Alter column types
86 + # auth_tokens.c.user_id.alter(type=String(length=255,
87 + # convert_unicode=False,
88 + # assert_unicode=None,
89 + # unicode_error=None,
90 + # _warn_on_bytestring=False))

I recognize you probably commented that stuff out thinking that we are only using a single 003_cactus.py migration file, but if that migration file is renamed to something more descriptive (and we can have >1 migration file per release), obviously I think you should remove all that commented out code :)

Missed i18n in these locations:

139 + LOG.warn("ISCSI provider_location not stored, using discovery")
175 + LOG.debug("ISCSI Discovery: Found %s" % (location))
552 + raise exception.Error("local_path not supported")

For 175 above, please use the following instead:

LOG.debug(_("ISCSI Discovery: Found %s"), location)

Not sure if you meant to i18n this or not :)

457 + message = ("Unexpected number of virtual ips for cluster %s. "
458 + "Result=%s" %
459 + (cluster_name, ElementTree.tostring(cluster_xml)))

But if so, should be:

message = _("Unexpected number of virtual ips for cluster %(cluster)s. "
            "Result=%(result)s" % {'cluster': cluster_name,
                           'result': ElementTree.tostring(cluster_xml)})

Other than those little nits above, the only other advice I could give would be to maybe sprinkle a few LOG.debug()s around HpSanISCSIDriver methods and maybe add a bit of docstring documentation to HpSanISCSIDriver that gives the rough flow of the commands executed against the volume controller?

Cheers, and thanks for a great contribution!
jay

review: Needs Fixing

[lp:~justin-fathomdb/nova/justinsb-hpsan]

659. By justinsb on 2011-02-14

i18n fixes

660. By justinsb on 2011-02-18

Merge branch 'master' into hpsan

Conflicts:
 nova/volume/manager.py

661. By justinsb on 2011-02-18

Renamed migration 003 to 006 (likely to be right number, I think)

662. By justinsb on 2011-02-18

Documentation, debug output & formatting improvements

Unmerged revisions

662. By justinsb on 2011-02-18

Documentation, debug output & formatting improvements

661. By justinsb on 2011-02-18

Renamed migration 003 to 006 (likely to be right number, I think)

660. By justinsb on 2011-02-18

Merge branch 'master' into hpsan

Conflicts:
 nova/volume/manager.py

659. By justinsb on 2011-02-14

i18n fixes

658. By justinsb on 2011-02-09

Fixed debug strings for i18n

657. By justinsb on 2011-02-09

Improved handling of missing project_id

656. By justinsb on 2011-02-09

Fix pylint/pep8 issues. Also store provider_location for Solaris iSCSI provider

655. By justinsb on 2011-02-09

Fixed whitespace for PEP8 compliance

654. By justinsb on 2011-02-09

Merge branch 'master' into hpsan

Conflicts:
 nova/volume/san.py

653. By justinsb on 2011-02-09

Small bugfixes from testing. Now works.

=== modified file 'nova/api/openstack/servers.py'
=== added file 'nova/api/openstack/zones.py'
--- nova/api/openstack/zones.py	1970-01-01 00:00:00 +0000
+++ nova/api/openstack/zones.py	2011-02-18 18:41:49 +0000
@@ -0,0 +1,80 @@
+# Copyright 2010 OpenStack LLC.
+# All Rights Reserved.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+import common
+import logging
+
+from nova import flags
+from nova import wsgi
+from nova import db
+
+
+FLAGS = flags.FLAGS
+
+
+def _filter_keys(item, keys):
+    """
+    Filters all model attributes except for keys
+    item is a dict
+
+    """
+    return dict((k, v) for k, v in item.iteritems() if k in keys)
+
+
+def _scrub_zone(zone):
+    return _filter_keys(zone, ('id', 'api_url'))
+
+
+class Controller(wsgi.Controller):
+
+    _serialization_metadata = {
+        'application/xml': {
+            "attributes": {
+                "zone": ["id", "api_url"]}}}
+
+    def index(self, req):
+        """Return all zones in brief"""
+        items = db.zone_get_all(req.environ['nova.context'])
+        items = common.limited(items, req)
+        items = [_scrub_zone(item) for item in items]
+        return dict(zones=items)
+
+    def detail(self, req):
+        """Return all zones in detail"""
+        return self.index(req)
+
+    def show(self, req, id):
+        """Return data about the given zone id"""
+        zone_id = int(id)
+        zone = db.zone_get(req.environ['nova.context'], zone_id)
+        return dict(zone=_scrub_zone(zone))
+
+    def delete(self, req, id):
+        zone_id = int(id)
+        db.zone_delete(req.environ['nova.context'], zone_id)
+        return {}
+
+    def create(self, req):
+        context = req.environ['nova.context']
+        env = self._deserialize(req.body, req)
+        zone = db.zone_create(context, env["zone"])
+        return dict(zone=_scrub_zone(zone))
+
+    def update(self, req, id):
+        context = req.environ['nova.context']
+        env = self._deserialize(req.body, req)
+        zone_id = int(id)
+        zone = db.zone_update(context, zone_id, env["zone"])
+        return dict(zone=_scrub_zone(zone))

=== renamed file 'nova/api/openstack/zones.py' => 'nova/api/openstack/zones.py.moved'
=== modified file 'nova/compute/api.py'
=== modified file 'nova/compute/manager.py'
=== modified file 'nova/db/api.py'
=== added file 'nova/db/sqlalchemy/migrate_repo/versions/003_add_label_to_networks.py'
--- nova/db/sqlalchemy/migrate_repo/versions/003_add_label_to_networks.py	1970-01-01 00:00:00 +0000
+++ nova/db/sqlalchemy/migrate_repo/versions/003_add_label_to_networks.py	2011-02-18 18:41:49 +0000
@@ -0,0 +1,51 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+
+# Copyright 2011 OpenStack LLC
+# All Rights Reserved.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+from sqlalchemy import *
+from migrate import *
+
+from nova import log as logging
+
+
+meta = MetaData()
+
+
+networks = Table('networks', meta,
+        Column('id', Integer(),  primary_key=True, nullable=False),
+        )
+
+
+#
+# New Tables
+#
+
+
+#
+# Tables to alter
+#
+
+networks_label = Column(
+        'label',
+        String(length=255, convert_unicode=False, assert_unicode=None,
+               unicode_error=None, _warn_on_bytestring=False))
+
+
+def upgrade(migrate_engine):
+    # Upgrade operations go here. Don't create your own engine;
+    # bind migrate_engine to your metadata
+    meta.bind = migrate_engine
+    networks.create_column(networks_label)

=== renamed file 'nova/db/sqlalchemy/migrate_repo/versions/003_add_label_to_networks.py' => 'nova/db/sqlalchemy/migrate_repo/versions/003_add_label_to_networks.py.moved'
=== added file 'nova/db/sqlalchemy/migrate_repo/versions/004_add_zone_tables.py'
--- nova/db/sqlalchemy/migrate_repo/versions/004_add_zone_tables.py	1970-01-01 00:00:00 +0000
+++ nova/db/sqlalchemy/migrate_repo/versions/004_add_zone_tables.py	2011-02-18 18:41:49 +0000
@@ -0,0 +1,61 @@
+# Copyright 2010 OpenStack LLC.
+# All Rights Reserved.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+from sqlalchemy import *
+from migrate import *
+
+from nova import log as logging
+
+
+meta = MetaData()
+
+
+#
+# New Tables
+#
+zones = Table('zones', meta,
+        Column('created_at', DateTime(timezone=False)),
+        Column('updated_at', DateTime(timezone=False)),
+        Column('deleted_at', DateTime(timezone=False)),
+        Column('deleted', Boolean(create_constraint=True, name=None)),
+        Column('id', Integer(),  primary_key=True, nullable=False),
+        Column('api_url',
+               String(length=255, convert_unicode=False, assert_unicode=None,
+                      unicode_error=None, _warn_on_bytestring=False)),
+        Column('username',
+               String(length=255, convert_unicode=False, assert_unicode=None,
+                      unicode_error=None, _warn_on_bytestring=False)),
+        Column('password',
+               String(length=255, convert_unicode=False, assert_unicode=None,
+                      unicode_error=None, _warn_on_bytestring=False)),
+        )
+
+
+#
+# Tables to alter
+#
+
+# (none currently)
+
+
+def upgrade(migrate_engine):
+    # Upgrade operations go here. Don't create your own engine;
+    # bind migrate_engine to your metadata
+    meta.bind = migrate_engine
+    for table in (zones, ):
+        try:
+            table.create()
+        except Exception:
+            logging.info(repr(table))

=== renamed file 'nova/db/sqlalchemy/migrate_repo/versions/004_add_zone_tables.py' => 'nova/db/sqlalchemy/migrate_repo/versions/004_add_zone_tables.py.moved'
=== added file 'nova/db/sqlalchemy/migrate_repo/versions/006_add_provider_data_to_volumes.py'
--- nova/db/sqlalchemy/migrate_repo/versions/006_add_provider_data_to_volumes.py	1970-01-01 00:00:00 +0000
+++ nova/db/sqlalchemy/migrate_repo/versions/006_add_provider_data_to_volumes.py	2011-02-18 18:41:49 +0000
@@ -0,0 +1,72 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+
+# Copyright 2011 Justin Santa Barbara.
+# All Rights Reserved.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+from sqlalchemy import *
+from migrate import *
+
+from nova import log as logging
+
+
+meta = MetaData()
+
+
+# Table stub-definitions
+# Just for the ForeignKey and column creation to succeed, these are not the
+# actual definitions of instances or services.
+#
+volumes = Table('volumes', meta,
+                Column('id', Integer(), primary_key=True, nullable=False),
+                )
+
+
+#
+# New Tables
+#
+# None
+
+#
+# Tables to alter
+#
+# None
+
+#
+# Columns to add to existing tables
+#
+
+volumes_provider_location = Column('provider_location',
+                                   String(length=256,
+                                          convert_unicode=False,
+                                          assert_unicode=None,
+                                          unicode_error=None,
+                                          _warn_on_bytestring=False))
+
+volumes_provider_auth = Column('provider_auth',
+                               String(length=256,
+                                      convert_unicode=False,
+                                      assert_unicode=None,
+                                      unicode_error=None,
+                                      _warn_on_bytestring=False))
+
+
+def upgrade(migrate_engine):
+    # Upgrade operations go here. Don't create your own engine;
+    # bind migrate_engine to your metadata
+    meta.bind = migrate_engine
+
+    # Add columns to existing tables
+    volumes.create_column(volumes_provider_location)
+    volumes.create_column(volumes_provider_auth)

=== modified file 'nova/db/sqlalchemy/models.py'
--- nova/db/sqlalchemy/models.py	2011-02-17 21:39:03 +0000
+++ nova/db/sqlalchemy/models.py	2011-02-18 18:41:49 +0000
@@ -243,6 +243,9 @@
     display_name = Column(String(255))
     display_description = Column(String(255))
 
+    provider_location = Column(String(255))
+    provider_auth = Column(String(255))
+
 
 class Quota(BASE, NovaBase):
     """Represents quota overrides for a project."""

=== modified file 'nova/network/manager.py'
=== added file 'nova/tests/api/openstack/test_common.py'
--- nova/tests/api/openstack/test_common.py	1970-01-01 00:00:00 +0000
+++ nova/tests/api/openstack/test_common.py	2011-02-18 18:41:49 +0000
@@ -0,0 +1,161 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+
+# Copyright 2010 OpenStack LLC.
+# All Rights Reserved.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+"""
+Test suites for 'common' code used throughout the OpenStack HTTP API.
+"""
+
+import unittest
+
+from webob import Request
+
+from nova.api.openstack.common import limited
+
+
+class LimiterTest(unittest.TestCase):
+    """
+    Unit tests for the `nova.api.openstack.common.limited` method which takes
+    in a list of items and, depending on the 'offset' and 'limit' GET params,
+    returns a subset or complete set of the given items.
+    """
+
+    def setUp(self):
+        """
+        Run before each test.
+        """
+        self.tiny = range(1)
+        self.small = range(10)
+        self.medium = range(1000)
+        self.large = range(10000)
+
+    def test_limiter_offset_zero(self):
+        """
+        Test offset key works with 0.
+        """
+        req = Request.blank('/?offset=0')
+        self.assertEqual(limited(self.tiny, req), self.tiny)
+        self.assertEqual(limited(self.small, req), self.small)
+        self.assertEqual(limited(self.medium, req), self.medium)
+        self.assertEqual(limited(self.large, req), self.large[:1000])
+
+    def test_limiter_offset_medium(self):
+        """
+        Test offset key works with a medium sized number.
+        """
+        req = Request.blank('/?offset=10')
+        self.assertEqual(limited(self.tiny, req), [])
+        self.assertEqual(limited(self.small, req), self.small[10:])
+        self.assertEqual(limited(self.medium, req), self.medium[10:])
+        self.assertEqual(limited(self.large, req), self.large[10:1010])
+
+    def test_limiter_offset_over_max(self):
+        """
+        Test offset key works with a number over 1000 (max_limit).
+        """
+        req = Request.blank('/?offset=1001')
+        self.assertEqual(limited(self.tiny, req), [])
+        self.assertEqual(limited(self.small, req), [])
+        self.assertEqual(limited(self.medium, req), [])
+        self.assertEqual(limited(self.large, req), self.large[1001:2001])
+
+    def test_limiter_offset_blank(self):
+        """
+        Test offset key works with a blank offset.
+        """
+        req = Request.blank('/?offset=')
+        self.assertEqual(limited(self.tiny, req), self.tiny)
+        self.assertEqual(limited(self.small, req), self.small)
+        self.assertEqual(limited(self.medium, req), self.medium)
+        self.assertEqual(limited(self.large, req), self.large[:1000])
+
+    def test_limiter_offset_bad(self):
+        """
+        Test offset key works with a BAD offset.
+        """
+        req = Request.blank(u'/?offset=\u0020aa')
+        self.assertEqual(limited(self.tiny, req), self.tiny)
+        self.assertEqual(limited(self.small, req), self.small)
+        self.assertEqual(limited(self.medium, req), self.medium)
+        self.assertEqual(limited(self.large, req), self.large[:1000])
+
+    def test_limiter_nothing(self):
+        """
+        Test request with no offset or limit
+        """
+        req = Request.blank('/')
+        self.assertEqual(limited(self.tiny, req), self.tiny)
+        self.assertEqual(limited(self.small, req), self.small)
+        self.assertEqual(limited(self.medium, req), self.medium)
+        self.assertEqual(limited(self.large, req), self.large[:1000])
+
+    def test_limiter_limit_zero(self):
+        """
+        Test limit of zero.
+        """
+        req = Request.blank('/?limit=0')
+        self.assertEqual(limited(self.tiny, req), self.tiny)
+        self.assertEqual(limited(self.small, req), self.small)
+        self.assertEqual(limited(self.medium, req), self.medium)
+        self.assertEqual(limited(self.large, req), self.large[:1000])
+
+    def test_limiter_limit_medium(self):
+        """
+        Test limit of 10.
+        """
+        req = Request.blank('/?limit=10')
+        self.assertEqual(limited(self.tiny, req), self.tiny)
+        self.assertEqual(limited(self.small, req), self.small)
+        self.assertEqual(limited(self.medium, req), self.medium[:10])
+        self.assertEqual(limited(self.large, req), self.large[:10])
+
+    def test_limiter_limit_over_max(self):
+        """
+        Test limit of 3000.
+        """
+        req = Request.blank('/?limit=3000')
+        self.assertEqual(limited(self.tiny, req), self.tiny)
+        self.assertEqual(limited(self.small, req), self.small)
+        self.assertEqual(limited(self.medium, req), self.medium)
+        self.assertEqual(limited(self.large, req), self.large[:1000])
+
+    def test_limiter_limit_and_offset(self):
+        """
+        Test request with both limit and offset.
+        """
+        items = range(2000)
+        req = Request.blank('/?offset=1&limit=3')
+        self.assertEqual(limited(items, req), items[1:4])
+        req = Request.blank('/?offset=3&limit=0')
+        self.assertEqual(limited(items, req), items[3:1003])
+        req = Request.blank('/?offset=3&limit=1500')
+        self.assertEqual(limited(items, req), items[3:1003])
+        req = Request.blank('/?offset=3000&limit=10')
+        self.assertEqual(limited(items, req), [])
+
+    def test_limiter_custom_max_limit(self):
+        """
+        Test a max_limit other than 1000.
+        """
+        items = range(2000)
+        req = Request.blank('/?offset=1&limit=3')
+        self.assertEqual(limited(items, req, max_limit=2000), items[1:4])
+        req = Request.blank('/?offset=3&limit=0')
+        self.assertEqual(limited(items, req, max_limit=2000), items[3:])
+        req = Request.blank('/?offset=3&limit=2500')
+        self.assertEqual(limited(items, req, max_limit=2000), items[3:])
+        req = Request.blank('/?offset=3000&limit=10')
+        self.assertEqual(limited(items, req, max_limit=2000), [])

=== renamed file 'nova/tests/api/openstack/test_common.py' => 'nova/tests/api/openstack/test_common.py.moved'
=== added file 'nova/tests/api/openstack/test_zones.py'
--- nova/tests/api/openstack/test_zones.py	1970-01-01 00:00:00 +0000
+++ nova/tests/api/openstack/test_zones.py	2011-02-18 18:41:49 +0000
@@ -0,0 +1,141 @@
+# Copyright 2010 OpenStack LLC.
+# All Rights Reserved.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+import unittest
+
+import stubout
+import webob
+import json
+
+import nova.db
+from nova import context
+from nova import flags
+from nova.api.openstack import zones
+from nova.tests.api.openstack import fakes
+
+
+FLAGS = flags.FLAGS
+FLAGS.verbose = True
+
+
+def zone_get(context, zone_id):
+    return dict(id=1, api_url='http://foo.com', username='bob',
+                password='xxx')
+
+
+def zone_create(context, values):
+    zone = dict(id=1)
+    zone.update(values)
+    return zone
+
+
+def zone_update(context, zone_id, values):
+    zone = dict(id=zone_id, api_url='http://foo.com', username='bob',
+                password='xxx')
+    zone.update(values)
+    return zone
+
+
+def zone_delete(context, zone_id):
+    pass
+
+
+def zone_get_all(context):
+    return [
+        dict(id=1, api_url='http://foo.com', username='bob',
+                 password='xxx'),
+        dict(id=2, api_url='http://blah.com', username='alice',
+                 password='qwerty')
+    ]
+
+
+class ZonesTest(unittest.TestCase):
+    def setUp(self):
+        self.stubs = stubout.StubOutForTesting()
+        fakes.FakeAuthManager.auth_data = {}
+        fakes.FakeAuthDatabase.data = {}
+        fakes.stub_out_networking(self.stubs)
+        fakes.stub_out_rate_limiting(self.stubs)
+        fakes.stub_out_auth(self.stubs)
+
+        self.allow_admin = FLAGS.allow_admin_api
+        FLAGS.allow_admin_api = True
+
+        self.stubs.Set(nova.db, 'zone_get', zone_get)
+        self.stubs.Set(nova.db, 'zone_get_all', zone_get_all)
+        self.stubs.Set(nova.db, 'zone_update', zone_update)
+        self.stubs.Set(nova.db, 'zone_create', zone_create)
+        self.stubs.Set(nova.db, 'zone_delete', zone_delete)
+
+    def tearDown(self):
+        self.stubs.UnsetAll()
+        FLAGS.allow_admin_api = self.allow_admin
+
+    def test_get_zone_list(self):
+        req = webob.Request.blank('/v1.0/zones')
+        res = req.get_response(fakes.wsgi_app())
+        res_dict = json.loads(res.body)
+
+        self.assertEqual(res.status_int, 200)
+        self.assertEqual(len(res_dict['zones']), 2)
+
+    def test_get_zone_by_id(self):
+        req = webob.Request.blank('/v1.0/zones/1')
+        res = req.get_response(fakes.wsgi_app())
+        res_dict = json.loads(res.body)
+
+        self.assertEqual(res_dict['zone']['id'], 1)
+        self.assertEqual(res_dict['zone']['api_url'], 'http://foo.com')
+        self.assertFalse('password' in res_dict['zone'])
+        self.assertEqual(res.status_int, 200)
+
+    def test_zone_delete(self):
+        req = webob.Request.blank('/v1.0/zones/1')
+        res = req.get_response(fakes.wsgi_app())
+
+        self.assertEqual(res.status_int, 200)
+
+    def test_zone_create(self):
+        body = dict(zone=dict(api_url='http://blah.zoo', username='fred',
+                        password='fubar'))
+        req = webob.Request.blank('/v1.0/zones')
+        req.method = 'POST'
+        req.body = json.dumps(body)
+
+        res = req.get_response(fakes.wsgi_app())
+        res_dict = json.loads(res.body)
+
+        self.assertEqual(res.status_int, 200)
+        self.assertEqual(res_dict['zone']['id'], 1)
+        self.assertEqual(res_dict['zone']['api_url'], 'http://blah.zoo')
+        self.assertFalse('username' in res_dict['zone'])
+
+    def test_zone_update(self):
+        body = dict(zone=dict(username='zeb', password='sneaky'))
+        req = webob.Request.blank('/v1.0/zones/1')
+        req.method = 'PUT'
+        req.body = json.dumps(body)
+
+        res = req.get_response(fakes.wsgi_app())
+        res_dict = json.loads(res.body)
+
+        self.assertEqual(res.status_int, 200)
+        self.assertEqual(res_dict['zone']['id'], 1)
+        self.assertEqual(res_dict['zone']['api_url'], 'http://foo.com')
+        self.assertFalse('username' in res_dict['zone'])
+
+
+if __name__ == '__main__':
+    unittest.main()

=== renamed file 'nova/tests/api/openstack/test_zones.py' => 'nova/tests/api/openstack/test_zones.py.moved'
=== modified file 'nova/utils.py'
=== modified file 'nova/virt/xenapi/vmops.py'
=== modified file 'nova/virt/xenapi_conn.py'
=== modified file 'nova/volume/driver.py'
--- nova/volume/driver.py	2011-02-04 17:04:55 +0000
+++ nova/volume/driver.py	2011-02-18 18:41:49 +0000
@@ -21,6 +21,7 @@
 """
 
 import time
+import os
 
 from nova import exception
 from nova import flags
@@ -36,6 +37,8 @@
                     'Which device to export the volumes on')
 flags.DEFINE_string('num_shell_tries', 3,
                     'number of times to attempt to run flakey shell commands')
+flags.DEFINE_string('num_iscsi_scan_tries', 3,
+                    'number of times to rescan iSCSI target to find volume')
 flags.DEFINE_integer('num_shelves',
                     100,
                     'Number of vblade shelves')
@@ -294,40 +297,133 @@
         self._execute("sudo ietadm --op delete --tid=%s" %
                       iscsi_target)
 
-    def _get_name_and_portal(self, volume):
-        """Gets iscsi name and portal from volume name and host."""
+    def _do_iscsi_discovery(self, volume):
+        #TODO(justinsb): Deprecate discovery and use stored info
+        #NOTE(justinsb): Discovery won't work with CHAP-secured targets (?)
+        LOG.warn(_("ISCSI provider_location not stored, using discovery"))
+
         volume_name = volume['name']
-        host = volume['host']
+
         (out, _err) = self._execute("sudo iscsiadm -m discovery -t "
-                                    "sendtargets -p %s" % host)
+                                    "sendtargets -p %s" % (volume['host']))
         for target in out.splitlines():
             if FLAGS.iscsi_ip_prefix in target and volume_name in target:
-                (location, _sep, iscsi_name) = target.partition(" ")
-                break
-        iscsi_portal = location.split(",")[0]
-        return (iscsi_name, iscsi_portal)
+                return target
+        return None
+
+    def _get_iscsi_properties(self, volume):
+        """Gets iscsi configuration, ideally from saved information in the
+        volume entity, but falling back to discovery if need be."""
+
+        properties = {}
+
+        location = volume['provider_location']
+
+        if location:
+            # provider_location is the same format as iSCSI discovery output
+            properties['target_discovered'] = False
+        else:
+            location = self._do_iscsi_discovery(volume)
+
+            if not location:
+                raise exception.Error(_("Could not find iSCSI export "
+                                        " for volume %s") %
+                                      (volume['name']))
+
+            LOG.debug(_("ISCSI Discovery: Found %s") % (location))
+            properties['target_discovered'] = True
+
+        (iscsi_target, _sep, iscsi_name) = location.partition(" ")
+
+        iscsi_portal = iscsi_target.split(",")[0]
+
+        properties['target_iqn'] = iscsi_name
+        properties['target_portal'] = iscsi_portal
+
+        auth = volume['provider_auth']
+
+        if auth:
+            (auth_method, auth_username, auth_secret) = auth.split()
+
+            properties['auth_method'] = auth_method
+            properties['auth_username'] = auth_username
+            properties['auth_password'] = auth_secret
+
+        return properties
+
+    def _run_iscsiadm(self, iscsi_properties, iscsi_command):
+        command = ("sudo iscsiadm -m node -T %s -p %s %s" %
+                   (iscsi_properties['target_iqn'],
+                    iscsi_properties['target_portal'],
+                    iscsi_command))
+        (out, err) = self._execute(command)
+        LOG.debug("iscsiadm %s: stdout=%s stderr=%s" %
+                  (iscsi_command, out, err))
+        return (out, err)
+
+    def _iscsiadm_update(self, iscsi_properties, property_key, property_value):
+        iscsi_command = ("--op update -n %s -v %s" %
+                         (property_key, property_value))
+        return self._run_iscsiadm(iscsi_properties, iscsi_command)
 
     def discover_volume(self, volume):
         """Discover volume on a remote host."""
-        iscsi_name, iscsi_portal = self._get_name_and_portal(volume)
-        self._execute("sudo iscsiadm -m node -T %s -p %s --login" %
-                      (iscsi_name, iscsi_portal))
-        self._execute("sudo iscsiadm -m node -T %s -p %s --op update "
-                      "-n node.startup -v automatic" %
-                      (iscsi_name, iscsi_portal))
-        return "/dev/disk/by-path/ip-%s-iscsi-%s-lun-0" % (iscsi_portal,
-                                                           iscsi_name)
+        iscsi_properties = self._get_iscsi_properties(volume)
+
+        if not iscsi_properties['target_discovered']:
+            self._run_iscsiadm(iscsi_properties, "--op new")
+
+        if iscsi_properties.get('auth_method'):
+            self._iscsiadm_update(iscsi_properties,
+                                  "node.session.auth.authmethod",
+                                  iscsi_properties['auth_method'])
+            self._iscsiadm_update(iscsi_properties,
+                                  "node.session.auth.username",
+                                  iscsi_properties['auth_username'])
+            self._iscsiadm_update(iscsi_properties,
+                                  "node.session.auth.password",
+                                  iscsi_properties['auth_password'])
+
+        self._run_iscsiadm(iscsi_properties, "--login")
+
+        self._iscsiadm_update(iscsi_properties, "node.startup", "automatic")
+
+        mount_device = ("/dev/disk/by-path/ip-%s-iscsi-%s-lun-0" %
+                        (iscsi_properties['target_portal'],
+                         iscsi_properties['target_iqn']))
+
+        # The /dev/disk/by-path/... node is not always present immediately
+        # TODO(justinsb): This retry-with-delay is a pattern, move to utils?
+        tries = 0
+        while not os.path.exists(mount_device):
+            if tries >= FLAGS.num_iscsi_scan_tries:
+                raise exception.Error(_("iSCSI device not found at %s") %
+                                      (mount_device))
+
+            LOG.warn(_("ISCSI volume not yet found at: %(mount_device)s. "
+                       "Will rescan & retry.  Try number: %(tries)s") %
+                     locals())
+
+            # The rescan isn't documented as being necessary(?), but it helps
+            self._run_iscsiadm(iscsi_properties, "--rescan")
+
+            tries = tries + 1
+            if not os.path.exists(mount_device):
+                time.sleep(tries ** 2)
+
+        if tries != 0:
+            LOG.debug(_("Found iSCSI node %(mount_device)s "
+                        "(after %(tries)s rescans)") %
+                      locals())
+
+        return mount_device
 
     def undiscover_volume(self, volume):
         """Undiscover volume on a remote host."""
-        iscsi_name, iscsi_portal = self._get_name_and_portal(volume)
-        self._execute("sudo iscsiadm -m node -T %s -p %s --op update "
-                      "-n node.startup -v manual" %
-                      (iscsi_name, iscsi_portal))
-        self._execute("sudo iscsiadm -m node -T %s -p %s --logout " %
-                      (iscsi_name, iscsi_portal))
-        self._execute("sudo iscsiadm -m node --op delete "
-                      "--targetname %s" % iscsi_name)
+        iscsi_properties = self._get_iscsi_properties(volume)
+        self._iscsiadm_update(iscsi_properties, "node.startup", "manual")
+        self._run_iscsiadm(iscsi_properties, "--logout")
+        self._run_iscsiadm(iscsi_properties, "--op delete")
 
 
 class FakeISCSIDriver(ISCSIDriver):

=== modified file 'nova/volume/manager.py'
--- nova/volume/manager.py	2011-02-15 18:19:52 +0000
+++ nova/volume/manager.py	2011-02-18 18:41:49 +0000
@@ -107,11 +107,20 @@
             vol_size = volume_ref['size']
             LOG.debug(_("volume %(vol_name)s: creating lv of"
                     " size %(vol_size)sG") % locals())
-            self.driver.create_volume(volume_ref)
+            db_update = self.driver.create_volume(volume_ref)
+            if db_update:
+                self.db.volume_update(context, volume_ref['id'], db_update)
 
             LOG.debug(_("volume %s: creating export"), volume_ref['name'])
+<<<<<<< TREE
             self.driver.create_export(context, volume_ref)
         except Exception:
+=======
+            db_update = self.driver.create_export(context, volume_ref)
+            if db_update:
+                self.db.volume_update(context, volume_ref['id'], db_update)
+        except Exception:
+>>>>>>> MERGE-SOURCE
             self.db.volume_update(context,
                                   volume_ref['id'], {'status': 'error'})
             raise

=== modified file 'nova/volume/san.py'
--- nova/volume/san.py	2011-02-09 19:25:18 +0000
+++ nova/volume/san.py	2011-02-18 18:41:49 +0000
@@ -23,6 +23,8 @@
 import os
 import paramiko
 
+from xml.etree import ElementTree
+
 from nova import exception
 from nova import flags
 from nova import log as logging
@@ -41,37 +43,15 @@
                     'Password for SAN controller')
 flags.DEFINE_string('san_privatekey', '',
                     'Filename of private key to use for SSH authentication')
+flags.DEFINE_string('san_clustername', '',
+                    'Cluster name to use for creating volumes')
+flags.DEFINE_integer('san_ssh_port', 22,
+                    'SSH port to use with SAN')
 
 
 class SanISCSIDriver(ISCSIDriver):
     """ Base class for SAN-style storage volumes
         (storage providers we access over SSH)"""
-    #Override because SAN ip != host ip
-    def _get_name_and_portal(self, volume):
-        """Gets iscsi name and portal from volume name and host."""
-        volume_name = volume['name']
-
-        # TODO(justinsb): store in volume, remerge with generic iSCSI code
-        host = FLAGS.san_ip
-
-        (out, _err) = self._execute("sudo iscsiadm -m discovery -t "
-                                    "sendtargets -p %s" % host)
-
-        location = None
-        find_iscsi_name = self._build_iscsi_target_name(volume)
-        for target in out.splitlines():
-            if find_iscsi_name in target:
-                (location, _sep, iscsi_name) = target.partition(" ")
-                break
-        if not location:
-            raise exception.Error(_("Could not find iSCSI export "
-                                    " for volume %s") %
-                                  volume_name)
-
-        iscsi_portal = location.split(",")[0]
-        LOG.debug("iscsi_name=%s, iscsi_portal=%s" %
-                  (iscsi_name, iscsi_portal))
-        return (iscsi_name, iscsi_portal)
 
     def _build_iscsi_target_name(self, volume):
         return "%s%s" % (FLAGS.iscsi_target_prefix, volume['name'])
@@ -85,6 +65,7 @@
         ssh.set_missing_host_key_policy(paramiko.AutoAddPolicy())
         if FLAGS.san_password:
             ssh.connect(FLAGS.san_ip,
+                        port=FLAGS.san_ssh_port,
                         username=FLAGS.san_login,
                         password=FLAGS.san_password)
         elif FLAGS.san_privatekey:
@@ -92,10 +73,11 @@
             # It sucks that paramiko doesn't support DSA keys
             privatekey = paramiko.RSAKey.from_private_key_file(privatekeyfile)
             ssh.connect(FLAGS.san_ip,
+                        port=FLAGS.san_ssh_port,
                         username=FLAGS.san_login,
                         pkey=privatekey)
         else:
-            raise exception.Error("Specify san_password or san_privatekey")
+            raise exception.Error(_("Specify san_password or san_privatekey"))
         return ssh
 
     def _run_ssh(self, command, check_exit_code=True):
@@ -124,10 +106,10 @@
     def check_for_setup_error(self):
         """Returns an error if prerequisites aren't met"""
         if not (FLAGS.san_password or FLAGS.san_privatekey):
-            raise exception.Error("Specify san_password or san_privatekey")
+            raise exception.Error(_("Specify san_password or san_privatekey"))
 
         if not (FLAGS.san_ip):
-            raise exception.Error("san_ip must be set")
+            raise exception.Error(_("san_ip must be set"))
 
 
 def _collect_lines(data):
@@ -306,6 +288,17 @@
             self._run_ssh("pfexec /usr/sbin/stmfadm add-view -t %s %s" %
                           (target_group_name, luid))
 
+        #TODO(justinsb): Is this always 1? Does it matter?
+        iscsi_portal_interface = '1'
+        iscsi_portal = FLAGS.san_ip + ":3260," + iscsi_portal_interface
+
+        db_update = {}
+        db_update['provider_location'] = ("%s %s" %
+                                          (iscsi_portal,
+                                           iscsi_name))
+
+        return db_update
+
     def remove_export(self, context, volume):
         """Removes an export for a logical volume."""
 
@@ -333,3 +326,239 @@
         if self._is_lu_created(volume):
             self._run_ssh("pfexec /usr/sbin/sbdadm delete-lu %s" %
                           (luid))
+
+
+class HpSanISCSIDriver(SanISCSIDriver):
+    """Executes commands relating to HP/Lefthand SAN ISCSI volumes.
+    We use the CLIQ interface, over SSH.
+
+    Rough overview of CLIQ commands used:
+    CLIQ createVolume (creates the volume)
+    CLIQ getVolumeInfo (to discover the IQN etc)
+    CLIQ getClusterInfo (to discover the iSCSI target IP address)
+    CLIQ assignVolumeChap (exports it with CHAP security)
+
+    The 'trick' here is that the HP SAN enforces security by default, so
+    normally a volume mount would need both to configure the SAN in the volume
+    layer and do the mount on the compute layer.  Multi-layer operations are
+    not catered for at the moment in the nova architecture, so instead we
+    share the volume using CHAP at volume creation time.  Then the mount need
+    only use those CHAP credentials, so can take place exclusively in the
+    compute layer"""
+
+    def _cliq_run(self, verb, cliq_args):
+        """Runs a CLIQ command over SSH, without doing any result parsing"""
+        cliq_arg_strings = []
+        for k, v in cliq_args.items():
+            cliq_arg_strings.append(" %s=%s" % (k, v))
+        cmd = verb + ''.join(cliq_arg_strings)
+
+        return self._run_ssh(cmd)
+
+    def _cliq_run_xml(self, verb, cliq_args, check_cliq_result=True):
+        """Runs a CLIQ command over SSH, parsing and checking the output"""
+        cliq_args['output'] = 'XML'
+        (out, _err) = self._cliq_run(verb, cliq_args)
+
+        LOG.debug(_("CLIQ command returned %s"), out)
+
+        result_xml = ElementTree.fromstring(out)
+        if check_cliq_result:
+            response_node = result_xml.find("response")
+            if response_node is None:
+                msg = (_("Malformed response to CLIQ command "
+                         "%(verb)s %(cliq_args)s. Result=%(out)s") %
+                       locals())
+                raise exception.Error(msg)
+
+            result_code = response_node.attrib.get("result")
+
+            if result_code != "0":
+                msg = (_("Error running CLIQ command %(verb)s %(cliq_args)s. "
+                         " Result=%(out)s") %
+                       locals())
+                raise exception.Error(msg)
+
+        return result_xml
+
+    def _cliq_get_cluster_info(self, cluster_name):
+        """Queries for info about the cluster (including IP)"""
+        cliq_args = {}
+        cliq_args['clusterName'] = cluster_name
+        cliq_args['searchDepth'] = '1'
+        cliq_args['verbose'] = '0'
+
+        result_xml = self._cliq_run_xml("getClusterInfo", cliq_args)
+
+        return result_xml
+
+    def _cliq_get_cluster_vip(self, cluster_name):
+        """Gets the IP on which a cluster shares iSCSI volumes"""
+        cluster_xml = self._cliq_get_cluster_info(cluster_name)
+
+        vips = []
+        for vip in cluster_xml.findall("response/cluster/vip"):
+            vips.append(vip.attrib.get('ipAddress'))
+
+        if len(vips) == 1:
+            return vips[0]
+
+        _xml = ElementTree.tostring(cluster_xml)
+        msg = (_("Unexpected number of virtual ips for cluster "
+                 " %(cluster_name)s. Result=%(_xml)s") %
+               locals())
+        raise exception.Error(msg)
+
+    def _cliq_get_volume_info(self, volume_name):
+        """Gets the volume info, including IQN"""
+        cliq_args = {}
+        cliq_args['volumeName'] = volume_name
+        result_xml = self._cliq_run_xml("getVolumeInfo", cliq_args)
+
+        # Result looks like this:
+        #<gauche version="1.0">
+        #  <response description="Operation succeeded." name="CliqSuccess"
+        #            processingTime="87" result="0">
+        #    <volume autogrowPages="4" availability="online" blockSize="1024"
+        #       bytesWritten="0" checkSum="false" clusterName="Cluster01"
+        #       created="2011-02-08T19:56:53Z" deleting="false" description=""
+        #       groupName="Group01" initialQuota="536870912" isPrimary="true"
+        #       iscsiIqn="iqn.2003-10.com.lefthandnetworks:group01:25366:vol-b"
+        #       maxSize="6865387257856" md5="9fa5c8b2cca54b2948a63d833097e1ca"
+        #       minReplication="1" name="vol-b" parity="0" replication="2"
+        #       reserveQuota="536870912" scratchQuota="4194304"
+        #       serialNumber="9fa5c8b2cca54b2948a63d833097e1ca0000000000006316"
+        #       size="1073741824" stridePages="32" thinProvision="true">
+        #      <status description="OK" value="2"/>
+        #      <permission access="rw"
+        #            authGroup="api-34281B815713B78-(trimmed)51ADD4B7030853AA7"
+        #            chapName="chapusername" chapRequired="true" id="25369"
+        #            initiatorSecret="" iqn="" iscsiEnabled="true"
+        #            loadBalance="true" targetSecret="supersecret"/>
+        #    </volume>
+        #  </response>
+        #</gauche>
+
+        # Flatten the nodes into a dictionary; use prefixes to avoid collisions
+        volume_attributes = {}
+
+        volume_node = result_xml.find("response/volume")
+        for k, v in volume_node.attrib.items():
+            volume_attributes["volume." + k] = v
+
+        status_node = volume_node.find("status")
+        if not status_node is None:
+            for k, v in status_node.attrib.items():
+                volume_attributes["status." + k] = v
+
+        # We only consider the first permission node
+        permission_node = volume_node.find("permission")
+        if not permission_node is None:
+            for k, v in status_node.attrib.items():
+                volume_attributes["permission." + k] = v
+
+        LOG.debug(_("Volume info: %(volume_name)s => %(volume_attributes)s") %
+                  locals())
+        return volume_attributes
+
+    def create_volume(self, volume):
+        """Creates a volume."""
+        cliq_args = {}
+        cliq_args['clusterName'] = FLAGS.san_clustername
+        #TODO(justinsb): Should we default to inheriting thinProvision?
+        cliq_args['thinProvision'] = '1' if FLAGS.san_thin_provision else '0'
+        cliq_args['volumeName'] = volume['name']
+        if int(volume['size']) == 0:
+            cliq_args['size'] = '100MB'
+        else:
+            cliq_args['size'] = '%sGB' % volume['size']
+
+        self._cliq_run_xml("createVolume", cliq_args)
+
+        volume_info = self._cliq_get_volume_info(volume['name'])
+        cluster_name = volume_info['volume.clusterName']
+        iscsi_iqn = volume_info['volume.iscsiIqn']
+
+        #TODO(justinsb): Is this always 1? Does it matter?
+        cluster_interface = '1'
+
+        cluster_vip = self._cliq_get_cluster_vip(cluster_name)
+        iscsi_portal = cluster_vip + ":3260," + cluster_interface
+
+        db_update = {}
+        db_update['provider_location'] = ("%s %s" %
+                                          (iscsi_portal,
+                                           iscsi_iqn))
+
+        return db_update
+
+    def delete_volume(self, volume):
+        """Deletes a volume."""
+        cliq_args = {}
+        cliq_args['volumeName'] = volume['name']
+        cliq_args['prompt'] = 'false'  # Don't confirm
+
+        self._cliq_run_xml("deleteVolume", cliq_args)
+
+    def local_path(self, volume):
+        # TODO(justinsb): Is this needed here?
+        raise exception.Error(_("local_path not supported"))
+
+    def ensure_export(self, context, volume):
+        """Synchronously recreates an export for a logical volume."""
+        return self._do_export(context, volume, force_create=False)
+
+    def create_export(self, context, volume):
+        return self._do_export(context, volume, force_create=True)
+
+    def _do_export(self, context, volume, force_create):
+        """Supports ensure_export and create_export"""
+        volume_info = self._cliq_get_volume_info(volume['name'])
+
+        is_shared = 'permission.authGroup' in volume_info
+
+        db_update = {}
+
+        should_export = False
+
+        if force_create or not is_shared:
+            should_export = True
+            # Check that we have a project_id
+            project_id = volume['project_id']
+            if not project_id:
+                project_id = context.project_id
+
+            if project_id:
+                #TODO(justinsb): Use a real per-project password here
+                chap_username = 'proj_' + project_id
+                # HP/Lefthand requires that the password be >= 12 characters
+                chap_password = 'project_secret_' + project_id
+            else:
+                msg = (_("Could not determine project for volume %s, "
+                         "can't export") %
+                         (volume['name']))
+                if force_create:
+                    raise exception.Error(msg)
+                else:
+                    LOG.warn(msg)
+                    should_export = False
+
+        if should_export:
+            cliq_args = {}
+            cliq_args['volumeName'] = volume['name']
+            cliq_args['chapName'] = chap_username
+            cliq_args['targetSecret'] = chap_password
+
+            self._cliq_run_xml("assignVolumeChap", cliq_args)
+
+            db_update['provider_auth'] = ("CHAP %s %s" %
+                                          (chap_username, chap_password))
+
+        return db_update
+
+    def remove_export(self, context, volume):
+        """Removes an export for a logical volume."""
+        cliq_args = {}
+        cliq_args['volumeName'] = volume['name']
+
+        self._cliq_run_xml("unassignVolume", cliq_args)

=== modified file 'plugins/xenserver/xenapi/etc/xapi.d/plugins/agent'
--- plugins/xenserver/xenapi/etc/xapi.d/plugins/agent	2011-02-17 22:09:26 +0000
+++ plugins/xenserver/xenapi/etc/xapi.d/plugins/agent	2011-02-18 18:41:49 +0000
@@ -91,6 +91,7 @@
     return resp
 
 
+<<<<<<< TREE
 @jsonify
 def resetnetwork(self, arg_dict):
     """Writes a resquest to xenstore that tells the agent
@@ -102,6 +103,20 @@
     xenstore.write_record(self, arg_dict)
 
 
+=======
+@jsonify
+def resetnetwork(self, arg_dict):
+    """
+    Writes a resquest to xenstore that tells the agent to reset networking.
+
+    """
+    arg_dict['value'] = json.dumps({'name': 'resetnetwork', 'value': ''})
+    request_id = arg_dict['id']
+    arg_dict['path'] = "data/host/%s" % request_id
+    xenstore.write_record(self, arg_dict)
+
+
+>>>>>>> MERGE-SOURCE
 def _wait_for_agent(self, request_id, arg_dict):
     """Periodically checks xenstore for a response from the agent.
     The request is always written to 'data/host/{id}', and