Adds Dnsimple as new provider #224

jose5918 · 2017-06-06T18:40:45Z

This PR adds Dnsimple as a provider option. I was just having a bit of trouble adding dnsimple's go library as a dependency with glide (Still a bit new with Go).

Edit: Got glide to run but seems it added a way more files than necessary

ideahitme · 2017-06-07T00:41:00Z

@jose5918 thanks for PR !

Kind request to either:

Not add the vendor before PR is reviewed (we can tolerate failing build), just include glide files so we can build it locally. Once PR is approved you can commit dependencies
Or
Add dependencies with a separate commit

jose5918 · 2017-06-07T16:18:08Z

@ideahitme Let me know if that looks better for you guys or if you'd prefer additional changes

linki · 2017-06-12T13:38:10Z

Glanced over it and looks good. I'll try it out this week. Thank you @jose5918.

ideahitme · 2017-06-22T14:52:53Z

provider/dnsimple_test.go

+
+type mockDnsimpleZonesService struct{}
+
+func (m *mockDnsimpleZonesService) ListZones(accountID string, options *dnsimple.ZoneListOptions) (*dnsimple.ZonesResponse, error) {


kind request to use testify/mock instead of mocking separate service for each type of expected response :)

i know we are using this pattern in other dns providers, but tests are getting incredibly bloated and difficult to read, so we need to take time to refactor existing tests as well

@ideahitme Sure thing. Also let me know if I should make any changes related to this issue #228

sure, sorry for keep you waiting. We will try to review this PR in the next few days. Thanks a lot for support ;)

linki · 2017-06-23T12:34:12Z

@jose5918 I merged in master and fixed some conflicts. Hope that's ok.

linki · 2017-06-23T13:44:09Z

Tested it out in my account. Initial record creation of records works fine:

$ external-dns --source service --domain-filter linki.space --provider dnsimple --namespace INFO[0001] Changing records: CREATE {0  0 A myapp.linki.space 146.148.123.148 0 0 false []  }
INFO[0001] Changing records: CREATE {0  0 TXT myapp.linki.space "heritage=external-dns,external-dns/owner=linki-local" 0 0 false []  }

But then I found a couple of issues:

On the second iteration it throw an error:

$ external-dns --source service --domain-filter linki.space --provider dnsimple --namespace external-dns-test --registry=txt --txt-owner-id=linki-local
INFO[0001] Changing records: CREATE {0  0 A myapp.linki.space 146.148.123.148 0 0 false []  }
ERRO[0001] POST https://api.dnsimple.com/v2/61844/zones/linki.space/records: 400 Zone record already exists

Then I removed the annotation so that it would delete the record. Also got an error:

$ external-dns --source service --domain-filter linki.space --provider dnsimple --namespace external-dns-test --registry=txt --txt-owner-id=linki-local
INFO[0001] Changing records: DELETE {0  0 A myapp 146.148.123.148 0 0 false []  }
ERRO[0001] GET https://api.dnsimple.com/v2/61844/zones//records: 404 Zone `records` not found

jose5918 · 2017-06-23T16:42:39Z

@linki Alright I will look into seeing what I broke. Looks like I had some mistakes in the unit test so I likely broke the delete functionality without realizing it when doing some cleanup.

jose5918 · 2017-06-26T23:02:34Z

@linki It should be fixed now. Here is an example test

$ external-dns --source service --domain-filter kube-external-test.io --provider dnsimple --namespace external-dns-jose --registry=txt
INFO[0008] All records are already up to date           
INFO[0069] Changing records: CREATE {0  0 A example.kube-external-test.io 35.185.193.99 0 0 false []  } in zone: kube-external-test.io 
INFO[0069] Changing records: CREATE {0  0 TXT example.kube-external-test.io "heritage=external-dns,external-dns/owner=default" 0 0 false []  } in zone: kube-external-test.io 
INFO[0130] Changing records: CREATE {0  0 A example2.kube-external-test.io 35.185.193.99 0 0 false []  } in zone: kube-external-test.io 
INFO[0130] Changing records: CREATE {0  0 TXT example2.kube-external-test.io "heritage=external-dns,external-dns/owner=default" 0 0 false []  } in zone: kube-external-test.io 
INFO[0130] Changing records: DELETE {0  0 A example.kube-external-test.io 35.185.193.99 0 0 false []  } in zone: kube-external-test.io 
INFO[0131] Changing records: DELETE {0  0 TXT example.kube-external-test.io "heritage=external-dns,external-dns/owner=default" 0 0 false []  } in zone: kube-external-test.io

