Skip to content

Commit

Permalink
Show update check to users once in 24 hours
Browse files Browse the repository at this point in the history
  • Loading branch information
tejal29 committed May 6, 2021
1 parent ce34201 commit 8a34a00
Show file tree
Hide file tree
Showing 5 changed files with 138 additions and 3 deletions.
3 changes: 3 additions & 0 deletions cmd/skaffold/app/cmd/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,9 @@ func NewSkaffoldCommand(out, errOut io.Writer) *cobra.Command {
PersistentPostRun: func(cmd *cobra.Command, args []string) {
select {
case msg := <-updateMsg:
if err := config.UpdateMsgDisplayed(opts.GlobalConfig); err != nil {
logrus.Debugf("could not update the 'last-prompted' config for 'update-config' section due to %s", err)
}
fmt.Fprintf(cmd.OutOrStderr(), "%s\n", msg)
default:
}
Expand Down
7 changes: 7 additions & 0 deletions pkg/skaffold/config/global_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ type ContextConfig struct {
KindDisableLoad *bool `yaml:"kind-disable-load,omitempty"`
K3dDisableLoad *bool `yaml:"k3d-disable-load,omitempty"`
CollectMetrics *bool `yaml:"collect-metrics,omitempty"`
UpdateCheckConfig *UpdateConfig `yaml:"update-config,omitempty"`
}

// SurveyConfig is the survey config information
Expand All @@ -45,3 +46,9 @@ type SurveyConfig struct {
LastTaken string `yaml:"last-taken,omitempty"`
LastPrompted string `yaml:"last-prompted,omitempty"`
}

// UpdateConfig is the update config information
type UpdateConfig struct {
// TODO (tejaldesai) Move update-check config within this struct
LastPrompted string `yaml:"last-prompted,omitempty"`
}
36 changes: 36 additions & 0 deletions pkg/skaffold/config/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,42 @@ func recentlyPromptedOrTaken(cfg *ContextConfig) bool {
return lessThan(cfg.Survey.LastTaken, 90*24*time.Hour) || lessThan(cfg.Survey.LastPrompted, 10*24*time.Hour)
}

func ShouldDisplayUpdateMsg(configfile string) bool {
cfg, err := GetConfigForCurrentKubectx(configfile)
if err != nil {
return true
}
if cfg == nil || cfg.UpdateCheckConfig == nil {
return true
}
return !lessThan(cfg.UpdateCheckConfig.LastPrompted, 24*time.Hour)
}

// UpdateMsgDisplayed updates the `last-prompted` config for `update-config` in
// the skaffold config
func UpdateMsgDisplayed(configFile string) error {
// Today's date
today := current().Format(time.RFC3339)

configFile, err := ResolveConfigFile(configFile)
if err != nil {
return err
}
fullConfig, err := ReadConfigFile(configFile)
if err != nil {
return err
}
if fullConfig.Global == nil {
fullConfig.Global = &ContextConfig{}
}
if fullConfig.Global.UpdateCheckConfig == nil {
fullConfig.Global.UpdateCheckConfig = &UpdateConfig{}
}
fullConfig.Global.UpdateCheckConfig.LastPrompted = today
err = WriteFullConfig(configFile, fullConfig)
return err
}

func lessThan(date string, duration time.Duration) bool {
t, err := time.Parse(time.RFC3339, date)
if err != nil {
Expand Down
89 changes: 89 additions & 0 deletions pkg/skaffold/config/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -764,3 +764,92 @@ kubeContexts: []`,
})
}
}

func TestUpdateMsgDisplayed(t *testing.T) {
tests := []struct {
description string
cfg string
expectedCfg *GlobalConfig
}{
{
description: "update global context when context is empty",
expectedCfg: &GlobalConfig{
Global: &ContextConfig{UpdateCheckConfig: &UpdateConfig{LastPrompted: "some date"}},
ContextConfigs: []*ContextConfig{},
},
},
{
description: "update global context when update config is not nil",
cfg: `
global:
update-config:
last-prompted: "some date"
kubeContexts: []`,
expectedCfg: &GlobalConfig{
Global: &ContextConfig{UpdateCheckConfig: &UpdateConfig{LastPrompted: "some date"}},
ContextConfigs: []*ContextConfig{},
},
},
{
description: "update global context when update config last taken is in past",
cfg: `
global:
update-config:
last-taken: "some date in past"
kubeContexts: []`,
expectedCfg: &GlobalConfig{
Global: &ContextConfig{Survey: &SurveyConfig{}},
ContextConfigs: []*ContextConfig{},
},
},
}
for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
cfg := t.TempFile("config", []byte(test.cfg))
testTime := time.Now()
t.Override(&ReadConfigFile, ReadConfigFileNoCache)
t.Override(&current, func() time.Time {
return testTime
})

// update the time
err := UpdateMsgDisplayed(cfg)
t.CheckNoError(err)

_, cfgErr := ReadConfigFile(cfg)
t.CheckNoError(cfgErr)
})
}
}

func TestShouldDisplayUpdateMsg(t *testing.T) {
Jan012021, _ := time.Parse(time.RFC3339, "2021-01-01T12:04:05Z")
tests := []struct {
description string
cfg *ContextConfig
expected bool
}{
{
description: "should not display prompt when prompt is displayed in last 24 hours",
cfg: &ContextConfig{
UpdateCheckConfig: &UpdateConfig{LastPrompted: "2021-01-01T00:00:00Z"},
},
},
{
description: "should display prompt when last prompted is before 24 hours",
cfg: &ContextConfig{
UpdateCheckConfig: &UpdateConfig{LastPrompted: "2020-12-22T00:00:00Z"},
},
expected: true,
},
}
for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
t.Override(&GetConfigForCurrentKubectx, func(string) (*ContextConfig, error) { return test.cfg, nil })
t.Override(&current, func() time.Time {
return Jan012021
})
t.CheckDeepEqual(test.expected, ShouldDisplayUpdateMsg("dummyconfig"))
})
}
}
6 changes: 3 additions & 3 deletions pkg/skaffold/update/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,16 +49,16 @@ func CheckVersionOnError(config string) (string, error) {
return checkVersion(config, true)
}

func checkVersion(config string, onError bool) (string, error) {
if !isUpdateCheckEnabled(config) {
func checkVersion(configfile string, onError bool) (string, error) {
if !isUpdateCheckEnabled(configfile) {
logrus.Debugf("Update check not enabled, skipping.")
return "", nil
}
latest, current, err := GetLatestAndCurrentVersion()
if err != nil {
return "", fmt.Errorf("getting latest and current skaffold versions: %w", err)
}
if !latest.GT(current) {
if !latest.GT(current) || !config.ShouldDisplayUpdateMsg(configfile) {
return "", nil
}
if onError {
Expand Down

0 comments on commit 8a34a00

Please sign in to comment.