-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow record types to be changed #287
Comments
mikenairn
added a commit
to mikenairn/kuadrant-operator
that referenced
this issue
Oct 30, 2024
DNSRecords are currently owned by the DNSPolicy that created them. We can't just update the ownership on policy change since the new policy may not be compatible with the current DNSRecord, instead we must re-create the DNSRecord resource. This is not ideal, issue to look into changing this Kuadrant/dns-operator#287 Signed-off-by: Michael Nairn <[email protected]>
mikenairn
added a commit
to mikenairn/kuadrant-operator
that referenced
this issue
Oct 30, 2024
DNSRecords are currently owned by the DNSPolicy that created them. We can't just update the ownership on policy change since the new policy may not be compatible with the current DNSRecord, instead we must re-create the DNSRecord resource. This is not ideal, issue to look into changing this Kuadrant/dns-operator#287 Signed-off-by: Michael Nairn <[email protected]>
mikenairn
added a commit
to mikenairn/kuadrant-operator
that referenced
this issue
Oct 30, 2024
DNSRecords are currently owned by the DNSPolicy that created them. We can't just update the ownership on policy change since the new policy may not be compatible with the current DNSRecord, instead we must re-create the DNSRecord resource. This is not ideal, issue to look into changing this Kuadrant/dns-operator#287 Signed-off-by: Michael Nairn <[email protected]>
mikenairn
added a commit
to Kuadrant/kuadrant-operator
that referenced
this issue
Oct 31, 2024
dnspolicy section name support * fix dsnpolicy: Don't change owner ref on update DNSRecords are currently owned by the DNSPolicy that created them. We can't just update the ownership on policy change since the new policy may not be compatible with the current DNSRecord, instead we must re-create the DNSRecord resource. This is not ideal, issue to look into changing this Kuadrant/dns-operator#287 * Add canUpdateDNSRecord function Returns true if an existing record can knowingly be updated to a desired state based on the differences of the specs. Current known reasons for not being able to update are: * RootHost updates * Endpoint record type changes (A -> CNAME etc..) In both these cases, and any others that may be added to `canUpdateDNSRecord` the current record should be deleted before doing a create of the desired record. --------- Signed-off-by: Michael Nairn <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Currently we deliberately prevent a record type being modified in a provider by the dns operator e.g. A -> CNAME, instead throwing an error from the reconcile in this scenario.
From what i can remember we did this for the distributed dns cases where one cluster could be configured differently from another and we didn't want to end up in a continuous cycle of updates that hit the provider API, instead choosing to have any updates to the type fail instead.
This has the knock on effect of meaning you have to delete and re-create a DNSRecord if you wish to change the type of endpoint from one you previously created, this is the case for records owned by any number of owners (distributed or not).
In the kuadrant operator, to change from simple to loadbalanced, or vice versa, it is expected that the DNSPolicy be deleted and re-created in order to remove the current DNSRecord before creating it again. Apart from not being a great user experience, it also pins us down in how the policy works as we currently expect that a single policy owns a dns record in order for us to know when to delete it due to a policy change which could be a potential switch from simple to loadbalanced.
The text was updated successfully, but these errors were encountered: