Repository: bacalhau-project/bacalhau (language: python)

Issue:
Automatically perform update checks on every command
In v1.1.1 we added an update check feature to `bacalhau version` command. This is useful but users don't run that command very often.

Now we want to run the check for every command. As ever, the update check should:

- print out a message at the end of the command if an update is required
- fail silently, no error message should be printed if the update check fails
- terminate silently if the command completes before the update check is finished
Limit update checks to once per day
In #2893 we made more commands check for updates. Strictly not all of these checks are necessary as the code is unlikely to have changed regularly. Instead we should limit the amount we check for updates to (perhaps) once per day. 

Suggestion: every time we perform an update check, write a config property into `config.yaml` that will be the `NextUpdateCheck` date and time. If the current time is behind this value, don't do any checking. If the current time is ahead of this value, do and update check and then move the time forward by 24 hours.


Discussion / hints:

I like this, though maybe not in `config.yaml` as thats is the config file the user should provide and we shouldn't use it to track state 
Yeah ok. It's also not very easy to write an individual key in the config file! So will just track state in some file in the repo.

Fix patch:
diff --git a/cmd/util/version.go b/cmd/util/version.go
index 720f4848ff..334cb1601c 100644
--- a/cmd/util/version.go
+++ b/cmd/util/version.go
@@ -2,7 +2,10 @@ package util
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
+	"os"
+	"time"
 
 	"github.com/Masterminds/semver"
 	"github.com/pkg/errors"
