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

Ignore synced config zones where no config item exists #7120

Merged
merged 1 commit into from Apr 16, 2019

Conversation

dnsmichi
Copy link
Contributor

@dnsmichi dnsmichi commented Apr 15, 2019

Story

User renames/deletes zone configuration in zones.conf on the client

This isn't related to the config sync per se, and requires configuration changes followed by reloads on the client itself (admin action).

Currently, the entire configuration in /var/lib/icinga2/api/zones is included during config compilation.
At this stage in the config compiler, we don't have any objects activated yet, so we cannot check against non-configured zone objects unfortunately.

/*
object Zone "global-templates" { global = true }
*/

This leads into this perpetum mobile situation:

The configuration is loaded and compiled ...

mbmif /usr/local/tests/icinga2/master-slave (master *+) # cat icinga2b/lib/icinga2/api/zones/global-templates/_etc/commands.conf
object CheckCommand "sleep" {
  command = [ "/bin/sleep", 30 ]
}

... but the config validation fails since the Zone object 'global-templates' does not exist.

[2018-09-28 11:45:47 +0200] critical/config: Error: Validation failed for object 'sleep' of type 'CheckCommand'; Attribute 'zone': Object 'global-templates' of type 'Zone' does not exist.
Location: in icinga2b/lib/icinga2/api/zones/global-templates/_etc/commands.conf: 1:0-1:26
icinga2b/lib/icinga2/api/zones/global-templates/_etc/commands.conf(1): object CheckCommand "sleep" {
                                                                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^
icinga2b/lib/icinga2/api/zones/global-templates/_etc/commands.conf(2):   command = [ "/bin/sleep", 30 ]
icinga2b/lib/icinga2/api/zones/global-templates/_etc/commands.conf(3): }

[2018-09-28 11:45:47 +0200] critical/config: 1 error

Solution

Really a problem, since we can only guess which zones may survive the config compiler. Since we would end in a broken configuration validation later on if zones contain errors, we can use this advantage to enforce a guess.

How?

By making the bold statement that you cannot sync zones via cluster zone sync for this specific instance.

This way, we can fairly assume that the configuration for zones was done statically/via API package.
The solution is to check whether a configitem of the type "Zone" and the directory name already exists.

/* We don't have an activated zone object yet. We may forcefully guess from configitems
 * to not include this specific synced zones directory.
 */
if(!ConfigItem::GetByTypeAndName(Type::GetByName("Zone"), zoneName)) {
        Log(LogWarning, "config")
                << "Ignoring directory '" << path << "' for unknown zone '" << zoneName << "'.";
        return;
}

This solves #3323

Upon next config sync, the directories are automatically purged then (requires the solution with staged syncs).

Technical Details

The culprit is that we're in compiling configuration stage here,
we don't have access to Zone::GetByName() as objects have not
been activated yet.

Our best guess is from a config item loaded before (e.g. from zones.conf)
since no-one can sync zones via cluster config sync either.

It may not be 100% correct since the zone object itself may be invalid.
Still, if the zone object validator fails later, the config breaks either way.

The problem with removal of these directories is dealt by the cluster
config sync with stages.

refs #6727
refs #6716

fixes #3323

The culprit is that we're in compiling configuration stage here,
we don't have access to `Zone::GetByName()` as objects have not
been activated yet.

Our best guess is from a config item loaded before (e.g. from zones.conf)
since no-one can sync zones via cluster config sync either.

It may not be 100% correct since the zone object itself may be invalid.
Still, if the zone object validator fails later, the config breaks either way.

The problem with removal of these directories is dealt by the cluster
config sync with stages.

refs #6727
refs #6716
@dnsmichi dnsmichi added bug Something isn't working area/distributed Distributed monitoring (master, satellites, clients) area/configuration DSL, parser, compiler, error handling labels Apr 15, 2019
@dnsmichi dnsmichi self-assigned this Apr 15, 2019
@dnsmichi dnsmichi added this to the 2.11.0 milestone Apr 15, 2019
@dnsmichi dnsmichi merged commit ab9b4b7 into master Apr 16, 2019
@dnsmichi dnsmichi deleted the bugfix/cluster-ignore-non-configured-zones branch April 16, 2019 09:08
@dnsmichi
Copy link
Contributor Author

ref/NC/509507

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/configuration DSL, parser, compiler, error handling area/distributed Distributed monitoring (master, satellites, clients) bug Something isn't working ref/NC
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[dev.icinga.com #10000] Removing zones crash remote icinga instances
1 participant