-
Notifications
You must be signed in to change notification settings - Fork 808
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 Dns Resolve Host Count HealthCheck #684 #686
Conversation
yes that looks great! |
So, if we don't care about max, only min, we could put MAX_INT as the value...or we could make max_hosts a nullable int, etc... |
@bakgerman changed. Signature: public DnsResolveCountOptions AddHost(string hostName, int minHosts, int? maxHosts)
{
if (!HostRegistrations.TryGetValue(hostName, out var _))
{
HostRegistrations.Add(hostName, (minHosts, maxHosts));
}
return this;
} Check: if (totalAddresses < minHosts || (maxHosts.HasValue && totalAddresses > maxHosts.Value))
{
failedResolutions.Add((entry.Key, totalAddresses));
} |
|
||
|
||
|
||
if (totalAddresses < minHosts || (maxHosts.HasValue && totalAddresses > maxHosts.Value)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (totalAddresses < minHosts || (maxHosts.HasValue && totalAddresses > maxHosts.Value)) | |
if (totalAddresses < minHosts || totalAddresses > maxHosts) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mmm. maxHosts.Value will crash if it has no value?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, my typo, fixed.
d96b39d
to
e92b8d9
Compare
Added new DNS healthchecks in Network package to allow configuring a target host and the minimum and maximum IP addresses that should be resolved for that host name.
What this PR does / why we need it:
Which issue(s) this PR fixes: #684
Please reference the issue this PR will close: #[issue number]
Special notes for your reviewer:
Does this PR introduce a user-facing change?:
Please make sure you've completed the relevant tasks for this PR, out of the following list: