Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

snap: implement new snap reboot command #9021

Merged
merged 21 commits into from
Sep 10, 2020
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
4469021
snap: implement new `snap reboot` command
mvo5 Jul 16, 2020
b0e0833
Merge remote-tracking branch 'upstream/master' into snap-cmd-reboot
bboozzoo Jul 28, 2020
1a5f7fe
cmd/snap: tweak reboot command help and errors, extend unit tests
bboozzoo Jul 28, 2020
2e4100f
snap: remove XXX comment that was discussed in the PR
mvo5 Aug 11, 2020
bab0ca2
snap: improve `snap reboot` output message (thanks to Ian)
mvo5 Aug 25, 2020
83ea27a
Merge branch 'api-system-post-reboot' into snap-cmd-reboot
mvo5 Sep 3, 2020
c98ad20
snap: make <label>/reboot-OPTIONS no longer required
mvo5 Sep 3, 2020
105ede5
client: implement DoSystemReboot()
mvo5 Sep 3, 2020
4485991
snap: make `snap reboot` use client.DoSystemReboot
mvo5 Sep 3, 2020
5a34eff
tests: update uc20-recovery test to match new "snap reboot" output
mvo5 Sep 3, 2020
5a99a04
Merge remote-tracking branch 'upstream/master' into snap-cmd-reboot
mvo5 Sep 7, 2020
c876309
snap: improve tests/output of snap reboot cmd
mvo5 Sep 7, 2020
4509e86
Merge branch 'systems-reboot-api-client-side' into snap-cmd-reboot
mvo5 Sep 7, 2020
9da1973
Merge branch 'systems-reboot-api-client-side' into snap-cmd-reboot
mvo5 Sep 7, 2020
688b602
Merge remote-tracking branch 'upstream/master' into snap-cmd-reboot
mvo5 Sep 8, 2020
421e70a
snap: use correct client.RebootToSystem API
mvo5 Sep 10, 2020
a37c40d
snap: improve long reboot doc
mvo5 Sep 10, 2020
4a9c98e
client,snap: rework error formating of reboot command (thanks to Samu…
mvo5 Sep 10, 2020
e88e6d5
snap: fix silly typo
mvo5 Sep 10, 2020
29d87b0
snap: add missing i18n.G() (thanks Pawel)
mvo5 Sep 10, 2020
5b57db1
tests: fix silly typo
mvo5 Sep 10, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmd/snap/cmd_help.go
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ var helpCategories = []helpCategory{
}, {
Label: i18n.G("Other"),
Description: i18n.G("miscellanea"),
Commands: []string{"version", "warnings", "okay", "ack", "known", "model", "create-cohort"},
Commands: []string{"version", "warnings", "okay", "ack", "known", "model", "create-cohort", "reboot"},
}, {
Label: i18n.G("Development"),
Description: i18n.G("developer-oriented features"),
Expand Down
110 changes: 110 additions & 0 deletions cmd/snap/cmd_reboot.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
// -*- Mode: Go; indent-tabs-mode: t -*-

/*
* Copyright (C) 2020 Canonical Ltd
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License version 3 as
* published by the Free Software Foundation.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*
*/

package main

import (
"fmt"

"github.com/jessevdk/go-flags"

"github.com/snapcore/snapd/client"
"github.com/snapcore/snapd/i18n"
)

type cmdReboot struct {
clientMixin
Positional struct {
Label string
} `positional-args:"true" required:"true"`

RunMode bool `long:"run"`
InstallMode bool `long:"install"`
RecoverMode bool `long:"recover"`
}

var shortRebootHelp = i18n.G("Reboot into selected system and mode")
var longRebootHelp = i18n.G(`
The reboot command reboots the system into a particular mode of the selected
recovery system.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should also explain what happens if mode or label are omitted

`)

func init() {
addCommand("reboot", shortRebootHelp, longRebootHelp, func() flags.Commander {
return &cmdReboot{}
}, map[string]string{
// TRANSLATORS: This should not start with a lowercase letter.
"run": i18n.G("Boot into run mode"),
// TRANSLATORS: This should not start with a lowercase letter.
"install": i18n.G("Boot into install mode"),
// TRANSLATORS: This should not start with a lowercase letter.
"recover": i18n.G("Boot into recover mode"),
}, []argDesc{
{
// TRANSLATORS: This needs to begin with < and end with >
name: i18n.G("<label>"),
// TRANSLATORS: This should not start with a lowercase letter.
desc: i18n.G("The recovery system label"),
},
})
}

func (x *cmdReboot) modeFromCommandline() (*client.SystemAction, error) {
var action client.SystemAction
for _, arg := range []struct {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice.

enabled bool
mode string
}{
{x.RunMode, "run"},
{x.RecoverMode, "recover"},
{x.InstallMode, "install"},
} {
if !arg.enabled {
continue
}
if action.Mode != "" {
return nil, fmt.Errorf("Please specify a single mode")
}
action.Mode = arg.mode
}
// XXX: should we have a default here?
mvo5 marked this conversation as resolved.
Show resolved Hide resolved
if action.Mode == "" {
return nil, fmt.Errorf("Please specify a mode, see --help")
}

return &action, nil
}

func (x *cmdReboot) Execute(args []string) error {
if len(args) > 0 {
return ErrExtraArgs
}

action, err := x.modeFromCommandline()
if err != nil {
return err
}

if err := x.client.DoSystemAction(x.Positional.Label, action); err != nil {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For run label can only be the current system, so it could also be omitted. Same for recover.

In general the label can be left out and default to current.

return fmt.Errorf("cannot reboot into system %q: %v", x.Positional.Label, err)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we maybe want a client.RebootSystem(label, mode) method, that should map to a different variant of the API that has action: "reboot"

the action: "do" setup was mostly meant for the chooser, where possibly some actions might not be reboots

}

fmt.Fprintf(Stdout, "Reboot into %q with mode %q scheduled.\n", x.Positional.Label, action.Mode)
mvo5 marked this conversation as resolved.
Show resolved Hide resolved
return nil
}
122 changes: 122 additions & 0 deletions cmd/snap/cmd_reboot_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
// -*- Mode: Go; indent-tabs-mode: t -*-

/*
* Copyright (C) 2020 Canonical Ltd
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License version 3 as
* published by the Free Software Foundation.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*
*/

package main_test

import (
"fmt"
"net/http"
"strings"

. "gopkg.in/check.v1"

snap "github.com/snapcore/snapd/cmd/snap"
)

func (s *SnapSuite) TestRebootHelp(c *C) {
msg := `Usage:
snap.test reboot [reboot-OPTIONS] <label>

The reboot command reboots the system into a particular mode of the selected
recovery system.

[reboot command options]
--run Boot into run mode
--install Boot into install mode
--recover Boot into recover mode

[reboot command arguments]
<label>: The recovery system label
`
s.testSubCommandHelp(c, "reboot", msg)
}

func (s *SnapSuite) TestRebootHappy(c *C) {
n := 0
s.RedirectClientToTestServer(func(w http.ResponseWriter, r *http.Request) {
switch n {
case 0:
c.Check(r.Method, Equals, "POST")
c.Check(r.URL.Path, Equals, "/v2/systems/20200101")
c.Check(r.URL.RawQuery, Equals, "")
fmt.Fprintln(w, `{"type": "sync", "result": {}}`)
default:
c.Fatalf("expected to get 1 requests, now on %d", n+1)
}

n++
})
rest, err := snap.Parser(snap.Client()).ParseArgs([]string{"reboot", "--recover", "20200101"})
c.Assert(err, IsNil)
c.Assert(rest, DeepEquals, []string{})
c.Check(s.Stdout(), Equals, `Reboot into "20200101" with mode "recover" scheduled.
`)
c.Check(s.Stderr(), Equals, "")
}

func (s *SnapSuite) TestRebootUnhappy(c *C) {
s.RedirectClientToTestServer(func(w http.ResponseWriter, r *http.Request) {
c.Fatalf("server should not be hit in this test")
})

var tc = []struct {
args []string
errStr string
}{
{
args: []string{"reboot", "20200101"},
errStr: "Please specify a mode, see --help",
},
{
args: []string{"reboot", "--run", "--recover", "20200101"},
errStr: "Please specify a single mode",
},
{
args: []string{"reboot", "--unknown-mode", "20200101"},
errStr: "unknown flag `unknown-mode'",
},
}

for _, t := range tc {
_, err := snap.Parser(snap.Client()).ParseArgs(t.args)
c.Check(err, ErrorMatches, t.errStr, Commentf(strings.Join(t.args, " ")))
}
}

func (s *SnapSuite) TestRebootAPIFail(c *C) {
n := 0
s.RedirectClientToTestServer(func(w http.ResponseWriter, r *http.Request) {
switch n {
case 0:
c.Check(r.Method, Equals, "POST")
c.Check(r.URL.Path, Equals, "/v2/systems/20200101")
c.Check(r.URL.RawQuery, Equals, "")
w.WriteHeader(404)
fmt.Fprintln(w, `{"type": "error", "status-code":404, "result": {"message":"requested system does not exist"}}`)
default:
c.Fatalf("expected to get 1 requests, now on %d", n+1)
}

n++
})
_, err := snap.Parser(snap.Client()).ParseArgs([]string{"reboot", "--recover", "20200101"})
c.Assert(err, ErrorMatches, `cannot reboot into system "20200101": cannot request system action: requested system does not exist`)
c.Check(s.Stdout(), Equals, "")
c.Check(s.Stderr(), Equals, "")
}
8 changes: 2 additions & 6 deletions tests/core/uc20-recovery/task.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,7 @@ execute: |
# request a recovery mode
echo "Request rebooting into recovery mode"
# response contains maintenance: {"maintenance":{"message":"system is restarting","kind":"system-restart"}}
test-snapd-curl.curl -X POST --data '{"action":"do","mode":"recover"}' \
-s --unix-socket /run/snapd.socket "http://localhost/v2/systems/$label" | \
MATCH '"system-restart"'
snap reboot --recover "$label" | MATCH "Reboot.*scheduled"
mvo5 marked this conversation as resolved.
Show resolved Hide resolved

# snapd schedules a slow timeout and an immediate one, however it is
# scheduled asynchronously, try keep the check simple
Expand Down Expand Up @@ -136,9 +134,7 @@ execute: |
mount -o bind "$PWD/mock-shutdown" /usr/sbin/shutdown

# request going back to run mode
test-snapd-curl.curl -X POST --data '{"action":"do","mode":"run"}' \
-s --unix-socket /run/snapd.socket "http://localhost/v2/systems/$label" | \
MATCH '"system-restart"'
snap reboot --run "$label" | MATCH "Reboot.*scheduled"
# XXX: is this a race between spread seeing REBOOT and machine rebooting?

# see earlier note about shutdown
Expand Down