Fixes LIBCLOUD-651, Route53 cant delete a multiple value record

[Itxaka]

Takes into consideration that when trying to delete multiple values you need to sent all the resources.

[Itxaka]

@Kami ?

[Kami]

@Itxaka Thanks. I need to test this one before merging.

I do remember this issue (we had it at DivvyCloud), but I thought I fixed it quite a while ago and submitted the patch upstream, but this might not be the case.

[Kami]

I just tested this change - the problem is it doesn't work correctly / as expected.

It deletes all the related multiple value records and not just one. It also looks like this might affect existing "ex_delete_all_records" method.

Probably a better approach would be to handle multiple records in "delete_record" method and if necessary, add a new private method (same as we do for update record).

As far as the delete logic goes - looks like we should use a similar logic to the one in _update_multi_value_record - we need to send DELETE action for the record we want to delete and CREATE for the other records we want to keep.

[Itxaka]

True, didnt notice that as I was just testing by creating and then deleting a single multi value record, and indeed it affetcs other ones.

Let me have a look at it and fix that :)

[Itxaka]

| It deletes all the related multiple value records and not just one.

@Kami Isn't this the intended end? If you wanted to modify it, you would call update_recordinstead, which would update what the record is pointing at (thus, deleting one of the multi values and keeping the others), but this is for deleting the record altogether.

Knowing that a multi value record is still viewed by libcloud as just one record, this should be the case when doing either dnsdriver.delete(record) or record.delete() as both methods will try to destroy the whole record.

[Itxaka]

@Kami In fact the reason for the ex_delete_all_records method to fail is because libcloud is retuning a multivalue record in 2 different ways.

zones.get_records() will return several records for a multivalue record, while dnsdriver.get_record(zone.id,'A:whatever') will return only one multivalued record.

IMO this should be unified somehow as to not be presented in different ways. Either a multi value record is one record or its several(which makes no sense). Or a change in the base driver to make multivalue records work across all drivers?

Let me know what you think.

[Kami]

@Kami Isn't this the intended end? If you wanted to modify it, you would call update_recordinstead, which would update what the record is pointing at (thus, deleting one of the multi values and keeping the others), but this is for deleting the record altogether.

Knowing that a multi value record is still viewed by libcloud as just one record, this should be the case when doing either dnsdriver.delete(record) or record.delete() as both methods will try to destroy the whole record.

@Itxaka The reason why each record should be presented separately is to be consistent with the standard Libcloud DNS API. Other providers drivers handle it in that matter and it makes more sense as well. Amazon not handling it in that manner is, imo, really just exposing an (unfriendly) implementation detail to the end user. The whole idea behind Libcloud is to try to hide (as much as possible) all the inconsistencies between the providers and expose a consistent standard API.

Deleting all the related records when user really just wants to delete one, breaks the API promise. Sadly in some cases we can't avoid that (in those cases we use ex_ prefixes), but in this one we can.

And update_record and all the other methods also work with a single record - that's the whole promise of the standard API.

[Kami]

zones.get_records() will return several records for a multivalue record, while dnsdriver.get_record(zone.id,'A:whatever') will return only one multivalued record.

IMO this should be unified somehow as to not be presented in different ways. Either a multi value record is one record or its several(which makes no sense). Or a change in the base driver to make multivalue records work across all drivers?

Let me know what you think.

This sounds like an inconsistency / bug in zone.get_record().

As far as Libcloud DNS API is concerned, there is no such thing as a record with multiple values. This is just an implementation detail in Route53 API which we need to handle and expose a consistent API to the end user.

As far as fixing get_record goes - it sounds like we will need to implement a different id scheme for records with multiple values - e.g. hash of type, name, data. This will allows us to expose a consistent API and allow users to retrieve a single record. This is also some what related to delete_record, but we can already implement single record deletion without doing the id change...

[Kami]

Also, forgot to say - good catch on get_record as well - that's another edge case we need to handle.