Merge pull request #2868 from hashicorp/f-feedback

Variety of UX fixes
This commit is contained in:
Alex Dadgar 2017-07-20 17:43:42 -07:00 committed by GitHub
commit 15fad96c21
25 changed files with 323 additions and 68 deletions

View File

@ -1,5 +1,9 @@
## 0.6.0 (Unreleased)
__BACKWARDS INCOMPATIBILITIES:__
* cli: When given a prefix that does not resolve to a particular object,
commands now return exit code 1 rather than 0.
IMPROVEMENTS:
* core: Rolling updates based on allocation health [GH-2621, GH-2634, GH-2799]
* core: New deployment object to track job updates [GH-2621, GH-2634, GH-2799]

View File

@ -42,7 +42,7 @@ func (c *DeploymentFailCommand) Synopsis() string {
func (c *DeploymentFailCommand) Run(args []string) int {
var detach, verbose bool
flags := c.Meta.FlagSet("deployment resume", FlagSetClient)
flags := c.Meta.FlagSet("deployment fail", FlagSetClient)
flags.Usage = func() { c.Ui.Output(c.Help()) }
flags.BoolVar(&detach, "detach", false, "")
flags.BoolVar(&verbose, "verbose", false, "")
@ -81,8 +81,8 @@ func (c *DeploymentFailCommand) Run(args []string) int {
}
if len(possible) != 0 {
c.Ui.Output(fmt.Sprintf("Prefix matched multiple deployments\n\n%s", formatDeployments(possible, length)))
return 0
c.Ui.Error(fmt.Sprintf("Prefix matched multiple deployments\n\n%s", formatDeployments(possible, length)))
return 1
}
u, _, err := client.Deployments().Fail(deploy.ID, nil)

View File

@ -73,8 +73,8 @@ func (c *DeploymentPauseCommand) Run(args []string) int {
}
if len(possible) != 0 {
c.Ui.Output(fmt.Sprintf("Prefix matched multiple deployments\n\n%s", formatDeployments(possible, length)))
return 0
c.Ui.Error(fmt.Sprintf("Prefix matched multiple deployments\n\n%s", formatDeployments(possible, length)))
return 1
}
if _, _, err := client.Deployments().Pause(deploy.ID, true, nil); err != nil {

View File

@ -30,12 +30,9 @@ General Options:
Promote Options:
-all
All promotes all task groups in the deployment.
-group
Group may be specified many times and is used to promote that particular
group.
group. If no specific groups are specified, all groups are promoted.
-detach
Return immediately instead of entering monitor mode. After deployment
@ -49,16 +46,15 @@ Promote Options:
}
func (c *DeploymentPromoteCommand) Synopsis() string {
return "Manually fail a deployment"
return "Promote canaries in a deployment"
}
func (c *DeploymentPromoteCommand) Run(args []string) int {
var all, detach, verbose bool
var detach, verbose bool
var groups []string
flags := c.Meta.FlagSet("deployment resume", FlagSetClient)
flags := c.Meta.FlagSet("deployment promote", FlagSetClient)
flags.Usage = func() { c.Ui.Output(c.Help()) }
flags.BoolVar(&all, "all", false, "")
flags.BoolVar(&detach, "detach", false, "")
flags.BoolVar(&verbose, "verbose", false, "")
flags.Var((*flaghelper.StringFlag)(&groups), "group", "")
@ -73,10 +69,6 @@ func (c *DeploymentPromoteCommand) Run(args []string) int {
c.Ui.Error(c.Help())
return 1
}
if !all && len(groups) == 0 {
c.Ui.Error("Either -all or one or more -group flags must be specified.")
return 1
}
dID := args[0]
// Truncate the id unless full length is requested
@ -100,19 +92,19 @@ func (c *DeploymentPromoteCommand) Run(args []string) int {
}
if len(possible) != 0 {
c.Ui.Output(fmt.Sprintf("Prefix matched multiple deployments\n\n%s", formatDeployments(possible, length)))
return 0
c.Ui.Error(fmt.Sprintf("Prefix matched multiple deployments\n\n%s", formatDeployments(possible, length)))
return 1
}
var u *api.DeploymentUpdateResponse
if all {
if len(groups) == 0 {
u, _, err = client.Deployments().PromoteAll(deploy.ID, nil)
} else {
u, _, err = client.Deployments().PromoteGroups(deploy.ID, groups, nil)
}
if err != nil {
c.Ui.Error(fmt.Sprintf("Error failing deployment: %s", err))
c.Ui.Error(fmt.Sprintf("Error promoting deployment: %s", err))
return 1
}

View File

@ -27,14 +27,6 @@ func TestDeploymentPromoteCommand_Fails(t *testing.T) {
if code := cmd.Run([]string{"-address=nope", "12"}); code != 1 {
t.Fatalf("expected exit code 1, got: %d", code)
}
if out := ui.ErrorWriter.String(); !strings.Contains(out, "flags must be specified") {
t.Fatalf("expected missing flags error, got: %s", out)
}
ui.ErrorWriter.Reset()
if code := cmd.Run([]string{"-address=nope", "-all", "12"}); code != 1 {
t.Fatalf("expected exit code 1, got: %d", code)
}
if out := ui.ErrorWriter.String(); !strings.Contains(out, "Error retrieving deployment") {
t.Fatalf("expected failed query error, got: %s", out)
}

View File

@ -79,8 +79,8 @@ func (c *DeploymentResumeCommand) Run(args []string) int {
}
if len(possible) != 0 {
c.Ui.Output(fmt.Sprintf("Prefix matched multiple deployments\n\n%s", formatDeployments(possible, length)))
return 0
c.Ui.Error(fmt.Sprintf("Prefix matched multiple deployments\n\n%s", formatDeployments(possible, length)))
return 1
}
u, _, err := client.Deployments().Pause(deploy.ID, false, nil)

View File

@ -84,8 +84,8 @@ func (c *DeploymentStatusCommand) Run(args []string) int {
}
if len(possible) != 0 {
c.Ui.Output(fmt.Sprintf("Prefix matched multiple deployments\n\n%s", formatDeployments(possible, length)))
return 0
c.Ui.Error(fmt.Sprintf("Prefix matched multiple deployments\n\n%s", formatDeployments(possible, length)))
return 1
}
if json || len(tmpl) > 0 {
@ -177,9 +177,12 @@ func formatDeploymentGroups(d *api.Deployment, uuidLength int) string {
if autorevert {
rowString += "Auto Revert|"
}
if canaries {
rowString += "Promoted|"
}
rowString += "Desired|"
if canaries {
rowString += "Canaries|Promoted|"
rowString += "Canaries|"
}
rowString += "Placed|Healthy|Unhealthy"
@ -191,10 +194,12 @@ func formatDeploymentGroups(d *api.Deployment, uuidLength int) string {
if autorevert {
row += fmt.Sprintf("%v|", state.AutoRevert)
}
if canaries {
row += fmt.Sprintf("%v|", state.Promoted)
}
row += fmt.Sprintf("%d|", state.DesiredTotal)
if canaries {
row += fmt.Sprintf("%d|", state.DesiredCanaries)
row += fmt.Sprintf("%v|", state.Promoted)
}
row += fmt.Sprintf("%d|%d|%d", state.PlacedAllocs, state.HealthyAllocs, state.UnhealthyAllocs)
rows[i] = row

View File

@ -137,8 +137,8 @@ func (c *EvalStatusCommand) Run(args []string) int {
failures,
)
}
c.Ui.Output(fmt.Sprintf("Prefix matched multiple evaluations\n\n%s", formatList(out)))
return 0
c.Ui.Error(fmt.Sprintf("Prefix matched multiple evaluations\n\n%s", formatList(out)))
return 1
}
// If we are in monitor mode, monitor and exit

View File

@ -160,8 +160,8 @@ func (f *FSCommand) Run(args []string) int {
if len(allocs) > 1 {
// Format the allocs
out := formatAllocListStubs(allocs, verbose, length)
f.Ui.Output(fmt.Sprintf("Prefix matched multiple allocations\n\n%s", out))
return 0
f.Ui.Error(fmt.Sprintf("Prefix matched multiple allocations\n\n%s", out))
return 1
}
// Prefix lookup matched a single allocation
alloc, _, err := client.Allocations().Info(allocs[0].ID, nil)

View File

@ -23,6 +23,9 @@ General Options:
Inspect Options:
-version <job version>
Display only the history for the given job version.
-json
Output the job in its JSON format.
@ -38,12 +41,13 @@ func (c *InspectCommand) Synopsis() string {
func (c *InspectCommand) Run(args []string) int {
var json bool
var tmpl string
var tmpl, versionStr string
flags := c.Meta.FlagSet("inspect", FlagSetClient)
flags.Usage = func() { c.Ui.Output(c.Help()) }
flags.BoolVar(&json, "json", false, "")
flags.StringVar(&tmpl, "t", "", "")
flags.StringVar(&versionStr, "version", "", "")
if err := flags.Parse(args); err != nil {
return 1
@ -93,12 +97,23 @@ func (c *InspectCommand) Run(args []string) int {
return 1
}
if len(jobs) > 1 && strings.TrimSpace(jobID) != jobs[0].ID {
c.Ui.Output(fmt.Sprintf("Prefix matched multiple jobs\n\n%s", createStatusListOutput(jobs)))
return 0
c.Ui.Error(fmt.Sprintf("Prefix matched multiple jobs\n\n%s", createStatusListOutput(jobs)))
return 1
}
var version *uint64
if versionStr != "" {
v, _, err := parseVersion(versionStr)
if err != nil {
c.Ui.Error(fmt.Sprintf("Error parsing version value %q: %v", versionStr, err))
return 1
}
version = &v
}
// Prefix lookup matched a single job
job, _, err := client.Jobs().Info(jobs[0].ID, nil)
job, err := getJob(client, jobs[0].ID, version)
if err != nil {
c.Ui.Error(fmt.Sprintf("Error inspecting job: %s", err))
return 1
@ -132,3 +147,25 @@ func (c *InspectCommand) Run(args []string) int {
c.Ui.Output(out)
return 0
}
// getJob retrieves the job optionally at a particular version.
func getJob(client *api.Client, jobID string, version *uint64) (*api.Job, error) {
if version == nil {
job, _, err := client.Jobs().Info(jobID, nil)
return job, err
}
versions, _, _, err := client.Jobs().Versions(jobID, false, nil)
if err != nil {
return nil, err
}
for _, j := range versions {
if *j.Version != *version {
continue
}
return j, nil
}
return nil, fmt.Errorf("job %q with version %d couldn't be found", jobID, *version)
}

View File

@ -82,8 +82,8 @@ func (c *JobDeploymentsCommand) Run(args []string) int {
return 1
}
if len(jobs) > 1 && strings.TrimSpace(jobID) != jobs[0].ID {
c.Ui.Output(fmt.Sprintf("Prefix matched multiple jobs\n\n%s", createStatusListOutput(jobs)))
return 0
c.Ui.Error(fmt.Sprintf("Prefix matched multiple jobs\n\n%s", createStatusListOutput(jobs)))
return 1
}
jobID = jobs[0].ID

View File

@ -100,8 +100,8 @@ func (c *JobHistoryCommand) Run(args []string) int {
return 1
}
if len(jobs) > 1 && strings.TrimSpace(jobID) != jobs[0].ID {
c.Ui.Output(fmt.Sprintf("Prefix matched multiple jobs\n\n%s", createStatusListOutput(jobs)))
return 0
c.Ui.Error(fmt.Sprintf("Prefix matched multiple jobs\n\n%s", createStatusListOutput(jobs)))
return 1
}
// Prefix lookup matched a single job

136
command/job_promote.go Normal file
View File

@ -0,0 +1,136 @@
package command
import (
"fmt"
"strings"
"github.com/hashicorp/nomad/api"
flaghelper "github.com/hashicorp/nomad/helper/flag-helpers"
)
type JobPromoteCommand struct {
Meta
}
func (c *JobPromoteCommand) Help() string {
helpText := `
Usage: nomad job promote [options] <job id>
Promote is used to promote task groups in the most recent deployment for the
given job. Promotion should occur when the deployment has placed canaries for a
task group and those canaries have been deemed healthy. When a task group is
promoted, the rolling upgrade of the remaining allocations is unblocked. If the
canaries are found to be unhealthy, the deployment may either be failed using
the "nomad deployment fail" command, the job can be failed forward by submitting
a new version or failed backwards by reverting to an older version using the
"nomad job revert" command.
General Options:
` + generalOptionsUsage() + `
Promote Options:
-group
Group may be specified many times and is used to promote that particular
group. If no specific groups are specified, all groups are promoted.
-detach
Return immediately instead of entering monitor mode. After deployment
resume, the evaluation ID will be printed to the screen, which can be used
to examine the evaluation using the eval-status command.
-verbose
Display full information.
`
return strings.TrimSpace(helpText)
}
func (c *JobPromoteCommand) Synopsis() string {
return "Promote a job's canaries"
}
func (c *JobPromoteCommand) Run(args []string) int {
var detach, verbose bool
var groups []string
flags := c.Meta.FlagSet("job promote", FlagSetClient)
flags.Usage = func() { c.Ui.Output(c.Help()) }
flags.BoolVar(&detach, "detach", false, "")
flags.BoolVar(&verbose, "verbose", false, "")
flags.Var((*flaghelper.StringFlag)(&groups), "group", "")
if err := flags.Parse(args); err != nil {
return 1
}
// Check that we got no arguments
args = flags.Args()
if l := len(args); l != 1 {
c.Ui.Error(c.Help())
return 1
}
// Truncate the id unless full length is requested
length := shortId
if verbose {
length = fullId
}
// Get the HTTP client
client, err := c.Meta.Client()
if err != nil {
c.Ui.Error(fmt.Sprintf("Error initializing client: %s", err))
return 1
}
// Check if the job exists
jobID := args[0]
jobs, _, err := client.Jobs().PrefixList(jobID)
if err != nil {
c.Ui.Error(fmt.Sprintf("Error promoting job: %s", err))
return 1
}
if len(jobs) == 0 {
c.Ui.Error(fmt.Sprintf("No job(s) with prefix or id %q found", jobID))
return 1
}
if len(jobs) > 1 && strings.TrimSpace(jobID) != jobs[0].ID {
c.Ui.Error(fmt.Sprintf("Prefix matched multiple jobs\n\n%s", createStatusListOutput(jobs)))
return 1
}
jobID = jobs[0].ID
// Do a prefix lookup
deploy, _, err := client.Jobs().LatestDeployment(jobID, nil)
if err != nil {
c.Ui.Error(fmt.Sprintf("Error retrieving deployment: %s", err))
return 1
}
if deploy == nil {
c.Ui.Error(fmt.Sprintf("Job %q has no deployment to promote", jobID))
return 1
}
var u *api.DeploymentUpdateResponse
if len(groups) == 0 {
u, _, err = client.Deployments().PromoteAll(deploy.ID, nil)
} else {
u, _, err = client.Deployments().PromoteGroups(deploy.ID, groups, nil)
}
if err != nil {
c.Ui.Error(fmt.Sprintf("Error promoting deployment %q for job %q: %s", deploy.ID, jobID, err))
return 1
}
// Nothing to do
evalCreated := u.EvalID != ""
if detach || !evalCreated {
return 0
}
mon := newMonitor(c.Ui, client, length)
return mon.monitor(u.EvalID, false)
}

View File

@ -0,0 +1,34 @@
package command
import (
"strings"
"testing"
"github.com/mitchellh/cli"
)
func TestJobPromoteCommand_Implements(t *testing.T) {
var _ cli.Command = &JobPromoteCommand{}
}
func TestJobPromoteCommand_Fails(t *testing.T) {
ui := new(cli.MockUi)
cmd := &JobPromoteCommand{Meta: Meta{Ui: ui}}
// Fails on misuse
if code := cmd.Run([]string{"some", "bad", "args"}); code != 1 {
t.Fatalf("expected exit code 1, got: %d", code)
}
if out := ui.ErrorWriter.String(); !strings.Contains(out, cmd.Help()) {
t.Fatalf("expected help output, got: %s", out)
}
ui.ErrorWriter.Reset()
if code := cmd.Run([]string{"-address=nope", "12"}); code != 1 {
t.Fatalf("expected exit code 1, got: %d", code)
}
if out := ui.ErrorWriter.String(); !strings.Contains(out, "Error promoting") {
t.Fatalf("expected failed to promote error, got: %s", out)
}
ui.ErrorWriter.Reset()
}

View File

@ -91,8 +91,8 @@ func (c *JobRevertCommand) Run(args []string) int {
return 1
}
if len(jobs) > 1 && strings.TrimSpace(jobID) != jobs[0].ID {
c.Ui.Output(fmt.Sprintf("Prefix matched multiple jobs\n\n%s", createStatusListOutput(jobs)))
return 0
c.Ui.Error(fmt.Sprintf("Prefix matched multiple jobs\n\n%s", createStatusListOutput(jobs)))
return 1
}
// Prefix lookup matched a single job

View File

@ -136,8 +136,8 @@ func (l *LogsCommand) Run(args []string) int {
if len(allocs) > 1 {
// Format the allocs
out := formatAllocListStubs(allocs, verbose, length)
l.Ui.Output(fmt.Sprintf("Prefix matched multiple allocations\n\n%s", out))
return 0
l.Ui.Error(fmt.Sprintf("Prefix matched multiple allocations\n\n%s", out))
return 1
}
// Prefix lookup matched a single allocation
alloc, _, err := client.Allocations().Info(allocs[0].ID, nil)

View File

@ -124,8 +124,8 @@ func (c *NodeDrainCommand) Run(args []string) int {
node.Status)
}
// Dump the output
c.Ui.Output(fmt.Sprintf("Prefix matched multiple nodes\n\n%s", formatList(out)))
return 0
c.Ui.Error(fmt.Sprintf("Prefix matched multiple nodes\n\n%s", formatList(out)))
return 1
}
// Prefix lookup matched a single node

View File

@ -231,8 +231,8 @@ func (c *NodeStatusCommand) Run(args []string) int {
node.Status)
}
// Dump the output
c.Ui.Output(fmt.Sprintf("Prefix matched multiple nodes\n\n%s", formatList(out)))
return 0
c.Ui.Error(fmt.Sprintf("Prefix matched multiple nodes\n\n%s", formatList(out)))
return 1
}
// Prefix lookup matched a single node
node, _, err := client.Nodes().Info(nodes[0].ID, nil)

View File

@ -121,8 +121,8 @@ func (c *StatusCommand) Run(args []string) int {
return 1
}
if len(jobs) > 1 && strings.TrimSpace(jobID) != jobs[0].ID {
c.Ui.Output(fmt.Sprintf("Prefix matched multiple jobs\n\n%s", createStatusListOutput(jobs)))
return 0
c.Ui.Error(fmt.Sprintf("Prefix matched multiple jobs\n\n%s", createStatusListOutput(jobs)))
return 1
}
// Prefix lookup matched a single job
job, _, err := client.Jobs().Info(jobs[0].ID, nil)

View File

@ -113,16 +113,18 @@ func TestStatusCommand_Run(t *testing.T) {
if !strings.Contains(out, "Created At") {
t.Fatal("should have created header")
}
ui.ErrorWriter.Reset()
ui.OutputWriter.Reset()
// Query jobs with prefix match
if code := cmd.Run([]string{"-address=" + url, "job"}); code != 0 {
t.Fatalf("expected exit 0, got: %d", code)
if code := cmd.Run([]string{"-address=" + url, "job"}); code != 1 {
t.Fatalf("expected exit 1, got: %d", code)
}
out = ui.OutputWriter.String()
out = ui.ErrorWriter.String()
if !strings.Contains(out, "job1_sfx") || !strings.Contains(out, "job2_sfx") {
t.Fatalf("expected job1_sfx and job2_sfx, got: %s", out)
}
ui.ErrorWriter.Reset()
ui.OutputWriter.Reset()
// Query a single job with prefix match

View File

@ -94,8 +94,8 @@ func (c *StopCommand) Run(args []string) int {
return 1
}
if len(jobs) > 1 && strings.TrimSpace(jobID) != jobs[0].ID {
c.Ui.Output(fmt.Sprintf("Prefix matched multiple jobs\n\n%s", createStatusListOutput(jobs)))
return 0
c.Ui.Error(fmt.Sprintf("Prefix matched multiple jobs\n\n%s", createStatusListOutput(jobs)))
return 1
}
// Prefix lookup matched a single job
job, _, err := client.Jobs().Info(jobs[0].ID, nil)

View File

@ -144,6 +144,11 @@ func Commands(metaPtr *command.Meta) map[string]cli.CommandFactory {
Meta: meta,
}, nil
},
"job promote": func() (cli.Command, error) {
return &command.JobPromoteCommand{
Meta: meta,
}, nil
},
"job revert": func() (cli.Command, error) {
return &command.JobRevertCommand{
Meta: meta,

View File

@ -30,7 +30,7 @@ func RunCustom(args []string, commands map[string]cli.CommandFactory) int {
"deployment resume", "deployment fail", "deployment promote":
case "executor":
case "fs ls", "fs cat", "fs stat":
case "job deployments", "job dispatch", "job history", "job revert":
case "job deployments", "job dispatch", "job history", "job promote", "job revert":
case "operator raft", "operator raft list-peers", "operator raft remove-peer":
case "syslog":
default:

View File

@ -122,7 +122,11 @@ func (d *Deployment) Pause(args *structs.DeploymentPauseRequest, reply *structs.
}
if !deploy.Active() {
return fmt.Errorf("can't pause terminal deployment")
if args.Pause {
return fmt.Errorf("can't pause terminal deployment")
}
return fmt.Errorf("can't resume terminal deployment")
}
// Call into the deployment watcher

View File

@ -78,6 +78,12 @@ job "docs" {
last stable job on deployment failure. A job is marked as stable if all the
allocations as part of its deployment were marked healthy.
- `canary` `(int: 0)` - Specifies that changes to the job that would result in
destructive updates should create the specified number of canaries without
stopping any previous allocations. Once the operator determines the canaries
are healthy, they can be promoted which unblocks a rolling update of the
remaining allocations at a rate of `max_parallel`.
- `stagger` `(string: "30s")` - Specifies the delay between migrating
allocations off nodes marked for draining. This is specified using a label
suffix like "30s" or "1h".
@ -120,10 +126,7 @@ update {
This example creates a canary allocation when the job is updated. The canary is
created without stopping any previous allocations from the job and allows
operators to determine if the new version of the job should be rolled out. Once
the operator has determined the new job should be deployed, the deployment can
be promoted and a rolling update will occur performing 3 updates at a time till
the remainder of the groups allocations have been rolled to the new version.
operators to determine if the new version of the job should be rolled out.
```hcl
update {
@ -132,6 +135,47 @@ update {
}
```
Once the operator has determined the new job should be deployed, the deployment
can be promoted and a rolling update will occur performing 3 updates at a time
until the remainder of the groups allocations have been rolled to the new
version.
```text
# Promote the canaries for the job.
$ nomad job promote <job-id>
```
### Blue/Green Upgrades
By setting the canary count equal to that of the task group, blue/green
deployments can be achieved. When a new version of the job is submitted, instead
of doing a rolling upgrade of the existing allocations, the new version of the
group is deployed along side the existing set. While this duplicates the
resources required during the upgrade process, it allows very safe deployments
as the original version of the group is untouched.
```hcl
group "api-server" {
count = 3
update {
canary = 3
max_parallel = 3
}
...
}
```
Once the operator is satisfied that the new version of the group is stable, the
group can be promoted which will result in all allocations for the old versions
of the group to be shutdown. This completes the upgrade from blue to green, or
old to new version.
```text
# Promote the canaries for the job.
$ nomad job promote <job-id>
```
### Serial Upgrades
This example uses a serial upgrade strategy, meaning exactly one task group will