From 48de53aed142fd716628d1e81cc007904cd92dd1 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Wed, 10 Feb 2016 17:54:43 -0800 Subject: [PATCH] Strip as much copystructure as possible --- client/alloc_runner.go | 5 +- client/config/config.go | 17 ++-- client/task_runner.go | 5 +- nomad/structs/funcs.go | 33 +++++++ nomad/structs/structs.go | 201 +++++++++++++++++++++++++++++++++++---- 5 files changed, 227 insertions(+), 34 deletions(-) diff --git a/client/alloc_runner.go b/client/alloc_runner.go index 67df55177..fb516b3f2 100644 --- a/client/alloc_runner.go +++ b/client/alloc_runner.go @@ -86,7 +86,10 @@ func NewAllocRunner(logger *log.Logger, config *config.Config, updater AllocStat // stateFilePath returns the path to our state file func (r *AllocRunner) stateFilePath() string { - return filepath.Join(r.config.StateDir, "alloc", r.alloc.ID, "state.json") + r.allocLock.Lock() + defer r.allocLock.Unlock() + path := filepath.Join(r.config.StateDir, "alloc", r.alloc.ID, "state.json") + return path } // RestoreState is used to restore the state of the alloc runner diff --git a/client/config/config.go b/client/config/config.go index c6a81fdfb..b78dc24f2 100644 --- a/client/config/config.go +++ b/client/config/config.go @@ -8,7 +8,6 @@ import ( "time" "github.com/hashicorp/nomad/nomad/structs" - "github.com/mitchellh/copystructure" ) // RPCHandler can be provided to the Client if there is a local server @@ -74,16 +73,12 @@ type Config struct { } func (c *Config) Copy() *Config { - log := c.LogOutput - c.LogOutput = nil - i, err := copystructure.Copy(c) - c.LogOutput = log - if err != nil { - return nil - } - copy := i.(*Config) - copy.LogOutput = log - return copy + nc := new(Config) + *nc = *c + nc.Node = nc.Node.Copy() + nc.Servers = structs.CopySliceString(nc.Servers) + nc.Options = structs.CopyMapStringString(nc.Options) + return nc } // Read returns the specified configuration value or "". diff --git a/client/task_runner.go b/client/task_runner.go index f27a38107..cbf4618ec 100644 --- a/client/task_runner.go +++ b/client/task_runner.go @@ -168,10 +168,7 @@ func (r *TaskRunner) setState(state string, event *structs.TaskEvent) { // createDriver makes a driver for the task func (r *TaskRunner) createDriver() (driver.Driver, error) { - // Create a copy of the node. - // TODO REMOVE - node := r.config.Node.Copy() - taskEnv, err := driver.GetTaskEnv(r.ctx.AllocDir, node, r.task) + taskEnv, err := driver.GetTaskEnv(r.ctx.AllocDir, r.config.Node, r.task) if err != nil { err = fmt.Errorf("failed to create driver '%s' for alloc %s: %v", r.task.Driver, r.alloc.ID, err) diff --git a/nomad/structs/funcs.go b/nomad/structs/funcs.go index edc30319a..d222cc41a 100644 --- a/nomad/structs/funcs.go +++ b/nomad/structs/funcs.go @@ -137,3 +137,36 @@ func GenerateUUID() string { buf[8:10], buf[10:16]) } + +// Helpers for copying generic structures. +func CopyMapStringString(m map[string]string) map[string]string { + c := make(map[string]string, len(m)) + for k, v := range m { + c[k] = v + } + return c +} + +func CopyMapStringInt(m map[string]int) map[string]int { + c := make(map[string]int, len(m)) + for k, v := range m { + c[k] = v + } + return c +} + +func CopyMapStringFloat64(m map[string]float64) map[string]float64 { + c := make(map[string]float64, len(m)) + for k, v := range m { + c[k] = v + } + return c +} + +func CopySliceString(s []string) []string { + c := make([]string, len(s)) + for i, v := range s { + c[i] = v + } + return c +} diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index da544b346..effb32905 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -543,11 +543,17 @@ type Node struct { } func (n *Node) Copy() *Node { - i, err := copystructure.Copy(n) - if err != nil { + if n == nil { return nil } - return i.(*Node) + nn := new(Node) + *nn = *n + nn.Attributes = CopyMapStringString(nn.Attributes) + nn.Resources = nn.Resources.Copy() + nn.Reserved = nn.Reserved.Copy() + nn.Links = CopyMapStringString(nn.Links) + nn.Meta = CopyMapStringString(nn.Meta) + return nn } // TerminalStatus returns if the current status is terminal and @@ -656,6 +662,9 @@ func (r *Resources) MeetsMinResources() error { // Copy returns a deep copy of the resources func (r *Resources) Copy() *Resources { + if r == nil { + return nil + } newR := new(Resources) *newR = *r n := len(r.Networks) @@ -750,6 +759,9 @@ func (n *NetworkResource) MeetsMinResources() error { // Copy returns a deep copy of the network resource func (n *NetworkResource) Copy() *NetworkResource { + if n == nil { + return nil + } newR := new(NetworkResource) *newR = *n if n.ReservedPorts != nil { @@ -909,12 +921,28 @@ func (j *Job) InitFields() { // Copy returns a deep copy of the Job. It is expected that callers use recover. // This job can panic if the deep copy failed as it uses reflection. func (j *Job) Copy() *Job { - i, err := copystructure.Copy(j) - if err != nil { - panic(err) + if j == nil { + return nil } + nj := new(Job) + *nj = *j + nj.Datacenters = CopySliceString(nj.Datacenters) - return i.(*Job) + con := make([]*Constraint, len(nj.Constraints)) + for i, c := range nj.Constraints { + con[i] = c.Copy() + } + nj.Constraints = con + + tgs := make([]*TaskGroup, len(nj.TaskGroups)) + for i, tg := range nj.TaskGroups { + tgs[i] = tg.Copy() + } + nj.TaskGroups = tgs + + nj.Periodic = nj.Periodic.Copy() + nj.Meta = CopyMapStringString(nj.Meta) + return nj } // Validate is used to sanity check a job input @@ -1074,6 +1102,15 @@ type PeriodicConfig struct { ProhibitOverlap bool `mapstructure:"prohibit_overlap"` } +func (p *PeriodicConfig) Copy() *PeriodicConfig { + if p == nil { + return nil + } + np := new(PeriodicConfig) + *np = *p + return np +} + func (p *PeriodicConfig) Validate() error { if !p.Enabled { return nil @@ -1194,6 +1231,15 @@ type RestartPolicy struct { Mode string } +func (r *RestartPolicy) Copy() *RestartPolicy { + if r == nil { + return nil + } + nrp := new(RestartPolicy) + *nrp = *r + return nrp +} + func (r *RestartPolicy) Validate() error { switch r.Mode { case RestartPolicyModeDelay, RestartPolicyModeFail: @@ -1254,12 +1300,28 @@ type TaskGroup struct { } func (tg *TaskGroup) Copy() *TaskGroup { - i, err := copystructure.Copy(tg) - if err != nil { - panic(err) + if tg == nil { + return nil } + ntg := new(TaskGroup) + *ntg = *tg - return i.(*TaskGroup) + con := make([]*Constraint, len(ntg.Constraints)) + for i, c := range ntg.Constraints { + con[i] = c.Copy() + } + ntg.Constraints = con + + ntg.RestartPolicy = ntg.RestartPolicy.Copy() + + tasks := make([]*Task, len(ntg.Tasks)) + for i, t := range ntg.Tasks { + tasks[i] = t.Copy() + } + ntg.Tasks = tasks + + ntg.Meta = CopyMapStringString(ntg.Meta) + return ntg } // InitFields is used to initialize fields in the TaskGroup. @@ -1356,6 +1418,15 @@ type ServiceCheck struct { Timeout time.Duration // Timeout of the response from the check before consul fails the check } +func (sc *ServiceCheck) Copy() *ServiceCheck { + if sc == nil { + return nil + } + nsc := new(ServiceCheck) + *nsc = *sc + return nsc +} + func (sc *ServiceCheck) Validate() error { t := strings.ToLower(sc.Type) if t != ServiceCheckTCP && t != ServiceCheckHTTP { @@ -1401,6 +1472,22 @@ type Service struct { Checks []*ServiceCheck // List of checks associated with the service } +func (s *Service) Copy() *Service { + if s == nil { + return nil + } + ns := new(Service) + *ns = *s + ns.Tags = CopySliceString(ns.Tags) + + checks := make([]*ServiceCheck, len(ns.Checks)) + for i, c := range ns.Checks { + checks[i] = c.Copy() + } + ns.Checks = checks + return ns +} + // InitFields interpolates values of Job, Task Group and Task in the Service // Name. This also generates check names, service id and check ids. func (s *Service) InitFields(job string, taskGroup string, task string) { @@ -1487,11 +1574,33 @@ type Task struct { } func (t *Task) Copy() *Task { - i, err := copystructure.Copy(t) - if err != nil { + if t == nil { return nil } - return i.(*Task) + nt := new(Task) + *nt = *t + nt.Env = CopyMapStringString(nt.Env) + + services := make([]*Service, len(nt.Services)) + for i, s := range nt.Services { + services[i] = s.Copy() + } + nt.Services = services + + con := make([]*Constraint, len(nt.Constraints)) + for i, s := range nt.Constraints { + con[i] = s.Copy() + } + nt.Constraints = con + + nt.Resources = nt.Resources.Copy() + nt.Meta = CopyMapStringString(nt.Meta) + + if i, err := copystructure.Copy(nt.Config); err != nil { + nt.Config = i.(map[string]interface{}) + } + + return nt } // InitFields initializes fields in the task. @@ -1544,6 +1653,9 @@ type TaskState struct { } func (ts *TaskState) Copy() *TaskState { + if ts == nil { + return nil + } copy := new(TaskState) copy.State = ts.State copy.Events = make([]*TaskEvent, len(ts.Events)) @@ -1588,6 +1700,9 @@ type TaskEvent struct { } func (te *TaskEvent) Copy() *TaskEvent { + if te == nil { + return nil + } copy := new(TaskEvent) *copy = *te return copy @@ -1680,6 +1795,15 @@ type Constraint struct { str string // Memoized string } +func (c *Constraint) Copy() *Constraint { + if c == nil { + return nil + } + nc := new(Constraint) + *nc = *c + return nc +} + func (c *Constraint) String() string { if c.str != "" { return c.str @@ -1788,12 +1912,35 @@ type Allocation struct { } func (a *Allocation) Copy() *Allocation { - i, err := copystructure.Copy(a) - if err != nil { - panic(err) + if a == nil { + return nil } + na := new(Allocation) + *na = *a - return i.(*Allocation) + na.Job = na.Job.Copy() + na.Resources = na.Resources.Copy() + + tr := make(map[string]*Resources, len(na.TaskResources)) + for task, resource := range na.TaskResources { + tr[task] = resource.Copy() + } + na.TaskResources = tr + + s := make(map[string]string, len(na.Services)) + for service, id := range na.Services { + s[service] = id + } + na.Services = s + + na.Metrics = na.Metrics.Copy() + + ts := make(map[string]*TaskState, len(na.TaskStates)) + for task, state := range na.TaskStates { + ts[task] = state.Copy() + } + na.TaskStates = ts + return na } // TerminalStatus returns if the desired or actual status is terminal and @@ -1926,6 +2073,21 @@ type AllocMetric struct { CoalescedFailures int } +func (a *AllocMetric) Copy() *AllocMetric { + if a == nil { + return nil + } + na := new(AllocMetric) + *na = *a + na.NodesAvailable = CopyMapStringInt(na.NodesAvailable) + na.ClassFiltered = CopyMapStringInt(na.ClassFiltered) + na.ConstraintFiltered = CopyMapStringInt(na.ConstraintFiltered) + na.ClassExhausted = CopyMapStringInt(na.ClassExhausted) + na.DimensionExhausted = CopyMapStringInt(na.DimensionExhausted) + na.Scores = CopyMapStringFloat64(na.Scores) + return na +} + func (a *AllocMetric) EvaluateNode() { a.NodesEvaluated += 1 } @@ -2090,6 +2252,9 @@ func (e *Evaluation) GoString() string { } func (e *Evaluation) Copy() *Evaluation { + if e == nil { + return nil + } ne := new(Evaluation) *ne = *e return ne