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

Allow working directory configuration in env of metadata plugins #585

Merged
merged 2 commits into from Jul 29, 2019

Conversation

@itchyny
Copy link
Contributor

commented Jul 24, 2019

The MACKEREL_PLUGIN_WORKDIR environment variable is handled by the plugin process for metrics and check plugins. So this value is often set in the env config within mackerel-agent.conf.
However, this environment variable is handled by the mackerel-agent process for metadata plugin. This is not intuitive when user want to configure the working directory for each metadata plugins.

@itchyny itchyny force-pushed the metadata-plugin-workdir branch from 7198813 to 97bcc36 Jul 24, 2019

@itchyny itchyny force-pushed the metadata-plugin-workdir branch from 97bcc36 to 8366178 Jul 24, 2019

@@ -14,12 +15,16 @@ import (
func metadataGenerators(conf *config.Config) []*metadata.Generator {
generators := make([]*metadata.Generator, 0, len(conf.MetadataPlugins))

workdir := pluginutil.PluginWorkDir()
workdir := filepath.Join(pluginutil.PluginWorkDir(), "mackerel-metadata")

This comment has been minimized.

Copy link
@susisu

susisu Jul 26, 2019

Member

[note] pluginutil.PluginWorkDir() does look MACKEREL_PLUGIN_WORKDIR.
https://github.com/mackerelio/golib/blob/master/pluginutil/tempfile.go

generator := &metadata.Generator{
Name: name,
Config: pluginConfig,
Cachefile: filepath.Join(workdir, "mackerel-metadata", name),
Cachefile: filepath.Join(workdir, name),

This comment has been minimized.

Copy link
@susisu

susisu Jul 26, 2019

Member

Is it ok to not append mackerel-metadata/ to the path when MACKEREL_PLUGIN_WORKDIR is configured per plugin?
filepath.Join(workdir, name) may conflict with the plugin executable?

This comment has been minimized.

Copy link
@itchyny

itchyny Jul 26, 2019

Author Contributor

The temporary file of metric plugin is stored at MACKEREL_PLUGIN_WORKDIR/pluginname (https://github.com/mackerelio/go-mackerel-plugin/blob/7d51210247a3f1a73fdd43098cda470b328cb58f/mackerel-plugin.go#L196) so I think this is consistent. The value is not expected to be set to the bin directory.

This comment has been minimized.

Copy link
@susisu

susisu Jul 26, 2019

Member

I understand!

This comment has been minimized.

Copy link
@itchyny

itchyny Jul 29, 2019

Author Contributor

I reconsidered this and prepended mackerel-plugin-metadata- to the plugin name. The file without the env variable config is not changed.

@susisu
susisu approved these changes Jul 26, 2019
prepend mackerel-plugin-metadata- to the cache file of metadata plugi…
…n when MACKEREL_PLUGIN_WORKDIR is specified
@itchyny

This comment has been minimized.

Copy link
Contributor Author

commented Jul 29, 2019

Thank you.

@itchyny itchyny merged commit 07d787a into master Jul 29, 2019

4 checks passed

continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@itchyny itchyny deleted the metadata-plugin-workdir branch Jul 29, 2019

@hayajo hayajo referenced this pull request Jul 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.