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

[dev.icinga.com #9623] missing config warning on empty port in endpoints #3149

Closed
icinga-migration opened this issue Jul 15, 2015 · 7 comments
Closed
Labels
Milestone

Comments

@icinga-migration
Copy link
Member

@icinga-migration icinga-migration commented Jul 15, 2015

This issue has been migrated from Redmine: https://dev.icinga.com/issues/9623

Created by VisionFhg on 2015-07-15 07:28:38 +00:00

Assignee: jflach
Status: Resolved (closed on 2015-08-25 14:44:45 +00:00)
Target Version: 2.3.9
Last Update: 2015-08-25 14:45:30 +00:00 (in Redmine)

Icinga Version: 2.3.6
Backport?: Already backported
Include in Changelog: 1

We discovered that a faulty configuration in the zones.conf lead to an inconsistency in the connection.

Our zones.conf were configured like this (on node and master):

object Endpoint "icinga.local" {
        host = "192.168.2.99"
        port = ""
}
object Zone "dmz" {
        endpoints = [ "icinga.local" ]
}
object Endpoint "webshop.local" {
}
object Zone "webshop.local" {
        endpoints = [ "webshop.local" ]
        parent = "dmz"
}

Despite the missing Ports configuration everything was working flawlessly until a restart of the node, where the connection was lost and never able to recover unit a restart of the Masternode, after which everything was working again.

Adding the correct port information lead to no more connection losses. We reproduced this with 2.3.6.

Although it was a configuration error on our end, it might be worth to fallback to a sane default (5665) or throw a config error (better). As it was quite irritating to have a functional setup in the beginning, just to fail after a service restart on the node.

Feel free to contact us for further questions.

Changesets

2015-08-24 15:19:12 +00:00 by jflach 9f88a24

Add error on missing port, needs review and testing

refs #9623

2015-08-25 14:43:52 +00:00 by jflach 0513be2

Add config error on empty port in Endpoints

fixes #9623

Conflicts:
	lib/remote/endpoint.cpp
	lib/remote/endpoint.hpp

2015-08-25 14:44:32 +00:00 by jflach 9b05304

Add config error on empty port in Endpoints

fixes #9623

2015-08-25 15:06:08 +00:00 by jflach 2a9ac26

Move endpoint error check to ti file

refs #9623

2015-08-25 15:10:20 +00:00 by jflach f71a2ca

Move endpoint error check to ti file

refs #9623

Conflicts:
	lib/remote/endpoint.cpp
	lib/remote/endpoint.hpp

2015-08-26 05:10:49 +00:00 by (unknown) d01330a

Build fix

refs #9623

Relations:

@icinga-migration

This comment has been minimized.

Copy link
Member Author

@icinga-migration icinga-migration commented Jul 15, 2015

Updated by mfriedrich on 2015-07-15 07:51:25 +00:00

  • Relates set to 9205
@icinga-migration

This comment has been minimized.

Copy link
Member Author

@icinga-migration icinga-migration commented Jul 15, 2015

Updated by mfriedrich on 2015-07-15 07:53:01 +00:00

  • Category set to CLI
  • Status changed from New to Feedback
  • Assigned to set to VisionFhg

Your configuration was probably created with a version before 2.3.6 (see the fix in #9205). Can you verify that the node wizard is working flawlessly (not setting the port attribute if empty) in 2.3.6+?

@icinga-migration

This comment has been minimized.

Copy link
Member Author

@icinga-migration icinga-migration commented Jul 15, 2015

Updated by VisionFhg on 2015-07-15 08:14:08 +00:00

Yes i can confirm this. We created the zones.conf with the wizard in a pre 2.3.6version.
A testrun of the wizard with 2.3.6 does infact not leave the port empty (Port = "5665" after just pressing enter in the setup process; see below).

We distributed the faulty zones.conf with puppet, which was kind of separate from the wizard process.
That's why i'd still prefer a config error on an empty (port) setting, no matter how the config file was created.

Please specify if this is a satellite setup ('n' installs a master setup) [Y/n]: 
Starting the Node setup routine...
Please specifiy the common name (CN) [webshop.local]: 
Please specifiy the local zone name [webshop.local]: 
Please specify the master endpoint(s) this node should connect to:
Master Common Name (CN from your master setup): icinga.local
Do you want to establish a connection to the master from this node? [Y/n]: 
Please fill out the master connection information:
Master endpoint host (Your master's IP address or FQDN): icinga.local
Master endpoint port [5665]: 
Add more master endpoints? [y/N]: 
Please specify the master connection for CSR auto-signing (defaults to master endpoint host):
Host [icinga.local]: 
Port [5665]:

results in:

object Endpoint "icinga.local" {
    host = "icinga.local"
    port = "5665"
}
@icinga-migration

This comment has been minimized.

Copy link
Member Author

@icinga-migration icinga-migration commented Jul 15, 2015

Updated by mfriedrich on 2015-07-15 20:47:56 +00:00

  • Subject changed from Satellites lose connection after restart due to faulty ports configuration in zones.conf to missing config warning on empty port in endpoints
  • Status changed from Feedback to New
  • Assigned to deleted VisionFhg
@icinga-migration

This comment has been minimized.

Copy link
Member Author

@icinga-migration icinga-migration commented Jul 16, 2015

Updated by gbeutner on 2015-07-16 08:46:18 +00:00

  • Status changed from New to Assigned
  • Assigned to set to jflach

We should add a custom validator for the port attribute. Empty values should not be allowed.

@icinga-migration

This comment has been minimized.

Copy link
Member Author

@icinga-migration icinga-migration commented Aug 25, 2015

Updated by jflach on 2015-08-25 14:44:45 +00:00

  • Status changed from Assigned to Resolved
  • Done % changed from 0 to 100

Applied in changeset 0513be2.

@icinga-migration

This comment has been minimized.

Copy link
Member Author

@icinga-migration icinga-migration commented Aug 25, 2015

Updated by jflach on 2015-08-25 14:45:30 +00:00

  • Target Version set to 2.3.9
  • Backport? changed from TBD to Yes
@icinga-migration icinga-migration added this to the 2.3.9 milestone Jan 17, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant
You can’t perform that action at this time.