From 0fbb9fdec1c7ce16525e5b935984f8a8828806cd Mon Sep 17 00:00:00 2001 From: "Joel D. Elkins" Date: Sat, 30 Jul 2022 14:26:23 -0500 Subject: [PATCH] Improve logging and output formatting with CommandSet --- cmd/create.go | 2 +- cmd/execute.go | 15 ++- cmd/pull.go | 2 +- cmd/recreate.go | 2 +- cmd/restart.go | 2 +- cmd/rm.go | 2 +- cmd/start.go | 2 +- cmd/stop.go | 2 +- cmd/update.go | 2 +- internal/pkg/command/command.go | 97 ++++++---------- internal/pkg/container/container.go | 172 +++++++++++++++------------- 11 files changed, 147 insertions(+), 153 deletions(-) diff --git a/cmd/create.go b/cmd/create.go index 2460eea..3236be7 100644 --- a/cmd/create.go +++ b/cmd/create.go @@ -38,7 +38,7 @@ var createCmd = &cobra.Command{ names or categories. Multiple arguments are supported.`, Run: func(_ *cobra.Command, args []string) { conts := config.Union(args, contMask) - getAndExecute("CREATE", conts, func(c *container.Container) command.Commands { return c.CreateCommands() }) + getAndExecute(conts, func(c *container.Container) command.CommandSet { return c.CreateCommands() }) }, } diff --git a/cmd/execute.go b/cmd/execute.go index c3640e0..6795408 100644 --- a/cmd/execute.go +++ b/cmd/execute.go @@ -22,23 +22,26 @@ THE SOFTWARE. package cmd import ( - "fmt" "sync" "gitea.elkins.co/Networking/ccl/internal/pkg/command" "gitea.elkins.co/Networking/ccl/internal/pkg/container" + "github.com/sirupsen/logrus" ) -func getAndExecute(action string, tgts []container.Container, getCmds func(*container.Container) command.Commands) { +func getAndExecute(tgts []container.Container, getSet func(*container.Container) command.CommandSet) { var wg sync.WaitGroup for i := range tgts { - fmt.Fprintln(output, action, tgts[i].Name) wg.Add(1) go func(cont *container.Container) { defer wg.Done() - for _, cmd := range getCmds(cont) { - if err := cmd.Execute(output, fake); err != nil { - cont.LogEntry().WithField("error", err).Errorln("Could not", action) + set := getSet(cont) + for _, cmd := range set.Commands { + if err := cmd.Execute(output, fake, set.ID); err != nil { + cont.LogEntry().WithFields(logrus.Fields{ + "error": err, + "action": set.ID, + }).Errorln("Failed") return } } diff --git a/cmd/pull.go b/cmd/pull.go index a0375b5..341e10d 100644 --- a/cmd/pull.go +++ b/cmd/pull.go @@ -39,7 +39,7 @@ affected: the old image will still remain, though untagged, and any defined cont will still use it.`, Run: func(cmd *cobra.Command, args []string) { conts := config.Union(args, contMask) - getAndExecute("PULL", conts, func(c *container.Container) command.Commands { return c.PullCommands() }) + getAndExecute(conts, func(c *container.Container) command.CommandSet { return c.PullCommands() }) }, } diff --git a/cmd/recreate.go b/cmd/recreate.go index bed4c1a..112bd1a 100644 --- a/cmd/recreate.go +++ b/cmd/recreate.go @@ -38,7 +38,7 @@ var recreateCmd = &cobra.Command{ one or more container names or categories. If empty, "all" is assumed.`, Run: func(_ *cobra.Command, args []string) { conts := config.Union(args, contMask) - getAndExecute("RECREATE", conts, func(c *container.Container) command.Commands { return c.RecreateCommands() }) + getAndExecute(conts, func(c *container.Container) command.CommandSet { return c.RecreateCommands() }) }, } diff --git a/cmd/restart.go b/cmd/restart.go index 6860d27..7360bdc 100644 --- a/cmd/restart.go +++ b/cmd/restart.go @@ -38,7 +38,7 @@ var restartCmd = &cobra.Command{ one or more container names or categories. If empty, "all" is assumed.`, Run: func(_ *cobra.Command, args []string) { conts := config.Union(args, contMask) - getAndExecute("RESTART", conts, func(c *container.Container) command.Commands { return c.RestartCommands() }) + getAndExecute(conts, func(c *container.Container) command.CommandSet { return c.RestartCommands() }) }, } diff --git a/cmd/rm.go b/cmd/rm.go index 5e7b6e3..ac883bc 100644 --- a/cmd/rm.go +++ b/cmd/rm.go @@ -39,7 +39,7 @@ var rmCmd = &cobra.Command{ If running, they will first be stopped.`, Run: func(cmd *cobra.Command, args []string) { conts := config.Union(args, contMask) - getAndExecute("REMOVE", conts, func(c *container.Container) command.Commands { return c.DestroyCommands() }) + getAndExecute(conts, func(c *container.Container) command.CommandSet { return c.RemoveCommands() }) }, } diff --git a/cmd/start.go b/cmd/start.go index 1f4b26f..8dd7f3e 100644 --- a/cmd/start.go +++ b/cmd/start.go @@ -38,7 +38,7 @@ var startCmd = &cobra.Command{ one or more container names or categories. If empty, "all" is assumed.`, Run: func(_ *cobra.Command, args []string) { conts := config.Union(args, contMask) - getAndExecute("START", conts, func(c *container.Container) command.Commands { return c.StartCommands() }) + getAndExecute(conts, func(c *container.Container) command.CommandSet { return c.StartCommands() }) }, } diff --git a/cmd/stop.go b/cmd/stop.go index 007ff4e..5529904 100644 --- a/cmd/stop.go +++ b/cmd/stop.go @@ -38,7 +38,7 @@ var stopCmd = &cobra.Command{ one or more container names or categories. If empty, "all" is assumed.`, Run: func(_ *cobra.Command, args []string) { conts := config.Union(args, contMask) - getAndExecute("STOP", conts, func(c *container.Container) command.Commands { return c.StopCommands() }) + getAndExecute(conts, func(c *container.Container) command.CommandSet { return c.StopCommands() }) }, } diff --git a/cmd/update.go b/cmd/update.go index 3bccbef..fdd047e 100644 --- a/cmd/update.go +++ b/cmd/update.go @@ -38,7 +38,7 @@ var updateCmd = &cobra.Command{ one or more container names or categories. If empty, "all" is assumed.`, Run: func(_ *cobra.Command, args []string) { conts := config.Union(args, contMask) - getAndExecute("UPDATE", conts, func(c *container.Container) command.Commands { return c.UpdateCommands() }) + getAndExecute(conts, func(c *container.Container) command.CommandSet { return c.UpdateCommands() }) }, } diff --git a/internal/pkg/command/command.go b/internal/pkg/command/command.go index 3b90f5d..b975b5c 100644 --- a/internal/pkg/command/command.go +++ b/internal/pkg/command/command.go @@ -25,15 +25,13 @@ import ( "fmt" "io" "os/exec" - - log "github.com/sirupsen/logrus" ) type CommandType int const ( CT_NOP CommandType = iota - CT_SH + CT_SHELL CT_FUNC CT_INDIRECT CT_SET @@ -45,7 +43,7 @@ func (ct CommandType) String() string { switch ct { case CT_NOP: return "NOP" - case CT_SH: + case CT_SHELL: return "SHELL" case CT_FUNC: return "FUNC" @@ -68,6 +66,11 @@ type Command struct { } type Commands []Command +type CommandSet struct{ + ID string + Commands +} + type errFunc struct { Name string Func func() error @@ -81,7 +84,7 @@ type conditional struct { } func NewShell(cmd string) Command { - return Command{CT_SH, cmd} + return Command{CT_SHELL, cmd} } func NewFunc(name string, f func() error) Command { @@ -92,8 +95,8 @@ func NewIndirect(c Command) Command { return Command{CT_INDIRECT, c} } -func NewSet(cs []Command) Command { - return Command{CT_SET, cs} +func NewSet(cs CommandSet) Command { + return Command{CT_SET, cs.Commands} } func NewDebug(msg string) Command { @@ -113,33 +116,22 @@ func NewConditional(name string, ifPart func() bool, thenPart Command, elsePart }} } -func (c Command) GetShell() (string, error) { - s, ok := c.Command.(string) - if ok { - return s, nil - } - return s, fmt.Errorf("Type error. Requested = %s, Type = %s", CT_SH, c.Type) -} - -func (cmds Commands) Execute(output io.Writer, fake bool) error { - for _, c := range cmds { - if err := c.Execute(output, fake); err != nil { +func (cmds CommandSet) Execute(output io.Writer, fake bool) error { + for _, c := range cmds.Commands { + if err := c.Execute(output, fake, cmds.ID); err != nil { return err } } return nil } -func (c Command) Execute(output io.Writer, fake bool) error { +func (c Command) Execute(output io.Writer, fake bool, commandSetID string) error { switch c.Type { case CT_NOP: - fmt.Fprintln(output, c.Type) - case CT_SH: - cmd, err := c.GetShell() - if err != nil { - return err - } - fmt.Fprintf(output, "%s sh -c \"%s\"\n", c.Type, cmd) + fmt.Fprintf(output, "%s: %s\n", commandSetID, c.Type) + case CT_SHELL: + cmd := c.Command.(string) + fmt.Fprintf(output, "%s: %s sh -c \"%s\"\n", commandSetID, c.Type, cmd) if !fake { out, err := exec.Command("sh", "-c", cmd).CombinedOutput() fmt.Fprint(output, string(out)) @@ -147,11 +139,7 @@ func (c Command) Execute(output io.Writer, fake bool) error { } case CT_FUNC: ef := c.Command.(errFunc) - log.WithFields(log.Fields{ - "type": CT_FUNC.String(), - "func": ef.Name, - }).Debugf("Calling") - fmt.Fprintln(output, c.Type, ef.Name) + fmt.Fprintf(output, "%s: %s %s\n", commandSetID, c.Type, ef.Name) if !fake { if err := ef.Func(); err != nil { return err @@ -159,50 +147,39 @@ func (c Command) Execute(output io.Writer, fake bool) error { return nil } case CT_INDIRECT: - ct, ok := c.Command.(Command) - if !ok { - return fmt.Errorf("Type error: Requested = %s, Type = %s", CT_INDIRECT, c.Type) - } - return ct.Execute(output, fake) + ct := c.Command.(Command) + return ct.Execute(output, fake, commandSetID) case CT_SET: - cs, ok := c.Command.([]Command) - if !ok { - return fmt.Errorf("Type error: Requested = %s, Type = %s", CT_SET, c.Type) - } + cs := c.Command.(Commands) for i := range cs { - err := cs[i].Execute(output, fake) + err := cs[i].Execute(output, fake, commandSetID) if err != nil { return err } } case CT_CONDITIONAL: - cond, ok := c.Command.(conditional) - if !ok { - return fmt.Errorf("Type error: Requested = %s, Type = %s", CT_CONDITIONAL, c.Type) - } + cond := c.Command.(conditional) if fake { - fmt.Fprintln(output, c.Type, cond.Name) - fmt.Fprintln(output, "-- True branch") - cond.ThenCmd.Execute(output, fake) - fmt.Fprintln(output, "-- False branch") - cond.ElseCmd.Execute(output, fake) - fmt.Fprintln(output, "-- End conditional") - return nil + // in a fake setting, we don't know which branch will be followed, + // so show what we would do in either branch + fmt.Fprintf(output, "%s: %s %s\n", commandSetID, c.Type, cond.Name) + fmt.Fprintf(output, "%s: -- True branch\n", commandSetID) + cond.ThenCmd.Execute(output, fake, commandSetID) + fmt.Fprintf(output, "%s: -- False branch\n", commandSetID) + cond.ElseCmd.Execute(output, fake, commandSetID) + fmt.Fprintf(output, "%s: -- End conditional\n", commandSetID) } else { branch := cond.Condition() - fmt.Fprintln(output, c.Type, cond.Name, ":", branch) + fmt.Fprintf(output, "%s: %s %s: %v\n", commandSetID, c.Type, cond.Name, branch) if branch { - return cond.ThenCmd.Execute(output, fake) + return cond.ThenCmd.Execute(output, fake, commandSetID) } else { - return cond.ElseCmd.Execute(output, fake) + return cond.ElseCmd.Execute(output, fake, commandSetID) } } case CT_DEBUG: - msg, ok := c.Command.(string) - if !ok { - return fmt.Errorf("Type error: Requested = %s, Type = %s", CT_DEBUG, c.Type) - } - fmt.Fprintln(output, c.Type, msg) + msg := c.Command.(string) + fmt.Fprintf(output, "%s: %s %s\n", commandSetID, c.Type, msg) } return nil } diff --git a/internal/pkg/container/container.go b/internal/pkg/container/container.go index 4aefb68..4360d4b 100644 --- a/internal/pkg/container/container.go +++ b/internal/pkg/container/container.go @@ -27,7 +27,7 @@ import ( "net" "os/exec" - "gitea.elkins.co/Networking/ccl/internal/pkg/command" + cmd "gitea.elkins.co/Networking/ccl/internal/pkg/command" "gitea.elkins.co/Networking/ccl/internal/pkg/network" "github.com/containers/common/libnetwork/types" "github.com/containers/podman/v4/libpod/define" @@ -44,7 +44,7 @@ type Container struct { Name string `toml:"name"` Image string `toml:"image"` Hostname string `toml:"hostname,omitempty"` - Command []string `toml:"command,omitempty"` + Command []string `toml:"cmd,omitempty"` Arguments string `toml:"arguments,omitempty"` Networks []network.Network `toml:"networks,omitempty"` Env map[string]string `toml:"env,omitempty"` @@ -114,21 +114,28 @@ func (c *Container) pull() error { return err } -func (c *Container) PullCommands() command.Commands { - return command.Commands{ - command.NewFunc("do_pull", func() error { - return c.pull() - }), +func (c *Container) newCommandSet(op string, cmds cmd.Commands) cmd.CommandSet { + return cmd.CommandSet { + ID: fmt.Sprintf("%s-%s", op, c.Name), + Commands: cmds, } } -func (c *Container) CreateCommands() command.Commands { +func (c *Container) PullCommands() cmd.CommandSet { + return c.newCommandSet("PULL", cmd.Commands{ + cmd.NewFunc("do_pull", func() error { + return c.pull() + }), + }) +} + +func (c *Container) CreateCommands() cmd.CommandSet { if c.Image == "" { - return command.Commands{ - command.NewFunc("image_error", func() error { + return c.newCommandSet("CREATE", cmd.Commands{ + cmd.NewFunc("image_error", func() error { return fmt.Errorf("Image not configured") }), - } + }) } sysctl := map[string]string{} nets := map[string]types.PerNetworkOptions{} @@ -176,8 +183,8 @@ func (c *Container) CreateCommands() command.Commands { }, } - return command.Commands{ - command.NewFunc("bail_if_exists", func() error { + return c.newCommandSet("CREATE", cmd.Commands{ + cmd.NewFunc("bail_if_exists", func() error { if ex, err := containers.Exists(c.conn, c.Name, &containers.ExistsOptions{}); err != nil || ex { if err != nil { return err @@ -186,7 +193,7 @@ func (c *Container) CreateCommands() command.Commands { } return nil }), - command.NewFunc("pull_if_necessary", func() error { + cmd.NewFunc("pull_if_necessary", func() error { if ex, err := images.Exists(c.conn, c.Image, &images.ExistsOptions{}); err != nil || !ex { if err != nil { return err @@ -195,51 +202,61 @@ func (c *Container) CreateCommands() command.Commands { } return nil }), - command.NewFunc("validate_spec", spec.Validate), - command.NewFunc("do_create", func() error { + cmd.NewFunc("validate_spec", spec.Validate), + cmd.NewFunc("do_create", func() error { _, err := containers.CreateWithSpec(c.conn, &spec, nil) if err != nil { return err } return c.populateCData() }), - } + }) } -func (c *Container) RecreateCommands() command.Commands { +func (c *Container) RecreateCommands() cmd.CommandSet { wasRunning := false - return command.Commands{ - command.NewFunc("stash_run_state", func() error { + return c.newCommandSet("RECREATE", cmd.Commands{ + cmd.NewFunc("stash_run_state", func() error { wasRunning = c.IsRunning() return nil }), - command.NewSet(c.DestroyCommands()), - command.NewSet(c.CreateCommands()), - command.NewConditional("start_if_was_running", + cmd.NewSet(c.StopCommands()), + cmd.NewSet(c.removeCommands()), + cmd.NewSet(c.CreateCommands()), + cmd.NewConditional("start_if_was_running", func() bool { return wasRunning }, - command.NewSet(c.StartCommands()), - command.NewNop(), + cmd.NewSet(c.StartCommands()), + cmd.NewNop(), ), - } + }) } -func (c *Container) DestroyCommands() command.Commands { - cmds := c.StopCommands() - cmds = append(cmds, command.NewFunc("remove_if_exists", func() error { - if c.cdata.ID == "" { - return nil - } - yes := true - _, err := containers.Remove(c.conn, c.cdata.ID, &containers.RemoveOptions{Force: &yes}) - return err - })) - return cmds +func (c *Container) RemoveCommands() cmd.CommandSet { + return c.newCommandSet("REMOVE", cmd.Commands{ + cmd.NewSet(c.StopCommands()), + cmd.NewSet(c.removeCommands()), + }) } -func (c *Container) StartCommands() command.Commands { - return command.Commands{ - command.NewFunc("start_container", func() error { - if c.cdata.State != nil && c.cdata.State.Running { +// unexported version just removes the container without attempting a stop. +func (c *Container) removeCommands() cmd.CommandSet { + return c.newCommandSet("remove", cmd.Commands{ + cmd.NewFunc("remove_if_exists", func() error { + if c.cdata.ID == "" { + return nil + } + yes := true + _, err := containers.Remove(c.conn, c.cdata.ID, &containers.RemoveOptions{Force: &yes}) + return err + }), + }) +} + +func (c *Container) StartCommands() cmd.CommandSet { + return c.newCommandSet("START", cmd.Commands{ + cmd.NewFunc("start_container", func() error { + if c.IsRunning() { + c.LogEntry().Debugln("Container start was commanded but it is already running. Not a problem.") return nil } err := containers.Start(c.conn, c.cdata.ID, nil) @@ -256,18 +273,18 @@ func (c *Container) StartCommands() command.Commands { } err = c.assureNetNS() if err != nil { - return err + c.LogEntry().WithField("error", err).Warnln("Failed to create network namespace") } return nil }), - } + }) } -func (c *Container) RestartCommands() command.Commands { - return command.Commands{ - command.NewSet(c.StopCommands()), - command.NewSet(c.StartCommands()), - } +func (c *Container) RestartCommands() cmd.CommandSet { + return c.newCommandSet("RESTART", cmd.Commands{ + cmd.NewSet(c.StopCommands()), + cmd.NewSet(c.StartCommands()), + }) } func (c *Container) IsRunning() bool { @@ -284,54 +301,51 @@ func (c *Container) IsCreated() bool { return true } -func (c *Container) UpdateCommands() command.Commands { +func (c *Container) UpdateCommands() cmd.CommandSet { wasRunning := false - return command.Commands{ - command.NewFunc("pull_and_stop", func() error { + return c.newCommandSet("UPDATE", cmd.Commands{ + cmd.NewFunc("pull_image", func() error { err := c.pull() if err != nil { return err } wasRunning = c.cdata != nil && c.cdata.State != nil && c.cdata.State.Running - if wasRunning { - var timeout uint = 10 - _ = containers.Stop(c.conn, c.cdata.ID, &containers.StopOptions{Timeout: &timeout}) - } return nil }), - command.NewSet(c.DestroyCommands()), - command.NewSet(c.CreateCommands()), - command.NewConditional("restart_if_was_running", + cmd.NewSet(c.StopCommands()), + cmd.NewSet(c.removeCommands()), + cmd.NewSet(c.CreateCommands()), + cmd.NewConditional("restart_if_was_running", func() bool { return wasRunning }, - command.NewSet(c.StartCommands()), - command.NewNop(), + cmd.NewSet(c.StartCommands()), + cmd.NewNop(), ), - } + }) } -func (c *Container) StopCommands() command.Commands { - return command.Commands{ - command.NewFunc("stop_if_running", func() error { - if c.IsRunning() { - var timeout uint = 10 - err := containers.Stop(c.conn, c.cdata.ID, &containers.StopOptions{Timeout: &timeout}) - if err != nil { - return err - } - _, err = containers.Wait(c.conn, c.cdata.ID, &containers.WaitOptions{Condition: []define.ContainerStatus{define.ContainerStateExited}}) - if err != nil { - return err - } - return c.populateCData() +func (c *Container) StopCommands() cmd.CommandSet { + return c.newCommandSet("STOP", cmd.Commands{ + cmd.NewFunc("stop_if_running", func() error { + if !c.IsRunning() { + c.LogEntry().Debugln("Container stop was commanded but it wasn't running. Not a problem.") + return nil } - c.LogEntry().Debugf("Container stopped but wasn't running. Not a problem.") - return nil + var timeout uint = 10 + err := containers.Stop(c.conn, c.cdata.ID, &containers.StopOptions{Timeout: &timeout}) + if err != nil { + return err + } + _, err = containers.Wait(c.conn, c.cdata.ID, &containers.WaitOptions{Condition: []define.ContainerStatus{define.ContainerStateExited}}) + if err != nil { + return err + } + return c.populateCData() }), - } + }) } func (c *Container) populateCData() error { - // TODO: locking + // TODO: locking? var err error no := false c.cdata, err = containers.Inspect(c.conn, c.Name, &containers.InspectOptions{Size: &no})