From 3ca97c474ca55078ff3cc9c0ed85232f28bead30 Mon Sep 17 00:00:00 2001 From: Reece Williams <31943163+Reecepbcups@users.noreply.github.com> Date: Mon, 16 Sep 2024 11:20:15 -0500 Subject: [PATCH] feat: check for panic in logs on start (#1258) --- dockerutil/container_lifecycle.go | 54 ++++++++++++++++++ examples/cosmos/bad_genesis_params_test.go | 64 ++++++++++++++++++++++ local-interchain/interchain/start.go | 22 ++++---- 3 files changed, 129 insertions(+), 11 deletions(-) create mode 100644 examples/cosmos/bad_genesis_params_test.go diff --git a/dockerutil/container_lifecycle.go b/dockerutil/container_lifecycle.go index b89d06acc..56f8c0135 100644 --- a/dockerutil/container_lifecycle.go +++ b/dockerutil/container_lifecycle.go @@ -3,8 +3,11 @@ package dockerutil import ( "context" "fmt" + "io" "net" + "regexp" "strings" + "time" dockertypes "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/container" @@ -18,6 +21,9 @@ import ( "github.com/strangelove-ventures/interchaintest/v8/ibc" ) +// Example Go/Cosmos-SDK panic format is `panic: bad Duration: time: invalid duration "bad"\n` +var panicRe = regexp.MustCompile(`panic:.*\n`) + type ContainerLifecycle struct { log *zap.Logger client *dockerclient.Client @@ -124,7 +130,55 @@ func (c *ContainerLifecycle) StartContainer(ctx context.Context) error { return err } + if err := c.CheckForFailedStart(ctx, time.Second*1); err != nil { + return err + } + c.log.Info("Container started", zap.String("container", c.containerName)) + return nil +} + +// CheckForFailedStart checks the logs of the container for a +// panic message after a wait period to allow the container to start. +func (c *ContainerLifecycle) CheckForFailedStart(ctx context.Context, wait time.Duration) error { + time.Sleep(wait) + containerLogs, err := c.client.ContainerLogs(ctx, c.id, dockertypes.ContainerLogsOptions{ + ShowStdout: true, + ShowStderr: true, + }) + if err != nil { + return fmt.Errorf("failed to read logs from container %s: %w", c.containerName, err) + } + defer containerLogs.Close() + + logs := new(strings.Builder) + _, err = io.Copy(logs, containerLogs) + if err != nil { + return fmt.Errorf("failed to read logs from container %s: %w", c.containerName, err) + } + + if err := ParseSDKPanicFromText(logs.String()); err != nil { + // Must use Println and not the logger as there are ascii escape codes in the logs. + fmt.Printf("\nContainer name: %s.\nerror: %s.\nlogs\n%s\n", c.containerName, err.Error(), logs.String()) + return fmt.Errorf("container %s failed to start: %w", c.containerName, err) + } + + return nil +} + +// ParsePanicFromText returns a panic line if it exists in the logs so +// that it can be returned to the user in a proper error message instead of +// hanging. +func ParseSDKPanicFromText(text string) error { + if !strings.Contains(text, "panic: ") { + return nil + } + + match := panicRe.FindString(text) + if match != "" { + panicMessage := strings.TrimSpace(match) + return fmt.Errorf("%s", panicMessage) + } return nil } diff --git a/examples/cosmos/bad_genesis_params_test.go b/examples/cosmos/bad_genesis_params_test.go new file mode 100644 index 000000000..274fd50d6 --- /dev/null +++ b/examples/cosmos/bad_genesis_params_test.go @@ -0,0 +1,64 @@ +package cosmos_test + +import ( + "context" + "testing" + + "github.com/strangelove-ventures/interchaintest/v8" + "github.com/strangelove-ventures/interchaintest/v8/chain/cosmos" + "github.com/strangelove-ventures/interchaintest/v8/ibc" + "github.com/stretchr/testify/require" + "go.uber.org/zap/zaptest" +) + +var ( + badGenesis = []cosmos.GenesisKV{ + cosmos.NewGenesisKV("app_state.gov.params.voting_period", "bad"), + } +) + +func TestBadInputParams(t *testing.T) { + cf := interchaintest.NewBuiltinChainFactory(zaptest.NewLogger(t), []*interchaintest.ChainSpec{ + { + Name: "juno", + ChainName: "juno", + Version: "v19.0.0-alpha.3", + ChainConfig: ibc.ChainConfig{ + Denom: "ujuno", + Bech32Prefix: "juno", + CoinType: "118", + ModifyGenesis: cosmos.ModifyGenesis(badGenesis), + GasPrices: "0ujuno", + }, + NumValidators: &numValsOne, + NumFullNodes: &numFullNodesZero, + }, + }) + + chains, err := cf.Chains(t.Name()) + require.NoError(t, err) + + chain := chains[0].(*cosmos.CosmosChain) + + ic := interchaintest.NewInterchain(). + AddChain(chain) + + ctx := context.Background() + client, network := interchaintest.DockerSetup(t) + + err = ic.Build(ctx, nil, interchaintest.InterchainBuildOptions{ + TestName: t.Name(), + Client: client, + NetworkID: network, + SkipPathCreation: true, + }) + + // failed to start chains: failed to start chain juno: PANIC: container juno-1-val-0-TestBadInputParams failed to start: panic: bad Duration: time: invalid duration "bad" + require.Error(t, err) + require.ErrorContains(t, err, "bad Duration") + t.Log("err", err) + + t.Cleanup(func() { + _ = ic.Close() + }) +} diff --git a/local-interchain/interchain/start.go b/local-interchain/interchain/start.go index 607bc6f58..e5c060ee3 100644 --- a/local-interchain/interchain/start.go +++ b/local-interchain/interchain/start.go @@ -47,14 +47,9 @@ func StartChain(installDir, chainCfgFile string, ac *types.AppStartConfig) { select { case <-c: fmt.Println("\nReceived signal to stop local-ic...") - removed := dockerutil.KillAllInterchaintestContainers(ctx) - for _, r := range removed { - fmt.Println(" - ", r) - } - cancel() - os.Exit(1) + killContainer(ctx) case <-ctx.Done(): - fmt.Println("Context is done") + killContainer(ctx) } }() @@ -186,10 +181,7 @@ func StartChain(installDir, chainCfgFile string, ac *types.AppStartConfig) { SkipPathCreation: false, }) if err != nil { - // calls the KillAllInterchaintestContainers(...) above - <-ctx.Done() - - logger.Fatal("ic.Build", zap.Error(err)) + log.Fatalf("ic.Build: %v", err) } if relayer != nil && len(ibcpaths) > 0 { @@ -299,3 +291,11 @@ func GetTestName(chainCfgFile string) string { return name + "ic" } + +func killContainer(ctx context.Context) { + removed := dockerutil.KillAllInterchaintestContainers(ctx) + for _, r := range removed { + fmt.Println(" - ", r) + } + os.Exit(1) +}