Provide DNSOverride for targets

[sfc-gh-raram]

The default behavior of cloudprober is to use the local DNS resolver for all targets. While this covers most use cases, some times we want to resolve a target using a DNS resolver that we provide such as Cloudflare's DNS resolver (1.1.1.1). This PR allows the user to specify the dns resolver for a probe and for static targets, it will use the specified dns resolver.

This change includes a unit test and was tested in a k8s environment where the cloudprober config included probes that used the dns resolver override and the default dns resolver.

[manugarg]

Hey Rahul,

Thanks for sending this. I think being able to override DNS resolver makes a lot of sense. I was wondering if you've considered adding the new option to TargetsDef proto instead:

cloudprober/targets/proto/targets.proto

Line 100 in 24587fb

message TargetsDef {

I think that's a better place for it and it will simplify the implementation quite a bit as we'll not have to pass this option around.

[sfc-gh-raram]

Hey Rahul,

Thanks for sending this. I think being able to override DNS resolver makes a lot of sense. I was wondering if you've considered adding the new option to TargetsDef proto instead:

cloudprober/targets/proto/targets.proto

Line 100 in 24587fb

message TargetsDef {

I think that's a better place for it and it will simplify the implementation quite a bit as we'll not have to pass this option around.

Agreed. Updated the PR with the new changes. Tested them locally and in k8s. When you get a chance, could you take another look? Thank you.

[manugarg]

Hi Rahul,

Few more comments. I am out of office until end of the week, so take your time.

Thanks,
Manu

[sfc-gh-raram]

I also have a question regarding the unit test. The unit test checks whether the correct DNSIP is present in the targets struct. Ideally, I would like to put this field in the resolver struct. However, I run into the issue that the resolver struct does not implement the Resolver interface since the resolve method signatures are different, preventing me from casting targets.resolver into the resolver struct. Is there a way to fix this, or am I missing something? Thank you.

[manugarg]

I also have a question regarding the unit test. The unit test checks whether the correct DNSIP is present in the targets struct. Ideally, I would like to put this field in the resolver struct. However, I run into the issue that the resolver struct does not implement the Resolver interface since the resolve method signatures are different, preventing me from casting targets.resolver into the resolver struct. Is there a way to fix this, or am I missing something? Thank you.

Ha, that's surprising. You're right.. adding a DNS IP field to resolver makes more sense, then you should be able to access it from target's resolver by type-casting.

Does the following not work?

in resolver.go:
struct Resolver {
   ...
   DNSOverrideIP string // Used for testing
}

in targets_test.go:

...
r, ok := t.resolver.(*dnsRes.Resolver)
if !ok {
  t.Fatalf(..)
}
assert.Equal(t, wantDNSOverrideIP, r.DNSOverrideIP, ..)
...

[sfc-gh-raram]

I also have a question regarding the unit test. The unit test checks whether the correct DNSIP is present in the targets struct. Ideally, I would like to put this field in the resolver struct. However, I run into the issue that the resolver struct does not implement the Resolver interface since the resolve method signatures are different, preventing me from casting targets.resolver into the resolver struct. Is there a way to fix this, or am I missing something? Thank you.

Ha, that's surprising. You're right.. adding a DNS IP field to resolver makes more sense, then you should be able to access it from target's resolver by type-casting.

Does the following not work?

in resolver.go:
struct Resolver {
   ...
   DNSOverrideIP string // Used for testing
}

in targets_test.go:

...
r, ok := t.resolver.(*dnsRes.Resolver)
if !ok {
  t.Fatalf(..)
}
assert.Equal(t, wantDNSOverrideIP, r.DNSOverrideIP, ..)
...

Unfortunately, it does not work. During runtime, I receive the following error panic: interface conversion: endpoint.Resolver is *targets.targets, not *resolver.Resolver [recovered].

[manugarg]

Looks good. Just last couple of minor comments if they make sense to you. Otherwise we can go with this too.

[manugarg]

LGTM. Thanks!