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

add a basic integration test for service/google #63

Closed
wants to merge 2 commits into from
Closed

add a basic integration test for service/google #63

wants to merge 2 commits into from

Conversation

linki
Copy link
Member

@linki linki commented Mar 6, 2017

This is a bash file doing a simple integration test with a Service and Google DNS.

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

should this bash script also create a docker container to run alongside the executed commands ?

@linki linki added the do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. label Mar 7, 2017
@linki
Copy link
Member Author

linki commented Mar 7, 2017

My current goal is to run this on travis. Would also mean that I have to configure credentials there and have proper cleanup. Probably have to clarify with some people. Whatever is needed is in-scope.

@linki linki changed the title add a basic integration test for service/google [WIP] add a basic integration test for service/google Mar 7, 2017
@linki linki changed the title [WIP] add a basic integration test for service/google add a basic integration test for service/google Mar 8, 2017
@linki linki added work-in-progress and removed do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. labels Mar 8, 2017
@linki
Copy link
Member Author

linki commented Mar 16, 2017

it now uses a built image of external-dns. Also grabs credentials from your local machine. @ideahitme you want to give it a shot? Putting that on our CI needs a couple more steps though.

@linki linki added this to the v0.4 milestone Apr 11, 2017
do
sleep 5

DNS_TARGET=$(dig +short nginx-ingress.external-dns-test.gcp.zalan.do.)
Copy link
Contributor

Choose a reason for hiding this comment

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

This could return more than one ip and then your == will fail.
I guess it does not happen, now but maybe in the future.
Maybe add a TODO for it.

do
sleep 5

DNS_TARGET=$(dig +short nginx.external-dns-test.gcp.zalan.do.)
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

do
sleep 5

DNS_TARGET=$(dig @ns-cloud-b1.googledomains.com. +short nginx.external-dns-test.gcp.zalan.do.)
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

@linki
Copy link
Member Author

linki commented Jun 1, 2017

Thnaks for your review @szuecs.

@linki
Copy link
Member Author

linki commented Jun 1, 2017

Closing to reduce work-in-progress. Will follow up at some point. A set of integration tests per DNS provider is fairly important, imo.

@linki linki closed this Jun 1, 2017
lou-lan pushed a commit to lou-lan/external-dns that referenced this pull request May 11, 2022
- remove access to internals of KrewPaths and provide methods
- rename KrewPaths to Paths (tentative)
- make Paths testable + add tests
- MustGetKrewPaths: add test for home dir detection

This PR paves the way to;
- add environment-variable overrides for krew base path and have it testable
- refactor common filepath.Join()s that happen around returned paths so they
  are not repeated in the plugin lifecycle logic.
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. exploration work-in-progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants