Change _member_role_exists to work with current upgrade flow
Because we now shut down all OpenStack services before upgrading the undercloud, we can't run API queries against them prior to running puppet. This means we now need to handle the _member_ role in a post config step. Note that this was only not failing before because the check in _stackrc_exists was always returning false, so the old code was never actually run. That also means it was not doing what it was supposed to, of course. Change-Id: Iaf81adc48321dbc985c50aeb5bfc0cc94810cb1d Closes-Bug: 1663719
This commit is contained in:
Ben Nemec
parent
66d12ccfff
commit
9f6465fe8c
|
|
@ -307,15 +307,6 @@ keystone_config {
|
|||
'ec2/driver': value => 'keystone.contrib.ec2.backends.sql.Ec2';
|
||||
}
|
||||
|
||||
if str2bool(hiera('member_role_exists', false)) {
|
||||
# Old deployments where assigning _member_ role to admin user.
|
||||
# The _member_ role is needed because it's delegated via heat trusts in
|
||||
# existing deployments, hence existing role assignments can't just be
|
||||
# deleted. This Puppet Collector will allow to update deployments with
|
||||
# admin role managed by Puppet.
|
||||
Keystone_user_role<| title == 'admin@admin' |> { roles +> ['_member_'] }
|
||||
}
|
||||
|
||||
# TODO: notifications, scrubber, etc.
|
||||
class { '::glance::api':
|
||||
enable_proxy_headers_parsing => $enable_proxy_headers_parsing,
|
||||
|
|
|
|||
|
|
@ -31,10 +31,6 @@ tripleo::profile::base::haproxy::certificates_specs:
|
|||
# CA defaults
|
||||
certmonger_ca: {{CERTIFICATE_GENERATION_CA}}
|
||||
|
||||
# Workaround for puppet deleting _member_ role assignment on old deployments
|
||||
member_role_exists: {{MEMBER_ROLE_EXISTS}}
|
||||
|
||||
|
||||
# Common Hiera data gets applied to all nodes
|
||||
ssh::server::storeconfigs_enabled: false
|
||||
|
||||
|
|
|
|||
|
|
@ -20,6 +20,7 @@ import subprocess
|
|||
import tempfile
|
||||
|
||||
import fixtures
|
||||
from keystoneauth1 import exceptions as ks_exceptions
|
||||
import mock
|
||||
from mistralclient.api import base as mistralclient_base
|
||||
from novaclient import exceptions
|
||||
|
|
@ -322,11 +323,6 @@ class TestGenerateEnvironment(BaseTestCase):
|
|||
self.useFixture(self.mock_distro)
|
||||
self.mock_distro.mock.return_value = [
|
||||
'Red Hat Enterprise Linux Server 7.1']
|
||||
stackrc_exists_patcher = mock.patch(
|
||||
'instack_undercloud.undercloud._stackrc_exists',
|
||||
return_value=False)
|
||||
stackrc_exists_patcher.start()
|
||||
self.addCleanup(stackrc_exists_patcher.stop)
|
||||
|
||||
@mock.patch('socket.gethostname')
|
||||
def test_hostname_set(self, mock_gethostname):
|
||||
|
|
@ -620,6 +616,7 @@ class TestConfigureSshKeys(base.BaseTestCase):
|
|||
|
||||
|
||||
class TestPostConfig(base.BaseTestCase):
|
||||
@mock.patch('instack_undercloud.undercloud._member_role_exists')
|
||||
@mock.patch('novaclient.client.Client', autospec=True)
|
||||
@mock.patch('mistralclient.api.client.client', autospec=True)
|
||||
@mock.patch('instack_undercloud.undercloud._delete_default_flavors')
|
||||
|
|
@ -631,7 +628,7 @@ class TestPostConfig(base.BaseTestCase):
|
|||
def test_post_config(self, mock_post_config_mistral, mock_ensure_flavor,
|
||||
mock_configure_ssh_keys, mock_get_auth_values,
|
||||
mock_copy_stackrc, mock_delete, mock_mistral_client,
|
||||
mock_nova_client):
|
||||
mock_nova_client, mock_member_role_exists):
|
||||
instack_env = {
|
||||
'UNDERCLOUD_ENDPOINT_MISTRAL_PUBLIC':
|
||||
'http://192.168.24.1:8989/v2',
|
||||
|
|
@ -746,13 +743,6 @@ class TestPostConfig(base.BaseTestCase):
|
|||
]
|
||||
mock_run.assert_has_calls(calls)
|
||||
|
||||
@mock.patch('instack_undercloud.undercloud._stackrc_exists')
|
||||
def test_member_role_exists_nostackrc(self, mock_stackrc_exists):
|
||||
mock_stackrc_exists.return_value = False
|
||||
instack_env = {}
|
||||
undercloud._member_role_exists(instack_env)
|
||||
self.assertEqual({'MEMBER_ROLE_EXISTS': 'False'}, instack_env)
|
||||
|
||||
def _mock_ksclient_roles(self, mock_auth_values, mock_ksdiscover, roles):
|
||||
mock_auth_values.return_value = ('user', 'password',
|
||||
'tenant', 'http://test:123')
|
||||
|
|
@ -769,29 +759,62 @@ class TestPostConfig(base.BaseTestCase):
|
|||
mock_client.roles = mock_roles
|
||||
mock_discover.create_client.return_value = mock_client
|
||||
|
||||
@mock.patch('keystoneclient.discover.Discover')
|
||||
@mock.patch('instack_undercloud.undercloud._get_auth_values')
|
||||
@mock.patch('instack_undercloud.undercloud._stackrc_exists')
|
||||
def test_member_role_exists(self, mock_stackrc_exists, mock_auth_values,
|
||||
mock_ksdiscover):
|
||||
mock_stackrc_exists.return_value = True
|
||||
self._mock_ksclient_roles(mock_auth_values, mock_ksdiscover,
|
||||
['admin'])
|
||||
instack_env = {}
|
||||
undercloud._member_role_exists(instack_env)
|
||||
self.assertEqual({'MEMBER_ROLE_EXISTS': 'False'}, instack_env)
|
||||
mock_tenant_list = [mock.Mock(), mock.Mock()]
|
||||
mock_tenant_list[0].name = 'admin'
|
||||
mock_tenant_list[0].id = 'admin-id'
|
||||
mock_tenant_list[1].name = 'service'
|
||||
mock_tenant_list[1].id = 'service-id'
|
||||
mock_client.tenants.list.return_value = mock_tenant_list
|
||||
|
||||
mock_user_list = [mock.Mock(), mock.Mock()]
|
||||
mock_user_list[0].name = 'admin'
|
||||
mock_user_list[1].name = 'nova'
|
||||
mock_client.users.list.return_value = mock_user_list
|
||||
return mock_client
|
||||
|
||||
@mock.patch('keystoneclient.discover.Discover')
|
||||
@mock.patch('instack_undercloud.undercloud._get_auth_values')
|
||||
@mock.patch('instack_undercloud.undercloud._stackrc_exists')
|
||||
def test_member_role_exists_true(self, mock_stackrc_exists,
|
||||
@mock.patch('os.path.isfile')
|
||||
def test_member_role_exists(self, mock_isfile, mock_auth_values,
|
||||
mock_ksdiscover):
|
||||
mock_isfile.return_value = True
|
||||
mock_client = self._mock_ksclient_roles(mock_auth_values,
|
||||
mock_ksdiscover,
|
||||
['admin'])
|
||||
undercloud._member_role_exists()
|
||||
self.assertFalse(mock_client.tenants.list.called)
|
||||
|
||||
@mock.patch('keystoneclient.discover.Discover')
|
||||
@mock.patch('instack_undercloud.undercloud._get_auth_values')
|
||||
@mock.patch('os.path.isfile')
|
||||
def test_member_role_exists_true(self, mock_isfile,
|
||||
mock_auth_values, mock_ksdiscover):
|
||||
mock_stackrc_exists.return_value = True
|
||||
self._mock_ksclient_roles(mock_auth_values, mock_ksdiscover,
|
||||
['admin', '_member_'])
|
||||
instack_env = {}
|
||||
undercloud._member_role_exists(instack_env)
|
||||
self.assertEqual({'MEMBER_ROLE_EXISTS': 'True'}, instack_env)
|
||||
mock_isfile.return_value = True
|
||||
mock_client = self._mock_ksclient_roles(mock_auth_values,
|
||||
mock_ksdiscover,
|
||||
['admin', '_member_'])
|
||||
undercloud._member_role_exists()
|
||||
mock_user = mock_client.users.list.return_value[0]
|
||||
mock_role = mock_client.roles.list.return_value[1]
|
||||
mock_client.roles.add_user_role.assert_called_once_with(
|
||||
mock_user, mock_role, 'admin-id')
|
||||
|
||||
@mock.patch('keystoneclient.discover.Discover')
|
||||
@mock.patch('instack_undercloud.undercloud._get_auth_values')
|
||||
@mock.patch('os.path.isfile')
|
||||
def test_has_member_role(self, mock_isfile, mock_auth_values,
|
||||
mock_ksdiscover):
|
||||
mock_isfile.return_value = True
|
||||
mock_client = self._mock_ksclient_roles(mock_auth_values,
|
||||
mock_ksdiscover,
|
||||
['admin', '_member_'])
|
||||
fake_exception = ks_exceptions.http.Conflict('test')
|
||||
mock_client.roles.add_user_role.side_effect = fake_exception
|
||||
undercloud._member_role_exists()
|
||||
mock_user = mock_client.users.list.return_value[0]
|
||||
mock_role = mock_client.roles.list.return_value[1]
|
||||
mock_client.roles.add_user_role.assert_called_once_with(
|
||||
mock_user, mock_role, 'admin-id')
|
||||
|
||||
def _create_flavor_mocks(self):
|
||||
mock_nova = mock.Mock()
|
||||
|
|
|
|||
|
|
@ -30,7 +30,7 @@ import tempfile
|
|||
import time
|
||||
import uuid
|
||||
|
||||
from keystoneclient import exceptions as ks_exceptions
|
||||
from keystoneauth1 import exceptions as ks_exceptions
|
||||
from keystoneclient import auth
|
||||
from keystoneclient import session
|
||||
from keystoneclient import discover
|
||||
|
|
@ -901,39 +901,40 @@ def _write_password_file(instack_env):
|
|||
password_file.write('%s=%s\n' % (opt.name, value))
|
||||
|
||||
|
||||
def _member_role_exists(instack_env):
|
||||
def _member_role_exists():
|
||||
# This is a workaround for puppet removing the deprecated _member_
|
||||
# role on upgrade - if it exists we must not remove role assignments
|
||||
# role on upgrade - if it exists we must restore role assignments
|
||||
# or trusts stored in the undercloud heat will break
|
||||
if not _stackrc_exists():
|
||||
instack_env['MEMBER_ROLE_EXISTS'] = 'False'
|
||||
return
|
||||
user, password, tenant, auth_url = _get_auth_values()
|
||||
role_exists = False
|
||||
# Note this is made somewhat verbose due to trying to handle
|
||||
# any format auth_url (versionless, v2,0/v3 suffix)
|
||||
auth_plugin_class = auth.get_plugin_class('password')
|
||||
auth_kwargs = {
|
||||
'auth_url': auth_url,
|
||||
'username': user,
|
||||
'password': password,
|
||||
'project_name': tenant}
|
||||
if 'v2.0' not in auth_url:
|
||||
auth_kwargs.update({
|
||||
'project_domain_name': 'default',
|
||||
'user_domain_name': 'default'})
|
||||
auth_plugin = auth_plugin_class(**auth_kwargs)
|
||||
sess = session.Session(auth=auth_plugin)
|
||||
disc = discover.Discover(session=sess)
|
||||
c = disc.create_client()
|
||||
try:
|
||||
# Note this is made somewhat verbose due to trying to handle
|
||||
# any format auth_url (versionless, v2,0/v3 suffix)
|
||||
auth_plugin_class = auth.get_plugin_class('password')
|
||||
auth_kwargs = {
|
||||
'auth_url': auth_url,
|
||||
'username': user,
|
||||
'password': password,
|
||||
'project_name': tenant}
|
||||
if 'v2.0' not in auth_url:
|
||||
auth_kwargs.update({
|
||||
'project_domain_name': 'default',
|
||||
'user_domain_name': 'default'})
|
||||
auth_plugin = auth_plugin_class(**auth_kwargs)
|
||||
sess = session.Session(auth=auth_plugin)
|
||||
disc = discover.Discover(session=sess)
|
||||
c = disc.create_client()
|
||||
role_names = [r.name for r in c.roles.list()]
|
||||
role_exists = '_member_' in role_names
|
||||
except ks_exceptions.ConnectionError:
|
||||
# This will happen on initial deployment, assume False
|
||||
# as no new deployments should have _member_
|
||||
role_exists = False
|
||||
instack_env['MEMBER_ROLE_EXISTS'] = six.text_type(role_exists)
|
||||
member_role = [r for r in c.roles.list() if r.name == '_member_'][0]
|
||||
except IndexError:
|
||||
# Do nothing if there is no _member_ role
|
||||
return
|
||||
admin_tenant = [t for t in c.tenants.list() if t.name == 'admin'][0]
|
||||
admin_user = [u for u in c.users.list() if u.name == 'admin'][0]
|
||||
try:
|
||||
c.roles.add_user_role(admin_user, member_role, admin_tenant.id)
|
||||
LOG.info('Added _member_ role to admin user')
|
||||
except ks_exceptions.http.Conflict:
|
||||
# They already had the role
|
||||
pass
|
||||
|
||||
|
||||
class InstackEnvironment(dict):
|
||||
|
|
@ -950,7 +951,7 @@ class InstackEnvironment(dict):
|
|||
DYNAMIC_KEYS = {'INSPECTION_COLLECTORS', 'INSPECTION_KERNEL_ARGS',
|
||||
'INSPECTION_NODE_NOT_FOUND_HOOK',
|
||||
'TRIPLEO_INSTALL_USER', 'TRIPLEO_UNDERCLOUD_CONF_FILE',
|
||||
'TRIPLEO_UNDERCLOUD_PASSWORD_FILE', 'MEMBER_ROLE_EXISTS'}
|
||||
'TRIPLEO_UNDERCLOUD_PASSWORD_FILE'}
|
||||
"""The variables we calculate in _generate_environment call."""
|
||||
|
||||
PUPPET_KEYS = DYNAMIC_KEYS | {opt.name.upper() for _, group in list_opts()
|
||||
|
|
@ -1089,8 +1090,6 @@ def _generate_environment(instack_root):
|
|||
instack_env['UNDERCLOUD_SERVICE_CERTIFICATE'] = (
|
||||
'/etc/pki/tls/certs/undercloud-%s.pem' % public_host)
|
||||
|
||||
_member_role_exists(instack_env)
|
||||
|
||||
return instack_env
|
||||
|
||||
|
||||
|
|
@ -1257,14 +1256,6 @@ def _ensure_flavor(nova, name, profile=None):
|
|||
LOG.info(message, name, profile)
|
||||
|
||||
|
||||
def _stackrc_exists():
|
||||
user_stackrc = os.path.expanduser('~/stackrc')
|
||||
# We gotta check if the copying of stackrc has already been done.
|
||||
if os.path.isfile('/root/stackrc') and os.path.isfile(user_stackrc):
|
||||
return True
|
||||
return False
|
||||
|
||||
|
||||
def _copy_stackrc():
|
||||
args = ['sudo', 'cp', '/root/stackrc', os.path.expanduser('~')]
|
||||
try:
|
||||
|
|
@ -1378,6 +1369,8 @@ def _post_config(instack_env):
|
|||
auth_url=auth_url)
|
||||
_post_config_mistral(instack_env, mistral)
|
||||
|
||||
_member_role_exists()
|
||||
|
||||
|
||||
def _handle_upgrade_fact(upgrade=False):
|
||||
"""Create an upgrade fact for use in puppet
|
||||
|
|
|
|||
|
|
@ -0,0 +1,9 @@
|
|||
---
|
||||
upgrade:
|
||||
- |
|
||||
The _member_ role (if it exists) on the admin user will now be retained
|
||||
automatically during undercloud upgrades. This functionality was
|
||||
originally added to work around an issue with upgrading very old versions
|
||||
of TripleO, but was broken by changes to the upgrade process. It will
|
||||
no longer be necessary to manually add the _member_ role to the admin user
|
||||
after upgrading an affected deployment.
|
||||
Loading…
Reference in New Issue