Skip to content
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

initial-design file #4

Merged
merged 6 commits into from
Feb 15, 2017
Merged

initial-design file #4

merged 6 commits into from
Feb 15, 2017

Conversation

ideahitme
Copy link

*** DO NOT MERGE ***
This is to start design documentation. The following contains the basic outline of what external-dns is and some goals/design decisions we should consider

@ideahitme ideahitme added the kind/support Categorizes issue or PR as a support question. label Feb 13, 2017
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Feb 13, 2017

External DNS should be compatible with annotations used by three above mentioned projects. The idea is that resources created and tagged with annotations for other projects should continue to be valid and now managed by External DNS.

TODO:*Add complete list here*
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

route53-kubernetes functions pretty simply and only works with properly annotated services. The README has more info: https://github.com/wearemolecule/route53-kubernetes

If I were to put it into the example table as given for Mate it would look like this:
Tag: domainName
Description: Hostname to be registered
Default: None (no DNS record would be created)
Example: foo.example.org

But, we would also need to include that it currently only functions on services (no support for ingress) and that a label dns: route53 must also be set on the service for which the user intends to create a DNS record.

All that being said, I could see how we may not want to support the above rules. It might be easier to support running route53-kubernetes and this project simultaneously until a user can correctly port everything.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@iterion I see your point and it feels right in route53-kubernetes case. in mate's case we do not tag resources which can/cannot be managed by mate. Hence running mate + external-dns at the same time might be problematic.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, agree that backwards compatibility for mate makes sense. If we can safely run both route53-kubernetes and external-dns I think it would be a good idea not to support its annotations. It's just less surface area that we'd ultimately probably just deprecate later. I'll add a small section documenting this.


User runs `kubectl create -f ingress.yaml`, this will create an ingress as normal.
Typically the user would then have to manually create a DNS record pointing the ingress endpoint
If the external-dns controller is running on the cluster, it could automatically configure the DNS records instead, by observing the host attribute in the ingress object.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we say we create records by observing the host attribute. I assume that means the host field specified here: https://kubernetes.io/docs/api-reference/v1.5/#ingressrule-v1beta1. But, later we specify an annotation to do the same (external-dns.kubernetes.io/hostname). Are both required?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my point of view only ingress host is required. In this doc it is mentioned this annotation is only required for other types of resources (which is only loadbalancer service at the moment). But this is certainly open for discussion

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see, to check my understanding. If you're using ingress resources ingress host is required. But, if you are using services then you must specify both of the annotations?

Copy link
Author

@ideahitme ideahitme Feb 13, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the host is specified when ingress is created. For services both annotations are required per my understanding

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To make this clear perhaps we should add a section under design, maybe something like:

### Configuration

DNS records will be automatically created in multiple situations:
1. Setting `spec.rules.host` on an ingress object.
2. Specifying two annotations (`external-dns.kubernetes.io/controller` and `external-dns.kubernetes.io/hostname`) on a `type=LoadBalancer` service object.

Copy link
Author

@ideahitme ideahitme Feb 13, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@iterion yup, seems good to me. You can push to this branch, this PR is for us to agree on same design basis :) we can fix it any time later. anyways as time goes we will probably need to create separate file(s) for configuration/how to run/use-cases/annotations etc

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, pushed.

@hjacobs hjacobs added the do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. label Feb 13, 2017

| Annotations | |
|---|---|
|Tag |external-dns.kubernetes.io/controller |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we make this the default (configurable)? I want users to have the least amount of annotations possible. There is no need for people to specify this annotation if only one controller is running in the cluster (which will be the case for 99% of the cases).


DNS records will be automatically created in multiple situations:
1. Setting `spec.rules.host` on an ingress object.
2. Specifying two annotations (`external-dns.kubernetes.io/controller` and `external-dns.kubernetes.io/hostname`) on a `type=LoadBalancer` service object.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here: why are two annotations needed? I would assume external-dns.kubernetes.io/controller to be optional in 99% of the use cases.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not necessary, I just grabbed it annotations section. It seems like external-dns.kubernetes.io/controller is optional since it has a default value. I'll correct this.

@k8s-ci-robot
Copy link
Contributor

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://github.com/kubernetes/kubernetes/wiki/CLA-FAQ to sign the CLA.

Once you've signed, please reply here (e.g. "I signed it!") and we'll verify. Thanks.


Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@k8s-ci-robot k8s-ci-robot added cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. and removed cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Feb 13, 2017
@@ -74,6 +74,10 @@ TODO:*Add complete list here*
|Default | Empty(falls back to template based approach) |
|Example|foo.example.org|

**route53-kubernetes*
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: missing star at the end of line

@iterion
Copy link
Contributor

iterion commented Feb 13, 2017

I signed it!

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Feb 13, 2017

## Goals

1. Support AWS Route53 and Google Cloud DNS providers
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is Azure out of scope?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, it's listed below, pls ignore.

@ideahitme
Copy link
Author

let's merge this for now and iterate on it later on. any objections ?

@linki
Copy link
Member

linki commented Feb 15, 2017

👍

@ideahitme ideahitme merged commit 786055f into master Feb 15, 2017
@ideahitme ideahitme removed the do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. label Feb 15, 2017
@ideahitme ideahitme deleted the docs/design branch February 24, 2017 16:55
ffledgling referenced this pull request in ffledgling/external-dns Jan 18, 2018
njuettner pushed a commit that referenced this pull request Jul 13, 2018
exoscale: fix boilerplate header
fraenkel added a commit to fraenkel/external-dns that referenced this pull request Apr 12, 2019
k8s-ci-robot pushed a commit that referenced this pull request Jul 30, 2020
Elbehery referenced this pull request in Elbehery/external-dns Oct 27, 2021
Add .ci-operator.yaml with base_root_image
seanmalloy referenced this pull request in KohlsTechnology/external-dns Nov 19, 2021
* chore: fix k8s-ci-robot license check on github

* fix: More linter fixes

* fix: linter issues

* Merge from github external-dns into release (#4)

* Allow multiple services to share same dns record

* NS record support

* Fix NS related provider test

* update comment to explain edge case better

* Add short tutorial on how to create NS record

* bumps cloudbuild image

Signed-off-by: GitHub <noreply@github.com>

* pre-pulls images due to cloudbuilder bug

* bump cloudbuild timeout for new release process

Signed-off-by: GitHub <noreply@github.com>

* aws-r53 adding Africa (Cape Town) ELB endpoints and hosted zone id's

* cloudflare: readme update for RBAC config

service account needs access to `watch` nodes.  GKE and AWS R53, the two stable providers, have this permission

* Update to Go 1.15 & update Azure dependencies

* fix: linter issues (#3)

* adds arm32v7 arch for images

Signed-off-by: Raffaele Di Fazio <difazio.raffaele@gmail.com>

* Add namespace for httpbin-gateway

Signed-off-by: 杨阳 10014842 <yangyang1@zte.com.cn>

* adds GOARM for arm32v7 images

Signed-off-by: GitHub <noreply@github.com>

* adds faq for multiarch images

Signed-off-by: GitHub <noreply@github.com>

* Fix OVH tutorial to match new permissions

External DNS now require permissions on endpoints resource. Adding it in the OVH tutorial manifest following this comment (kubernetes-sigs#961 (comment)) for making it continue to work out of the box.

Co-authored-by: Pavel Tumik <pavel.tumik@gmail.com>
Co-authored-by: Yury Tsarev <yury.tsarev@absa.africa>
Co-authored-by: Kubernetes Prow Robot <k8s-ci-robot@users.noreply.github.com>
Co-authored-by: Raffaele Di Fazio <difazio.raffaele@gmail.com>
Co-authored-by: Mic <mzingarelli@gmail.com>
Co-authored-by: R. Aidan Campbell <raidancampbell@users.noreply.github.com>
Co-authored-by: Jonas-Taha El Sesiy <github@elsesiy.com>
Co-authored-by: 杨阳 10014842 <yangyang1@zte.com.cn>
Co-authored-by: Victor Coutellier <victor.coutellier@gmail.com>

* adding e2e test stage

* fixing the yaml error

* parameterize target branch

* bump version

* doc: Update Readme to add F5 DNS Load Balancer Cloud service

Co-authored-by: Pavel Tumik <pavel.tumik@gmail.com>
Co-authored-by: Yury Tsarev <yury.tsarev@absa.africa>
Co-authored-by: Kubernetes Prow Robot <k8s-ci-robot@users.noreply.github.com>
Co-authored-by: Raffaele Di Fazio <difazio.raffaele@gmail.com>
Co-authored-by: Mic <mzingarelli@gmail.com>
Co-authored-by: R. Aidan Campbell <raidancampbell@users.noreply.github.com>
Co-authored-by: Jonas-Taha El Sesiy <github@elsesiy.com>
Co-authored-by: 杨阳 10014842 <yangyang1@zte.com.cn>
Co-authored-by: Victor Coutellier <victor.coutellier@gmail.com>
Co-authored-by: Rajeev Panda <r.panda@f5.com>
lou-lan pushed a commit to lou-lan/external-dns that referenced this pull request May 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/support Categorizes issue or PR as a support question.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants