-
Notifications
You must be signed in to change notification settings - Fork 13
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
Improve gatherers documentation and add rhai example outputs #160
Conversation
7b6d2a4
to
e7d1ca4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! I just found an inconsistency in hosts
gatherer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great. Only a copy/paste error in the hosts gatherer.
Ready to merge once that is fixed
| :------------------------------------------------------------- | :----------------------------------------------------------------- | | ||
| `cib.configuration` | complete cib configuration entry as a map | | ||
| `cib.configuration.resources.primitive.0` | first available primitive resource | | ||
| `cib.configuration.crm_config.cluster_property_set.0.nvpair.1` | second nvpair value from the first element of cluster_property_set | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the cluster_property_set
is a potential list, we will need to include it here:
https://github.com/trento-project/agent/blob/main/internal/factsengine/gatherers/cibadmin.go#L48
Sample arguments: | ||
| Name | Return value | | ||
| :--------------------------- | :------------------------------- | | ||
| `totem.token` | extracted value from the command | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that we have the new implementation of this gatherer done, we should include new examples. @jamie-suse could you do that in an additional PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep @arbulu89, I can add examples featuring an output that contains a list.
62ae5d3
to
28577f7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💯
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
Only a type with a quote and I think the value in the host gatherer should be a list
28577f7
to
b626af3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just that minor comment; looks good otherwise
b626af3
to
2754d78
Compare
This PR improves gatherers documentation, spellcheck, and format and adds rhai example outputs in place of golang test links.