@@ -69,19 +72,75 @@ var printMessage *string = nil
 // background. There should be no output if the check fails or the context is
 // cancelled before the check can complete.
 func StartUpdateCheck(cmd *cobra.Command, args []string) {
-	go func(ctx context.Context) {
-		if skip, err := config.Get[bool](types.SkipUpdateCheck); skip || err != nil {
-			log.Ctx(ctx).Debug().Err(err).Bool(types.SkipUpdateCheck, skip).Msg("Skipping update check")
-			return
-		}
+	if skip, err := config.Get[bool](types.UpdateSkipChecks); skip || err != nil {
+		log.Ctx(cmd.Context()).Debug().Err(err).Bool(types.UpdateSkipChecks, skip).Msg("Skipping update check due to config")
+		return
+	}
+
+	lastCheck, err := readLastCheckTime(cmd.Context())
+	if err == nil && time.Since(lastCheck) < config.GetUpdateCheckFrequency() {
+		log.Ctx(cmd.Context()).Debug().Time("LastCheck", lastCheck).Msg("Skipping update check as there was a recent enough check")
+		return
+	} else if err != nil {
+		log.Ctx(cmd.Context()).Warn().Err(err).Msg("Error reading update check state – will perform check anyway")
+	}
 
+	go func(ctx context.Context) {
 		versions, err := GetAllVersions(ctx)
 		if err == nil {
 			printMessage = &versions.UpdateMessage
+			err = writeNewLastCheckTime(ctx)
+			log.Ctx(ctx).Debug().Err(err).Str("LatestVersion", versions.LatestVersion.GitVersion).Msg("Performed update check")
 		}
 	}(cmd.Context())
 }
 
+type updateState struct {
+	LastCheck time.Time
+}
+
+func readLastCheckTime(ctx context.Context) (time.Time, error) {
+	path, err := config.Get[string](types.UpdateCheckStatePath)
+	if err != nil {
+		return time.Now(), errors.Wrap(err, "error getting repo path")
+	}
+
+	file, err := os.Open(path)
+	if err != nil {
+		return time.Now(), errors.Wrap(err, "error opening update state file")
+	}
+	defer file.Close()
+
+	var state updateState
+	err = json.NewDecoder(file).Decode(&state)
+	if err != nil {
+		return time.Now(), errors.Wrap(err, "error reading update state")
+	}
+
+	return state.LastCheck, nil
+}
+
+func writeNewLastCheckTime(ctx context.Context) error {
+	path, err := config.Get[string](types.UpdateCheckStatePath)
+	if err != nil {
+		return errors.Wrap(err, "error getting repo path")
+	}
+
+	file, err := os.Create(path)
+	if err != nil {
+		return errors.Wrap(err, "error creating update state file")
+	}
+	defer file.Close()
+
+	state := updateState{LastCheck: time.Now()}
+	err = json.NewEncoder(file).Encode(&state)
+	if err != nil {
+		return errors.Wrap(err, "error writing update state")
+	}
+
+	return nil
+}
+
 // PrintUpdateCheck is a Cobra post run hook to print the results of an update
 // check. The message will be a non-nil pointer only if the update check
 // succeeds and should only have visible output if the message is non-empty.
diff --git a/pkg/config/configenv/dev.go b/pkg/config/configenv/dev.go
index 92ffa29b59..0a333de568 100644
--- a/pkg/config/configenv/dev.go
+++ b/pkg/config/configenv/dev.go
@@ -16,6 +16,9 @@ var Development = types.BacalhauConfig{
 		Libp2pTracerPath: os.DevNull,
 		EventTracerPath:  os.DevNull,
 	},
+	Update: types.UpdateConfig{
+		SkipChecks: true,
+	},
 	Node: types.NodeConfig{
 		ClientAPI: types.APIConfig{
 			Host: "bootstrap.development.bacalhau.org",
diff --git a/pkg/config/configenv/local.go b/pkg/config/configenv/local.go
index 41d717f9bf..d988210825 100644
--- a/pkg/config/configenv/local.go
+++ b/pkg/config/configenv/local.go
@@ -16,6 +16,9 @@ var Local = types.BacalhauConfig{
 		Libp2pTracerPath: os.DevNull,
 		EventTracerPath:  os.DevNull,
 	},
+	Update: types.UpdateConfig{
+		SkipChecks: true,
+	},
 	Node: types.NodeConfig{
 		ClientAPI: types.APIConfig{
 			Host: "0.0.0.0",
diff --git a/pkg/config/configenv/production.go b/pkg/config/configenv/production.go
index fee6fc0bf7..83d24f01e2 100644
--- a/pkg/config/configenv/production.go
+++ b/pkg/config/configenv/production.go
@@ -16,6 +16,10 @@ var Production = types.BacalhauConfig{
 		Libp2pTracerPath: os.DevNull,
 		EventTracerPath:  os.DevNull,
 	},
+	Update: types.UpdateConfig{
+		SkipChecks:     false,
+		CheckFrequency: types.Duration(24 * time.Hour),
+	},
 	Node: types.NodeConfig{
 		ClientAPI: types.APIConfig{
 			Host: "bootstrap.production.bacalhau.org",
diff --git a/pkg/config/configenv/staging.go b/pkg/config/configenv/staging.go
index 372faf5341..992e0d5af9 100644
--- a/pkg/config/configenv/staging.go
+++ b/pkg/config/configenv/staging.go
@@ -16,6 +16,10 @@ var Staging = types.BacalhauConfig{
 		Libp2pTracerPath: os.DevNull,
 		EventTracerPath:  os.DevNull,
 	},
+	Update: types.UpdateConfig{
+		SkipChecks:     false,
+		CheckFrequency: types.Duration(24 * time.Hour),
+	},
 	Node: types.NodeConfig{
 		ClientAPI: types.APIConfig{
 			Host: "bootstrap.staging.bacalhau.org",
diff --git a/pkg/config/getters.go b/pkg/config/getters.go
index 24ad8e4704..f5ec5bbe37 100644
--- a/pkg/config/getters.go
+++ b/pkg/config/getters.go
@@ -142,6 +142,10 @@ func GetVolumeSizeRequestTimeout() time.Duration {
 	return viper.GetDuration(types.NodeVolumeSizeRequestTimeout)
 }
 
+func GetUpdateCheckFrequency() time.Duration {
+	return viper.GetDuration(types.UpdateCheckFrequency)
+}
+
 func GetStoragePath() string {
 	path := viper.GetString(types.NodeComputeStoragePath)
 	if path == "" {
diff --git a/pkg/config/setters.go b/pkg/config/setters.go
index 11d5cc4909..b7472bb5df 100644
--- a/pkg/config/setters.go
+++ b/pkg/config/setters.go
@@ -25,3 +25,7 @@ func SetComputeStoragesPath(path string) {
 func SetAutoCertCachePath(path string) {
 	viper.Set(types.NodeServerAPITLSAutoCertCachePath, path)
 }
+
+func SetUpdateCheckStatePath(path string) {
+	viper.Set(types.UpdateCheckStatePath, path)
+}
diff --git a/pkg/config/types/config.go b/pkg/config/types/config.go
index 327a90e8bb..5555c67f57 100644
--- a/pkg/config/types/config.go
+++ b/pkg/config/types/config.go
@@ -6,8 +6,7 @@ type BacalhauConfig struct {
 	Node    NodeConfig    `yaml:"Node"`
 	User    UserConfig    `yaml:"User"`
 	Metrics MetricsConfig `yaml:"Metrics"`
-
-	SkipUpdateCheck bool `yaml:"SkipUpdateCheck"`
+	Update  UpdateConfig  `yaml:"UpdateConfig"`
 }
 
 type UserConfig struct {
@@ -19,3 +18,9 @@ type MetricsConfig struct {
 	Libp2pTracerPath string `yaml:"Libp2PTracerPath"`
 	EventTracerPath  string `yaml:"EventTracerPath"`
 }
+
+type UpdateConfig struct {
+	SkipChecks     bool     `yaml:"SkipChecks"`
+	CheckStatePath string   `yaml:"StatePath"`
+	CheckFrequency Duration `yaml:"CheckFrequency"`
+}
diff --git a/pkg/config/types/generated_constants.go b/pkg/config/types/generated_constants.go
index 253305c100..4a1e3c5133 100644
--- a/pkg/config/types/generated_constants.go
+++ b/pkg/config/types/generated_constants.go
@@ -120,4 +120,7 @@ const UserLibp2pKeyPath = "User.Libp2pKeyPath"
 const Metrics = "Metrics"
 const MetricsLibp2pTracerPath = "Metrics.Libp2pTracerPath"
 const MetricsEventTracerPath = "Metrics.EventTracerPath"
-const SkipUpdateCheck = "SkipUpdateCheck"
+const Update = "Update"
+const UpdateSkipChecks = "Update.SkipChecks"
+const UpdateCheckStatePath = "Update.CheckStatePath"
+const UpdateCheckFrequency = "Update.CheckFrequency"
diff --git a/pkg/config/types/generated_viper_defaults.go b/pkg/config/types/generated_viper_defaults.go
index 2a0b3aea0f..631893ffae 100644
--- a/pkg/config/types/generated_viper_defaults.go
+++ b/pkg/config/types/generated_viper_defaults.go
@@ -123,7 +123,10 @@ func SetDefaults(cfg BacalhauConfig) {
 	viper.SetDefault(Metrics, cfg.Metrics)
 	viper.SetDefault(MetricsLibp2pTracerPath, cfg.Metrics.Libp2pTracerPath)
 	viper.SetDefault(MetricsEventTracerPath, cfg.Metrics.EventTracerPath)
-	viper.SetDefault(SkipUpdateCheck, cfg.SkipUpdateCheck)
+	viper.SetDefault(Update, cfg.Update)
+	viper.SetDefault(UpdateSkipChecks, cfg.Update.SkipChecks)
+	viper.SetDefault(UpdateCheckStatePath, cfg.Update.CheckStatePath)
+	viper.SetDefault(UpdateCheckFrequency, cfg.Update.CheckFrequency.String())
 }
 
 func Set(cfg BacalhauConfig) {
@@ -245,5 +248,8 @@ func Set(cfg BacalhauConfig) {
 	viper.Set(Metrics, cfg.Metrics)
 	viper.Set(MetricsLibp2pTracerPath, cfg.Metrics.Libp2pTracerPath)
 	viper.Set(MetricsEventTracerPath, cfg.Metrics.EventTracerPath)
-	viper.Set(SkipUpdateCheck, cfg.SkipUpdateCheck)
+	viper.Set(Update, cfg.Update)
+	viper.Set(UpdateSkipChecks, cfg.Update.SkipChecks)
+	viper.Set(UpdateCheckStatePath, cfg.Update.CheckStatePath)
+	viper.Set(UpdateCheckFrequency, cfg.Update.CheckFrequency.String())
 }
diff --git a/pkg/repo/fs.go b/pkg/repo/fs.go
index 4a31c80578..35e3dafff8 100644
--- a/pkg/repo/fs.go
+++ b/pkg/repo/fs.go
@@ -41,6 +41,9 @@ const (
 
 	// requester paths
 	AutoCertCachePath = "autocert-cache"
+
+	// update check paths
+	UpdateCheckStatePath = "update.json"
 )
 
 type FsRepo struct {
@@ -154,12 +157,19 @@ func (fsr *FsRepo) Open() error {
 		config.SetComputeStoragesPath(cfg.Node.ComputeStoragePath)
 	}
 
+	if cfg.Update.CheckStatePath == "" {
+		cfg.Update.CheckStatePath = filepath.Join(fsr.path, UpdateCheckStatePath)
+		config.SetUpdateCheckStatePath(cfg.Update.CheckStatePath)
+	}
+
 	// Using a slice of paths to minimize repetitive checks
 	pathsToCheck := []string{
 		cfg.User.KeyPath,
 		cfg.User.Libp2pKeyPath,
 		cfg.Node.ExecutorPluginPath,
 		cfg.Node.ComputeStoragePath,
+		// Note that cfg.Update.StateCheckPath is deliberately omitted here:
+		// this file will get created on first run of the update check
 	}
 
 	for _, path := range pathsToCheck {


Test patch:
diff --git a/cmd/testing/base.go b/cmd/testing/base.go
index 6c239e1182..6861edd47c 100644
--- a/cmd/testing/base.go
+++ b/cmd/testing/base.go
@@ -3,14 +3,11 @@ package cmdtesting
 import (
 	"context"
 	"fmt"
-	"os"
 	"time"
 
 	"github.com/stretchr/testify/suite"
 
 	"github.com/bacalhau-project/bacalhau/pkg/bidstrategy/semantic"
-	"github.com/bacalhau-project/bacalhau/pkg/config"
-	"github.com/bacalhau-project/bacalhau/pkg/config/types"
 	"github.com/bacalhau-project/bacalhau/pkg/publicapi/client"
 	clientv2 "github.com/bacalhau-project/bacalhau/pkg/publicapi/client/v2"
 
@@ -36,8 +33,6 @@ func (s *BaseSuite) SetupTest() {
 	logger.ConfigureTestLogging(s.T())
 	util.Fatal = util.FakeFatalErrorHandler
 
-	os.Setenv(config.KeyAsEnvVar(types.SkipUpdateCheck), "1")
-
 	computeConfig, err := node.NewComputeConfigWith(node.ComputeConfigParams{
 		JobSelectionPolicy: node.JobSelectionPolicy{
 			Locality: semantic.Anywhere,
diff --git a/pkg/config/configenv/test.go b/pkg/config/configenv/test.go
index 443c3f6ffd..71b2371941 100644
--- a/pkg/config/configenv/test.go
+++ b/pkg/config/configenv/test.go
@@ -16,6 +16,9 @@ var Testing = types.BacalhauConfig{
 		Libp2pTracerPath: os.DevNull,
 		EventTracerPath:  os.DevNull,
 	},
+	Update: types.UpdateConfig{
+		SkipChecks: true,
+	},
 	Node: types.NodeConfig{
 		ClientAPI: types.APIConfig{
 			Host: "test",