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

feat(run): Add the --dry option to the run command #550

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 20 additions & 1 deletion internals/cli/cmd_run.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package cli
import (
"errors"
"fmt"
"maps"
"os"
"os/signal"
"strconv"
Expand Down Expand Up @@ -62,22 +63,29 @@ var sharedRunEnterArgsHelp = map[string]string{
"--args": "Provide additional arguments to a service",
"--identities": "Seed identities from file (like update-identities --replace)",
}
var runArgsHelp = map[string]string{
"--dry": "Start {{.DisplayName}} without side-effects",
Copy link
Contributor

Choose a reason for hiding this comment

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

It doesn't "start" Pebble though, right? I think we need a more accurate description here.

}

type cmdRun struct {
client *client.Client

socketPath string
pebbleDir string

DryRun bool `long:"dry"`
sharedRunEnterOpts
}

func init() {
argsHelp := runArgsHelp
maps.Copy(argsHelp, sharedRunEnterArgsHelp)

AddCommand(&CmdInfo{
Name: "run",
Summary: cmdRunSummary,
Description: cmdRunDescription,
ArgsHelp: sharedRunEnterArgsHelp,
ArgsHelp: argsHelp,
New: func(opts *CmdOptions) flags.Commander {
return &cmdRun{
client: opts.Client,
Expand Down Expand Up @@ -185,6 +193,7 @@ func runDaemon(rcmd *cmdRun, ch chan os.Signal, ready chan<- func()) error {
dopts := daemon.Options{
Dir: rcmd.pebbleDir,
SocketPath: rcmd.socketPath,
DryRun: rcmd.DryRun,
}
if rcmd.Verbose {
dopts.ServiceOutput = os.Stdout
Expand All @@ -195,6 +204,16 @@ func runDaemon(rcmd *cmdRun, ch chan os.Signal, ready chan<- func()) error {
if err != nil {
return err
}

if rcmd.DryRun {
// Validation has been performed by initializing the daemon above, so we
// don't need the rest of the code to execute for dry-run.
// If a hook for manager-specific dry run logic is added in the future, it
// should be run immediately above this block.o
logger.Noticef("No error encountered: dry-run successful.")
return nil
}

if err := d.Init(); err != nil {
return err
}
Expand Down
4 changes: 4 additions & 0 deletions internals/daemon/daemon.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,9 @@ type Options struct {
// OverlordExtension is an optional interface used to extend the capabilities
// of the Overlord.
OverlordExtension overlord.Extension

// If true, no state will be written to file.
DryRun bool
}

// A Daemon listens for requests and routes them to the right command
Expand Down Expand Up @@ -852,6 +855,7 @@ func New(opts *Options) (*Daemon, error) {
RestartHandler: d,
ServiceOutput: opts.ServiceOutput,
Extension: opts.OverlordExtension,
DryRun: opts.DryRun,
}

ovld, err := overlord.New(&ovldOptions)
Expand Down
6 changes: 6 additions & 0 deletions internals/overlord/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,15 @@ type overlordStateBackend struct {
path string
ensureBefore func(d time.Duration)
requestRestart func(t restart.RestartType)

// If this is true, the backend will not actually save anything to file.
DryRun bool
}

func (osb *overlordStateBackend) Checkpoint(data []byte) error {
if osb.DryRun {
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of checking a flag in Checkpoint, could we provide a different type for the state backend, a do-nothing implementation?

return nil
}
return osutil.AtomicWriteFile(osb.path, data, 0600, 0)
}

Expand Down
8 changes: 8 additions & 0 deletions internals/overlord/overlord.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,9 @@ type Options struct {
ServiceOutput io.Writer
// Extension allows extending the overlord with externally defined features.
Extension Extension
// DryRun must be true if state in storage is not meant to be altered.
// Otherwise, the Overlord will operate normally.
DryRun bool
}

// Overlord is the central manager of the system, keeping track
Expand Down Expand Up @@ -106,6 +109,9 @@ type Overlord struct {
logMgr *logstate.LogManager

extension Extension

// If true, no state will be written to file.
DryRun bool
}

// New creates an Overlord with all its state managers.
Expand All @@ -116,6 +122,7 @@ func New(opts *Options) (*Overlord, error) {
loopTomb: new(tomb.Tomb),
inited: true,
extension: opts.Extension,
DryRun: opts.DryRun,
}

if !filepath.IsAbs(o.pebbleDir) {
Expand All @@ -133,6 +140,7 @@ func New(opts *Options) (*Overlord, error) {
backend := &overlordStateBackend{
path: statePath,
ensureBefore: o.ensureBefore,
DryRun: opts.DryRun,
}
s, restartMgr, err := loadState(statePath, opts.RestartHandler, backend)
if err != nil {
Expand Down
Loading