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

Load default configuration file from template instead of old configuration file #6968

Closed
jsternberg opened this issue Jul 6, 2016 · 8 comments
Milestone

Comments

@jsternberg
Copy link
Contributor

Feature Request

Proposal: Modify InfluxDB to load the default configuration file from a template file instead of the main configuration file. This allows someone to customize the default values and use influxd config to generate a functioning configuration file. It also fixes the bug that happens when you run this command: influxd config > /etc/influxdb/influxdb.conf.

Current behavior: It will load the default from one of three common locations and then output the file again. If the file gets truncated due to a shell redirection, this causes the output to fail since the original input is bad.

Desired behavior: To keep a static file that can be used for config output generation but not be the same file that is read when starting the process.

Use case: This allows an installation to customize the output of influxd config. With Docker as an example, we want a different default location for the various directories because the volumes are located in a specific location. We want any call to influxd config to output these as the defaults instead of the ones hard-coded into the influxd binary. But, when we load these from the existing configuration, we can end up propagating a bad configuration file or accidentally truncate the current configuration file while trying to regenerate a new one. This allows a packager or administrator to change the behavior of influxd config without affecting the output generation.

@gunnaraasen
Copy link
Member

What do you think about setting the base /influxdb directory with a build flag, e.g. -ldflags '-X main.dir=$DEFAULT_DIR'? Then the build script can set the default canonical path for each OS/distribution.

I'm in favor of switching to a template to allow the output to be similar to the telegraf -sample-config output, which provides useful comments about each section.

@jsternberg
Copy link
Contributor Author

@gunnaraasen I don't think setting it through a linker flag would be a good idea personally since I think a config file for the binary is a better idea that doesn't require adding additional steps to the linker configuration file. We also need this kind of functionality for some distributions like Docker where it may require a custom location.

This change unfortunately won't output the file with comments, but it will allow us to distribute a file to /etc/influxdb/influxdb.conf.in or /etc/influxdb/influxdb.conf.tmpl and on every upgrade, we can automatically generate /etc/influxdb/influxdb.conf from that file if it's present. At the same time, someone can just do that right now by removing the sections where they want the default value so I'm not sure how necessary this change is.

@nathanielc thinking about this again, how would it be if we modified the influxd config and kapacitord config commands to use the demo config as the default when it is loading a config file to print out? I've pushed an example of what I mean to js-6968-load-default-configuration-file.

@nathanielc
Copy link
Contributor

@jsternberg How does using the demo config solve the problem?

@jsternberg
Copy link
Contributor Author

@nathanielc the demo config has default values for the required values so loading an empty configuration file works. One of the current problems is that it does these steps that are different from each other when printing out a config.

  1. Check for existing configuration file.
  2. If existing configuration file exists, load it and overwrite values with the environment variables.
  3. Output configuration file.
  4. If existing configuration doesn't exist, output the demo configuration file.

The problem is step 2 causes a weird issue. Since the command you ran created the file, it went to the number 2 path instead of the number 4 path. If you load the demo configuration by default, then it doesn't matter if you have a blank configuration file present in the path.

@nathanielc
Copy link
Contributor

But in step 4 the demo config is still going to create dirs in /root right?

@nathanielc
Copy link
Contributor

Ah, I see by using hte demo config we can fix the bash redirection issue. Then are you suggesting that nothing else has to change?

@jsternberg
Copy link
Contributor Author

I think so. We can then just keep the current behavior and warn people in the documentation that, if they are going to regenerate their config, to regenerate it to a temporary file. I think that should be good enough?

@nathanielc
Copy link
Contributor

OK sounds good, so long as there is a clear log message that a specific config file was loaded and used as the defaults. aka step 2. happened.

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

No branches or pull requests

4 participants