Skip to content

Commit

Permalink
fleetctl: make load & start run only for necessary units
Browse files Browse the repository at this point in the history
fleetctl load and start have been working for all given units, no matter
whether each unit's target state is more activated than the current state
or not. That means, for example, fleetctl load on activated units
results in stopping the units, which could be unexpected for users.

To fix that, check that each unit really needs to be waited, by running
unitToBeChanged().

Fixes coreos#1428
  • Loading branch information
Dongsu Park committed Jul 28, 2016
1 parent 32a9033 commit 235dbce
Showing 1 changed file with 37 additions and 4 deletions.
41 changes: 37 additions & 4 deletions fleetctl/fleetctl.go
Original file line number Diff line number Diff line change
Expand Up @@ -890,18 +890,26 @@ func isLocalUnitDifferent(cCmd *cobra.Command, file string, su *schema.Unit, fat

func lazyLoadUnits(args []string) ([]*schema.Unit, error) {
units := make([]string, 0, len(args))
js := job.JobStateLoaded
for _, j := range args {
units = append(units, unitNameMangle(j))
uName := unitNameMangle(j)
if unitToBeChanged(uName, js) {
units = append(units, uName)
}
}
return setTargetStateOfUnits(units, job.JobStateLoaded)
return setTargetStateOfUnits(units, js)
}

func lazyStartUnits(args []string) ([]*schema.Unit, error) {
units := make([]string, 0, len(args))
js := job.JobStateLaunched
for _, j := range args {
units = append(units, unitNameMangle(j))
uName := unitNameMangle(j)
if unitToBeChanged(uName, js) {
units = append(units, uName)
}
}
return setTargetStateOfUnits(units, job.JobStateLaunched)
return setTargetStateOfUnits(units, js)
}

// setTargetStateOfUnits ensures that the target state for the given Units is set
Expand Down Expand Up @@ -1158,6 +1166,31 @@ func machineState(machID string) (*machine.MachineState, error) {
return nil, nil
}

// unitToBeChanged returns true if state of a given unit is a more activated
// state than the current state. A wrapper of jsToBeChanged.
func unitToBeChanged(name string, js job.JobState) bool {
var state string

u, err := cAPI.Unit(name)
if err != nil {
log.Warningf("Error retrieving Unit(%s) from Registry: %v", name, err)
return false
}
if u == nil {
log.Warningf("Unit %s not found", name)
return false
}

// If this is a global unit, CurrentState will never be set. Instead, wait for DesiredState.
if suToGlobal(*u) {
state = u.DesiredState
} else {
state = u.CurrentState
}

return jsToBeChanged(job.JobState(state), js)
}

// jsToBeChanged returns true if the target state is a more activated state
// than the current state. For example, it returns true if it's a transition
// from inactive to launched, but false for other way around.
Expand Down

0 comments on commit 235dbce

Please sign in to comment.