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 TTL annotation check for azure records. #436

Merged
merged 2 commits into from
Jan 18, 2018

Conversation

stromming
Copy link
Contributor

No description provided.

@k8s-ci-robot k8s-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Jan 8, 2018
@hjacobs
Copy link
Contributor

hjacobs commented Jan 8, 2018

Thanks for the PR! Does it make sense to add a test for this?

@ideahitme
Copy link

yes, this definitely needs a test

@stromming please add it

@stromming
Copy link
Contributor Author

stromming commented Jan 10, 2018

Running into problems while writing tests;

provider.Records() creates an endpoint slice which ignores the TTL of the records (see e.g. external-dns/provider/azure.go on row 134). This is true for at least google and azure providers. Is this intended behaviour?

If not I'll add fix to this PR

Edit: Nevermind, will mimic behaviour from AWS provider.

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jan 10, 2018
@linki
Copy link
Member

linki commented Jan 16, 2018

LGTM 👍 @peterhuene @TsuyoshiUshio What do you think?

@peterhuene
Copy link
Contributor

peterhuene commented Jan 16, 2018

LGTM 👍 Thanks for the work, @stromming!

@njuettner njuettner merged commit a80d755 into kubernetes-sigs:master Jan 18, 2018
grimmy pushed a commit to grimmy/external-dns that referenced this pull request Apr 10, 2018
Added TTL annotation check for azure records.
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. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants