diff --git a/.travis.yml b/.travis.yml index f0a7b4e57..48e736d62 100644 --- a/.travis.yml +++ b/.travis.yml @@ -22,8 +22,8 @@ deploy: api_key: secure: NtpNjquqjnwpeVQQM1GTHTTU7YOo8fEIyoBtMf3Vf1ayZjuWVZxwNfM77E596TG52a8pnZtpapXyHT0M4e1zms7F5KVCrOEfOB0OrA4IDzoATelVqdONnN3lbRJeVJVdSmK8/FNKwjI24tQZTaTQcIOioNqh7ZRcrEYlatGCuAw= file: - - /home/travis/rpmbuild/RPMS/noarch/mackerel-agent-0.18.0-1.noarch.rpm - - packaging/mackerel-agent_0.18.0-1_all.deb + - /home/travis/rpmbuild/RPMS/noarch/mackerel-agent-0.18.1-1.noarch.rpm + - packaging/mackerel-agent_0.18.1-1_all.deb - snapshot/mackerel-agent_darwin_386.zip - snapshot/mackerel-agent_darwin_amd64.zip - snapshot/mackerel-agent_freebsd_386.zip diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f9c50bf7..c6a8ae776 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,13 @@ # Changelog +## 0.18.1 (2015-07-16) + +* s/ami_id/ami-id/ in spec/cloud.go #112 (Songmu) +* remove `UpdateHost()` process from `prepareHost()` for simplicity #116 (Songmu) +* filter invalid roleFullNames with warning logs #117 (Songmu) +* allow using spaces as delimiter for custom metric values #119 (Songmu) + + ## 0.18.0 (2015-07-08) * Retry in prepare #108 (Songmu) diff --git a/command/command.go b/command/command.go index 21a80209a..70a888d2a 100644 --- a/command/command.go +++ b/command/command.go @@ -108,7 +108,7 @@ func prepareHost(root string, api *mackerel.API, roleFullnames []string, checks if lastErr != nil { return nil, fmt.Errorf("Failed to find this host on mackerel: %s", lastErr.Error()) } - } else { // update + } else { // check the hostID is valid or not doRetry(func() error { result, lastErr = api.FindHost(hostID) return filterErrorForRetry(lastErr) @@ -116,21 +116,6 @@ func prepareHost(root string, api *mackerel.API, roleFullnames []string, checks if lastErr != nil { return nil, fmt.Errorf("Failed to find this host on mackerel (You may want to delete file \"%s\" to register this host to an another organization): %s", idFilePath(root), lastErr.Error()) } - - doRetry(func() error { - lastErr = api.UpdateHost(hostID, mackerel.HostSpec{ - Name: hostname, - Meta: meta, - Interfaces: interfaces, - RoleFullnames: roleFullnames, - Checks: checks, - DisplayName: displayName, - }) - return filterErrorForRetry(lastErr) - }) - if lastErr != nil { - return nil, fmt.Errorf("Failed to update this host: %s", lastErr.Error()) - } } if hostSt != "" && hostSt != result.Status { @@ -321,11 +306,12 @@ func loop(c *context, termCh chan struct{}) int { func updateHostSpecsLoop(c *context, quit chan struct{}) { for { + UpdateHostSpecs(c.conf, c.api, c.host) select { case <-quit: return case <-time.After(specsUpdateInterval): - UpdateHostSpecs(c.conf, c.api, c.host) + // nop } } } @@ -475,7 +461,12 @@ func collectHostSpecs() (string, map[string]interface{}, []map[string]interface{ return "", nil, nil, fmt.Errorf("failed to obtain hostname: %s", err.Error()) } - meta := spec.Collect(specGenerators()) + specGens := specGenerators() + cGen := spec.SuggestCloudGenerator() + if cGen != nil { + specGens = append(specGens, cGen) + } + meta := spec.Collect(specGens) interfacesSpec, err := interfaceGenerator().Generate() if err != nil { diff --git a/command/command_linux.go b/command/command_linux.go index 8ad6ee397..5920a12c6 100644 --- a/command/command_linux.go +++ b/command/command_linux.go @@ -9,20 +9,13 @@ import ( ) func specGenerators() []spec.Generator { - specs := []spec.Generator{ + return []spec.Generator{ &specLinux.KernelGenerator{}, &specLinux.CPUGenerator{}, &specLinux.MemoryGenerator{}, &specLinux.BlockDeviceGenerator{}, &specLinux.FilesystemGenerator{}, } - cloudGenerator, err := specLinux.NewCloudGenerator("") - if err != nil { - logger.Errorf("Failed to create cloudGenerator: %s", err.Error()) - } else { - specs = append(specs, cloudGenerator) - } - return specs } func interfaceGenerator() spec.Generator { diff --git a/command/command_test.go b/command/command_test.go index a00e98f7f..d1a34713e 100644 --- a/command/command_test.go +++ b/command/command_test.go @@ -284,6 +284,11 @@ func TestLoop(t *testing.T) { "success": true, } } + mockHandlers["PUT /api/v0/hosts/xyzabc12345"] = func(req *http.Request) (int, jsonObject) { + return 200, jsonObject{ + "result": "OK", + } + } // Prepare required objects... ag := &agent.Agent{ diff --git a/main.go b/main.go index f8b77c944..d8e32990c 100644 --- a/main.go +++ b/main.go @@ -22,7 +22,7 @@ import ( // allow options like -role=... -role=... type roleFullnamesFlag []string -var roleFullnamePattern = regexp.MustCompile(`^[\w-]+:\s*[\w-]+$`) +var roleFullnamePattern = regexp.MustCompile(`^[a-zA-Z0-9][-_a-zA-Z0-9]*:\s*[a-zA-Z0-9][-_a-zA-Z0-9]*$`) func (r *roleFullnamesFlag) String() string { return fmt.Sprint(*r) @@ -30,15 +30,7 @@ func (r *roleFullnamesFlag) String() string { func (r *roleFullnamesFlag) Set(input string) error { inputRoles := strings.Split(input, ",") - - for _, inputRole := range inputRoles { - if roleFullnamePattern.MatchString(inputRole) == false { - return fmt.Errorf("Bad format for role fullname (expecting :): %s", inputRole) - } - } - *r = append(*r, inputRoles...) - return nil } @@ -88,14 +80,14 @@ func resolveConfig() (*config.Config, *otherOptions) { otherOptions := &otherOptions{} var ( - conffile = flag.String("conf", config.DefaultConfig.Conffile, "Config file path (Configs in this file are over-written by command line options)") - apibase = flag.String("apibase", config.DefaultConfig.Apibase, "API base") - pidfile = flag.String("pidfile", config.DefaultConfig.Pidfile, "File containing PID") - root = flag.String("root", config.DefaultConfig.Root, "Directory containing variable state information") - apikey = flag.String("apikey", "", "API key from mackerel.io web site") - diagnostic = flag.Bool("diagnostic", false, "Enables diagnostic features") - runOnce = flag.Bool("once", false, "Show spec and metrics to stdout once") - printVersion = flag.Bool("version", false, "Prints version and exit") + conffile = flag.String("conf", config.DefaultConfig.Conffile, "Config file path (Configs in this file are over-written by command line options)") + apibase = flag.String("apibase", config.DefaultConfig.Apibase, "API base") + pidfile = flag.String("pidfile", config.DefaultConfig.Pidfile, "File containing PID") + root = flag.String("root", config.DefaultConfig.Root, "Directory containing variable state information") + apikey = flag.String("apikey", "", "API key from mackerel.io web site") + diagnostic = flag.Bool("diagnostic", false, "Enables diagnostic features") + runOnce = flag.Bool("once", false, "Show spec and metrics to stdout once") + printVersion = flag.Bool("version", false, "Prints version and exit") ) var verbose bool @@ -145,6 +137,15 @@ func resolveConfig() (*config.Config, *otherOptions) { } }) + r := []string{} + for _, roleFullName := range conf.Roles { + if !roleFullnamePattern.MatchString(roleFullName) { + logger.Errorf("Bad format for role fullname (expecting :. Alphabet, numbers, hyphens and underscores are acceptable, but the first character must not be a hyphen or an underscore.): '%s'", roleFullName) + } else { + r = append(r, roleFullName) + } + } + conf.Roles = r return conf, nil } diff --git a/main_test.go b/main_test.go index 7bcdf09d4..b3b2173e4 100644 --- a/main_test.go +++ b/main_test.go @@ -23,7 +23,7 @@ diagnostic=false confFile.Close() defer os.Remove(confFile.Name()) - os.Args = []string{"mackerel-agent", "-conf=" + confFile.Name(), "-role=My-Service:default", "-verbose", "-diagnostic"} + os.Args = []string{"mackerel-agent", "-conf=" + confFile.Name(), "-role=My-Service:default,INVALID#SERVICE", "-verbose", "-diagnostic"} // Overrides Args from go test command flag.CommandLine = flag.NewFlagSet(os.Args[0], flag.PanicOnError) diff --git a/metrics/plugin.go b/metrics/plugin.go index 97a000c49..6d551c5a7 100644 --- a/metrics/plugin.go +++ b/metrics/plugin.go @@ -212,6 +212,8 @@ func (g *pluginGenerator) makeCreateGraphDefsPayload() []mackerel.CreateGraphDef return payloads } +var delimReg = regexp.MustCompile(`[\s\t]+`) + func (g *pluginGenerator) collectValues() (Values, error) { command := g.Config.Command pluginLogger.Debugf("Executing plugin: command = \"%s\"", command) @@ -228,7 +230,7 @@ func (g *pluginGenerator) collectValues() (Values, error) { for _, line := range strings.Split(stdout, "\n") { // Key, value, timestamp // ex.) tcp.CLOSING 0 1397031808 - items := strings.Split(line, "\t") + items := delimReg.Split(line, 3) if len(items) != 3 { continue } diff --git a/metrics/plugin_linux_test.go b/metrics/plugin_test.go similarity index 89% rename from metrics/plugin_linux_test.go rename to metrics/plugin_test.go index 8dfe6aa03..cb94b48a5 100644 --- a/metrics/plugin_linux_test.go +++ b/metrics/plugin_test.go @@ -1,4 +1,4 @@ -// +build linux +// +build linux darwin freebsd package metrics @@ -73,6 +73,30 @@ func TestPluginCollectValuesCommand(t *testing.T) { } } +func TestPluginCollectValuesCommandWithSpaces(t *testing.T) { + g := &pluginGenerator{Config: config.PluginConfig{ + Command: `echo "just.echo.2 2 1397822016"`, + }} + + values, err := g.collectValues() + if err != nil { + t.Errorf("should not raise error: %v", err) + } + + if len(values) != 1 { + t.Error("Only 1 value shoud be generated") + } + + for name, value := range values { + if name != "custom.just.echo.2" { + t.Errorf("Wrong name: %s", name) + } + if value != 2.0 { + t.Errorf("Wrong value: %+v", value) + } + } +} + func TestPluginLoadPluginMeta(t *testing.T) { g := &pluginGenerator{ Config: config.PluginConfig{ diff --git a/metrics/windows/plugin.go b/metrics/windows/plugin.go index 26cf1fc68..fcc5aa414 100644 --- a/metrics/windows/plugin.go +++ b/metrics/windows/plugin.go @@ -6,6 +6,7 @@ import ( "bytes" "errors" "os/exec" + "regexp" "strconv" "strings" @@ -43,6 +44,8 @@ func (g *PluginGenerator) Generate() (metrics.Values, error) { return results, nil } +var delimReg = regexp.MustCompile(`[\s\t]+`) + func (g *PluginGenerator) collectValues(command string) (metrics.Values, error) { pluginLogger.Debugf("Executing plugin: command = \"%s\"", command) @@ -63,7 +66,7 @@ func (g *PluginGenerator) collectValues(command string) (metrics.Values, error) for _, line := range strings.Split(string(outBuffer.Bytes()), "\n") { // Key, value, timestamp // ex.) localhost.localdomain.tcp.CLOSING 0 1397031808 - items := strings.Split(line, "\t") + items := delimReg.Split(line, 3) if len(items) != 3 { continue } diff --git a/packaging/deb/debian/changelog b/packaging/deb/debian/changelog index 4deb05d25..f5cddd663 100644 --- a/packaging/deb/debian/changelog +++ b/packaging/deb/debian/changelog @@ -1,3 +1,16 @@ +mackerel-agent (0.18.1-1) stable; urgency=low + + * s/ami_id/ami-id/ in spec/cloud.go (by Songmu) + + * remove `UpdateHost()` process from `prepareHost()` for simplicity (by Songmu) + + * filter invalid roleFullNames with warning logs (by Songmu) + + * allow using spaces as delimiter for custom metric values (by Songmu) + + + -- Songmu Thu, 16 Jul 2015 17:05:43 +0900 + mackerel-agent (0.18.0-1) stable; urgency=low * Retry in prepare (by Songmu) diff --git a/packaging/rpm/mackerel-agent.spec b/packaging/rpm/mackerel-agent.spec index 2b386e3a3..161a317fb 100644 --- a/packaging/rpm/mackerel-agent.spec +++ b/packaging/rpm/mackerel-agent.spec @@ -5,7 +5,7 @@ %define _localbindir /usr/local/bin Name: mackerel-agent -Version: 0.18.0 +Version: 0.18.1 Release: 1 License: Commercial Summary: macekrel.io agent @@ -73,6 +73,12 @@ fi %{_sysconfdir}/logrotate.d/%{name} %changelog +* Thu Jul 16 2015 - 0.18.1-1 +- s/ami_id/ami-id/ in spec/cloud.go (by Songmu) +- remove `UpdateHost()` process from `prepareHost()` for simplicity (by Songmu) +- filter invalid roleFullNames with warning logs (by Songmu) +- allow using spaces as delimiter for custom metric values (by Songmu) + * Wed Jul 08 2015 - 0.18.0-1 - Retry in prepare (by Songmu) - [WORKAROUND] downgrade golang version for windows (by Sixeight) diff --git a/spec/linux/cloud.go b/spec/cloud.go similarity index 59% rename from spec/linux/cloud.go rename to spec/cloud.go index ba2126356..03a734254 100644 --- a/spec/linux/cloud.go +++ b/spec/cloud.go @@ -1,13 +1,12 @@ -// +build linux - -package linux +package spec import ( - "github.com/mackerelio/mackerel-agent/logging" "io/ioutil" "net/http" "net/url" "time" + + "github.com/mackerelio/mackerel-agent/logging" ) // This Generator collects metadata about cloud instances. @@ -18,7 +17,12 @@ import ( // CloudGenerator definition type CloudGenerator struct { - baseURL *url.URL + CloudMetaGenerator +} + +// CloudMetaGenerator interface of metadata generator for each cloud platform +type CloudMetaGenerator interface { + Generate() (interface{}, error) } // Key is a root key for the generator. @@ -28,22 +32,50 @@ func (g *CloudGenerator) Key() string { var cloudLogger = logging.GetLogger("spec.cloud") -// NewCloudGenerator creates a Cloud Generator instance with specified baseurl. -func NewCloudGenerator(baseurl string) (*CloudGenerator, error) { - if baseurl == "" { - baseurl = "http://169.254.169.254/latest/meta-data" +const ( + ec2BaseURL = "http://169.254.169.254/latest/meta-data" + gcpBaseURL = "http://metadata.google.internal/computeMetadata/v1" + digitalOceanBaseURL = "http://169.254.169.254/metadata/v1" // has not been yet used +) + +var timeout = 100 * time.Millisecond + +// SuggestCloudGenerator returns suitable CloudGenerator +func SuggestCloudGenerator() *CloudGenerator { + if isEC2() { + return &CloudGenerator{NewEC2Generator()} + } + + return nil +} + +func isEC2() bool { + cl := http.Client{ + Timeout: timeout, } - u, err := url.Parse(baseurl) + // '/ami-id` is may be aws specific URL + resp, err := cl.Get(ec2BaseURL + "/ami-id") if err != nil { - return nil, err + return false } - return &CloudGenerator{u}, nil + defer resp.Body.Close() + + return resp.StatusCode == 200 } -// Generate collects metadata from cloud platform. -func (g *CloudGenerator) Generate() (interface{}, error) { +// EC2Generator meta generator for EC2 +type EC2Generator struct { + baseURL *url.URL +} - timeout := time.Duration(100 * time.Millisecond) +// NewEC2Generator returns new instance of EC2Generator +func NewEC2Generator() *EC2Generator { + url, _ := url.Parse(ec2BaseURL) + return &EC2Generator{url} +} + +// Generate collects metadata from cloud platform. +func (g *EC2Generator) Generate() (interface{}, error) { client := http.Client{ Timeout: timeout, } @@ -53,7 +85,7 @@ func (g *CloudGenerator) Generate() (interface{}, error) { "instance-type", "placement/availability-zone", "security-groups", - "ami_id", + "ami-id", "hostname", "local-hostname", "public-hostname", @@ -81,7 +113,7 @@ func (g *CloudGenerator) Generate() (interface{}, error) { metadata[key] = string(body) cloudLogger.Debugf("results %s:%s", key, string(body)) } else { - cloudLogger.Warningf("Status code of the result of requesting metadata is '%d'", resp.StatusCode) + cloudLogger.Warningf("Status code of the result of requesting metadata '%s' is '%d'", key, resp.StatusCode) } } diff --git a/spec/linux/cloud_test.go b/spec/cloud_test.go similarity index 76% rename from spec/linux/cloud_test.go rename to spec/cloud_test.go index 68cf32f50..c4312faa4 100644 --- a/spec/linux/cloud_test.go +++ b/spec/cloud_test.go @@ -1,28 +1,13 @@ -// +build linux - -package linux +package spec import ( "fmt" "net/http" "net/http/httptest" + "net/url" "testing" ) -func TestNewCloudGenerator(t *testing.T) { - g, err := NewCloudGenerator( - "http://example.com", - ) - - if err != nil { - t.Errorf("should not raise error: %v", err) - } - - if g.baseURL.String() != "http://example.com" { - t.Error("should return URL") - } -} - func TestCloudKey(t *testing.T) { g := &CloudGenerator{} @@ -40,10 +25,12 @@ func TestCloudGenerate(t *testing.T) { })) defer ts.Close() - g, err := NewCloudGenerator(ts.URL) + u, err := url.Parse(ts.URL) if err != nil { t.Errorf("should not raise error: %s", err) } + g := &CloudGenerator{&EC2Generator{u}} + value, err := g.Generate() if err != nil { t.Errorf("should not raise error: %s", err)