Skip to content

Commit

Permalink
Remove event GenericSource, and address other code review comments. A…
Browse files Browse the repository at this point in the history
…lso added deprecation info in comments.
  • Loading branch information
Preetha Appan committed Nov 3, 2017
1 parent 17584d9 commit b3631f3
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 41 deletions.
6 changes: 3 additions & 3 deletions client/alloc_runner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ func TestAllocRunner_DeploymentHealth_Unhealthy_BadStart(t *testing.T) {
assert.NotNil(state)
assert.NotEmpty(state.Events)
last := state.Events[len(state.Events)-1]
assert.Equal(allocHealthEventSource, last.GenericSource)
assert.Equal(allocHealthEventSource, last.Type)
assert.Contains(last.Message, "failed task")
}

Expand Down Expand Up @@ -202,7 +202,7 @@ func TestAllocRunner_DeploymentHealth_Unhealthy_Deadline(t *testing.T) {
assert.NotNil(state)
assert.NotEmpty(state.Events)
last := state.Events[len(state.Events)-1]
assert.Equal(allocHealthEventSource, last.GenericSource)
assert.Equal(allocHealthEventSource, last.Type)
assert.Contains(last.Message, "not running by deadline")
}

Expand Down Expand Up @@ -412,7 +412,7 @@ func TestAllocRunner_DeploymentHealth_Unhealthy_Checks(t *testing.T) {
assert.NotNil(state)
assert.NotEmpty(state.Events)
last := state.Events[len(state.Events)-1]
assert.Equal(allocHealthEventSource, last.GenericSource)
assert.Equal(allocHealthEventSource, last.Type)
assert.Contains(last.Message, "Services not healthy by deadline")
}

