service-create allows 2 services with the same name

[Hoang Xuan Phu]

While `service-get <name>` seems to be confused by the duplicated name. The same thing happens to `service-delete <name>`. Of course, getting and deleting services by ID is not affected.

Following http://docs.openstack.org/trunk/install-guide/install/apt/content/cinder-controller.html:

$ keystone service-create --name=cinder --type=volume --description="Cinder Volume Service"
+-------------+----------------------------------+
| Property | Value |
+-------------+----------------------------------+
| description | Cinder Volume Service |
| id | 54f9153b21b54a908562d85392784992 |
| name | cinder |
| type | volume |
+-------------+----------------------------------+
$ keystone service-create --name=cinder --type=volumev2 --description="Cinder Volume Service V2"
+-------------+----------------------------------+
| Property | Value |
+-------------+----------------------------------+
| description | Cinder Volume Service V2 |
| id | dd425789606b4472b32d78a63942aefc |
| name | cinder |
| type | volumev2 |
+-------------+----------------------------------+
$ keystone service-get cinder
global name 'exc' is not defined

Debug output is here http://pastebin.com/kVJGUCwA

I'm not sure whether duplicated names are allowed/recommended, but at least one of the following is needed:

  - service-create should check the name and stop when a service with the same name exists
  - service-get and service-delete should be changed so that they work with duplicated names, maybe by showing multiple services or reporting an appropriate error
  - The help text of service-get and service-delete should not say that name is allowed

[Dolph Mathews (dolph)]

"service.name" isn't actually an indexed column, much less unique, in the SQL backend. This will require a migration.

[Juan Manuel Ollé (juan-m-olle)]

Some questions regarding this issue:

- As you indicate, a DB Table should be migrated. In this table the only new column will be “name”? and maintain “extra”, or just split “extra” into [name, description].

- Should “name” be unique? (actually nova is creating 2 services named nova)
         o If the answer is yes, during migration, how must be managed the actual services with duplicated names (remove duplicated entries doesn’t fill as a viable idea)
         o If the answer is no, actually for services with duplicate <name> the response is

  Multiple service matches found for <name>, use an ID to be more specific.

Should it respond in the same manner or just return all service that match?

[Florent Flament (florentflament)]

Hi,
I couldn't reproduce the bug with the latest python-keystoneclient version:

$ keystone service-list
+----------------------------------+------------+--------------+---------------------------+
| id | name | type | description |
+----------------------------------+------------+--------------+---------------------------+
| c15b275cf0d6409eaaa47388033399cb | cinder | volume | Cinder Volume Service |
| fbe94b37fa324705a64f6806d9167904 | cinder | volumev2 | Cinder Volume Service V2 |
+----------------------------------+------------+--------------+---------------------------+
$ keystone service-get cinder
Multiple service matches found for 'cinder', use an ID to be more specific.
$

[Hoang Xuan Phu (phunehehe)]

Hmm I'm using the package provided by http://ubuntu-cloud.archive.canonical.com/, which may be a little outdated. What is the prefered way to install the latest version of python-keystoneclient?

[Dolph Mathews (dolph)]

Juan: Maintain 'extra' as it may have proprietary data in it. 'name' should be removed and put into an indexable column, which should then be made unique. I think the migration should fail with an appropriate error message. Nova/devstack should not be creating duplicate services in the catalog -- they should move towards a single unversioned endpoint and use version discovery on the client side (this has been discussed to death on -dev).

Juan/Florent: your service-list illustrates that you've already reproduced it -- the duplicate service names is the bug, not the subsequent behavior of service-get <name> which is behaving correctly, given the state of the underlying bug.

Hoang: $ pip install python-keystoneclient
or install from from https://pypi.python.org/pypi/python-keystoneclient/

[Juan Manuel Ollé (juan-m-olle)]

Dolph: In the following example added duplicated cinder services (for testing), but I have never added a nova service.
In this case the upgrade will fail. the name needs to be unique or perhaps the name/type union

