Skip to content
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

Telegraf: remove unused mosquitto dependency #554

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ukkopahis
Copy link

@ukkopahis ukkopahis commented Apr 29, 2022

The mosquitto dependency for Telegraf is a misconfiguration.

There is a mosquitto example for reading from from mosquitto (.templates/telegraf/iotstack_defaults/additions/inputs.mqtt_consumer.conf), but this is NOT enabled by default. As such there shouldn't be a default dependency either. Docs updated with instructions to add this dependency if you use the the addition.

Reasons to remove the dependency:

  • The generated docker compose won't work if telegraf is selected, but mosquitto isn't selected in the menu! The Menu should always produce a working docker-compose.yml.
  • nothing will break even if there's a mqtt consumer, resulting in the wrong startup order. Telegraf will try reconnecting to to the mqtt. At worst a spurious error is logged.

Resolves #550

@Paraphraser
Copy link

I really hate to be "All Mr Negative" twice in the space of 24 hours but I also can't endorse this proposed change.

Let's work backwards.

Q1: Why does Telegraf's service definition contain:

depends_on:
  - mosquitto

A1: Because of ❶ in the following:

/home/pi/IOTstack/.templates/telegraf/iotstack_defaults/
├── additions
│   └── inputs.mqtt_consumer.conf ❶
└── auto_include
    ├── inputs.cpu_temp.conf
    └── inputs.docker.conf

Q2: How does inputs.mqtt_consumer.conf create a dependency on Mosquitto?

A2: Because of line 4 in that file:

servers = ["tcp://mosquitto:1883"]

Q3: Why is that line there?

A3: It is the direct descendent of line 171 in Graham Garner's original implementation.

Q4: When did Graham Garner create that dependency?

A4: November 2019. That's the initial commit, unmodified since.

So, the way I see it is, every single IOTstack user who has ever used IOTstack to instantiate a Telegraf container has had a default configuration that assumes Telegraf is capable of subscribing to topics distributed by a broker responding on the URL tcp://mosquitto:1883 which, in practice, means a Mosquitto container running alongside Telegraf on the same host.

I don't see how that logical chain meets any reasonable definition of "unused". The presence of mosquitto in the depends-on clause simply formalises the expectation set by Telegraf's default configuration that the mosquitto broker will be there.

It may not be a Telegraf dependency. But it's an IOTStack implementation of Telegraf dependency.

The dependency is documented in the Telegraf documentation. If there are situations where it would be better for the Telegraf container to not have this linkage with Mosquitto then perhaps the documentation should be expanded to explain the how-to:

  1. Comment-out the lines in inputs.mqtt_consumer.conf; and then
  2. Remove mosquito from the depends_on clause in the service definition.

With the usual caveats that I'm not in charge of Jack Split, and that this is all just my 2¢ …

👎

@ukkopahis
Copy link
Author

"using an optional addition absolutely needs this dependency" (my takeaway)

Good point, docs updated with instructions to add this dependency back, if you use the the addition.

Telegraf container to not have this linkage with Mosquitto .. Comment-out the lines in inputs.mqtt_consumer.conf

Umm.. that is an optional addition, not enabled by default.

@ukkopahis ukkopahis force-pushed the telegraf-dependency-cleanup branch from 2bb2248 to 41c3f3e Compare May 5, 2022 16:50
Seems like remnant misconfiguration from the beginning.

Fixes docker-compose.yml to be valid even if you don't choose mosquitto
fron the menu.

Added documentation about adding the mosquitto 'depends_on' back if you
use the mqtt_consumer addition.

Resolves SensorsIot#550
@ukkopahis
Copy link
Author

PR cleaned up to only include changes regarding the dependency. Other important Telegraf changes split into their own pull-requests.

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.

Telegraf depends on mosquitto ?
2 participants