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

Document host Common Runtime Attribute #9883

Merged
merged 1 commit into from
Oct 25, 2023

Conversation

oxzi
Copy link
Member

@oxzi oxzi commented Oct 24, 2023

While being present in the attribute list for the Service object, the Host's name field is missing in the list of all attributes, even when being referred to in multiple examples of assign where.

It is also present in the REST API's response:

$ curl -s -k -u root:icinga 'https://localhost:5665/v1/objects/hosts' | jq '.results[0].name'

@cla-bot cla-bot bot added the cla/signed label Oct 24, 2023
@julianbrost
Copy link
Contributor

That attribute is not just present in Host objects but everywhere. However, https://icinga.com/docs/icinga-2/latest/doc/09-object-types/#common-runtime-attributes doesn't list it either, but that should be the better place to document it.

@oxzi oxzi force-pushed the doc-09-object-types-host-name branch from 454dc87 to bdfb1dd Compare October 24, 2023 09:27
@oxzi
Copy link
Member Author

oxzi commented Oct 24, 2023

@julianbrost: Thanks for the clarification. I found it in the Service object's documentation and copied it to the Host object as I recently missed it.
I have altered this PR to move it to the Common Runtime Attributes section and pruned it from its only other appearance in the Service object section.

@oxzi oxzi changed the title Document Host object's name field Document host Common Runtime Attribute Oct 24, 2023
Copy link
Member

@Al2Klimov Al2Klimov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Julian, what do you think?

@@ -731,7 +732,6 @@ Configuration Attributes:
event\_command | Object name | **Optional.** The name of an event command that should be executed every time the service's state changes or the service is in a `SOFT` state.
volatile | Boolean | **Optional.** Treat all state changes as HARD changes. See [here](08-advanced-topics.md#volatile-services-hosts) for details. Defaults to `false`.
zone | Object name | **Optional.** The zone this object is a member of. Please read the [distributed monitoring](06-distributed-monitoring.md#distributed-monitoring) chapter for details.
name | String | **Required.** The service name. Must be unique on a per-host basis. For advanced usage in [apply rules](03-monitoring-basics.md#using-apply) only.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure whether to remove it. This one is on a per-host basis, ...

@@ -34,6 +34,7 @@ the [Icinga 2 API](12-icinga2-api.md#icinga2-api-config-objects).
templates | Array | Templates imported on object compilation.
package | String | [Configuration package name](12-icinga2-api.md#icinga2-api-config-management) this object belongs to. Local configuration is set to `_etc`, runtime created objects use `_api`.
source\_location | Dictionary | Location information where the configuration files are stored.
name | String | Object name. Must be unique on a per-type basis. Might be used for advanced usage in [apply rules](03-monitoring-basics.md#using-apply) only.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... but this one is unique on a per-type basis.

@julianbrost
Copy link
Contributor

I mean I was somewhat surprised that the Service object actually documented the name attribute (being the only type doing that). That makes it look like you should do name = "...", so that doesn't look like the right place for that information.

There should be some place in the documentation from which you should know that services on the same host need distinct names but multiple hosts can have services with identical names (or is that too obvious to explicitly write down?), but the service attributes don't look like the correct place for that, also given that the same restriction applies to Dependency, Notification and ScheduledDowntime as well (and Comment and Downtime, but you shouldn't create those object manually).

@Al2Klimov
Copy link
Member

That makes it look like you should do name = "...", so that doesn't look like the right place for that information.

Full ack! name=x enables you to name config objects not like config items. Which is already scary enough even if it doesn't break anything. So shall we document it at all?

services on the same host need distinct names but multiple hosts can have services with identical names (or is that too obvious to explicitly write down?

Yep.

@oxzi
Copy link
Member Author

oxzi commented Oct 25, 2023

services on the same host need distinct names but multiple hosts can have services with identical names (or is that too obvious to explicitly write down?

Yep.

Which part of the logical OR question was that a yes to?

However, as the text in question is now part of the Common Runtime Attributes section, couldn't we just remove the uniqueness note?

@julianbrost
Copy link
Contributor

However, as the text in question is now part of the Common Runtime Attributes section, couldn't we just remove the uniqueness note?

Yes, the text before the table with these attributes says "which cannot be modified by the user", so I mean that isn't the place you look for naming your objects anyways.

@oxzi oxzi force-pushed the doc-09-object-types-host-name branch from bdfb1dd to 2b259ca Compare October 25, 2023 08:05
@oxzi
Copy link
Member Author

oxzi commented Oct 25, 2023

Yes, the text before the table with these attributes says "which cannot be modified by the user", so I mean that isn't the place you look for naming your objects anyways.

Okay thanks. I have just pushed an updated version.

Al2Klimov
Al2Klimov previously approved these changes Oct 25, 2023
@Al2Klimov Al2Klimov added area/documentation End-user or developer help consider backporting Should be considered for inclusion in a bugfix release labels Oct 25, 2023
@Al2Klimov Al2Klimov added this to the 2.15.0 milestone Oct 25, 2023
doc/09-object-types.md Outdated Show resolved Hide resolved
@julianbrost julianbrost merged commit 3e8752d into master Oct 25, 2023
25 checks passed
@icinga-probot icinga-probot bot deleted the doc-09-object-types-host-name branch October 25, 2023 15:29
@Al2Klimov Al2Klimov mentioned this pull request Nov 24, 2023
3 tasks
@Al2Klimov Al2Klimov added backported Fix was included in a bugfix release and removed consider backporting Should be considered for inclusion in a bugfix release labels Jun 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/documentation End-user or developer help backported Fix was included in a bugfix release cla/signed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants