refactor (2/3): Remove responsibility for target DNS from GSLB assistant #505
Conversation
kuritka
requested review from
donovanmuller,
k0da,
somaritane and
ytsarev
as code owners
| // assert | ||
| assert.NoError(t, err) | ||
| assert.Equal(t, config.GetClusterNSName(), edgeDNSServer) | ||
| assert.True(t, reflect.DeepEqual(config.GetExternalClusterNSNames(), map[string]string{"uk": edgeDNSServer, "eu": edgeDNSServer})) |
There was a problem hiding this comment.
https://github.com/k8gb-io/k8gb/pull/505/files#diff-384d90ba8267cb88a0aab2845e0778c734551e380ce8040b033b006584153fdeR46 - hm. I didn't realize we check to Brexit in our tests :D
| @@ -69,6 +69,7 @@ var predefinedConfig = depresolver.Config{ | |||
| ClusterGeoTag: "us-west-1", | |||
| ExtClustersGeoTags: []string{"us-east-1"}, | |||
| EdgeDNSServer: "8.8.8.8", | |||
There was a problem hiding this comment.
Shouldn't we make localhost here? Or use 53 and override EdgeDNSServerPort in specific tests?
Otherwise 8.8.8.8 with 7753 is actually never working combination.
There was a problem hiding this comment.
@ytsarev , thx, amended!
regions:
I moved uk -> za
EdgeDNServer I put there more reasonable domain name. Tests for localhost EdgeDNSServer are already implemented: https://github.com/k8gb-io/k8gb/pull/505/files#diff-384d90ba8267cb88a0aab2845e0778c734551e380ce8040b033b006584153fdeR1418
GSLB assistant decides whether mock DNS (localhost:7753) or calculated NSServerName (gslb-ns-uk-cloud.example.com:53) will be used for DNS exchange. OVERRIDE_WITH_FAKE_EXT_DNS decides if NSServerNAme or localhost will be used. This PR removes such functionality from gslb assistant. Depresolver getNSName returns NSName based on EdgeDNSServer. If edgeDNSServer is localhost than nsname is localhost for any region, otherwise proper nsname is returned. I also removed OVERRIDE_WITH_FAKE_EXT_DNS and introduced EDGE_DNS_SERVER_PORT with default value 53. Now we can request DNS on any port. Signed-off-by: kuritka <[email protected]>
kuritka
force-pushed
the
refactor-remove-override-with-fakeDNS
branch
from
2a075a8 to
fde7998
Compare
GSLB assistant with help of
OVERRIDE_WITH_FAKE_EXT_DNSdecides whether to use mock DNS (localhost:7753) or calculated NSServerName (gslb-ns-uk-cloud.example.com:53).This PR removes such functionality from gslb assistant. Depresolver
getNSNamereturns NSName based on EdgeDNSServer.If edgeDNSServer is localhost than nsname is localhost for any region, otherwise proper nsname is returned.
I also removed
OVERRIDE_WITH_FAKE_EXT_DNSand introducedEDGE_DNS_SERVER_PORT.Now we can request edge DNS server on any port. All current settings are backward compatible as default 53 port is used.
Because OverrideWithFakeDNS was removed I had to change Assistant interface contract and regenerate mocks, covered by tests.
Signed-off-by: kuritka [email protected]