digitalocean: dns operations should be idempotent #4882
Conversation
k8s-ci-robot
added
cncf-cla: yes
| } | ||
|
|
||
| // digitalocean record.Name returns record without the zone prefix | ||
| // so normalize record by removing zone suffix |
There was a problem hiding this comment.
I think "prefix" in the comment here should be "suffix" (?)
|
|
||
| glog.V(2).Infof("record change set additions complete") | ||
| } | ||
|
|
||
| if len(r.removals) > 0 { |
There was a problem hiding this comment.
This could be problematic, if we temporarily delete a DNS record that is important, and there's a delay before we recreate it.
Hopefully everything is using upserts instead of add / remove pairs, but the distinction hasn't mattered as much where we've had batch operations.
We inherited this API from the federation code, and I added upsert IIRC. We could also look at how external-dns is handling it - the long term goal is to get closer to that code :-)
There was a problem hiding this comment.
Agreed, I don't love this implementation and it seems error prone, but DNS API is not setup in a way that plays nicely with the interface from the federation code. I'll take a look at external-dns and let you know if something better comes up :)
|
/lgtm (though tests are failing anyway, so it won't really help) Two comments though... |
k8s-ci-robot
added
the
lgtm
andrewsykim
force-pushed
the
digitalocean-improve-dns
branch
from
a11c861 to
65fb2bc
Compare
k8s-ci-robot
removed
the
lgtm
andrewsykim
force-pushed
the
digitalocean-improve-dns
branch
from
65fb2bc to
61dd131
Compare
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: andrewsykim, justinsb The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Modifies the DNS code for DigitalOcean to conform to what is expected from kops - where applying a record set overwrites any previous records.
#2150