From 2088152019f9c48aa27911deda6ebb07b444a55c Mon Sep 17 00:00:00 2001 From: Tejal Desai Date: Mon, 7 Oct 2019 15:00:29 -0700 Subject: [PATCH 1/3] add display logic --- pkg/skaffold/config/global_config.go | 18 +++- pkg/skaffold/config/util.go | 35 +++++++ pkg/skaffold/config/util_test.go | 146 +++++++++++++++++++++++++++ 3 files changed, 194 insertions(+), 5 deletions(-) diff --git a/pkg/skaffold/config/global_config.go b/pkg/skaffold/config/global_config.go index 57bba1777ad..348a7d7f899 100644 --- a/pkg/skaffold/config/global_config.go +++ b/pkg/skaffold/config/global_config.go @@ -26,9 +26,17 @@ type GlobalConfig struct { // ContextConfig is the context-specific config information provided in // the global Skaffold config. type ContextConfig struct { - Kubecontext string `yaml:"kube-context,omitempty"` - DefaultRepo string `yaml:"default-repo,omitempty"` - LocalCluster *bool `yaml:"local-cluster,omitempty"` - InsecureRegistries []string `yaml:"insecure-registries,omitempty"` - UpdateCheck *bool `yaml:"update-check,omitempty"` + Kubecontext string `yaml:"kube-context,omitempty"` + DefaultRepo string `yaml:"default-repo,omitempty"` + LocalCluster *bool `yaml:"local-cluster,omitempty"` + InsecureRegistries []string `yaml:"insecure-registries,omitempty"` + UpdateCheck *bool `yaml:"update-check,omitempty"` + Survey *SurveyConfig `yaml:"survey,omitempty"` +} + +// SurveyConfig is the survey config information +type SurveyConfig struct { + DisablePrompt *bool `yaml:"disable-prompt,omitempty"` + LastTaken string `yaml:"last-taken,omitempty"` + LastPrompted string `yaml:"last-prompted,omitempty"` } diff --git a/pkg/skaffold/config/util.go b/pkg/skaffold/config/util.go index 0da27a8d4bc..98a70c874be 100644 --- a/pkg/skaffold/config/util.go +++ b/pkg/skaffold/config/util.go @@ -21,6 +21,7 @@ import ( "path/filepath" "strings" "sync" + "time" "github.com/imdario/mergo" "github.com/mitchellh/go-homedir" @@ -36,6 +37,8 @@ import ( const ( defaultConfigDir = ".skaffold" defaultConfigFile = "config" + tenDays = time.Duration(time.Hour * 24 * 10) + threeMonths = time.Duration(time.Hour * 24 * 90) ) var ( @@ -51,6 +54,7 @@ var ( ReadConfigFile = readConfigFileCached GetConfigForCurrentKubectx = getConfigForCurrentKubectx + current = time.Now ) // readConfigFileCached reads the specified file and returns the contents @@ -223,3 +227,34 @@ func IsUpdateCheckEnabled(configfile string) bool { } return cfg == nil || cfg.UpdateCheck == nil || *cfg.UpdateCheck } + +func ShouldDisplayPrompt(configfile string) bool { + if cfg, disabled := isSurveyPromptDisabled(configfile); !disabled { + return ifNotSurveyTakenOrPromptNotDisplayed(cfg) + } + return false +} + +func isSurveyPromptDisabled(configfile string) (*ContextConfig, bool) { + cfg, err := GetConfigForCurrentKubectx(configfile) + if err != nil { + return nil, false + } + return cfg, cfg != nil && cfg.Survey != nil && *cfg.Survey.DisablePrompt +} + +func ifNotSurveyTakenOrPromptNotDisplayed(cfg *ContextConfig) bool { + if cfg == nil && cfg.Survey == nil { + return false + } + return !isRecent(cfg.Survey.LastTaken, threeMonths) && !isRecent(cfg.Survey.LastPrompted, tenDays) +} + +func isRecent(date string, duration time.Duration) bool { + t, err := time.Parse(time.RFC3339, date) + if err != nil { + logrus.Debugf("could not parse data %s", date) + return false + } + return current().Sub(t) < duration +} diff --git a/pkg/skaffold/config/util_test.go b/pkg/skaffold/config/util_test.go index 53435c74d85..6b0e2db6965 100644 --- a/pkg/skaffold/config/util_test.go +++ b/pkg/skaffold/config/util_test.go @@ -21,6 +21,7 @@ import ( "path/filepath" "strings" "testing" + "time" "gopkg.in/yaml.v2" @@ -309,3 +310,148 @@ func TestIsKindCluster(t *testing.T) { }) } } + +func TestIsSurveyPromptDisabled(t *testing.T) { + tests := []struct { + description string + cfg *ContextConfig + readErr error + expected bool + }{ + { + description: "config disable-prompt is nil returns false", + cfg: &ContextConfig{}, + }, + { + description: "config disable-prompt is true", + cfg: &ContextConfig{Survey: &SurveyConfig{DisablePrompt: util.BoolPtr(true)}}, + expected: true, + }, + { + description: "config disable-prompt is false", + cfg: &ContextConfig{Survey: &SurveyConfig{DisablePrompt: util.BoolPtr(false)}}, + }, + { + description: "config is nil", + cfg: nil, + }, + { + description: "config has err", + cfg: nil, + readErr: fmt.Errorf("error while reading"), + }, + } + for _, test := range tests { + testutil.Run(t, test.description, func(t *testutil.T) { + t.Override(&GetConfigForCurrentKubectx, func(string) (*ContextConfig, error) { return test.cfg, test.readErr }) + _, actual := isSurveyPromptDisabled("dummyconfig") + t.CheckDeepEqual(test.expected, actual) + }) + } +} + +func TestIsRecent(t *testing.T) { + tests := []struct { + description string + date string + duration time.Duration + expected bool + }{ + { + description: "date is recent than 10 days from 01/30/2019", + date: "2019-01-22T13:04:05Z", + duration: time.Duration(10 * 24 * time.Hour), + expected: true, + }, + { + description: "date is not recent than 10 days from 01/30/2019", + date: "2019-01-19T13:04:05Z", + duration: time.Duration(10 * 24 * time.Hour), + }, + { + description: "date is not right format", + date: "01-19=20129", + expected: false, + }, + } + for _, test := range tests { + testutil.Run(t, test.description, func(t *testutil.T) { + t.Override(¤t, func() time.Time { + t, _ := time.Parse(time.RFC3339, "2019-01-30T12:04:05Z") + return t + }) + t.CheckDeepEqual(test.expected, isRecent(test.date, test.duration)) + }) + } +} + +func TestShouldDisplayPrompt(t *testing.T) { + tests := []struct { + description string + cfg *ContextConfig + expected bool + }{ + { + description: "should not display prompt when prompt is disabled", + cfg: &ContextConfig{Survey: &SurveyConfig{DisablePrompt: util.BoolPtr(true)}}, + }, + { + description: "should not display prompt when last prompted is less than 2 weeks", + cfg: &ContextConfig{ + Survey: &SurveyConfig{ + DisablePrompt: util.BoolPtr(false), + LastPrompted: "2019-01-22T00:00:00Z", + }, + }, + }, + { + description: "should not display prompt when last taken in less than 3 months", + cfg: &ContextConfig{ + Survey: &SurveyConfig{ + DisablePrompt: util.BoolPtr(false), + LastTaken: "2018-11-22T00:00:00Z", + }, + }, + }, + { + description: "should display prompt when last prompted is before 2 weeks", + cfg: &ContextConfig{ + Survey: &SurveyConfig{ + DisablePrompt: util.BoolPtr(false), + LastPrompted: "2019-01-10T00:00:00Z", + }, + }, + expected: true, + }, + { + description: "should display prompt when last taken is before than 3 months ago", + cfg: &ContextConfig{ + Survey: &SurveyConfig{ + DisablePrompt: util.BoolPtr(false), + LastTaken: "2017-11-10T00:00:00Z", + }, + }, + expected: true, + }, + { + description: "should not display prompt when last taken is recent than 3 months ago", + cfg: &ContextConfig{ + Survey: &SurveyConfig{ + DisablePrompt: util.BoolPtr(false), + LastTaken: "2019-01-10T00:00:00Z", + LastPrompted: "2019-01-10T00:00:00Z", + }, + }, + }, + } + 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(¤t, func() time.Time { + t, _ := time.Parse(time.RFC3339, "2019-01-30T12:04:05Z") + return t + }) + t.CheckDeepEqual(test.expected, ShouldDisplayPrompt("dummyconfig")) + }) + } +} From 50738a82cf422a5dd2874591750488ee16530533 Mon Sep 17 00:00:00 2001 From: Tejal Desai Date: Mon, 7 Oct 2019 16:07:28 -0700 Subject: [PATCH 2/3] add linter --- pkg/skaffold/config/util.go | 6 +++--- pkg/skaffold/config/util_test.go | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/skaffold/config/util.go b/pkg/skaffold/config/util.go index 98a70c874be..9ae61a85704 100644 --- a/pkg/skaffold/config/util.go +++ b/pkg/skaffold/config/util.go @@ -37,8 +37,8 @@ import ( const ( defaultConfigDir = ".skaffold" defaultConfigFile = "config" - tenDays = time.Duration(time.Hour * 24 * 10) - threeMonths = time.Duration(time.Hour * 24 * 90) + tenDays = time.Hour * 24 * 10 + threeMonths = time.Hour * 24 * 90 ) var ( @@ -244,7 +244,7 @@ func isSurveyPromptDisabled(configfile string) (*ContextConfig, bool) { } func ifNotSurveyTakenOrPromptNotDisplayed(cfg *ContextConfig) bool { - if cfg == nil && cfg.Survey == nil { + if cfg == nil || cfg.Survey == nil { return false } return !isRecent(cfg.Survey.LastTaken, threeMonths) && !isRecent(cfg.Survey.LastPrompted, tenDays) diff --git a/pkg/skaffold/config/util_test.go b/pkg/skaffold/config/util_test.go index 6b0e2db6965..5aadf270c08 100644 --- a/pkg/skaffold/config/util_test.go +++ b/pkg/skaffold/config/util_test.go @@ -360,13 +360,13 @@ func TestIsRecent(t *testing.T) { { description: "date is recent than 10 days from 01/30/2019", date: "2019-01-22T13:04:05Z", - duration: time.Duration(10 * 24 * time.Hour), + duration: 10 * 24 * time.Hour, expected: true, }, { description: "date is not recent than 10 days from 01/30/2019", date: "2019-01-19T13:04:05Z", - duration: time.Duration(10 * 24 * time.Hour), + duration: 10 * 24 * time.Hour, }, { description: "date is not right format", From fff715405e46cb5a7abd4a35351fa17e91c10ea4 Mon Sep 17 00:00:00 2001 From: Tejal Desai Date: Thu, 30 Jan 2020 11:12:25 -0800 Subject: [PATCH 3/3] address balint's comments --- pkg/skaffold/config/util.go | 12 +++++------- pkg/skaffold/config/util_test.go | 8 ++++---- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/pkg/skaffold/config/util.go b/pkg/skaffold/config/util.go index 9ae61a85704..4fb5324a295 100644 --- a/pkg/skaffold/config/util.go +++ b/pkg/skaffold/config/util.go @@ -229,10 +229,8 @@ func IsUpdateCheckEnabled(configfile string) bool { } func ShouldDisplayPrompt(configfile string) bool { - if cfg, disabled := isSurveyPromptDisabled(configfile); !disabled { - return ifNotSurveyTakenOrPromptNotDisplayed(cfg) - } - return false + cfg, disabled := isSurveyPromptDisabled(configfile) + return !disabled && !recentlyPromptedOrTaken(cfg) } func isSurveyPromptDisabled(configfile string) (*ContextConfig, bool) { @@ -243,14 +241,14 @@ func isSurveyPromptDisabled(configfile string) (*ContextConfig, bool) { return cfg, cfg != nil && cfg.Survey != nil && *cfg.Survey.DisablePrompt } -func ifNotSurveyTakenOrPromptNotDisplayed(cfg *ContextConfig) bool { +func recentlyPromptedOrTaken(cfg *ContextConfig) bool { if cfg == nil || cfg.Survey == nil { return false } - return !isRecent(cfg.Survey.LastTaken, threeMonths) && !isRecent(cfg.Survey.LastPrompted, tenDays) + return lessThan(cfg.Survey.LastTaken, threeMonths) || lessThan(cfg.Survey.LastPrompted, tenDays) } -func isRecent(date string, duration time.Duration) bool { +func lessThan(date string, duration time.Duration) bool { t, err := time.Parse(time.RFC3339, date) if err != nil { logrus.Debugf("could not parse data %s", date) diff --git a/pkg/skaffold/config/util_test.go b/pkg/skaffold/config/util_test.go index 5aadf270c08..21a7666bbc3 100644 --- a/pkg/skaffold/config/util_test.go +++ b/pkg/skaffold/config/util_test.go @@ -350,7 +350,7 @@ func TestIsSurveyPromptDisabled(t *testing.T) { } } -func TestIsRecent(t *testing.T) { +func TestLessThan(t *testing.T) { tests := []struct { description string date string @@ -358,13 +358,13 @@ func TestIsRecent(t *testing.T) { expected bool }{ { - description: "date is recent than 10 days from 01/30/2019", + description: "date is less than 10 days from 01/30/2019", date: "2019-01-22T13:04:05Z", duration: 10 * 24 * time.Hour, expected: true, }, { - description: "date is not recent than 10 days from 01/30/2019", + description: "date is not less than 10 days from 01/30/2019", date: "2019-01-19T13:04:05Z", duration: 10 * 24 * time.Hour, }, @@ -380,7 +380,7 @@ func TestIsRecent(t *testing.T) { t, _ := time.Parse(time.RFC3339, "2019-01-30T12:04:05Z") return t }) - t.CheckDeepEqual(test.expected, isRecent(test.date, test.duration)) + t.CheckDeepEqual(test.expected, lessThan(test.date, test.duration)) }) } }