@ideahitme Is this sort of what you had in mind with the testify mock tests?

ideahitme · 2017-07-06T15:14:22Z

@jose5918 sorry for keep you waiting. I will merge the conflict on this branch and review your PR :)

ideahitme · 2017-07-06T16:01:05Z

Hi @jose5918 could you please remove the provider/mocks/ package entirely and define mocks in the provider/dnsimple_test.go I would avoid using auto-generated code for a simple task of defining a mock

jose5918 · 2017-07-11T16:30:19Z

@ideahitme Closing was an accident, meant to just let you know that I would make the changes.

linki · 2017-08-18T12:08:24Z

@jose5918 will test again and then come back here

arno01 · 2017-10-08T15:18:44Z

@linki any news?

linki · 2017-10-09T14:40:47Z

@jose5918 I merged master, fixed some conflicts, added dnsimple's go code, removed the need for suitableZone from the provider package and added the provider to the changelog.

I just tested all the combinations I can think of against an AWS and GKE cluster (CNAME vs. A record) and it worked perfectly fine.

This is a strong good to go. Thanks @jose5918, great work! 👍

linki · 2017-10-11T10:12:34Z

Thank you, @jose5918 😻

MichielDeMey · 2017-10-13T14:44:04Z

You might want to add a markdown document here as well https://github.com/kubernetes-incubator/external-dns/tree/master/docs/tutorials to help other users setup DNSimple with external-dns.

linki · 2017-10-13T15:05:32Z

@MichielDeMey Good suggestion. Would you be willing to write a tutorial for DNSimple @jose5918?

jose5918 · 2017-11-14T00:34:58Z

@linki I can write a tutorial, but I no longer have a dnsimple account to verify against so I can't verify that my documentation will be correct

arno01 · 2017-12-02T09:30:25Z

@jose5918 I could verify your steps. I have a DNSimple account and running Kubernetes 1.8.4.
I never used the external-dns module and have no idea (yet) on how to plug it into my K8s and use it.
So I would greatly appreciate if you could give some steps in a way:

how do I use external-dns with my current K8s deployment? (the simplest and straight-forward way);
how do I setup external-dns so that it would use my DNSimple account?;

Please let me know.
If you drop the steps here or in a gist, I will verify and comment.

Thanks for your work!

jose5918 · 2017-12-04T20:08:02Z

@arno01 https://gist.github.com/jose5918/26d184d90fb4b9c7c045c8d71e2ec4c5

Let me know if it works with those instructions. It's been a while and I think I first tried it on Kubernetes 1.6

arno01 · 2017-12-04T22:20:57Z

@jose5918 thanks for the instruction!

I have tried it with my configuration, unfortunately it did not work as I expected. Maybe I am doing something wrong...

I do not have a LoadBalancer since I am running K8s on my own server, so I am using ClusterIP for the services and the Ingress resource to expose them.

I followed the instruction with the following changes:

--source=ingress for external-dns;
--domain-filter=nixaid.com as I am the owner of it;
DNSIMPLE_OAUTH set to my Account access token (not the User one);

Then, I had to create the RBAC file dnsimple-rbac.yaml with the following contents in order to get rid of:

$ kubectl -n nginx-ingress logs -f external-dns-5c86f88ff8-q7zcp
...
time="2017-12-04T21:40:29Z" level=info msg="Connected to cluster at https://10.96.0.1:443" 
time="2017-12-04T21:40:30Z" level=error msg="ingresses.extensions is forbidden: User "system:serviceaccount:default:default" cannot list ingresses.extensions at the cluster scope"

dnsimple-rbac.yaml:

---
apiVersion: v1
kind: ServiceAccount
metadata:
  name: sa-external-dns
  labels:
    app: external-dns
---
apiVersion: rbac.authorization.k8s.io/v1beta1
kind: ClusterRole
metadata:
  name: sa-external-dns
  labels:
    app: external-dns
rules:
  - apiGroups:
      - ""
      - extensions
    resources:
      - ingresses
      - services
    verbs:
      - get
      - list
      - watch
---
apiVersion: rbac.authorization.k8s.io/v1beta1
kind: ClusterRoleBinding
metadata:
  name: sa-external-dns
  labels:
    app: external-dns
roleRef:
  apiGroup: rbac.authorization.k8s.io
  kind: ClusterRole
  name: sa-external-dns
subjects:
  - kind: ServiceAccount
    name: sa-external-dns
    namespace: default

nginx-test1.yaml:

apiVersion: extensions/v1beta1
kind: Deployment
metadata:
  name: nginx-test1
spec:
  template:
    metadata:
      labels:
        app: nginx-test1
    spec:
      containers:
      - image: nginx:alpine
        name: nginx-test1
        ports:
        - containerPort: 80
---
apiVersion: v1
kind: Service
metadata:
  name: nginx-test1
spec:
  selector:
    app: nginx-test1
  type: ClusterIP
  ports:
    - protocol: TCP
      port: 80
      targetPort: 80
---
apiVersion: extensions/v1beta1
kind: Ingress
metadata:
  name: nginx-test1
  annotations:
    ingress.kubernetes.io/rewrite-target: /
    kubernetes.io/ingress.class: "nginx"
    kubernetes.io/tls-acme: "true"
    external-dns.alpha.kubernetes.io/hostname: test1.nixaid.com
spec:
  rules:
  - host: "test1.nixaid.com"
    http:
      paths:
      - backend:
          serviceName: nginx-test1
          servicePort: 80
        path: /
  tls:
    - hosts:
      - test1.nixaid.com
      secretName: test1.nixaid.com

My K8s nodes and their external IPs:

$ kubectl get nodes -o wide
NAME          STATUS    ROLES     AGE       VERSION   EXTERNAL-IP   OS-IMAGE                KERNEL-VERSION              CONTAINER-RUNTIME
k8s-master    Ready     master    6d        v1.8.4    <none>        REDACTED                REDACTED   docker://Unknown
k8s-worker1   Ready     <none>    6d        v1.8.4    10.0.2.111    REDACTED                REDACTED   docker://Unknown
k8s-worker2   Ready     <none>    6d        v1.8.4    10.0.2.112    REDACTED                REDACTED   docker://Unknown

The logs from external-dns v0.4.8:

$ kubectl logs -f external-dns-668c745544-r8dt5
time="2017-12-04T21:50:51Z" level=info msg="config: &{Master: KubeConfig: Sources:[ingress] Namespace: AnnotationFilter: FQDNTemplate: Compatibility: PublishInternal:false Provider:dnsimple GoogleProject: DomainFilter:[nixaid.com] AWSZoneType: AzureConfigFile:/etc/kubernetes/azure.json AzureResourceGroup: CloudflareProxied:false InfobloxGridHost: InfobloxWapiPort:443 InfobloxWapiUsername:admin InfobloxWapiPassword: InfobloxWapiVersion:2.3.1 InfobloxSSLVerify:true InMemoryZones:[] Policy:sync Registry:txt TXTOwnerID:default TXTPrefix: Interval:1m0s Once:false DryRun:false LogFormat:text MetricsAddress::7979 LogLevel:info}" 
time="2017-12-04T21:50:51Z" level=info msg="Connected to cluster at https://10.96.0.1:443" 
time="2017-12-04T21:50:52Z" level=info msg="Changing records: CREATE {0  0 A echo.nixaid.com 10.0.2.112 0 0 false []  } in zone: nixaid.com" 
time="2017-12-04T21:50:52Z" level=info msg="Changing records: CREATE {0  0 A nixaid.com 10.0.2.112 0 0 false []  } in zone: nixaid.com" 
time="2017-12-04T21:50:52Z" level=info msg="Changing records: CREATE {0  0 A old.nixaid.com 10.0.2.112 0 0 false []  } in zone: nixaid.com" 
time="2017-12-04T21:50:53Z" level=info msg="Changing records: CREATE {0  0 TXT echo.nixaid.com "heritage=external-dns,external-dns/owner=default" 0 0 false []  } in zone: nixaid.com" 
time="2017-12-04T21:50:54Z" level=info msg="Changing records: CREATE {0  0 TXT nixaid.com "heritage=external-dns,external-dns/owner=default" 0 0 false []  } in zone: nixaid.com" 
time="2017-12-04T21:50:54Z" level=info msg="Changing records: CREATE {0  0 TXT old.nixaid.com "heritage=external-dns,external-dns/owner=default" 0 0 false []  } in zone: nixaid.com" 
time="2017-12-04T21:51:56Z" level=info msg="Changing records: CREATE {0  0 A echo.nixaid.com 10.0.2.112 0 0 false []  } in zone: nixaid.com" 
time="2017-12-04T21:51:56Z" level=error msg="POST https://api.dnsimple.com/v2/REDACTED/zones/nixaid.com/records: 400 Zone record already exists" 
time="2017-12-04T21:52:57Z" level=info msg="Changing records: CREATE {0  0 A echo.nixaid.com 10.0.2.112 0 0 false []  } in zone: nixaid.com" 
time="2017-12-04T21:52:57Z" level=error msg="POST https://api.dnsimple.com/v2/REDACTED/zones/nixaid.com/records: 400 Zone record already exists" 
time="2017-12-04T21:53:58Z" level=info msg="Changing records: CREATE {0  0 A echo.nixaid.com 10.0.2.112 0 0 false []  } in zone: nixaid.com" 
time="2017-12-04T21:53:58Z" level=error msg="POST https://api.dnsimple.com/v2/REDACTED/zones/nixaid.com/records: 400 Zone record already exists" 
time="2017-12-04T21:54:59Z" level=info msg="Changing records: CREATE {0  0 A echo.nixaid.com 10.0.2.112 0 0 false []  } in zone: nixaid.com" 
time="2017-12-04T21:54:59Z" level=error msg="POST https://api.dnsimple.com/v2/REDACTED/zones/nixaid.com/records: 400 Zone record already exists" 
time="2017-12-04T21:55:59Z" level=info msg="Changing records: CREATE {0  0 A echo.nixaid.com 10.0.2.112 0 0 false []  } in zone: nixaid.com" 
time="2017-12-04T21:56:00Z" level=error msg="POST https://api.dnsimple.com/v2/REDACTED/zones/nixaid.com/records: 400 Zone record already exists" 
time="2017-12-04T21:57:00Z" level=info msg="Changing records: CREATE {0  0 A echo.nixaid.com 10.0.2.112 0 0 false []  } in zone: nixaid.com" 
time="2017-12-04T21:57:00Z" level=error msg="POST https://api.dnsimple.com/v2/REDACTED/zones/nixaid.com/records: 400 Zone record already exists" 
...
... looping over the echo.nixaid.com (I have an ingress resource for it, but I did not annotate it for the external-dns dnsimple to register it.. not sure if it should be trying to POST it...)
...

I can see that external-dns is trying to set the External IP of my K8s nodes which is not helping much since it is in a Class A (private internet), e.g. 10.0.2.112.

Probably there has to be some way that would be finding out what is my actual public IP or a way to set it in the configuration.

Then there was no message coming regarding the before mentioned nginx-test1 ingress resource, like if the external-dns does not see it is there.. is this normal?

Not sure why is it continuously trying to do POST into my zones (for some, but not all, already existing ingresses), saying that the record already exists... I would not expect it to update my zones unless I say so, e.g. using the annotation external-dns.alpha.kubernetes.io/hostname: test1.nixaid.com.

Please let me know if all this is expected or if I am doing something wrong.

Thanks!

arno01 · 2017-12-04T22:33:35Z

Humm, it appears it somehow created a duplicate record with the same Name under my DNSimple account... Probably, the good point that it did create some record there, though, I was not explicitly asking to go through already existing Ingress resources.. I presume that there should be the annotation external-dns.alpha.kubernetes.io/hostname in the first place, before it would be let POST...

$ dig +noall +answer A echo.nixaid.com @8.8.8.8
echo.nixaid.com.	299	IN	A	217.182.136.169
echo.nixaid.com.	299	IN	A	10.0.2.112

See the screenshot:

Upd I have also noticed one A record added was nixaid.com.nixaid.com, I presume that this could be some issue when it was reading the hostname from the Ingress resource for nixaid.com.

@jose5918

* Adds Dnsimple as a provider * chore(vendor): remove vendor for smaller diff * fix(config): make dnsimple selectable via flags * Fix delete and update * Dnsimple testify mock tests * remove leaked file * Move and simplify mock functions * chore: use lowercase for logrus repository * chore: update dependencies using glide * chore: vendor dnsimple-go package * ref: isolate suitable type in source package * add support for DNSimple, thx @jose5918 :D

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jun 6, 2017

jose5918 force-pushed the dnsimple branch 4 times, most recently from 6f819a3 to e091def Compare June 7, 2017 00:09

jose5918 force-pushed the dnsimple branch from e091def to 05cb748 Compare June 7, 2017 16:15

linki mentioned this pull request Jun 12, 2017

Dnsimple as new provider #215

Closed

Adds Dnsimple as a provider

fa1d0b7

jose5918 force-pushed the dnsimple branch from 05cb748 to fa1d0b7 Compare June 13, 2017 21:15

linki added size/large provider labels Jun 16, 2017

ideahitme reviewed Jun 22, 2017

View reviewed changes

Merge remote-tracking branch 'upstream/master' into dnsimple

026a17f

linki force-pushed the dnsimple branch from 9da3355 to 026a17f Compare June 23, 2017 12:37

chore(vendor): remove vendor for smaller diff

cb22016

fix(config): make dnsimple selectable via flags

ab7b25b

Jose Aguirre added 2 commits June 26, 2017 09:39

Fix delete and update

e621c9e

Dnsimple testify mock tests

c3c79c7

merge with master

38a83e7

remove leaked file

b6ff7ba

linki self-assigned this Aug 18, 2017

Merge remote-tracking branch 'upstream/master' into jose5918/dnsimple

da2cc21

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/large labels Oct 9, 2017

Martin Linkhorst added 2 commits October 9, 2017 14:43

chore: use lowercase for logrus repository

673f3b0

chore: update dependencies using glide

e7ac0f3

linki mentioned this pull request Oct 9, 2017

Fix issue with uppercase logrus import #351

Merged

Martin Linkhorst added 3 commits October 9, 2017 15:51

Merge remote-tracking branch 'origin/logrus' into jose5918/dnsimple

a30914c

chore: vendor dnsimple-go package

018e189

ref: isolate suitable type in source package

5b16649

k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Oct 9, 2017

linki force-pushed the dnsimple branch from 44985c3 to 045457a Compare October 9, 2017 14:44

add support for DNSimple, thx @jose5918 :D

6fc9067

linki force-pushed the dnsimple branch from e39e129 to 6fc9067 Compare October 11, 2017 09:43

Merge remote-tracking branch 'upstream/master' into jose5918/dnsimple

ff61669

linki added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 11, 2017

linki merged commit 09c35b6 into kubernetes-sigs:master Oct 11, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds Dnsimple as new provider #224

Adds Dnsimple as new provider #224

jose5918 commented Jun 6, 2017 •

edited

Loading

ideahitme commented Jun 7, 2017

jose5918 commented Jun 7, 2017

linki commented Jun 12, 2017

ideahitme Jun 22, 2017

ideahitme Jun 22, 2017

jose5918 Jun 22, 2017

ideahitme Jun 22, 2017

linki commented Jun 23, 2017

linki commented Jun 23, 2017

jose5918 commented Jun 23, 2017

jose5918 commented Jun 26, 2017

ideahitme commented Jul 6, 2017

ideahitme commented Jul 6, 2017

jose5918 commented Jul 11, 2017 •

edited

Loading

linki commented Aug 18, 2017

arno01 commented Oct 8, 2017

linki commented Oct 9, 2017

linki commented Oct 11, 2017

MichielDeMey commented Oct 13, 2017

linki commented Oct 13, 2017

jose5918 commented Nov 14, 2017

arno01 commented Dec 2, 2017

jose5918 commented Dec 4, 2017 •

edited

Loading

arno01 commented Dec 4, 2017 •

edited

Loading

arno01 commented Dec 4, 2017 •

edited

Loading


		type mockDnsimpleZonesService struct{}

		func (m mockDnsimpleZonesService) ListZones(accountID string, options dnsimple.ZoneListOptions) (*dnsimple.ZonesResponse, error) {

Adds Dnsimple as new provider #224

Adds Dnsimple as new provider #224

Conversation

jose5918 commented Jun 6, 2017 • edited Loading

ideahitme commented Jun 7, 2017

jose5918 commented Jun 7, 2017

linki commented Jun 12, 2017

ideahitme Jun 22, 2017

Choose a reason for hiding this comment

ideahitme Jun 22, 2017

Choose a reason for hiding this comment

jose5918 Jun 22, 2017

Choose a reason for hiding this comment

ideahitme Jun 22, 2017

Choose a reason for hiding this comment

linki commented Jun 23, 2017

linki commented Jun 23, 2017

jose5918 commented Jun 23, 2017

jose5918 commented Jun 26, 2017

ideahitme commented Jul 6, 2017

ideahitme commented Jul 6, 2017

jose5918 commented Jul 11, 2017 • edited Loading

linki commented Aug 18, 2017

arno01 commented Oct 8, 2017

linki commented Oct 9, 2017

linki commented Oct 11, 2017

MichielDeMey commented Oct 13, 2017

linki commented Oct 13, 2017

jose5918 commented Nov 14, 2017

arno01 commented Dec 2, 2017

jose5918 commented Dec 4, 2017 • edited Loading

arno01 commented Dec 4, 2017 • edited Loading

arno01 commented Dec 4, 2017 • edited Loading

jose5918 commented Jun 6, 2017 •

edited

Loading

jose5918 commented Jul 11, 2017 •

edited

Loading

jose5918 commented Dec 4, 2017 •

edited

Loading

arno01 commented Dec 4, 2017 •

edited

Loading

arno01 commented Dec 4, 2017 •

edited

Loading