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

Switch to coredns with DNSendpoint plugin #292

Merged
merged 1 commit into from Feb 22, 2021
Merged

Switch to coredns with DNSendpoint plugin #292

merged 1 commit into from Feb 22, 2021

Conversation

k0da
Copy link
Collaborator

@k0da k0da commented Feb 5, 2021

  • put dnstype as label
  • Drop etcd, internal extdns instance, and coredns with etcd plugin
  • Use CoreDNS with DNSEndpoint plugin

Signed-off-by: Dinar Valeev dinar.valeev@absa.africa

@k0da k0da force-pushed the coredns_tune branch 8 times, most recently from 51f258c to 3d92452 Compare February 10, 2021 22:58
@k0da k0da changed the title WIP: switch to coredns with DNSendpoint plugin Switch to coredns with DNSendpoint plugin Feb 10, 2021
somaritane
somaritane previously approved these changes Feb 11, 2021
Copy link
Contributor

@somaritane somaritane left a comment

Choose a reason for hiding this comment

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

lgtm 👍

@k0da
Copy link
Collaborator Author

k0da commented Feb 12, 2021

@ytsarev thoughts?

@k0da
Copy link
Collaborator Author

k0da commented Feb 12, 2021

image (6)

kuritka
kuritka previously approved these changes Feb 12, 2021
Copy link
Member

@ytsarev ytsarev left a comment

Choose a reason for hiding this comment

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

It looks super cool - but how we test this shotgun-surgery level change? With current terratest test suite almost as it is.
@k0da please clarify the test automation approach

@@ -94,6 +94,7 @@ func (r *GslbReconciler) gslbDNSEndpoint(gslb *k8gbv1beta1.Gslb) (*externaldns.D
Name: gslb.Name,
Namespace: gslb.Namespace,
Annotations: map[string]string{"k8gb.absa.oss/dnstype": "local"},
Labels: map[string]string{"k8gb.absa.oss/dnstype": "local"},
Copy link
Member

Choose a reason for hiding this comment

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

Do we keep both labels and annotations?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes for backward compatibilty.

@k0da
Copy link
Collaborator Author

k0da commented Feb 21, 2021

@ytsarev terratest brings coredns up, terratest runs dig against new CoreDNS and receives expected result from it.

Change isn't huge. We just cutted of ETCD as medium for CoreDNS and use DNSEndpoint object directly.

* put dnstype as label
* Drop etcd, internal extdns instance, and coredns with etcd plugin
* Use CoreDNS with DNSEndpoint plugin

Signed-off-by: Dinar Valeev <dinar.valeev@absa.africa>
@k0da k0da dismissed stale reviews from kuritka and somaritane via 7a0701c February 21, 2021 20:36
@k0da k0da force-pushed the coredns_tune branch 2 times, most recently from 7a0701c to cb79e1e Compare February 21, 2021 21:12
@ytsarev
Copy link
Member

ytsarev commented Feb 22, 2021

@k0da could you point me to the tests you are talking about?

@k0da
Copy link
Collaborator Author

k0da commented Feb 22, 2021

@ytsarev
Copy link
Member

ytsarev commented Feb 22, 2021

@k0da so pretty much existing regression test suite. Ok, it makes sense

@k0da k0da merged commit e82f322 into master Feb 22, 2021
@ytsarev ytsarev mentioned this pull request Feb 22, 2021
@somaritane somaritane deleted the coredns_tune branch February 22, 2021 19:24
k0da referenced this pull request Feb 28, 2021
During CoreDNS refactor https://github.com/AbsaOSS/k8gb/pull/292
local external-dns instance was removed, while Route53 and NS1
external-dns instances relies on service account created by local
instance. This commit brings back external-dns service account.

Fixes: https://github.com/AbsaOSS/k8gb/issues/328

Signed-off-by: Dinar Valeev <dinar.valeev@absa.africa>
ytsarev referenced this pull request Mar 1, 2021
During CoreDNS refactor https://github.com/AbsaOSS/k8gb/pull/292
local external-dns instance was removed, while Route53 and NS1
external-dns instances relies on service account created by local
instance. This commit brings back external-dns service account.

Fixes: https://github.com/AbsaOSS/k8gb/issues/328

Signed-off-by: Dinar Valeev <dinar.valeev@absa.africa>
k0da referenced this pull request Mar 1, 2021
* Bring back external-dns service account

During CoreDNS refactor https://github.com/AbsaOSS/k8gb/pull/292
local external-dns instance was removed, while Route53 and NS1
external-dns instances relies on service account created by local
instance. This commit brings back external-dns service account.

Fixes: https://github.com/AbsaOSS/k8gb/issues/328

Signed-off-by: Dinar Valeev <dinar.valeev@absa.africa>

* Add missing RBAC for external-dns

Signed-off-by: Yury Tsarev <yury.tsarev@absa.africa>

Co-authored-by: Yury Tsarev <yury.tsarev@absa.africa>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants