Skip to content

Commit

Permalink
Add support for service action reload (#443)
Browse files Browse the repository at this point in the history
* Add support for service action reload

Adds support for reloading a systemd/unknown service via an
`ACTION_RELOAD` service action request.

* Update ReloadRemoteService to support multiple targets

Updates the `ReloadRemoteService` helper function to support multiple
targets in its invocation.

* Remove inaccurate comment on ReloadRemoteService

This function now _does_ support multiple targets.
  • Loading branch information
sfc-gh-daspencer authored Jun 3, 2024
1 parent 92e023d commit 7370745
Show file tree
Hide file tree
Showing 6 changed files with 86 additions and 24 deletions.
1 change: 1 addition & 0 deletions services/service/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ func (*serviceCmd) GetSubpackage(f *flag.FlagSet) *subcommands.Commander {
c.Register(&actionCmd{action: pb.Action_ACTION_STOP}, "")
c.Register(&actionCmd{action: pb.Action_ACTION_ENABLE}, "")
c.Register(&actionCmd{action: pb.Action_ACTION_DISABLE}, "")
c.Register(&actionCmd{action: pb.Action_ACTION_RELOAD}, "")
return c
}

Expand Down
14 changes: 14 additions & 0 deletions services/service/client/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,3 +155,17 @@ func DisableRemoteService(ctx context.Context, conn *proxy.Conn, system pb.Syste
}
return nil
}

// ReloadRemoteService is a helper function for reloading a service on a remote target
// using a proxy.Conn.
func ReloadRemoteService(ctx context.Context, conn *proxy.Conn, system pb.SystemType, service string) error {
c := pb.NewServiceClientProxy(conn)
if _, err := c.ActionOneMany(ctx, &pb.ActionRequest{
ServiceName: service,
SystemType: system,
Action: pb.Action_ACTION_RELOAD,
}); err != nil {
return fmt.Errorf("can't reload service %s - %v", service, err)
}
return nil
}
5 changes: 4 additions & 1 deletion services/service/server/server_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ type systemdConnection interface {
DisableUnitFilesContext(ctx context.Context, files []string, runtime bool) ([]dbus.DisableUnitFileChange, error)
EnableUnitFilesContext(ctx context.Context, files []string, runtime bool, force bool) (bool, []dbus.EnableUnitFileChange, error)
ReloadContext(ctx context.Context) error
ReloadUnitContext(ctx context.Context, name string, mode string, ch chan<- string) (int, error)
Close()
}

Expand Down Expand Up @@ -332,6 +333,8 @@ func (s *server) Action(ctx context.Context, req *pb.ActionRequest) (*pb.ActionR
_, err = conn.RestartUnitContext(ctx, unitName, modeReplace, resultChan)
case pb.Action_ACTION_STOP:
_, err = conn.StopUnitContext(ctx, unitName, modeReplace, resultChan)
case pb.Action_ACTION_RELOAD:
_, err = conn.ReloadUnitContext(ctx, unitName, modeReplace, resultChan)
case pb.Action_ACTION_ENABLE:
_, _, err = conn.EnableUnitFilesContext(ctx, []string{unitName}, false, true)
case pb.Action_ACTION_DISABLE:
Expand All @@ -351,7 +354,7 @@ func (s *server) Action(ctx context.Context, req *pb.ActionRequest) (*pb.ActionR
// Enable/disable don't use this method so we skip the channel (since it would hang)
// and instead force a reload which is what systemctl does when it enables/disables.
switch req.Action {
case pb.Action_ACTION_START, pb.Action_ACTION_RESTART, pb.Action_ACTION_STOP:
case pb.Action_ACTION_START, pb.Action_ACTION_RESTART, pb.Action_ACTION_STOP, pb.Action_ACTION_RELOAD:
result := <-resultChan
if result != operationResultDone {
recorder.CounterOrLog(ctx, serviceActionFailureCounter, 1, attribute.String("reason", "action_err"))
Expand Down
38 changes: 38 additions & 0 deletions services/service/server/server_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,9 @@ func (e errConn) EnableUnitFilesContext(context.Context, []string, bool, bool) (
func (e errConn) ReloadContext(ctx context.Context) error {
return errors.New(string(e))
}
func (e errConn) ReloadUnitContext(context.Context, string, string, chan<- string) (int, error) {
return 0, errors.New(string(e))
}
func (errConn) Close() {}

func TestDialError(t *testing.T) {
Expand Down Expand Up @@ -124,6 +127,9 @@ func (l listConn) EnableUnitFilesContext(context.Context, []string, bool, bool)
func (l listConn) ReloadContext(ctx context.Context) error {
return notImplementedError
}
func (l listConn) ReloadUnitContext(context.Context, string, string, chan<- string) (int, error) {
return 0, notImplementedError
}
func (listConn) Close() {}

func wantStatusErr(code codes.Code, message string) func(string, error, *testing.T) {
Expand Down Expand Up @@ -253,6 +259,9 @@ func (g getConn) EnableUnitFilesContext(context.Context, []string, bool, bool) (
func (g getConn) ReloadContext(ctx context.Context) error {
return notImplementedError
}
func (g getConn) ReloadUnitContext(context.Context, string, string, chan<- string) (int, error) {
return 0, notImplementedError
}
func (getConn) Close() {}

func TestStatus(t *testing.T) {
Expand Down Expand Up @@ -518,6 +527,12 @@ func (a actionConn) EnableUnitFilesContext(context.Context, []string, bool, bool
func (a actionConn) ReloadContext(ctx context.Context) error {
return a.reloadErr
}
func (a actionConn) ReloadUnitContext(ctx context.Context, name string, mode string, c chan<- string) (int, error) {
go func() {
c <- string(a.action)
}()
return 1, a.err
}
func (actionConn) Close() {}

func TestAction(t *testing.T) {
Expand Down Expand Up @@ -635,6 +650,16 @@ func TestAction(t *testing.T) {
want: nil,
errFunc: wantStatusErr(codes.Internal, "error reloading"),
},
{
name: "reload failed",
conn: actionConn{"failed", nil, nil},
req: &pb.ActionRequest{
ServiceName: "foo",
Action: pb.Action_ACTION_RELOAD,
},
want: nil,
errFunc: wantStatusErr(codes.Internal, "error performing action"),
},
{
name: "start success",
conn: actionConn{operationResultDone, nil, nil},
Expand Down Expand Up @@ -700,6 +725,19 @@ func TestAction(t *testing.T) {
},
errFunc: testutil.FatalOnErr,
},
{
name: "reload success",
conn: actionConn{operationResultDone, nil, nil},
req: &pb.ActionRequest{
ServiceName: "foo",
Action: pb.Action_ACTION_RELOAD,
},
want: &pb.ActionReply{
SystemType: pb.SystemType_SYSTEM_TYPE_SYSTEMD,
ServiceName: "foo",
},
errFunc: testutil.FatalOnErr,
},
} {
tc := tc
t.Run(tc.name, func(t *testing.T) {
Expand Down
51 changes: 28 additions & 23 deletions services/service/service.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions services/service/service.proto
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ enum Action {
ACTION_RESTART = 3;
ACTION_ENABLE = 4;
ACTION_DISABLE = 5;
ACTION_RELOAD = 6;
}

// ServiceStatus pairs a service with it's current status.
Expand Down

0 comments on commit 7370745

Please sign in to comment.