There are some parts of DnsManager that are worth keeping, but we should definitely replace the _* private methods that are hacking in support for the X-Auth-Sudo-Tenant-ID header with the proper upstream client library.

Well that would leave these:

def zones(self, name=None, params=None):
def create_zone(
def ensure_zone(
def recordsets(self, uuid, name=None, params=None):
def create_recordset(
def update_recordset(
def ensure_recordset(
def delete_recordset(self, uuid, rs):

All of this is replaceable with the stuff under designateclient.zones and designateclient.recordsets, except the ensure stuff which probably belongs in applications rather than the library.

Furthermore there is no reason to have all these extra functions for designate if we're not going to both complete it to cover all functionality of the Designate API and to do the same for every other OpenStack service we have installed.

Looked at which stuff runs jessie - based on parsing the dhcpd files it's, cloudservices1003, and a bunch of cloudvirt/labpuppetmaster/labmon/labstore hosts
Users of mwopenstackclients.DnsManager are wmcs-dns-floating-ip-updater.py which runs on control hosts, and the util::admin_scripts scripts which all seem to run on control too.

So actually this might all already be okay to do?

In T224708#5225397, @Krenair wrote:

All of this is replaceable with the stuff under designateclient.zones and designateclient.recordsets, except the ensure stuff which probably belongs in applications rather than the library.

The ensure stuff is definitely what I would want to keep. I don't understand the "probably belongs in applications" part of your argument. Libraries exist to reduce code duplication and make proper implementation of algorithms easier. ensure_recordset is used in 3 utility tools. I don't see why that code should be copied rather than living in our shim library that makes using the upstream libraries easier.

Furthermore there is no reason to have all these extra functions for designate if we're not going to both complete it to cover all functionality of the Designate API and to do the same for every other OpenStack service we have installed.

I'm not personally convinced that this argument holds. We can easily expose an initialized instance of the upstream designate client via a new mwopenstackclients.Clients.designateclient method mirroring the helpers for other upstream libraries. DnsManager can be reworked to use designateclient internally and remove the unneeded private methods. I don't understand why it would be necessary for this utility class to be extended to cover the entire designate api preemptively nor why we would need to add utility wrappers for all other openstack apis. There probably are other nice utility methods that you, Andrew, and others have made sprinkled throughout our various openstack automation scripts and hook handlers. It would be great to promote them up to the mwopenstackclients library, but I don't see how that having not been done yet bars us from continuing to use work that has already been done to move the DnsManager wrapper I originally wrote for wmcs-wikireplica-dns.py from the mwopenstackclients library.

In T224708#5225422, @bd808 wrote:

In T224708#5225397, @Krenair wrote:

All of this is replaceable with the stuff under designateclient.zones and designateclient.recordsets, except the ensure stuff which probably belongs in applications rather than the library.

The ensure stuff is definitely what I would want to keep. I don't understand the "probably belongs in applications" part of your argument. Libraries exist to reduce code duplication and make proper implementation of algorithms easier. ensure_recordset is used in 3 utility tools. I don't see why that code should be copied rather than living in our shim library that makes using the upstream libraries easier.

You're right. It would be out of place in designateclient as it does not map directly to a single request but it is a utility function.
ensure_zone is unused but ensure_recordset has 3 users.
On an unrelated note the current implementation of ensure_recordset appears dangerous as type is not taken into account when determining whether a record exists, should be updated, etc. I wouldn't want to ensure_recordset on tools.wmflabs.org SPF and have it overwrite the tools.wmflabs.org A record if that happens to get returned as the first item.

In T224708#5225422, @bd808 wrote:

Furthermore there is no reason to have all these extra functions for designate if we're not going to both complete it to cover all functionality of the Designate API and to do the same for every other OpenStack service we have installed.

I'm not personally convinced that this argument holds. We can easily expose an initialized instance of the upstream designate client via a new mwopenstackclients.Clients.designateclient method mirroring the helpers for other upstream libraries. DnsManager can be reworked to use designateclient internally and remove the unneeded private methods. I don't understand why it would be necessary for this utility class to be extended to cover the entire designate api preemptively nor why we would need to add utility wrappers for all other openstack apis. There probably are other nice utility methods that you, Andrew, and others have made sprinkled throughout our various openstack automation scripts and hook handlers. It would be great to promote them up to the mwopenstackclients library, but I don't see how that having not been done yet bars us from continuing to use work that has already been done to move the DnsManager wrapper I originally wrote for wmcs-wikireplica-dns.py from the mwopenstackclients library.

If we're just going to keep ensure_recordset and maybe ensure_zone then ok.

Change 513752 had a related patch set uploaded (by Alex Monk; owner: Alex Monk):
[operations/puppet@production] openstack mwopenstackclients: Add designateclient

https://gerrit.wikimedia.org/r/513752

Change 513909 had a related patch set uploaded (by Alex Monk; owner: Alex Monk):
[operations/puppet@production] openstack mwopenstackclients: Use designateclient in ensure functions

https://gerrit.wikimedia.org/r/513909

Change 513910 had a related patch set uploaded (by Alex Monk; owner: Alex Monk):
[operations/puppet@production] openstack scripts: Use designateclient where possible

https://gerrit.wikimedia.org/r/513910

Change 513911 had a related patch set uploaded (by Alex Monk; owner: Alex Monk):
[operations/puppet@production] openstack mwopenstackclients: Remove unused methods provided by designateclient now

https://gerrit.wikimedia.org/r/513911

Change 513752 merged by Andrew Bogott:
[operations/puppet@production] openstack mwopenstackclients: Add designateclient

https://gerrit.wikimedia.org/r/513752

Change 513909 merged by Andrew Bogott:
[operations/puppet@production] openstack mwopenstackclients: Use designateclient in ensure functions

https://gerrit.wikimedia.org/r/513909

https://gerrit.wikimedia.org/r/#/c/operations/puppet/+/522196/ needs newton