-
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
Add external-dns(v0.14.0) as a dependency #43
Conversation
@@ -15,7 +15,7 @@ import ( | |||
"sigs.k8s.io/controller-runtime/pkg/client" | |||
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" | |||
"sigs.k8s.io/controller-runtime/pkg/log" | |||
gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" | |||
gatewayapiv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See description for the reason why this is downgraded
} | ||
|
||
return domain, nil | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We might want to replace this by adding a new rootDomain
field to the dnsrecord spec, but this will work for now.
a706103
to
34a4294
Compare
34a4294
to
70a4d25
Compare
Adds external-dns (latest released version v0.14.0) as a go module dependency and updates the dnsrecord controller and dns provider logic to align with an external-dns approach to dns management. dnsrecord changes: * Replace v1alpha1.Endpoint with externaldns Endpoint * Add GetRootDomain to DNSRecord, returns the shortest domain that is shared across all spec.Endpoints dns names. provider changes: * Remove Ensure and Delete and instead include externaldnsprovider.Provider in provider.Provider interface. * Add provider.Config struct to hold common provider configuration, currently it only handles filters. Update provider Factory constructors accept the config as input. * Set DomainFilter, ZoneTypeFilter and ZoneIDFilter in provider implementations from provider.Config. provider aws changes: * Implement AdjustEndpoints to change our generic provider specific fields for weight and geo into aws ones understood by external-dns. provider google changes: * Copy external-dns google provider * Adds implementations of funtions required to translate endpoints and resourceRecordSets into the different states. - endpointsToProviderFormat - converts a list of endpoints into a google specific format. - endpointsFromResourceRecordSets - converts a list of `ResourceRecordSet` into endpoints (google format). - resourceRecordSetFromEndpoint - converts an endpoint(google format) into a `ResourceRecordSet`. controller changes: * Update DNSRecord controller to use applyChanges which follows an external-dns process to determine a set of changes(Create/Update/Delete) that are required to be made against the provider. * Use noop registry, no owner is currently set or checked for any records. * Setup provider filters: - DomainFilter = root domain for endpoints determined by calling dnsRecord.GetRootDomain() - ZoneTypeFilter = "", public or private zones considered. - ZoneIDFilter = set to single hosted zone id (managedZone.Status.ID) * Use sync policy, all changes will be applied (Create, Update and Deletes) * Use plan with above and using the current zone(plan.Current) and spec endpoints(plan.Desired) as input. additional changes: downgrade gatewayapiv1 to v0.7.1. This is required since the external dns aws provider is including the external dns source package which has a dependency on Gateway API v0.7.1. Downgrading is less than ideal, but dns-operator shouldn't have a dependency on gateway api, and the health check code that is currently causing it will likely be removed soon anyway in favour of provider health checks. There is also an opportunity to change external-dns to prevent it including the source package since it's including here for no reason other than getting a version https://github.com/kubernetes-sigs/external-dns/blob/master/provider/aws/session.go#L72
70a4d25
to
5ecdc28
Compare
Could you put the samples you showed us in the demo somewhere, so I can try this out locally? |
Added a "Validation" section to the end of the description. Has links to the example DNSRecords i was using. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
Adds external-dns (latest released version v0.14.0) as a go module dependency and updates the dnsrecord controller and dns provider logic to align with an external-dns approach to dns management.
Updates
dnsrecord changes:
v1alpha1.Endpoint
with externaldns EndpointGetRootDomain
method toDNSRecord
, returns the shortest domain that is shared across allspec.Endpoints
dns names.provider changes:
externaldnsprovider.Provider
inprovider.Provider
interface.provider.Config
struct to hold common provider configuration, currently it only handles filters. Update provider Factory constructors accept the config as input.provider.Config
.AWS provider changes:
AdjustEndpoints
to change our generic provider specific fields for weight and geo into aws ones understood by external-dns.Google provider changes:
ResourceRecordSet
into endpoints (google format).ResourceRecordSet
.controller changes:
DNSRecord
controller to use applyChanges which follows an external-dns process to determine a set of changes(Create/Update/Delete) that are required to be made against the provider.additional changes:
downgrade gatewayapiv1 to v0.7.1. This is required since the external dns aws provider is including the external dns source package which has a dependency on Gateway API v0.7.1. Downgrading is less than ideal, but dns-operator shouldn't have a dependency on gateway api, and the health check code that is currently causing it will likely be removed soon anyway in favour of provider health checks. There is also an opportunity to change external-dns to prevent it including the source package since it's including here for no reason other than getting a version
https://github.com/kubernetes-sigs/external-dns/blob/master/provider/aws/session.go#L72
Validation
As part of the POC several example dnsrecords were created to run through the various different scenarios on Google and AWS. These were all specific to my own accounts so are not added here. I did add them to the poc branch though so can be used, with modification, for testing here.
A test script is also available in this branch to step through each of the scenarios manually.
A test suite based on these will be created in the very near future,
Note: The tests for "multicluster with no control plane" are not expected to work currently
closes #47