Open troll-os opened 1 month ago
Welcome @troll-os!
It looks like this is your first PR to kubernetes-sigs/external-dns 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.
You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.
You can also check if kubernetes-sigs/external-dns has its own contribution guidelines.
You may want to refer to our testing guide if you run into trouble with your tests not passing.
If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!
Thank you, and welcome to Kubernetes. :smiley:
Hi @troll-os. Thanks for your PR.
I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test
on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.
Once the patch is verified, the new status will be reflected by the ok-to-test
label.
I understand the commands that are listed here.
Thanks @troll-os. It looks good. Would you please add some documentation on it ?
@mloiseleur Sure !
Where do you think it would fit the best? TXT registry ?
With this PR, it works only with TXT registry, so yes.
/ok-to-test
/lgtm
/assign @Raffo
@troll-os answered to your comments. Could you please also provide a way to test this manually with manifests and kubectl commands? I would be eager to verify myself that the migration works and then keep an end to end test around.
New changes are detected. LGTM label has been removed.
[APPROVALNOTIFIER] This PR is NOT APPROVED
This pull-request has been approved by: Once this PR has been reviewed and has the lgtm label, please ask for approval from mloiseleur. For more information see the Kubernetes Code Review Process.
The full list of commands accepted by this bot can be found here.
So I had to make a change for the OVH provider to ensure record targets are properly formatted, I ran another batch of test to migrate from the default
owner and for some reason, records returned by the OVH API don't have their targets formatted like ExternalDNS expects
Meaning that we couldn't map the record ID and the deletion would fail
I don't know why this happens on a specific zones, the other zones covered seem fine... When I make direct calls to the OVH API the formatting is good...
I didn't forget your query for manifests to tests the changes @Raffo, I'll make them ASAP
So I did a little of homework and made this work with Gandi with a domain I own for personal use and minikube
Steps used to reproduce on another setup than my work dev and prod environments :
make build
on the branch of this PR (eventually push to a repo you own) minikube enable addons ingress
apiVersion: apps/v1
kind: Deployment
metadata:
name: nginx
spec:
replicas: 1
selector:
matchLabels:
app: nginx
template:
metadata:
labels:
app: nginx
spec:
containers:
- image: nginx
name: nginx
ports:
- containerPort: 80
---
apiVersion: v1
kind: Service
metadata:
name: nginx
annotations:
external-dns.alpha.kubernetes.io/hostname: helloworld.whatever.tld
spec:
selector:
app: nginx
type: LoadBalancer
ports:
- protocol: TCP
port: 80
targetPort: 80
---
apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
name: nginx
spec:
ingressClassName: nginx
rules:
- host: helloworld.whatever.tld
http:
paths:
- path: /
pathType: Prefix
backend:
service:
name: nginx
port:
number: 80
apiVersion: apps/v1
kind: Deployment
metadata:
name: external-dns
spec:
replicas: 1
selector:
matchLabels:
app: external-dns
strategy:
type: Recreate
template:
metadata:
labels:
app: external-dns
spec:
containers:
- name: external-dns
image: yourRegistry.com/external-dns:v-someBuildNumber # <<<<<<<<<<<<
args:
- "--txt-prefix=%{record_type}-"
- "--txt-cache-interval=15m"
- "--log-level=info"
- "--log-format=text"
- "--txt-owner-id=old-owner"
- "--policy=sync"
- "--provider=youDecide" # <<<<<<<<<<<<<<<<
- "--registry=txt"
- "--interval=1m"
- "--domain-filter=whatever.tld"
- "--source=ingress"
env:
- name: whatever_env_var_provider_needs
value: "hello"
apiVersion: apps/v1
kind: Deployment
metadata:
name: external-dns
spec:
replicas: 1
selector:
matchLabels:
app: external-dns
strategy:
type: Recreate
template:
metadata:
labels:
app: external-dns
spec:
containers:
- name: external-dns
image: yourRegistry.com/external-dns:v-someBuildNumber # <<<<<<<<<<<<
args:
- "--txt-prefix=%{record_type}-"
- "--txt-cache-interval=15m"
- "--log-level=info"
- "--log-format=text"
- "--txt-owner-id=new-owner"
- "--from-txt-owner=old-owner"
- "--migrate-txt-owner"
- "--policy=sync"
- "--provider=youDecide" # <<<<<<<<<<<<
- "--registry=txt"
- "--interval=1m"
- "--domain-filter=whatever.tld"
- "--source=ingress"
env:
- name: whatever_env_var_provider_needs
value: "hello"
Also I tried to follow the CoreDNS setup from the external-dns docs to have a non "external" provider dependent way to tests but didn't manage to make it work for some reason and I didn't want to spend too much time on it (probably a minikube issue) knowing that it worked with a classic provider
Description
This pull requests aims to complete the work initiated in these PR : https://github.com/kubernetes-sigs/external-dns/pull/2466 and https://github.com/kubernetes-sigs/external-dns/pull/3631
Most credits go to the creators of the initial work, as I mostly just rebased their work on the current state of master
Initial PR description:
As for me I've successfully managed records without any issue using the newly added flags on the OVH provider If anyone is willing to try these changes on other providers I'd be happy to troubleshoot their issues
Regarding the documentation I'm not sure either in which category these changes are relevant, so taking any recommendations on that
Note: this is my first contribution to such a project and first time using Go aswell, I've went through the contribution guidelines but if I missed something please let me know so that I can fix it
Fixes #2036
Checklist