Andrew Bogott has uploaded a new change for review. https://gerrit.wikimedia.org/r/312127
Change subject: Make the nova_ldap notification handler thread-safe ...................................................................... Make the nova_ldap notification handler thread-safe It turns out that this is getting called by eventlet, and was re-entering all over the place. Change-Id: I58ca5f93d0f6e481e0467038585d0f8947d787df --- M modules/openstack/files/kilo/designate/nova_ldap/base.py M modules/openstack/files/liberty/designate/nova_ldap/base.py M modules/openstack/files/mitaka/designate/nova_ldap/base.py 3 files changed, 126 insertions(+), 111 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/operations/puppet refs/changes/27/312127/1 diff --git a/modules/openstack/files/kilo/designate/nova_ldap/base.py b/modules/openstack/files/kilo/designate/nova_ldap/base.py index 036fd5a..7ae9a46 100644 --- a/modules/openstack/files/kilo/designate/nova_ldap/base.py +++ b/modules/openstack/files/kilo/designate/nova_ldap/base.py @@ -33,7 +33,8 @@ class BaseAddressLdapHandler(BaseAddressHandler): - def _get_ip_data(self, addr_dict): + @staticmethod + def _get_ip_data(addr_dict): ip = addr_dict['address'] version = addr_dict['version'] @@ -49,7 +50,8 @@ data["octet%s" % i] = ip_data[i] return data - def _getLdapInfo(self, attr, conffile="/etc/ldap.conf"): + @staticmethod + def _getLdapInfo(attr, conffile="/etc/ldap.conf"): try: f = open(conffile) except IOError: @@ -64,39 +66,35 @@ return line.split(None, 1)[1].strip() break - def _initLdap(self): - self.base = self._getLdapInfo("base") - self.ldapHost = self._getLdapInfo("uri") - self.sslType = self._getLdapInfo("ssl") - - self.binddn = cfg.CONF[self.name].get('ldapusername') - self.bindpw = cfg.CONF[self.name].get('ldappassword') - def _openLdap(self): - self.ds = ldap.initialize(self.ldapHost) - self.ds.protocol_version = ldap.VERSION3 - if self.sslType == "start_tls": - self.ds.start_tls_s() + ldapHost = self._getLdapInfo("uri") + sslType = self._getLdapInfo("ssl") + + binddn = cfg.CONF[self.name].get('ldapusername') + bindpw = cfg.CONF[self.name].get('ldappassword') + ds = ldap.initialize(ldapHost) + ds.protocol_version = ldap.VERSION3 + if sslType == "start_tls": + ds.start_tls_s() try: - self.ds.simple_bind_s(self.binddn, self.bindpw) - return self.ds + ds.simple_bind_s(binddn, bindpw) + return ds except ldap.CONSTRAINT_VIOLATION: LOG.debug("LDAP bind failure: Too many failed attempts.\n") except ldap.INVALID_DN_SYNTAX: LOG.debug("LDAP bind failure: The bind DN is incorrect... \n") except ldap.NO_SUCH_OBJECT: - LOG.debug("LDAP bind failure: Unable to locate the bind DN account.\n") + LOG.debug("LDAP bind failure: " + "Unable to locate the bind DN account.\n") except ldap.UNWILLING_TO_PERFORM as msg: - LOG.debug("LDAP bind failure: The LDAP server was unwilling to perform the action" + LOG.debug("LDAP bind failure: " + "The LDAP server was unwilling to perform the action" " requested.\nError was: %s\n" % msg[0]["info"]) except ldap.INVALID_CREDENTIALS: LOG.debug("LDAP bind failure: Password incorrect.\n") return None - - def _closeLdap(self): - self.ds.unbind() def _create(self, addresses, extra, managed=True, resource_type=None, resource_id=None): @@ -110,8 +108,8 @@ :param resource_type: The managed resource type :param resource_id: The managed resource ID """ - self._initLdap() - if not self._openLdap(): + ds = self._openLdap() + if not ds: return LOG.debug('Using DomainID: %s' % cfg.CONF[self.name].domain_id) @@ -153,23 +151,26 @@ hostEntry['puppetVar'].append(var) hostEntry['associatedDomain'] = [] hostEntry['puppetVar'].append('instanceproject=%s' % - event_data['project_name'].encode('utf8')) + event_data['project_name'].encode( + 'utf8')) hostEntry['puppetVar'].append('instancename=%s' % - event_data['hostname'].encode('utf8')) + event_data['hostname'].encode( + 'utf8')) for fmt in cfg.CONF[self.name].get('format'): - hostEntry['associatedDomain'].append((fmt % event_data).rstrip('.').encode('utf8')) + hostEntry['associatedDomain'].append( + (fmt % event_data).rstrip('.').encode('utf8')) if managed: LOG.debug('Creating ldap record') modlist = ldap.modlist.addModlist(hostEntry) try: - self.ds.add_s(dn, modlist) + ds.add_s(dn, modlist) except ldap.LDAPError as e: LOG.debug('Ldap exception %s' % e) - self._closeLdap() + ds.unbind() def _delete(self, extra, managed=True, resource_id=None, resource_type='instance', criterion={}): @@ -178,9 +179,8 @@ :param criterion: Criterion to search and destroy records """ - LOG.debug('Initializing ldap') - self._initLdap() - if not self._openLdap(): + ds = self._openLdap() + if not ds: return LOG.debug('Delete using DomainID: %s' % cfg.CONF[self.name].domain_id) @@ -202,11 +202,11 @@ LOG.debug('Deleting ldap record: %s' % dn) try: - self.ds.delete_s(dn) + ds.delete_s(dn) except ldap.NO_SUCH_OBJECT: LOG.debug('Warning: %s not found in ldap. Not deleted.' % dn) - self._closeLdap() + ds.unbind() # WMF-specific add-on: Clean salt and puppet keys for deleted # instance @@ -217,7 +217,8 @@ LOG.debug('Cleaning puppet key %s' % puppetkey) self._run_remote_command(cfg.CONF[self.name].puppet_master_host, cfg.CONF[self.name].certmanager_user, - 'sudo puppet cert clean %s' % pipes.quote(puppetkey)) + 'sudo puppet cert clean %s' % + pipes.quote(puppetkey)) if (cfg.CONF[self.name].salt_key_format and cfg.CONF[self.name].salt_master_host): @@ -226,12 +227,16 @@ LOG.debug('Cleaning salt key %s' % saltkey) self._run_remote_command(cfg.CONF[self.name].salt_master_host, cfg.CONF[self.name].certmanager_user, - 'sudo salt-key -y -d %s' % pipes.quote(saltkey)) + 'sudo salt-key -y -d %s' % + pipes.quote(saltkey)) - def _run_remote_command(self, server, username, command): + @staticmethod + def _run_remote_command(server, username, command): ssh_command = ['/usr/bin/ssh', '-l%s' % username, server, command] - p = subprocess.Popen(ssh_command, stdout=subprocess.PIPE, stderr=subprocess.PIPE) + p = subprocess.Popen(ssh_command, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE) (out, error) = p.communicate() rcode = p.wait() return out, error, rcode diff --git a/modules/openstack/files/liberty/designate/nova_ldap/base.py b/modules/openstack/files/liberty/designate/nova_ldap/base.py index 036fd5a..7ae9a46 100644 --- a/modules/openstack/files/liberty/designate/nova_ldap/base.py +++ b/modules/openstack/files/liberty/designate/nova_ldap/base.py @@ -33,7 +33,8 @@ class BaseAddressLdapHandler(BaseAddressHandler): - def _get_ip_data(self, addr_dict): + @staticmethod + def _get_ip_data(addr_dict): ip = addr_dict['address'] version = addr_dict['version'] @@ -49,7 +50,8 @@ data["octet%s" % i] = ip_data[i] return data - def _getLdapInfo(self, attr, conffile="/etc/ldap.conf"): + @staticmethod + def _getLdapInfo(attr, conffile="/etc/ldap.conf"): try: f = open(conffile) except IOError: @@ -64,39 +66,35 @@ return line.split(None, 1)[1].strip() break - def _initLdap(self): - self.base = self._getLdapInfo("base") - self.ldapHost = self._getLdapInfo("uri") - self.sslType = self._getLdapInfo("ssl") - - self.binddn = cfg.CONF[self.name].get('ldapusername') - self.bindpw = cfg.CONF[self.name].get('ldappassword') - def _openLdap(self): - self.ds = ldap.initialize(self.ldapHost) - self.ds.protocol_version = ldap.VERSION3 - if self.sslType == "start_tls": - self.ds.start_tls_s() + ldapHost = self._getLdapInfo("uri") + sslType = self._getLdapInfo("ssl") + + binddn = cfg.CONF[self.name].get('ldapusername') + bindpw = cfg.CONF[self.name].get('ldappassword') + ds = ldap.initialize(ldapHost) + ds.protocol_version = ldap.VERSION3 + if sslType == "start_tls": + ds.start_tls_s() try: - self.ds.simple_bind_s(self.binddn, self.bindpw) - return self.ds + ds.simple_bind_s(binddn, bindpw) + return ds except ldap.CONSTRAINT_VIOLATION: LOG.debug("LDAP bind failure: Too many failed attempts.\n") except ldap.INVALID_DN_SYNTAX: LOG.debug("LDAP bind failure: The bind DN is incorrect... \n") except ldap.NO_SUCH_OBJECT: - LOG.debug("LDAP bind failure: Unable to locate the bind DN account.\n") + LOG.debug("LDAP bind failure: " + "Unable to locate the bind DN account.\n") except ldap.UNWILLING_TO_PERFORM as msg: - LOG.debug("LDAP bind failure: The LDAP server was unwilling to perform the action" + LOG.debug("LDAP bind failure: " + "The LDAP server was unwilling to perform the action" " requested.\nError was: %s\n" % msg[0]["info"]) except ldap.INVALID_CREDENTIALS: LOG.debug("LDAP bind failure: Password incorrect.\n") return None - - def _closeLdap(self): - self.ds.unbind() def _create(self, addresses, extra, managed=True, resource_type=None, resource_id=None): @@ -110,8 +108,8 @@ :param resource_type: The managed resource type :param resource_id: The managed resource ID """ - self._initLdap() - if not self._openLdap(): + ds = self._openLdap() + if not ds: return LOG.debug('Using DomainID: %s' % cfg.CONF[self.name].domain_id) @@ -153,23 +151,26 @@ hostEntry['puppetVar'].append(var) hostEntry['associatedDomain'] = [] hostEntry['puppetVar'].append('instanceproject=%s' % - event_data['project_name'].encode('utf8')) + event_data['project_name'].encode( + 'utf8')) hostEntry['puppetVar'].append('instancename=%s' % - event_data['hostname'].encode('utf8')) + event_data['hostname'].encode( + 'utf8')) for fmt in cfg.CONF[self.name].get('format'): - hostEntry['associatedDomain'].append((fmt % event_data).rstrip('.').encode('utf8')) + hostEntry['associatedDomain'].append( + (fmt % event_data).rstrip('.').encode('utf8')) if managed: LOG.debug('Creating ldap record') modlist = ldap.modlist.addModlist(hostEntry) try: - self.ds.add_s(dn, modlist) + ds.add_s(dn, modlist) except ldap.LDAPError as e: LOG.debug('Ldap exception %s' % e) - self._closeLdap() + ds.unbind() def _delete(self, extra, managed=True, resource_id=None, resource_type='instance', criterion={}): @@ -178,9 +179,8 @@ :param criterion: Criterion to search and destroy records """ - LOG.debug('Initializing ldap') - self._initLdap() - if not self._openLdap(): + ds = self._openLdap() + if not ds: return LOG.debug('Delete using DomainID: %s' % cfg.CONF[self.name].domain_id) @@ -202,11 +202,11 @@ LOG.debug('Deleting ldap record: %s' % dn) try: - self.ds.delete_s(dn) + ds.delete_s(dn) except ldap.NO_SUCH_OBJECT: LOG.debug('Warning: %s not found in ldap. Not deleted.' % dn) - self._closeLdap() + ds.unbind() # WMF-specific add-on: Clean salt and puppet keys for deleted # instance @@ -217,7 +217,8 @@ LOG.debug('Cleaning puppet key %s' % puppetkey) self._run_remote_command(cfg.CONF[self.name].puppet_master_host, cfg.CONF[self.name].certmanager_user, - 'sudo puppet cert clean %s' % pipes.quote(puppetkey)) + 'sudo puppet cert clean %s' % + pipes.quote(puppetkey)) if (cfg.CONF[self.name].salt_key_format and cfg.CONF[self.name].salt_master_host): @@ -226,12 +227,16 @@ LOG.debug('Cleaning salt key %s' % saltkey) self._run_remote_command(cfg.CONF[self.name].salt_master_host, cfg.CONF[self.name].certmanager_user, - 'sudo salt-key -y -d %s' % pipes.quote(saltkey)) + 'sudo salt-key -y -d %s' % + pipes.quote(saltkey)) - def _run_remote_command(self, server, username, command): + @staticmethod + def _run_remote_command(server, username, command): ssh_command = ['/usr/bin/ssh', '-l%s' % username, server, command] - p = subprocess.Popen(ssh_command, stdout=subprocess.PIPE, stderr=subprocess.PIPE) + p = subprocess.Popen(ssh_command, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE) (out, error) = p.communicate() rcode = p.wait() return out, error, rcode diff --git a/modules/openstack/files/mitaka/designate/nova_ldap/base.py b/modules/openstack/files/mitaka/designate/nova_ldap/base.py index 036fd5a..7ae9a46 100644 --- a/modules/openstack/files/mitaka/designate/nova_ldap/base.py +++ b/modules/openstack/files/mitaka/designate/nova_ldap/base.py @@ -33,7 +33,8 @@ class BaseAddressLdapHandler(BaseAddressHandler): - def _get_ip_data(self, addr_dict): + @staticmethod + def _get_ip_data(addr_dict): ip = addr_dict['address'] version = addr_dict['version'] @@ -49,7 +50,8 @@ data["octet%s" % i] = ip_data[i] return data - def _getLdapInfo(self, attr, conffile="/etc/ldap.conf"): + @staticmethod + def _getLdapInfo(attr, conffile="/etc/ldap.conf"): try: f = open(conffile) except IOError: @@ -64,39 +66,35 @@ return line.split(None, 1)[1].strip() break - def _initLdap(self): - self.base = self._getLdapInfo("base") - self.ldapHost = self._getLdapInfo("uri") - self.sslType = self._getLdapInfo("ssl") - - self.binddn = cfg.CONF[self.name].get('ldapusername') - self.bindpw = cfg.CONF[self.name].get('ldappassword') - def _openLdap(self): - self.ds = ldap.initialize(self.ldapHost) - self.ds.protocol_version = ldap.VERSION3 - if self.sslType == "start_tls": - self.ds.start_tls_s() + ldapHost = self._getLdapInfo("uri") + sslType = self._getLdapInfo("ssl") + + binddn = cfg.CONF[self.name].get('ldapusername') + bindpw = cfg.CONF[self.name].get('ldappassword') + ds = ldap.initialize(ldapHost) + ds.protocol_version = ldap.VERSION3 + if sslType == "start_tls": + ds.start_tls_s() try: - self.ds.simple_bind_s(self.binddn, self.bindpw) - return self.ds + ds.simple_bind_s(binddn, bindpw) + return ds except ldap.CONSTRAINT_VIOLATION: LOG.debug("LDAP bind failure: Too many failed attempts.\n") except ldap.INVALID_DN_SYNTAX: LOG.debug("LDAP bind failure: The bind DN is incorrect... \n") except ldap.NO_SUCH_OBJECT: - LOG.debug("LDAP bind failure: Unable to locate the bind DN account.\n") + LOG.debug("LDAP bind failure: " + "Unable to locate the bind DN account.\n") except ldap.UNWILLING_TO_PERFORM as msg: - LOG.debug("LDAP bind failure: The LDAP server was unwilling to perform the action" + LOG.debug("LDAP bind failure: " + "The LDAP server was unwilling to perform the action" " requested.\nError was: %s\n" % msg[0]["info"]) except ldap.INVALID_CREDENTIALS: LOG.debug("LDAP bind failure: Password incorrect.\n") return None - - def _closeLdap(self): - self.ds.unbind() def _create(self, addresses, extra, managed=True, resource_type=None, resource_id=None): @@ -110,8 +108,8 @@ :param resource_type: The managed resource type :param resource_id: The managed resource ID """ - self._initLdap() - if not self._openLdap(): + ds = self._openLdap() + if not ds: return LOG.debug('Using DomainID: %s' % cfg.CONF[self.name].domain_id) @@ -153,23 +151,26 @@ hostEntry['puppetVar'].append(var) hostEntry['associatedDomain'] = [] hostEntry['puppetVar'].append('instanceproject=%s' % - event_data['project_name'].encode('utf8')) + event_data['project_name'].encode( + 'utf8')) hostEntry['puppetVar'].append('instancename=%s' % - event_data['hostname'].encode('utf8')) + event_data['hostname'].encode( + 'utf8')) for fmt in cfg.CONF[self.name].get('format'): - hostEntry['associatedDomain'].append((fmt % event_data).rstrip('.').encode('utf8')) + hostEntry['associatedDomain'].append( + (fmt % event_data).rstrip('.').encode('utf8')) if managed: LOG.debug('Creating ldap record') modlist = ldap.modlist.addModlist(hostEntry) try: - self.ds.add_s(dn, modlist) + ds.add_s(dn, modlist) except ldap.LDAPError as e: LOG.debug('Ldap exception %s' % e) - self._closeLdap() + ds.unbind() def _delete(self, extra, managed=True, resource_id=None, resource_type='instance', criterion={}): @@ -178,9 +179,8 @@ :param criterion: Criterion to search and destroy records """ - LOG.debug('Initializing ldap') - self._initLdap() - if not self._openLdap(): + ds = self._openLdap() + if not ds: return LOG.debug('Delete using DomainID: %s' % cfg.CONF[self.name].domain_id) @@ -202,11 +202,11 @@ LOG.debug('Deleting ldap record: %s' % dn) try: - self.ds.delete_s(dn) + ds.delete_s(dn) except ldap.NO_SUCH_OBJECT: LOG.debug('Warning: %s not found in ldap. Not deleted.' % dn) - self._closeLdap() + ds.unbind() # WMF-specific add-on: Clean salt and puppet keys for deleted # instance @@ -217,7 +217,8 @@ LOG.debug('Cleaning puppet key %s' % puppetkey) self._run_remote_command(cfg.CONF[self.name].puppet_master_host, cfg.CONF[self.name].certmanager_user, - 'sudo puppet cert clean %s' % pipes.quote(puppetkey)) + 'sudo puppet cert clean %s' % + pipes.quote(puppetkey)) if (cfg.CONF[self.name].salt_key_format and cfg.CONF[self.name].salt_master_host): @@ -226,12 +227,16 @@ LOG.debug('Cleaning salt key %s' % saltkey) self._run_remote_command(cfg.CONF[self.name].salt_master_host, cfg.CONF[self.name].certmanager_user, - 'sudo salt-key -y -d %s' % pipes.quote(saltkey)) + 'sudo salt-key -y -d %s' % + pipes.quote(saltkey)) - def _run_remote_command(self, server, username, command): + @staticmethod + def _run_remote_command(server, username, command): ssh_command = ['/usr/bin/ssh', '-l%s' % username, server, command] - p = subprocess.Popen(ssh_command, stdout=subprocess.PIPE, stderr=subprocess.PIPE) + p = subprocess.Popen(ssh_command, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE) (out, error) = p.communicate() rcode = p.wait() return out, error, rcode -- To view, visit https://gerrit.wikimedia.org/r/312127 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I58ca5f93d0f6e481e0467038585d0f8947d787df Gerrit-PatchSet: 1 Gerrit-Project: operations/puppet Gerrit-Branch: production Gerrit-Owner: Andrew Bogott <abog...@wikimedia.org> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits