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 hostAlias to real edgeDNS #726

Merged
merged 1 commit into from
Dec 1, 2021
Merged

switch hostAlias to real edgeDNS #726

merged 1 commit into from
Dec 1, 2021

Conversation

k0da
Copy link
Collaborator

@k0da k0da commented Nov 10, 2021

This runs rfc2136 provider and remove all local hacks

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

@k0da k0da force-pushed the edge branch 5 times, most recently from 922be2b to 421c8d8 Compare November 15, 2021 19:03
@k0da k0da force-pushed the edge branch 8 times, most recently from 1ed3319 to ecab83d Compare November 19, 2021 12:53
@k0da k0da changed the title switch host alias to real edgeDNS switch hostAlias to real edgeDNS Nov 23, 2021
@k0da k0da force-pushed the edge branch 6 times, most recently from 0d3002b to f178bcc Compare November 23, 2021 15:32
deploy/edge/zone.yaml Outdated Show resolved Hide resolved
@k0da k0da force-pushed the edge branch 4 times, most recently from 1594489 to ed6a7a5 Compare November 23, 2021 17:41
@k0da k0da force-pushed the edge branch 9 times, most recently from e701529 to 1094152 Compare November 23, 2021 22:54
Copy link
Member

@jkremser jkremser left a comment

Choose a reason for hiding this comment

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

pls, leave the support for host aliases in helm

We should probably update the docs later on (local playground needs to be changed/improved to make podinfo working in the browser) and also mention somewhere that it's not meant to be used in production, unless the credentials are changed, like in the original tutorial.

otherwise looks good to me. I've tested the branch locally by:

make deploy-full-local-setup CLUSTERS_NUMBER=2 CHART='./chart/k8gb'
# CHART='./chart/k8gb' is required here, because the changes in helm haven't been released yet

dig @localhost -p 1053 roundrobin.cloud.example.com +short
172.19.0.8
172.19.0.7
172.19.0.3
172.19.0.4
172.19.0.5
172.19.0.6
k scale deployment frontend-podinfo -n test-gslb --replicas 0 --context k3d-test-gslb1

dig @localhost -p 1053 roundrobin.cloud.example.com +short
172.19.0.8
172.19.0.7
172.19.0.5
172.19.0.6

k scale deployment frontend-podinfo -n test-gslb --replicas 0 --context k3d-test-gslb2
dig @localhost -p 1053 roundrobin.cloud.example.com +short
172.19.0.8
172.19.0.7

k scale deployment frontend-podinfo -n test-gslb --replicas 1 --context k3d-test-gslb1
dig @localhost -p 1053 roundrobin.cloud.example.com +short
172.19.0.3
172.19.0.4
172.19.0.5
172.19.0.6

good stuff @k0da 👍

@@ -209,6 +215,9 @@ deploy-k8gb-with-helm:
--set k8gb.imageTag=${VERSION:"stable"=""} \
--set k8gb.log.format=$(LOG_FORMAT) \
--set k8gb.log.level=$(LOG_LEVEL) \
--set rfc2136.enabled=true \
--set k8gb.edgeDNSServers[0]=$(CLUSTER_GSLB_GATEWAY):1053 \
--set externaldns.image=absaoss/external-dns:rfc-ns1 \
Copy link
Member

Choose a reason for hiding this comment

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

what were the changes in absaoss/external-dns:rfc-ns1, where is it being built (/dockerfile)? Is it the upstream repo + this PR kubernetes-sigs/external-dns#2439 ?

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, exactly. This is built from branch make build.mini

Makefile Outdated Show resolved Hide resolved
{{- if .Values.k8gb.hostAliases }}
hostAliases:
{{- toYaml .Values.k8gb.hostAliases | nindent 8 }}
{{- end }}
Copy link
Member

Choose a reason for hiding this comment

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

Can't we leave this here and also in values.yaml as an alternative option? These still can co-exist, right? 0 support on the "makefile level", but leaving the option there would be a safe thing to do, imho

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'd like to drop fall back from controller as well, right now it is quite confusing

Makefile Outdated Show resolved Hide resolved
jkremser
jkremser previously approved these changes Nov 26, 2021
@k0da
Copy link
Collaborator Author

k0da commented Nov 26, 2021

rebased against master to include k3d-action@v2 and drop temporary k3d hack

This runs rfc2136 provider and remove all local hacks

Signed-off-by: Dinar Valeev <dinar.valeev@absa.africa>
Makefile Show resolved Hide resolved
Copy link
Collaborator

@kuritka kuritka left a comment

Choose a reason for hiding this comment

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

👍 nice job!

Copy link
Member

@jkremser jkremser left a comment

Choose a reason for hiding this comment

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

👍

@k0da k0da merged commit e70ba53 into k8gb-io:master Dec 1, 2021
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.

4 participants