If a modifier fails the host should be discarded

[paalbra]

If a modifier fails, the processing will just continue without those modifications:

zabbix-auto-config/zabbix_auto_config/processing.py

Lines 302 to 311 in 2b45f1c

	try:
	modified_host = host_modifier["module"].modify(host.copy(deep=True))
	assert isinstance(modified_host, models.Host)
	assert hostname == modified_host.hostname, f"Modifier changed the hostname, '{hostname}' -> '{modified_host.hostname}'"
	host = modified_host
	except AssertionError as e:
	logging.warning("Host, '%s', was modified to be invalid by modifier: '%s'. Error: %s", hostname, host_modifier["name"], str(e))
	except Exception as e:
	logging.warning("Error when running modifier %s on host '%s': %s", host_modifier["name"], hostname, str(e))
	# TODO: Do more?

This could lead to a weird state/unknown problems (very dependent on the modifier, obviously).

I think that the current host, that is being modified, should be discarded from the current update run (kind of like a failing collector), since we do not know what happened/what it really should look like. This would mean: Keep the host as is in the database.

This issue is also mentioned in #57

[paalbra]

Example by changing what is in the current README:

zabbix-auto-config/README.md

Lines 84 to 89 in 2b45f1c

	cat > path/to/host_modifier_dir/mod.py << EOF
	def modify(host):
	if host.hostname == "bar.example.com":
	host.properties.add("barry")
	return host
	EOF

Just add something like this to the modify function:

if random.randint(0, 1):
    raise Exception("FAILED "*50)

Then you will see Template-barry having a 50% chance of being linked to bar.example.com. It would be better to discard the host 50% of the time, and always keep the linked template.

[pederhan]

What if there is a syntax error or something that causes the modifer to fail 100% of the time? You would effectively remove all hosts, wouldn't you? Wouldn't this cause a non-trivial strain on the database from constantly adding and removing hosts, while also never actually recovering from the error? Or do you just mean that the host modifier process itself should just stop applying modifiers to that host, and otherwise continue?

Either way, if a failing modifier should cause a host to be discarded, I think there also needs to be some way to detect a situation in which a modifier fails repeatedly: see my comment on #57. Although that comment was related to source collectors, I think we can apply the same concept to host modifiers.

[paalbra]

Oh, I might not have been clear.

By "discarded" I don't at all mean delete. I just mean that it should be discarded from the current update run. E.g., in my example, the host bar.example.com should be kept as is, and not loose the barry property, and therefore not lose the Template-barry.

This would mean not updating the database with the failed modification.

EDIT: I tried to edit the issue post to be more precise.

[paalbra]

And if something fails 100% of the time I still think it is fair to discard all updates. You should seriously look into something that fails 100% of the time. That's no good.

[pederhan]

Yes, but your suggestion does not provide a failure path for bad modifiers. Yes, it prevents unpredictable state stemming from a modifier that fails, but the problematic modifier itself will never be removed, and it will generate potentially thousands of log entries per minute as long as it's active.

[paalbra]

That's true. They will just keep on failing.

This is mostly a hypothetical worst case scenario though. Generally this shouldn't happen, but if it does I think discarding is the best solution.

I don't see thousands of log entries as any issue if you compare it to erroneously modifying thousands of hosts, won't you agree?