Expand Down
5 changes: 3 additions & 2 deletions client/task_runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -543,6 +543,7 @@ func (r *TaskRunner) DestroyState() error {
// setState is used to update the state of the task runner
func (r *TaskRunner) setState(state string, event *structs.TaskEvent, lazySync bool) {
event.PopulateEventDisplayMessage()

// Persist our state to disk.
if err := r.SaveState(); err != nil {
r.logger.Printf("[ERR] client: failed to save state of Task Runner for task %q: %v", r.task.Name, err)
Expand Down Expand Up @@ -1752,8 +1753,8 @@ func (r *TaskRunner) Kill(source, reason string, fail bool) {
}

func (r *TaskRunner) EmitEvent(source, message string) {
event := structs.NewTaskEvent(structs.TaskGenericMessage).
SetGenericSource(source).SetMessage(message)
event := structs.NewTaskEvent(source).
SetMessage(message)
r.setState("", event, false)
r.logger.Printf("[DEBUG] client: event from %q for task %q in alloc %q: %v",
source, r.task.Name, r.alloc.ID, message)
Expand Down
17 changes: 7 additions & 10 deletions command/alloc_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -325,20 +325,18 @@ func (c *AllocStatusCommand) outputTaskStatus(state *api.TaskState) {

size := len(state.Events)
for i, event := range state.Events {
if event.DisplayMessage != "" {
formattedTime := formatUnixNanoTime(event.Time)
formattedDisplayMsg := fmt.Sprintf("%s|%s|%s", formattedTime, event.Type, event.DisplayMessage)
events[size-i] = formattedDisplayMsg
} else {
events[size-i] = buildDisplayMessage(event)
msg := event.DisplayMessage
if msg == "" {
msg = buildDisplayMessage(event)
}
formattedTime := formatUnixNanoTime(event.Time)
events[size-i] = fmt.Sprintf("%s|%s|%s", formattedTime, event.Type, msg)
// Reverse order so we are sorted by time
}
c.Ui.Output(formatList(events))
}

func buildDisplayMessage(event *api.TaskEvent) string {
formattedTime := formatUnixNanoTime(event.Time)

// Build up the description based on the event type.
var desc string
switch event.Type {
Expand Down Expand Up @@ -447,8 +445,7 @@ func buildDisplayMessage(event *api.TaskEvent) string {
desc = event.Message
}

// Reverse order so we are sorted by time
return fmt.Sprintf("%s|%s|%s", formattedTime, event.Type, desc)
return desc
}

// outputTaskResources prints the task resources for the passed task and if
Expand Down
70 changes: 44 additions & 26 deletions nomad/structs/structs.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,6 @@ const (
ACLTokenUpsertRequestType
ACLTokenDeleteRequestType
ACLTokenBootstrapRequestType

// Constant for restart events that are within policy
ReasonWithinPolicy = "Restart within policy"
)

const (
Expand Down Expand Up @@ -2487,6 +2484,9 @@ const (
// RestartPolicyMinInterval is the minimum interval that is accepted for a
// restart policy.
RestartPolicyMinInterval = 5 * time.Second

// ReasonWithinPolicy describes restart events that are within policy
ReasonWithinPolicy = "Restart within policy"
)

// RestartPolicy configures how Tasks are restarted when they crash or fail.
Expand Down Expand Up @@ -3753,9 +3753,6 @@ const (

// TaskLeaderDead indicates that the leader task within the has finished.
TaskLeaderDead = "Leader Task Dead"

// TaskGenericMessage is used by various subsystems to emit a message.
TaskGenericMessage = "Generic"
)

// TaskEvent is an event that effects the state of a task and contains meta-data
Expand All @@ -3764,75 +3761,105 @@ type TaskEvent struct {
Type string
Time int64 // Unix Nanosecond timestamp

// FailsTask marks whether this event fails the task
Message string // A possible message explaining the termination of the task.

// DisplayMessage is a human friendly message about the event
DisplayMessage string

// Details is a map with annotated info about the event
Details map[string]string

// DEPRECATION NOTICE: The following fields are deprecated and will be removed
// in a future release. Field values are available in the Details map.

// FailsTask marks whether this event fails the task.
// Deprecated, use Details["fails_task"] to access this.
FailsTask bool

// Restart fields.
// Deprecated, use Details["restart_reason"] to access this.
RestartReason string

// Setup Failure fields.
// Deprecated, use Details["setup_error"] to access this.
SetupError string

// Driver Failure fields.
// Deprecated, use Details["driver_error"] to access this.
DriverError string // A driver error occurred while starting the task.

// Task Terminated Fields.
ExitCode int // The exit code of the task.
Signal int // The signal that terminated the task.
Message string // A possible message explaining the termination of the task.

// Deprecated, use Details["exit_code"] to access this.
ExitCode int // The exit code of the task.

// Deprecated, use Details["signal"] to access this.
Signal int // The signal that terminated the task.

// Killing fields
// Deprecated, use Details["kill_timeout"] to access this.
KillTimeout time.Duration

// Task Killed Fields.
// Deprecated, use Details["kill_error"] to access this.
KillError string // Error killing the task.

// KillReason is the reason the task was killed
// Deprecated, use Details["kill_reason"] to access this.
KillReason string

// TaskRestarting fields.
// Deprecated, use Details["start_delay"] to access this.
StartDelay int64 // The sleep period before restarting the task in unix nanoseconds.

// Artifact Download fields
// Deprecated, use Details["download_error"] to access this.
DownloadError string // Error downloading artifacts

// Validation fields
// Deprecated, use Details["validation_error"] to access this.
ValidationError string // Validation error

// The maximum allowed task disk size.
// Deprecated, use Details["disk_limit"] to access this.
DiskLimit int64

// Name of the sibling task that caused termination of the task that
// the TaskEvent refers to.
// Deprecated, use Details["failed_sibling"] to access this.
FailedSibling string

// VaultError is the error from token renewal
// Deprecated, use Details["vault_renewal_error"] to access this.
VaultError string

// TaskSignalReason indicates the reason the task is being signalled.
// Deprecated, use Details["task_signal_reason"] to access this.
TaskSignalReason string

// TaskSignal is the signal that was sent to the task
// Deprecated, use Details["task_signal"] to access this.
TaskSignal string

// DriverMessage indicates a driver action being taken.
// Deprecated, use Details["driver_message"] to access this.
DriverMessage string

// GenericSource is the source of a message.
// Deprecated, is redundant with event type.
GenericSource string

// DisplayMessage is a human friendly message about the event
DisplayMessage string

// Details is a map with annotated info about the event
Details map[string]string
}

func (event *TaskEvent) PopulateEventDisplayMessage() {
// Build up the description based on the event type.
if event == nil { //TODO PA needs investigation alloc_runner's Run method sends a nil event when sigterming nomad. Why?
if event == nil { //TODO(preetha) needs investigation alloc_runner's Run method sends a nil event when sigterming nomad. Why?
return
}

if event.DisplayMessage != "" {
return
}

var desc string
switch event.Type {
case TaskSetup:
Expand Down Expand Up @@ -3935,9 +3962,6 @@ func (event *TaskEvent) PopulateEventDisplayMessage() {
desc = event.DriverMessage
case TaskLeaderDead:
desc = "Leader Task in Group dead"
case TaskGenericMessage:
event.Type = event.GenericSource
desc = event.Message
default:
desc = ""
}
Expand Down Expand Up @@ -4103,12 +4127,6 @@ func (e *TaskEvent) SetDriverMessage(m string) *TaskEvent {
return e
}

func (e *TaskEvent) SetGenericSource(s string) *TaskEvent {
e.GenericSource = s
e.Details["generic_source"] = s
return e
}

// TaskArtifact is an artifact to download before running the task.
type TaskArtifact struct {
// GetterSource is the source to download an artifact using go-getter
Expand Down

0 comments on commit b3631f3

Please sign in to comment.