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

generator: add a --config-file= option #638

Closed
wants to merge 1 commit into from

Conversation

bootc
Copy link

@bootc bootc commented Apr 4, 2021

This allows easier use of multiple generator config files, for example
for different sets of authentication credentials or to break up the
giant snmp.yml file into smaller chunks that fit better in a Kubernetes
ConfigMap.

This allows easier use of multiple generator config files, for example
for different sets of authentication credentials or to break up the
giant snmp.yml file into smaller chunks that fit better in a Kubernetes
ConfigMap.

Signed-off-by: Chris Boot <[email protected]>
@bootc bootc force-pushed the generator-config-option branch from 2da4947 to 56d56b7 Compare April 4, 2021 18:13
@RichiH
Copy link
Member

RichiH commented Apr 5, 2021

This would be superseded by https://docs.google.com/document/d/1BK_Gc3ixoWyxr9F5qGC07HEcfDPtb6z96mfqoGyz52Y . If you feel strongly, we could add it as --experimental-config-file to make clear it will go away; hopefully we will see what's in the doc within a month, though.

@bootc
Copy link
Author

bootc commented Apr 5, 2021

No I don't feel strongly about it, and it can wait to be solved properly. It just felt wrong to apply this locally without offering it up more widely.

@xkilian
Copy link

xkilian commented Apr 19, 2021

The work being done on other PRs is in fact to split the processing of each family of devices associated with a generator.yml file and its associated snmp mibs as distinct independant blobs.

We all wat to move away from a big generator.yml, with a massive directory of conflicting snmp mibs.

By having a structure to handle distinct device families we can deliver a bunch of mini generator_family.yml, their own mibs directory containing only the required mibs require and have their output consolidated in a destination snmp.d directory. The mibs directory are not distributed with the Exporter, but would be a listing of minimum required mibs as is done today.

It will be the collectors job to join/process the different snmp_family.yml files together for running the collector.

@xkilian
Copy link

xkilian commented Apr 6, 2023

This issue is still a valid concern all the more as we add scalability features to handle networking gear. We have used this for a couple years and have seen great value in seperating each device family in its own directory and specific generator yml file. Another specific way of dealing with it can be raised, but the monolithic generator file is not the answer.

@@ -95,6 +100,7 @@ func generateConfig(nodes *Node, nameToNode map[string]*Node, logger log.Logger)

var (
failOnParseErrors = kingpin.Flag("fail-on-parse-errors", "Exit with a non-zero status if there are MIB parsing errors").Default("false").Bool()
configPath = kingpin.Flag("config-file", "Path to configuration file").Default("generator.yml").Short('c').String()
Copy link
Member

Choose a reason for hiding this comment

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

We generally don't do short flags for things. We also prefix things so it's more clear what the flag is for.

Suggested change
configPath = kingpin.Flag("config-file", "Path to configuration file").Default("generator.yml").Short('c').String()
configPath = kingpin.Flag("generator.config.file", "Path to configuration file").Default("generator.yml").String()

Copy link
Member

@SuperQ SuperQ left a comment

Choose a reason for hiding this comment

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

Any chance you want to finish this?

@SuperQ
Copy link
Member

SuperQ commented Nov 24, 2023

Superseded by #1028

@SuperQ SuperQ closed this Nov 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants