Move INI config parsing to common agent config util #264
Conversation
|
|
||
| func TestGetProxySettings(t *testing.T) { |
There was a problem hiding this comment.
This whole set of INI proxy tests was modified - namely there is no default proxy port anymore.
Both the trace and core agents operate without a default proxy port - not sure why we were so behind/different. I'll investigate a bit more and ask the core-agent team if our implementation has been wrong.
|
|
||
| [[constraint]] | ||
| name = "github.com/DataDog/gopsutil" | ||
| revision = "c8f74f1344dd41bc49ec4d3c2377c526aaedfd20" | ||
|
|
||
| [[constraint]] | ||
| name = "github.com/gogo/protobuf" | ||
| revision = "d76fbc1373015ced59b43ac267f28d546b955683" | ||
| version = "~1.0.0" |
There was a problem hiding this comment.
This is required due to a change in datadog-agent. We'll have to test this well.
There was a problem hiding this comment.
Any way to pin this to a hard version?
There was a problem hiding this comment.
I'd like to keep it in-sync with datadog-agent:
https://github.com/DataDog/datadog-agent/blob/master/Gopkg.toml#L41
| if strings.HasSuffix(path, ".yaml") { // If they set a config file directly, let's try to honor that | ||
| config.Datadog.SetConfigFile(path) | ||
| } | ||
| func (a *AgentConfig) loadProcessConfig(iniPath, yamlPath string) error { |
There was a problem hiding this comment.
Since this takes care of loading the ini config aswell should we rename the file ?
There was a problem hiding this comment.
I was going to merge this all into config.go after the environment piece is done to reduce noise :)
| if strings.HasSuffix(path, ".yaml") { // If they set a config file directly, let's try to honor that | ||
| config.Datadog.SetConfigFile(path) | ||
| } | ||
| func (a *AgentConfig) loadProcessConfig(iniPath, yamlPath string) error { |
There was a problem hiding this comment.
Should we add a check to make sure iniPath and yamlPath are not both set, since this will behave unexpectedly in case they are. Or even accept one string and infer the type based on the extension?
There was a problem hiding this comment.
The behavior will change to prefer the YAML if both exist, similar to how the main + trace agents operate. We're going to list this as a breaking change in our release notes.
| @@ -75,19 +77,33 @@ func (a *AgentConfig) loadNetworkYamlConfig(path string) error { | |||
| // Pull additional parameters from the global config file. | |||
| a.LogLevel = config.Datadog.GetString("log_level") | |||
| a.StatsdPort = config.Datadog.GetInt("dogstatsd_port") | |||
| if config.Datadog.IsSet("bind_host") { | |||
There was a problem hiding this comment.
Did this move from somewhere, or was it left out before?
There was a problem hiding this comment.
It was left out before. Noticed it when moving things over
| @@ -169,48 +142,60 @@ func TestGetHostname(t *testing.T) { | |||
| } | |||
|
|
|||
| func TestDDAgentMultiAPIKeys(t *testing.T) { | |||
| // if no endpoint is given but api_keys are there, match the first api_key | |||
| // with the default endpoint | |||
| config.Datadog = config.NewConfig("datadog", "DD", strings.NewReplacer(".", "_")) | |||
There was a problem hiding this comment.
do we need to do this given that NewAgentConfig will set the global config.Datadog ? Same comment on all of these tests
There was a problem hiding this comment.
We really just want it reset after the last test, but is more of a best practice to add it to each test to ensure state is reset.
| config.Datadog.AddConfigPath(yamlPath) | ||
| // If they set a config file directly, let's try to honor that | ||
| if strings.HasSuffix(yamlPath, ".yaml") { | ||
| config.Datadog.SetConfigFile(yamlPath) |
There was a problem hiding this comment.
yml is sometimes used as an extension. I see similar code was elsewhere in the agent config parsing, so I guess we're intentionally only looking for yaml
There was a problem hiding this comment.
Yeah, our yaml config example is generated with the .yaml suffix. I don't think we've run into any cases where a customer has provided .yml or another flavor
|
Closing this as we no longer need it (agent 5 is no longer being built anymore for updated versions) |
There's actually more deletions then additions but the
Gopkg.*changes add noise as well as moving INI configurations to separate test resource files.This PR is dependent on: DataDog/datadog-agent#3132
Will bump datadog-agent version when thats merged.
@DataDog/burrito