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.

Sign up for GitHub

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

Remove upgrade script callbacks - not to be used #3

Merged
merged 1 commit into from
Aug 27, 2019
Merged

Remove upgrade script callbacks - not to be used #3

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
94 changes: 1 addition & 93 deletions x/upgrade/keeper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,11 @@
package upgrade

import (
"encoding/json"
"fmt"

"github.com/cosmos/cosmos-sdk/codec"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/spf13/viper"
abci "github.com/tendermint/tendermint/abci/types"
"github.com/tendermint/tendermint/libs/cli"
"os"
"os/exec"
"path/filepath"
)

// Keeper of the upgrade module
Expand All @@ -30,17 +25,6 @@ type Keeper interface {
// upgrade or false if there is none
GetUpgradePlan(ctx sdk.Context) (plan Plan, havePlan bool)

// SetWillUpgrader sets a custom function to be run whenever an upgrade is scheduled. This
// can be used to notify the node that an upgrade will be happen in the future so that it
// can download any software ahead of time in the background.
// It does not indicate that an upgrade is happening now and should just be used for preparation,
// not the actual upgrade.
SetWillUpgrader(willUpgrader func(ctx sdk.Context, plan Plan))

// SetOnUpgrader sets a custom function to be called right before the chain halts and the
// upgrade needs to be applied. This can be used to initiate an automatic upgrade process.
SetOnUpgrader(onUpgrader func(ctx sdk.Context, plan Plan))

// BeginBlocker should be called inside the BeginBlocker method of any app using the upgrade module. Scheduled upgrade
// plans are cached in memory so the overhead of this method is trivial.
BeginBlocker(ctx sdk.Context, req abci.RequestBeginBlock)
Expand All @@ -53,8 +37,6 @@ type keeper struct {
haveCache bool
//haveCachedPlan bool
//plan Plan
willUpgrader func(ctx sdk.Context, plan Plan)
onUpgrader func(ctx sdk.Context, plan Plan)
}

const (
Expand All @@ -75,14 +57,6 @@ func (keeper *keeper) SetUpgradeHandler(name string, upgradeHandler Handler) {
keeper.upgradeHandlers[name] = upgradeHandler
}

func (keeper *keeper) SetWillUpgrader(willUpgrader func(ctx sdk.Context, plan Plan)) {
keeper.willUpgrader = willUpgrader
}

func (keeper *keeper) SetOnUpgrader(onUpgrader func(ctx sdk.Context, plan Plan)) {
keeper.onUpgrader = onUpgrader
}

func (keeper *keeper) ScheduleUpgrade(ctx sdk.Context, plan Plan) sdk.Error {
err := plan.ValidateBasic()
if err != nil {
Expand Down Expand Up @@ -166,75 +140,9 @@ func (keeper *keeper) BeginBlocker(ctx sdk.Context, req abci.RequestBeginBlock)
} else {
// We don't have an upgrade handler for this upgrade name, meaning this software is out of date so shutdown
ctx.Logger().Error(fmt.Sprintf("UPGRADE \"%s\" NEEDED at height %d: %s", plan.Name, blockHeight, plan.Info))
if keeper.onUpgrader != nil {
keeper.onUpgrader(ctx, plan)
} else {
DefaultOnUpgrader(ctx, plan)
}
panic("UPGRADE REQUIRED!")
}
}
// TODO will upgraded is disabled for now because of inconsistent behavior
//else if justRetrievedFromCache {
// // In this case we are notifying the system that an upgrade is planned but not scheduled to happen yet
// if keeper.willUpgrader != nil {
// keeper.willUpgrader(ctx, keeper.plan)
// } else {
// DefaultWillUpgrader(ctx, keeper.plan)
// }
//}
}


// DefaultWillUpgrader asynchronously runs a script called prepare-upgrade from $COSMOS_HOME/config if such a script exists,
// with plan serialized to JSON as the first argument and the current block height as the second argument.
// The environment variable $COSMOS_HOME will be set to the home directory of the daemon.
func DefaultWillUpgrader(ctx sdk.Context, plan Plan) {
CallUpgradeScript(ctx, plan, "prepare-upgrade", true)
}

// DefaultOnUpgrader synchronously runs a script called do-upgrade from $COSMOS_HOME/config if such a script exists,
// with plan serialized to JSON as the first argument and the current block height as the second argument.
// The environment variable $COSMOS_HOME will be set to the home directory of the daemon.
func DefaultOnUpgrader(ctx sdk.Context, plan Plan) {
CallUpgradeScript(ctx, plan, "do-upgrade", false)
}

// CallUpgradeScript runs a script called script from $COSMOS_HOME/config if such a script exists,
// with plan serialized to JSON as the first argument and the current block height as the second argument.
// The environment variable $COSMOS_HOME will be set to the home directory of the daemon.
// If async is true, the command will be run in a separate go-routine.
func CallUpgradeScript(ctx sdk.Context, plan Plan, script string, async bool) {
f := func() {
home := viper.GetString(cli.HomeFlag)
file := filepath.Join(home, "config", script)
ctx.Logger().Info(fmt.Sprintf("Looking for upgrade script %s", file))
if _, err := os.Stat(file); err == nil {
ctx.Logger().Info(fmt.Sprintf("Applying upgrade script %s", file))
err = os.Setenv("COSMOS_HOME", home)
if err != nil {
ctx.Logger().Error(fmt.Sprintf("Error setting env var COSMOS_HOME: %v", err))
}


planJson, err := json.Marshal(plan)
if err != nil {
ctx.Logger().Error(fmt.Sprintf("Error marshaling upgrade plan to JSON: %v", err))
}
cmd := exec.Command(file, string(planJson), fmt.Sprintf("%d", ctx.BlockHeight()))
cmd.Stdout = logWriter{ctx, script, false}
cmd.Stderr = logWriter{ctx, script, false}
err = cmd.Run()
if err != nil {
ctx.Logger().Error(fmt.Sprintf("Error running script %s: %v", file, err))
}
}
}
if async {
go f()
} else {
f()
}
}

type logWriter struct {
Expand Down
40 changes: 3 additions & 37 deletions x/upgrade/keeper_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
package upgrade

import (
"testing"
"time"

"github.com/cosmos/cosmos-sdk/codec"
"github.com/cosmos/cosmos-sdk/store"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/stretchr/testify/suite"
abci "github.com/tendermint/tendermint/abci/types"
dbm "github.com/tendermint/tendermint/libs/db"
"github.com/tendermint/tendermint/libs/log"
"testing"
"time"
)

type TestSuite struct {
Expand Down Expand Up @@ -112,41 +113,6 @@ func (s *TestSuite) TestCantApplySameUpgradeTwice() {
s.Require().Equal(sdk.CodeUnknownRequest, err.Code())
}

func (s *TestSuite) TestCustomCallbacks() {
s.T().Log("Set custom WillUpgrader and OnUpgrader")
willUpgraderCalled := false
onUpgraderCalled := false
s.keeper.SetWillUpgrader(func(ctx sdk.Context, plan Plan) {
willUpgraderCalled = true
})
s.keeper.SetOnUpgrader(func(ctx sdk.Context, plan Plan) {
onUpgraderCalled = true
})

s.T().Log("Run an upgrade and verify that the custom WillUpgrader and OnUpgrader's are called")
err := s.keeper.ScheduleUpgrade(s.ctx, Plan{Name: "test", Height: s.ctx.BlockHeight() + 2})
s.Require().Nil(err)

header := abci.Header{Height: s.ctx.BlockHeight() + 1}
newCtx := sdk.NewContext(s.cms, header, false, log.NewNopLogger())
req := abci.RequestBeginBlock{Header: header}
s.Require().NotPanics(func() {
s.keeper.BeginBlocker(newCtx, req)
})
//s.Require().True(willUpgraderCalled)
s.Require().False(onUpgraderCalled)

willUpgraderCalled = false
header = abci.Header{Height: s.ctx.BlockHeight() + 2}
newCtx = sdk.NewContext(s.cms, header, false, log.NewNopLogger())
req = abci.RequestBeginBlock{Header: header}
s.Require().Panics(func() {
s.keeper.BeginBlocker(newCtx, req)
})
s.Require().True(onUpgraderCalled)
s.Require().False(willUpgraderCalled)
}

func (s *TestSuite) TestNoSpuriousUpgrades() {
s.T().Log("Verify that no upgrade panic is triggered in the BeginBlocker when we haven't scheduled an upgrade")
req := abci.RequestBeginBlock{Header: s.ctx.BlockHeader()}
Expand Down