Introduce dynamic configuartion reload global option

[Cropi]

The pull request encapsulates multiple changes, let me break it down for smaller parts:

Input threads

Name of a new input thread now consists of its name(e.g. imtcp) and its serial(e.g. 1 - in which dynamic conf reload has the thread been created). The purpose of this is to better distinguish between input threads that have been created during runtime.

Globals

New global directive is introduced - hup.reload.config. If set to "on", rsyslog will dynamically reload its configuration(right now only imtcp is implemented to do so). Otherwise, old behavior is used.

Modules

New function, reloadCnf() has been added to the interface. If a certain module implements it, the module will be signaled when rsyslog is HUPed, requesting the module to dynamically reload its config.

The imtcp module - conf reload

1. If there is just a slight difference between the old and new imtcp module configs we can not use old instances -> we need to reload all of them, note that not all of the module parameters force us to reload all instances, but for simplicity I think it makes sense to do it this way and later improve it
2. Check if new instance is part of old config. If indeed is, remove newly allocated resources and reconfigure pointers(relocate it from the running config into the config being loaded)
3. Remove old instance when not part of the new config.
   It would be a lot easier if I just closed all instances and restarted them later but in that case existing TCP connections would have been disturbed.
   The idea behind this is to not touch existing TCP connections, just relocate them from the old config to the new one.

Comparing two imtcp module/instance configs

I am open to any ideas for some change here. I could not come up with a better way when comparing them, also I am not sure where to place macros used for comparing two instances/configs -- other modules will need them as well.

[lgtm-com]

This pull request introduces 2 alerts when merging e4887d2 into 66a2692 - view on LGTM.com

new alerts:

• 2 for Commented-out code

[rgerhards]

@davidelang your feedback on the topic is highly appreciated!

[rgerhards]

@Cropi I think ubuntu 20 [T]SAN have valid complaints ;-)

[rgerhards]

New global directive is introduced - hup.reload.config. If set to "on", rsyslog will dynamically reload its configuration(right now only imtcp is implemented to do so). Otherwise, old behavior is used.

would it make sense to split this off to a separate PR or at least commit? I think this is laying ground for many changes to come and so IMHO it deserves to stand out in git history.

[Cropi]

@Cropi I think ubuntu 20 [T]SAN have valid complaints ;-)

Yes, you are right:D

would it make sense to split this off to a separate PR or at least commit? I think this is laying ground for many changes to come and so IMHO it deserves to stand out in git history.

Of course, I will divide it into smaller chunks. There are some parts that will require changes but I wanted to execute the CI.

[davidelang]