+----------------------------------+----------+-----------+---------------------------+
| id | name | type | description |
+----------------------------------+----------+-----------+---------------------------+
| 126bb6055cd8408d8d17d1b6c9af5023 | cinder | volumev2 | Cinder Volume Service V2 |
| 1c455e26b664407ba587cb4cab8fe4f3 | cinder | volume | Cinder Volume Service |
| ab3fe57178d44f7eb20fb939a949a440 | cinder | volumev2 | Cinder Volume Service V2 |
| ae723ac2d29341948013324f07dab386 | cinder | volume | Cinder Volume Service |
| 5697684cdadf4b7db5107e02c662d997 | ec2 | ec2 | EC2 Compatibility Layer |
| 6721d49123a148cc8bf30190fcdb8a17 | glance | image | Glance Image Service |
| 8442f8d00ed540ee8c821a3bdc41cfd6 | keystone | identity | Keystone Identity Service |
| 4fa88c02c07d4721b8078575204c9dfe | nova | computev3 | Nova Compute Service V3 |
| f061660af97a46e897df3f51f3eb2e2a | nova | compute | Nova Compute Service |
| 3017caf299d4469391ac68b5035eda6f | s3 | s3 | S3 |
+----------------------------------+----------+-----------+---------------------------+

[Juan Manuel Ollé (juan-m-olle)]

Other Question: If the migration fails and raise a ConflictException, is the migration method responsible to keep consistency in the DB or the migration mechanism should roll back all existing changes?.

Example: if cinder name is duplicated after "keystone-manage db_sync" raise the error
2013-12-16 11:50:52.263 11164 CRITICAL keystone [-] Conflict occurred attempting to store service. Service name cinder already exists
the final version is the previous one but the table remain with the new column.

[Dolph Mathews (dolph)]

If the migration can't be applied, then it should exit cleanly without making changes (an inconsistent state is a terrible place to be!).

I think the migration should wait for manual intervention to correct unexpected data (duplicate service names). Adding devstack to this bug because it sounds like it's behavior needs to be changed prior to landing a fix in keystone.

[Dolph Mathews (dolph)]

When were potential conflicts introduced and by what projects? Depending on their presence in the havana release, we might need to handle this differently.

[Juan Manuel Ollé (juan-m-olle)]

It seems that nova and cinder create services with duplicated names (this is what devstack has setup)

+----------------------------------+----------+----------------+-----------------------------+
| id | name | type | description |
+----------------------------------+----------+----------------+-----------------------------+
| 0862be25f69e43199e333d56eb3453dd | cinder | volumev2 | Cinder Volume Service V2 |
| dd3323fbb6af4a94acc94cdab1e29e22 | cinder | volume | Cinder Volume Service |
| 6b792fab7bd24321a84cdec1f8076c18 | ec2 | ec2 | EC2 Compatibility Layer |
| 5b005a8e96884ff8b5d01c596c65688c | glance | image | Glance Image Service |
| c2f8bae1c8a548a3ab0a5ffa4ad9a331 | heat | orchestration | Heat Service |
| 383e9d4c90a54c328b89825e24bc9291 | heat-cfn | cloudformation | Heat CloudFormation Service |
| a3150e6c07234881a51efe64cb72e09a | keystone | identity | Keystone Identity Service |
| 5bd52073090547c0b6e8d3d9ef722d1a | neutron | network | Neutron Service |
| 2a784573d96348c2a91f60b6181320a3 | nova | computev3 | Nova Compute Service V3 |
| a04f2e9e80184fa79449945b6265cf5a | nova | compute | Nova Compute Service |
| 93ee35ab254746ceb46a6b3703ef8471 | s3 | s3 | S3 |
+----------------------------------+----------+----------------+-----------------------------+

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/62894

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/65121

[Ben Nemec (bnemec)]

The general pattern with names in OpenStack is to allow them to be duplicated and require use of the id if the names are not unique. I wonder if that wouldn't be a safer way to handle this. Is there a pressing need for not allowing duplicated service names (other than the fact the keystoneclient doesn't seem to handle that correctly right now, which obviously would need to be fixed)? As an added bonus, that would eliminate the need for a db migration and worrying about what to do with existing setups that have duplicate names.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/65121
Committed: https://git.openstack.org/cgit/openstack-dev/devstack/commit/?id=4ad37630a2f938b19697f6e310def046a4dcca48
Submitter: Jenkins
Branch: master

commit 4ad37630a2f938b19697f6e310def046a4dcca48
Author: Juan Manuel Olle <email address hidden>
Date: Mon Jan 6 15:07:09 2014 -0300

    Remove duplicated name services

    Due to the fact that keystone will not allow services with
    duplicated names, cinder and nova services names were
    changed

    Closes-Bug: #1259425
    Change-Id: I988aef477b418a289426e02e5e108aa57dd1076b

[Juan Manuel Ollé (juan-m-olle)]

Devstack already change cinder and nova names to avoid duplicated names.
But now upgrade from previous version fails. According to a comment before (from Dolph) manual resolution should be apply. But Automatic test will also fail.

Due to the fact that keystoneclient does not allow duplicated names and only by UUID could be retrieve a service. What about to add a number to the name during Upgrade?

Ex:

original:
cinder
cinder

final:
cinder
cinder2

[Dolph Mathews (dolph)]

I'd rather not muck with existing data in a deployment like that -- it'll just result in more things breaking, likely outside the scope of openstack.

What automatic tests are you referring to? devstack? tempest? grenade? Do we need to add them to this bug, and patch them first?

[Juan Manuel Ollé (juan-m-olle)]

Hi Dolph,

Grenade test: check-grenade-dsvm

I don't think changing names are a good idea (to do it in the upgrade). Perhaps the previous patch has the correct behaviour

part of the test log

2014-01-13 21:06:04.235 | + upgrade_project keystone /opt/stack/new/grenade stable/havana
2014-01-13 21:06:04.235 | + project=keystone
2014-01-13 21:06:04.235 | + base_dir=/opt/stack/new/grenade
2014-01-13 21:06:04.235 | + branch=stable/havana
2014-01-13 21:06:04.266 | ++ get_release_name_from_branch stable/havana
2014-01-13 21:06:04.266 | ++ local branch=stable/havana
2014-01-13 21:06:04.266 | ++ [[ stable/havana =~ stable/ ]]
2014-01-13 21:06:04.266 | ++ echo havana
2014-01-13 21:06:04.267 | + upgrade_dir=havana
2014-01-13 21:06:04.267 | + upgrade_file=/opt/stack/new/grenade/from-havana/upgrade-keystone
2014-01-13 21:06:04.267 | + [[ -e /opt/stack/new/grenade/from-havana/upgrade-keystone ]]
2014-01-13 21:06:04.267 | + source /opt/stack/new/grenade/from-havana/upgrade-keystone
2014-01-13 21:06:04.267 | + configure_keystone_upgrade
2014-01-13 21:06:04.267 | + set -o xtrace
2014-01-13 21:06:04.268 | + set +o xtrace
2014-01-13 21:06:04.898 | 2014-01-13 21:06:04.886 2326 CRITICAL keystone [-] Conflict occurred attempting to store service. Service name cinder already exists
2014-01-13 21:06:04.931 | [Call Trace]
2014-01-13 21:06:04.931 | /opt/stack/new/grenade/upgrade-keystone:84:die
2014-01-13 21:06:04.934 | [ERROR] /opt/stack/new/grenade/upgrade-keystone:84 DB sync error
2014-01-13 21:06:04.935 | + die 253 'Failure in upgrade-keystone'
2014-01-13 21:06:04.935 | + local exitcode=1
2014-01-13 21:06:04.935 | + set +o xtrace
2014-01-13 21:06:04.936 | [Call Trace]
2014-01-13 21:06:04.936 | ./grenade.sh:253:die

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/136881

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/136917

[OpenStack Infra (hudson-openstack)]

Change abandoned by Lance Bragstad (<email address hidden>) on branch: master
Review: https://review.openstack.org/136881
Reason: Going to abandon this per IRC discussion with morganfainberg and jamielennox in #openstack-keystone on Freenode.

Service name shouldn't be required since service type is already a required attribute of service and *should* be unique.

A patch should be proposed to make service type completely unique.

[OpenStack Infra (hudson-openstack)]

Change abandoned by Lance Bragstad (<email address hidden>) on branch: master
Review: https://review.openstack.org/136917
Reason: Going to abandon this per IRC discussion with morganfainberg and jamielennox in #openstack-keystone on Freenode.

Service name shouldn't be required since service type is already a required attribute of service and *should* be unique.

A patch should be proposed to make service type completely unique.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/138130

[OpenStack Infra (hudson-openstack)]

Change abandoned by Lance Bragstad (<email address hidden>) on branch: master
Review: https://review.openstack.org/138130
Reason: Going to abandon this for now until I can come back to it.

[David Stanek (dstanek)]

Unassigned due to inactivity

[Steve Martinelli (stevemar)]

unassigning due to inactivity

[OpenStack Infra (hudson-openstack)]

Change abandoned by lei zhang ([e-mail]) on branch: master
Review: https://review.openstack.org/263197

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/307593

[OpenStack Infra (hudson-openstack)]

Change abandoned by David Stanek (<email address hidden>) on branch: master
Review: https://review.openstack.org/307593
Reason: I don't think this is something we want to do. Feel free to bring it back with updates if you disagree.

[Lance Bragstad (lbragstad)]

Unassigning due to inactivity.

[Steve Martinelli (stevemar)]

This is a limitation that we cannot fix without causing a backwards incompatible change. It is very rare to have 2 service names that are the same.