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

Added Exoscale as Provider #625

Merged
merged 16 commits into from Jul 13, 2018

Conversation

FaKod
Copy link
Contributor

@FaKod FaKod commented Jul 4, 2018

Exoscale is a trademark of Akenes SA, a private company founded in 2011 and headquartered in Switzerland. My company uses their cloud offers to host Kubernetes clusters.

With this PR I implemented Exoscale's DNS API as additional provider.
Let me know what you think...

@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://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment 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. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jul 4, 2018
FaKod and others added 2 commits July 4, 2018 14:59
Signed-off-by: Yoan Blanc <yoan.blanc@exoscale.ch>
@FaKod
Copy link
Contributor Author

FaKod commented Jul 4, 2018

Signed the CLA and changed my author email accordingly.

@greut
Copy link
Contributor

greut commented Jul 4, 2018

@FaKod my e-mail address should be okay as well.

@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 Jul 4, 2018
@njuettner
Copy link
Member

Thank for your PR @FaKod 🙂

It would be nice to have a tutorial to get started with Exoscale.

I'll have a look at the code.

@njuettner njuettner self-assigned this Jul 4, 2018
}
}
}
for _, epoint := range changes.UpdateNew {
Copy link
Member

Choose a reason for hiding this comment

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

Just to understand it a bit better, why do you ignore the updates? I just only see that you're looping over update changes even if you would use a logging level which is not debug?

Copy link
Contributor Author

@FaKod FaKod Jul 5, 2018

Choose a reason for hiding this comment

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

I must admit that for my use case it seems to be sufficient to only support create and delete. Is update mandatory?

Copy link
Member

Choose a reason for hiding this comment

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

I'm just only asking because I never saw that someone wouldn't want to update their records. Could there people which might need it?

If you don't use it at all I'm not seeing any point looping over updates which doesn't do anything execept logging.

@FaKod
Copy link
Contributor Author

FaKod commented Jul 6, 2018

@njuettner I've added UpdateNew, while still iterating over UpdateOld, just for logging reasons (like it is done in pdns.go)

@FaKod
Copy link
Contributor Author

FaKod commented Jul 10, 2018

The label docs-missing means actually tutorial missing?

@njuettner
Copy link
Member

@FaKod yes sorry for the confusion. Tutorial is missing, do you mind to add it as well and add it also in the README.md. There are already other links pointing to the provider tutorials.

@FaKod
Copy link
Contributor Author

FaKod commented Jul 10, 2018

@njuettner for the tutorial... Which external-dns version should I refer to?

@k8s-ci-robot k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jul 10, 2018
@njuettner
Copy link
Member

v0.5.5

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 12, 2018
@njuettner njuettner removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 12, 2018
@njuettner
Copy link
Member

@FaKod it's looks like the tests are failing in types.go. Could you take a look again?

Yoan Blanc and others added 4 commits July 12, 2018 13:30
Signed-off-by: Yoan Blanc <yoan.blanc@exoscale.ch>
exoscale: fix boilerplate header
@FaKod
Copy link
Contributor Author

FaKod commented Jul 12, 2018

@njuettner Sorry... should work now

@njuettner
Copy link
Member

/LGTM

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 13, 2018
verbs: ["get","watch","list"]
- apiGroups: ["extensions"]
resources: ["ingresses"]
verbs: ["get","watch","list"]
Copy link
Member

Choose a reason for hiding this comment

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

Sorry I wasn't seeing it but you have to add

- apiGroups: [""]
  resources: ["nodes"]
  verbs: ["list"]

to run external-dns with RBAC

@njuettner njuettner removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 13, 2018
@njuettner njuettner added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 13, 2018
@njuettner
Copy link
Member

Thanks again for your PR!

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. lgtm "Looks good to me", indicates that a PR is ready to be merged. new-provider size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants