Skip to content

fix: fallback hostname to ip like add_target and get_target#51

Merged
nic-6443 merged 5 commits into
masterfrom
nic/fix-target-list
Feb 5, 2026
Merged

fix: fallback hostname to ip like add_target and get_target#51
nic-6443 merged 5 commits into
masterfrom
nic/fix-target-list

Conversation

@nic-6443

@nic-6443 nic-6443 commented Feb 4, 2026

Copy link
Copy Markdown

Because when using add_target and get_target, if hostname is not specified, it will fallback to ip:

function checker:add_target(ip, port, hostname, is_healthy, hostheader, tbl_meta)
ip = tostring(assert(ip, "no ip address provided"))
port = assert(tonumber(port), "no port number provided")
hostname = hostname or ip

hostname = hostname or ip

However, the same logic is missing in the incr_counter function. This causes the hostname to be missing in the counter key in the shared memory when using the report_http_status function to report passive health check data, which further results in the get_target_list function being unable to correctly retrieve the counter data:

image The screenshot above shows the data in the shared dict when using passive health. It can be seen that all entries in `target_list` contain `hostname`. However, because the `incr_counter` lacks fallback code for `hostname`, both the counter and state are recorded in the shared memory key that does not contain `hostname`.

local counter = self.shm:get(key_for(self.TARGET_COUNTER,
target.ip, target.port, target.hostname))
target.counter = {
success = ctr_get(counter, CTR_SUCCESS),
http_failure = ctr_get(counter, CTR_HTTP),
tcp_failure = ctr_get(counter, CTR_TCP),
timeout_failure = ctr_get(counter, CTR_TIMEOUT),
}

However, get_target_list needs to retrieve data from the shared dict through a key that includes hostname. Since the keys do not match, it cannot work.

@nic-6443 nic-6443 force-pushed the nic/fix-target-list branch from 4f50958 to 4cc586e Compare February 5, 2026 04:09
Signed-off-by: Nic <qianyong@api7.ai>
@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ nic-6443
❌ locao
You have signed the CLA already but the status is still pending? Let us recheck it.

Signed-off-by: Nic <qianyong@api7.ai>
r
Signed-off-by: Nic <qianyong@api7.ai>
--- no_error_log
healthy SUCCESS increment
event: target status '127.0.0.1 (127.0.0.1:2119)' from 'false' to 'true'
event: target status '127.0.0.1(127.0.0.1:2119)' from 'false' to 'true'

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

why this change?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

self:log(DEBUG, "event: target status '", hostname or "", "(", ip, ":",

Actually, there is no space between hostname and ( .

Because the assertion is no_error_log, the mistake wasn't detected. So the previous test was actually ineffective; if there was really a problem with the code, this test wouldn't have found it.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

nice!

@shreemaan-abhishek shreemaan-abhishek left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

great PR!

@nic-6443 nic-6443 merged commit f34b621 into master Feb 5, 2026
2 of 3 checks passed
@nic-6443 nic-6443 deleted the nic/fix-target-list branch February 5, 2026 08:03
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.

5 participants