reloading configs is both an easy thing and a horribly hard thing, depending on how much the configs are changing. Can you go into a little detail about what sorts of things you are expecting to change? for example, adding additional inputs feeding into existing queue is relatively easy (and even for such a simple thing, it's an extremely useful feature to have) but making major changes to the flow of logs through multiple queues, changing queue names, and outputting to multiple places is an extremely complex problem that can leave logs orphaned in queues very easily. When you have the new config using different modules, it gets even messier as the allowed syntax changes depending on what modules are loaded. In between you have a 'normal' config where you don't have a complex flow, and are just working through the main queue outputting to different destinations (where any additional queues that are in use are only tied to a particular action, so if that queue doesn't appear in the new config, you could leave it in place with it's existing action to finish processing pending messages) I'll look through this PR later today, but if you could give us an idea of the scope that you are looking at (and a rough roadmap of the steps that you are thinking of) it would be useful. David Lang

[davidelang]

> New global directive is introduced - hup.reload.config. If set to "on", rsyslog will dynamically reload its configuration(right now only imtcp is implemented to do so). Otherwise, old behavior is used.

I think we want this directive to be more nuanced than 'on' 'off' (see my other comment about the possible complexity) David Lang

[rgerhards]

I think we want this directive to be more nuanced than 'on' 'off' (see my other comment about the possible complexity)

let me chime in. The goal here is actually to do the hard and complex thing, so we probably do not need the nuances. As code and PR develops, of course, I think @Cropi will go from least to more complex things.

[Cropi]

Thank you @davidelang for the constructive feedback. I did not specify in the description what I really want to accomplish with this PR, sorry for that. My long term goal would be to dynamically reload the entire config file via sending a HUP signal to rsyslog. I know that this task is extremely large and will certainly require lot of changes in the project.

All I wanted to accomplish in this PR, is to provide a way to dynamically reload the imtcp input module in the following manner: every single input(type="imtcp") instance, which

• is both present in old and new config is not disturbed at all
• is present in the old config but not in the new one is gracefully shut down
• is present in the new config but not in the old one will begin to run
  Please note that at this point, I do not take into consideration that the ruleset can change for an imtcp input. I know it's a bit silly but I think that will require a separate PR due to its complexity.

I totally agree that the hup.reload.config directive will require some changes and it won't be as simple as it is now.

I will divide the PR into smaller parts later today and address issues discovered by CI.

[lgtm-com]

This pull request introduces 2 alerts when merging 13ba861 into 66a2692 - view on LGTM.com

new alerts:

• 2 for Commented-out code

[Cropi]

Commit 1 e84efad - introduce reloadCnf() for modules, each module capable of reloading its config should implement it.
Commit 2 c6cf04d - make input thread names consist of name+ID in which dynamic config reload the thread has been started
Commit 3 ce90804 - extend tcpsrv interface to be able to terminate a single tcpsrv object
Commit 4 32974b1 - add functionality to compare two peers if they equal, will be used when comparing two imtcp instances
Commit 5 32974b1 - tcpsrv_etry_t instances were kept in a separate linked list. We need to connect them to the instanceConf struct, otherwise we can not dynamically reload the imtcp config because we are not able to determine which instance is connected to which tcpsrv_etry.
Commit 6 13ba861 - implement reloadCnf() for imtcp module(support for ruleset excluded)

The CI failed because of the following reasons: PR contains more than 1 commit, commented out code and elasticsearch issue connection issue.

Let me know if there is something I forgot to answer.

[davidelang]

On Wed, 26 Jan 2022, Attila Lakatos wrote: All I wanted to accomplish in this PR, is to provide a way to dynamically reload the ```imtcp``` input module in the following manner: every single ```input(type="imtcp")``` instance, which - is both present in old and new config is not disturbed at all - is present in the old config but not in the new one is gracefully shut down - is present in the new config but not in the old one will begin to run Please note that at this point, I do not take into consideration that the ruleset can change for an imtcp input. I know it's a bit silly but I think that will require a separate PR due to its complexity.

This is a good starting point. Another very useful place to do early would be imfile. will your initial version work with inputs going to rulesets? existing inputs with different rulesets? crypto changes? etc? or are you just looking at additions/removals? David Lang

[lgtm-com]

This pull request introduces 2 alerts when merging 805f886 into 66a2692 - view on LGTM.com

new alerts:

• 2 for Commented-out code

[Cropi]

On Wed, 26 Jan 2022, Attila Lakatos wrote: All I wanted to accomplish in this PR, is to provide a way to dynamically reload the imtcp input module in the following manner: every single input(type="imtcp") instance, which - is both present in old and new config is not disturbed at all - is present in the old config but not in the new one is gracefully shut down - is present in the new config but not in the old one will begin to run Please note that at this point, I do not take into consideration that the ruleset can change for an imtcp input. I know it's a bit silly but I think that will require a separate PR due to its complexity.
This is a good starting point. Another very useful place to do early would be imfile. will your initial version work with inputs going to rulesets? existing inputs with different rulesets? crypto changes? etc? or are you just looking at additions/removals? David Lang

The current version works with inputs going to rulesets. When config is reloaded, new inputs will deliver logs to whatever the ruleset is configured to. It has only one drawback: when a certain input is present in both old and new configs, then the ruleset is not updated(its content might have changed). In order to make it work, some changes in the tcpsrv interface might be needed. I can provide a fix for that but I do not really want to make this PR more complicated as it is now.

There are some globals which may affect imtcp instances. I've included some checks(thanks for notificing it) if these globals change, e.g. default netstream driver.
Some plans for next steps: as you mentioned, I would like to provide the ability to reload more than just one input module, maybe the next one can be imfile. Next, I plan to focus on other rsyslog directives as well starting from easier(dynstats, perctile stats) to more complex(rulesets). I wonder if we should track these steps somewhere else, to not get lost under a pull request. Separate github issue, mailing list or somewhere else?

[rgerhards]

Some plans for next steps: as you mentioned, I would like to provide the ability to reload more than just one input module, maybe the next one can be imfile. Next, I plan to focus on other rsyslog directives as well starting from easier(dynstats, perctile stats) to more complex(rulesets). I wonder if we should track these steps somewhere else, to not get lost under a pull request. Separate github issue, mailing list or somewhere else?

sorry, I overlooked this. This sounds like we should/could use github projects. Will look into it - there is some new beta feature, but not sure if this suits us well. The "old" one is clumsy but worked so far sufficiently well.

[rgerhards]

lol... I began to create the project this morning and was than dragged away. It's there, and I have assigned this PR to it. Pls try if you can assign your other work (else we need to look into permission settings).

[rgerhards]

@Cropi I plan the next scheduled stable release for the 15th. do you think we should include this PR then? If so, we may need to do a little of finish-up work. Pls let me know your thoughts.

[Cropi]

lol... I began to create the project this morning and was than dragged away. It's there, and I have assigned this PR to it. Pls try if you can assign your other work (else we need to look into permission settings).

Unfortunately, I am not able to assign a PR to the project.

[rgerhards]

Unfortunately, I am not able to assign a PR to the project.

thx, I'll have a look into permission requirements

[Cropi]

@Cropi I plan the next scheduled stable release for the 15th. do you think we should include this PR then? If so, we may need to do a little of finish-up work. Pls let me know your thoughts.

I think this PR will need additional changes, so it would be a good idea to postpone it to a later release.