From 81cfdd50305e6e926e606a0f5b43f7628f26b3b4 Mon Sep 17 00:00:00 2001 From: Brian Picciano Date: Tue, 10 Dec 2024 22:00:55 +0100 Subject: [PATCH] Refactor how daemon.RPC is passed through sub-commands --- go/cmd/entrypoint/garage.go | 4 +-- go/cmd/entrypoint/host.go | 6 ++--- go/cmd/entrypoint/main.go | 19 ++++++++++++-- go/cmd/entrypoint/main_test.go | 3 +-- go/cmd/entrypoint/network.go | 10 ++++---- go/cmd/entrypoint/storage.go | 10 ++++---- go/cmd/entrypoint/sub_cmd.go | 41 +++++++++---------------------- go/cmd/entrypoint/vpn.go | 2 +- go/cmd/entrypoint/vpn_firewall.go | 8 +++--- 9 files changed, 50 insertions(+), 53 deletions(-) diff --git a/go/cmd/entrypoint/garage.go b/go/cmd/entrypoint/garage.go index 68c317e..efc4eec 100644 --- a/go/cmd/entrypoint/garage.go +++ b/go/cmd/entrypoint/garage.go @@ -51,7 +51,7 @@ var subCmdGarageMC = subCmd{ return fmt.Errorf("parsing flags: %w", err) } - clientParams, err := ctx.getDaemonRPC().GetGarageClientParams(ctx) + clientParams, err := ctx.daemonRPC.GetGarageClientParams(ctx) if err != nil { return fmt.Errorf("calling GetGarageClientParams: %w", err) } @@ -122,7 +122,7 @@ var subCmdGarageCLI = subCmd{ return fmt.Errorf("parsing flags: %w", err) } - clientParams, err := ctx.getDaemonRPC().GetGarageClientParams(ctx) + clientParams, err := ctx.daemonRPC.GetGarageClientParams(ctx) if err != nil { return fmt.Errorf("calling GetGarageClientParams: %w", err) } diff --git a/go/cmd/entrypoint/host.go b/go/cmd/entrypoint/host.go index 6e97e1f..4f485f8 100644 --- a/go/cmd/entrypoint/host.go +++ b/go/cmd/entrypoint/host.go @@ -40,7 +40,7 @@ var subCmdHostCreate = subCmd{ return errors.New("--hostname is required") } - res, err := ctx.getDaemonRPC().CreateHost( + res, err := ctx.daemonRPC.CreateHost( ctx, hostName.V, network.CreateHostOpts{ IP: ip.V, CanCreateHosts: *canCreateHosts, @@ -63,7 +63,7 @@ var subCmdHostList = subCmd{ return nil, fmt.Errorf("parsing flags: %w", err) } - currBoostrap, err := ctx.getDaemonRPC().GetBootstrap(ctx) + currBoostrap, err := ctx.daemonRPC.GetBootstrap(ctx) if err != nil { return nil, fmt.Errorf("calling GetBootstrap: %w", err) } @@ -131,7 +131,7 @@ var subCmdHostRemove = subCmd{ return errors.New("--hostname is required") } - if err := ctx.getDaemonRPC().RemoveHost(ctx, hostName.V); err != nil { + if err := ctx.daemonRPC.RemoveHost(ctx, hostName.V); err != nil { return fmt.Errorf("calling RemoveHost: %w", err) } diff --git a/go/cmd/entrypoint/main.go b/go/cmd/entrypoint/main.go index dfd1412..9886fab 100644 --- a/go/cmd/entrypoint/main.go +++ b/go/cmd/entrypoint/main.go @@ -3,6 +3,8 @@ package main import ( "context" "fmt" + "isle/daemon" + "isle/daemon/jsonrpc2" "isle/toolkit" "os" "os/signal" @@ -65,9 +67,10 @@ var rootCmd = subCmd{ func doRootCmd( ctx context.Context, logger *mlog.Logger, + daemonRPC daemon.RPC, opts *subCmdCtxOpts, ) error { - subCmdCtx := newSubCmdCtx(ctx, logger, rootCmd, opts) + subCmdCtx := newSubCmdCtx(ctx, logger, daemonRPC, rootCmd, opts) return subCmdCtx.subCmd.do(subCmdCtx) } @@ -93,7 +96,19 @@ func main() { logger.FatalString(ctx, "second signal received, force quitting, there may be zombie children left behind, good luck!") }() - if err := doRootCmd(ctx, logger, nil); err != nil { + httpClient, baseURL := toolkit.NewUnixHTTPClient( + logger.WithNamespace("http-client"), + daemon.HTTPSocketPath(), + ) + defer httpClient.Close() + + baseURL.Path = daemonHTTPRPCPath + + daemonRPC := daemon.RPCFromClient( + jsonrpc2.NewHTTPClient(httpClient, baseURL.String()), + ) + + if err := doRootCmd(ctx, logger, daemonRPC, nil); err != nil { fmt.Fprintln(os.Stderr, err) } } diff --git a/go/cmd/entrypoint/main_test.go b/go/cmd/entrypoint/main_test.go index 84a663c..4adb7d0 100644 --- a/go/cmd/entrypoint/main_test.go +++ b/go/cmd/entrypoint/main_test.go @@ -58,9 +58,8 @@ func (h *runHarness) run(t *testing.T, args ...string) error { jsonrpc2.NewHTTPClient(httpClient, h.daemonRPCServer.URL), ) - return doRootCmd(h.ctx, h.logger, &subCmdCtxOpts{ + return doRootCmd(h.ctx, h.logger, daemonRPCClient, &subCmdCtxOpts{ args: args, - daemonRPC: daemonRPCClient, stdout: h.stdout, changeStager: h.changeStager, }) diff --git a/go/cmd/entrypoint/network.go b/go/cmd/entrypoint/network.go index 742c705..26e7ffd 100644 --- a/go/cmd/entrypoint/network.go +++ b/go/cmd/entrypoint/network.go @@ -57,7 +57,7 @@ var subCmdNetworkCreate = subCmd{ return errors.New("--name, --domain, --ip-net, and --hostname are required") } - err = ctx.getDaemonRPC().CreateNetwork( + err = ctx.daemonRPC.CreateNetwork( ctx, *name, *domain, ipNet.V, hostName.V, ) if err != nil { @@ -93,7 +93,7 @@ var subCmdNetworkJoin = subCmd{ ) } - return ctx.getDaemonRPC().JoinNetwork(ctx, newBootstrap) + return ctx.daemonRPC.JoinNetwork(ctx, newBootstrap) }, } @@ -107,7 +107,7 @@ var subCmdNetworkList = subCmd{ return nil, fmt.Errorf("parsing flags: %w", err) } - networkCreationParams, err := ctx.getDaemonRPC().GetNetworks(ctx) + networkCreationParams, err := ctx.daemonRPC.GetNetworks(ctx) if err != nil { return nil, fmt.Errorf("calling GetNetworks: %w", err) } @@ -125,7 +125,7 @@ var subCmdNetworkList = subCmd{ } var ( - daemonRPC = ctx.getDaemonRPC() + daemonRPC = ctx.daemonRPC networkViews = make([]networkView, len(networkCreationParams)) ) @@ -188,7 +188,7 @@ var subCmdNetworkGetConfig = subCmd{ return nil, fmt.Errorf("parsing flags: %w", err) } - return ctx.getDaemonRPC().GetConfig(ctx) + return ctx.daemonRPC.GetConfig(ctx) }), } diff --git a/go/cmd/entrypoint/storage.go b/go/cmd/entrypoint/storage.go index ac7a787..7ed73ec 100644 --- a/go/cmd/entrypoint/storage.go +++ b/go/cmd/entrypoint/storage.go @@ -98,14 +98,14 @@ var subCmdStorageAllocationAdd = subCmd{ ) } - config, err := ctx.getDaemonRPC().GetConfig(ctx) + config, err := ctx.daemonRPC.GetConfig(ctx) if err != nil { return fmt.Errorf("getting network config: %w", err) } config.Storage.Allocations = append(config.Storage.Allocations, alloc) - if err := ctx.getDaemonRPC().SetConfig(ctx, config); err != nil { + if err := ctx.daemonRPC.SetConfig(ctx, config); err != nil { return fmt.Errorf("updating the network config: %w", err) } @@ -123,7 +123,7 @@ var subCmdStorageAllocationList = subCmd{ return nil, fmt.Errorf("parsing flags: %w", err) } - config, err := ctx.getDaemonRPC().GetConfig(ctx) + config, err := ctx.daemonRPC.GetConfig(ctx) if err != nil { return nil, fmt.Errorf("getting network config: %w", err) } @@ -153,7 +153,7 @@ var subCmdStorageAllocationRemove = subCmd{ return errors.New("At least one --index must be specified") } - config, err := ctx.getDaemonRPC().GetConfig(ctx) + config, err := ctx.daemonRPC.GetConfig(ctx) if err != nil { return fmt.Errorf("getting network config: %w", err) } @@ -187,7 +187,7 @@ var subCmdStorageAllocationRemove = subCmd{ ) config.Storage.Allocations = newAllocs - if err := ctx.getDaemonRPC().SetConfig(ctx, config); err != nil { + if err := ctx.daemonRPC.SetConfig(ctx, config); err != nil { return fmt.Errorf("updating the network config: %w", err) } diff --git a/go/cmd/entrypoint/sub_cmd.go b/go/cmd/entrypoint/sub_cmd.go index 56845ba..bd412d4 100644 --- a/go/cmd/entrypoint/sub_cmd.go +++ b/go/cmd/entrypoint/sub_cmd.go @@ -6,9 +6,7 @@ import ( "fmt" "io" "isle/daemon" - "isle/daemon/jsonrpc2" "isle/jsonutil" - "isle/toolkit" "os" "strings" @@ -47,7 +45,6 @@ func (c subCmd) fullName() string { type subCmdCtxOpts struct { args []string // command-line arguments, excluding the subCmd itself. subCmdNames []string // names of subCmds so far, including this one - daemonRPC daemon.RPC stdout io.Writer changeStager *changeStager } @@ -75,9 +72,10 @@ func (o *subCmdCtxOpts) withDefaults() *subCmdCtxOpts { // subCmdCtx contains all information available to a subCmd's do method. type subCmdCtx struct { context.Context - logger *mlog.Logger - subCmd subCmd // the subCmd itself - opts *subCmdCtxOpts + logger *mlog.Logger + daemonRPC daemon.RPC + subCmd subCmd // the subCmd itself + opts *subCmdCtxOpts flags *pflag.FlagSet } @@ -85,17 +83,19 @@ type subCmdCtx struct { func newSubCmdCtx( ctx context.Context, logger *mlog.Logger, + daemonRPC daemon.RPC, subCmd subCmd, opts *subCmdCtxOpts, ) subCmdCtx { opts = opts.withDefaults() return subCmdCtx{ - Context: ctx, - logger: logger, - subCmd: subCmd, - opts: opts, - flags: pflag.NewFlagSet(subCmd.name, pflag.ExitOnError), + Context: ctx, + logger: logger, + daemonRPC: daemonRPC, + subCmd: subCmd, + opts: opts, + flags: pflag.NewFlagSet(subCmd.name, pflag.ExitOnError), } } @@ -108,23 +108,6 @@ func usagePrefix(subCmdNames []string) string { return fmt.Sprintf("USAGE:\n %s %s", os.Args[0], subCmdNamesStr) } -func (ctx subCmdCtx) getDaemonRPC() daemon.RPC { - if ctx.opts.daemonRPC == nil { - // TODO Close is not being called on the HTTPClient - httpClient, baseURL := toolkit.NewUnixHTTPClient( - ctx.logger.WithNamespace("http-client"), - daemon.HTTPSocketPath(), - ) - - baseURL.Path = daemonHTTPRPCPath - - ctx.opts.daemonRPC = daemon.RPCFromClient( - jsonrpc2.NewHTTPClient(httpClient, baseURL.String()), - ) - } - return ctx.opts.daemonRPC -} - func (ctx subCmdCtx) withParsedFlags() (subCmdCtx, error) { logLevel := logLevelFlag{mlog.LevelInfo} ctx.flags.VarP( @@ -257,7 +240,7 @@ func (ctx subCmdCtx) doSubCmd(subCmds ...subCmd) error { nextSubCmdCtxOpts.subCmdNames = append(ctx.opts.subCmdNames, subCmdName) nextSubCmdCtx := newSubCmdCtx( - ctx.Context, ctx.logger, subCmd, &nextSubCmdCtxOpts, + ctx.Context, ctx.logger, ctx.daemonRPC, subCmd, &nextSubCmdCtxOpts, ) if err := subCmd.do(nextSubCmdCtx); err != nil { diff --git a/go/cmd/entrypoint/vpn.go b/go/cmd/entrypoint/vpn.go index be24b05..49abe69 100644 --- a/go/cmd/entrypoint/vpn.go +++ b/go/cmd/entrypoint/vpn.go @@ -43,7 +43,7 @@ var subCmdVPNCreateCert = subCmd{ return fmt.Errorf("unmarshaling public key as PEM: %w", err) } - res, err := ctx.getDaemonRPC().CreateNebulaCertificate( + res, err := ctx.daemonRPC.CreateNebulaCertificate( ctx, hostName.V, hostPub, ) if err != nil { diff --git a/go/cmd/entrypoint/vpn_firewall.go b/go/cmd/entrypoint/vpn_firewall.go index 94589e9..a7c4f07 100644 --- a/go/cmd/entrypoint/vpn_firewall.go +++ b/go/cmd/entrypoint/vpn_firewall.go @@ -15,7 +15,7 @@ const vpnFirewallConfigChangeStagerName = "vpn-firewall-config" // vpnFirewallGetConfigWithStaged returns the network config along with any // staged firewall configuration changes, if there are any. func vpnFirewallGetConfig(ctx subCmdCtx) (daecommon.NetworkConfig, error) { - config, err := ctx.getDaemonRPC().GetConfig(ctx) + config, err := ctx.daemonRPC.GetConfig(ctx) if err != nil { return daecommon.NetworkConfig{}, err } @@ -151,14 +151,14 @@ var subCmdVPNFirewallCommit = subCmd{ return errors.New("no changes staged, use 'add' or 'remove' to stage changes") } - config, err := ctx.getDaemonRPC().GetConfig(ctx) + config, err := ctx.daemonRPC.GetConfig(ctx) if err != nil { return fmt.Errorf("getting network config: %w", err) } config.VPN.Firewall = firewallConfig - return ctx.getDaemonRPC().SetConfig(ctx, config) + return ctx.daemonRPC.SetConfig(ctx, config) }, } @@ -309,7 +309,7 @@ var subCmdVPNFirewallShow = subCmd{ } if !foundStaged { - config, err := ctx.getDaemonRPC().GetConfig(ctx) + config, err := ctx.daemonRPC.GetConfig(ctx) if err != nil { return nil, fmt.Errorf("getting network config: %w", err) }