From 2728216f6fb141cd73f335cefc78647785f13b4a Mon Sep 17 00:00:00 2001 From: Michael Schurter Date: Thu, 13 Apr 2017 16:43:38 -0700 Subject: [PATCH] Plumb alloc id + task name into script check logs --- command/agent/consul/client.go | 47 +++++++++++------------------ command/agent/consul/script.go | 24 ++++++++++----- command/agent/consul/script_test.go | 6 ++-- 3 files changed, 38 insertions(+), 39 deletions(-) diff --git a/command/agent/consul/client.go b/command/agent/consul/client.go index d6c125a5b..3e73b75ca 100644 --- a/command/agent/consul/client.go +++ b/command/agent/consul/client.go @@ -376,31 +376,6 @@ func (c *ServiceClient) RegisterAgent(role string, services []*structs.Service) return nil } -// makeCheckReg adds a check reg to operations. -func (c *ServiceClient) makeCheckReg(ops *operations, check *structs.ServiceCheck, - service *api.AgentServiceRegistration, exec driver.ScriptExecutor, parseAddr addrParser) error { - - checkID := createCheckID(service.ID, check) - if check.Type == structs.ServiceCheckScript { - if exec == nil { - return fmt.Errorf("driver doesn't support script checks") - } - ops.scripts = append(ops.scripts, newScriptCheck( - checkID, check, exec, c.client, c.logger, c.shutdownCh)) - - } - host, port := service.Address, service.Port - if check.PortLabel != "" { - host, port = parseAddr(check.PortLabel) - } - checkReg, err := createCheckReg(service.ID, checkID, check, host, port) - if err != nil { - return fmt.Errorf("failed to add check %q: %v", check.Name, err) - } - ops.regChecks = append(ops.regChecks, checkReg) - return nil -} - // serviceRegs creates service registrations, check registrations, and script // checks from a service. func (c *ServiceClient) serviceRegs(ops *operations, allocID string, service *structs.Service, @@ -421,10 +396,24 @@ func (c *ServiceClient) serviceRegs(ops *operations, allocID string, service *st ops.regServices = append(ops.regServices, serviceReg) for _, check := range service.Checks { - err := c.makeCheckReg(ops, check, serviceReg, exec, task.FindHostAndPortFor) - if err != nil { - return err + checkID := createCheckID(id, check) + if check.Type == structs.ServiceCheckScript { + if exec == nil { + return fmt.Errorf("driver doesn't support script checks") + } + ops.scripts = append(ops.scripts, newScriptCheck( + allocID, task.Name, checkID, check, exec, c.client, c.logger, c.shutdownCh)) + } + host, port := serviceReg.Address, serviceReg.Port + if check.PortLabel != "" { + host, port = task.FindHostAndPortFor(check.PortLabel) + } + checkReg, err := createCheckReg(id, checkID, check, host, port) + if err != nil { + return fmt.Errorf("failed to add check %q: %v", check.Name, err) + } + ops.regChecks = append(ops.regChecks, checkReg) } return nil } @@ -497,7 +486,7 @@ func (c *ServiceClient) UpdateTask(allocID string, existing, newTask *structs.Ta return fmt.Errorf("driver doesn't support script checks") } ops.scripts = append(ops.scripts, newScriptCheck( - checkID, check, exec, c.client, c.logger, c.shutdownCh)) + existingID, newTask.Name, checkID, check, exec, c.client, c.logger, c.shutdownCh)) } host, port := parseAddr(existingSvc.PortLabel) if check.PortLabel != "" { diff --git a/command/agent/consul/script.go b/command/agent/consul/script.go index 4d0a66b01..3e96df00d 100644 --- a/command/agent/consul/script.go +++ b/command/agent/consul/script.go @@ -32,6 +32,9 @@ func (s *scriptHandle) wait() <-chan struct{} { // scriptCheck runs script checks via a ScriptExecutor and updates the // appropriate check's TTL when the script succeeds. type scriptCheck struct { + allocID string + taskName string + id string check *structs.ServiceCheck exec driver.ScriptExecutor @@ -46,11 +49,14 @@ type scriptCheck struct { // newScriptCheck creates a new scriptCheck. run() should be called once the // initial check is registered with Consul. -func newScriptCheck(id string, check *structs.ServiceCheck, exec driver.ScriptExecutor, agent heartbeater, - logger *log.Logger, shutdownCh <-chan struct{}) *scriptCheck { +func newScriptCheck(allocID, taskName, checkID string, check *structs.ServiceCheck, + exec driver.ScriptExecutor, agent heartbeater, logger *log.Logger, + shutdownCh <-chan struct{}) *scriptCheck { return &scriptCheck{ - id: id, + allocID: allocID, + taskName: taskName, + id: checkID, check: check, exec: exec, agent: agent, @@ -92,7 +98,8 @@ func (s *scriptCheck) run() *scriptHandle { case context.DeadlineExceeded: // Log deadline exceeded every time, but flip last check to false s.lastCheckOk = false - s.logger.Printf("[WARN] consul.checks: check %q timed out (%s)", s.check.Name, s.check.Timeout) + s.logger.Printf("[WARN] consul.checks: check %q for task %q alloc %q timed out (%s)", + s.check.Name, s.taskName, s.allocID, s.check.Timeout) } // cleanup context @@ -126,15 +133,18 @@ func (s *scriptCheck) run() *scriptHandle { if err != nil { if s.lastCheckOk { s.lastCheckOk = false - s.logger.Printf("[WARN] consul.checks: update for check %q failed: %v", s.check.Name, err) + s.logger.Printf("[WARN] consul.checks: update for task %q alloc %q check %q failed: %v", + s.taskName, s.allocID, s.check.Name, err) } else { - s.logger.Printf("[DEBUG] consul.checks: update for check %q still failing: %v", s.check.Name, err) + s.logger.Printf("[DEBUG] consul.checks: update for task %q alloc %q check %q still failing: %v", + s.taskName, s.allocID, s.check.Name, err) } } else if !s.lastCheckOk { // Succeeded for the first time or after failing; log s.lastCheckOk = true - s.logger.Printf("[INFO] consul.checks: update for check %q succeeded", s.check.Name) + s.logger.Printf("[INFO] consul.checks: update for task %q alloc %q check %q succeeded", + s.taskName, s.allocID, s.check.Name) } select { diff --git a/command/agent/consul/script_test.go b/command/agent/consul/script_test.go index fdb2e241d..3d713f0c1 100644 --- a/command/agent/consul/script_test.go +++ b/command/agent/consul/script_test.go @@ -57,7 +57,7 @@ func TestConsulScript_Exec_Cancel(t *testing.T) { exec := newBlockingScriptExec() // pass nil for heartbeater as it shouldn't be called - check := newScriptCheck("checkid", &serviceCheck, exec, nil, testLogger(), nil) + check := newScriptCheck("allocid", "testtask", "checkid", &serviceCheck, exec, nil, testLogger(), nil) handle := check.run() // wait until Exec is called @@ -101,7 +101,7 @@ func TestConsulScript_Exec_Timeout(t *testing.T) { exec := newBlockingScriptExec() hb := newFakeHeartbeater() - check := newScriptCheck("checkid", &serviceCheck, exec, hb, testLogger(), nil) + check := newScriptCheck("allocid", "testtask", "checkid", &serviceCheck, exec, hb, testLogger(), nil) handle := check.run() defer handle.cancel() // just-in-case cleanup <-exec.running @@ -148,7 +148,7 @@ func TestConsulScript_Exec_Shutdown(t *testing.T) { hb := newFakeHeartbeater() shutdown := make(chan struct{}) - check := newScriptCheck("checkid", &serviceCheck, noopExec{}, hb, testLogger(), shutdown) + check := newScriptCheck("allocid", "testtask", "checkid", &serviceCheck, noopExec{}, hb, testLogger(), shutdown) handle := check.run() defer handle.cancel() // just-in-case cleanup