Merge pull request #4306 from tejal29/add_survey_check

hook up showing survey prompt if not taken or recently prompted

cmd/skaffold/app/cmd/cmd.go

@@ -33,6 +33,7 @@ import (
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/server"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/survey"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/update"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/version"
 )
@@ -47,6 +48,7 @@ var (
 
 func NewSkaffoldCommand(out, err io.Writer) *cobra.Command {
 	updateMsg := make(chan string)
+	surveyPrompt := make(chan bool)
 	var shutdownAPIServer func() error
 
 	rootCmd := &cobra.Command{
@@ -84,14 +86,15 @@ func NewSkaffoldCommand(out, err io.Writer) *cobra.Command {
 
 			switch {
 			case quietFlag:
-				logrus.Debugf("Update check is disabled because of quiet mode")
+				logrus.Debugf("Update check and survey prompt disabled in quiet mode")
 			case analyze:
-				logrus.Debugf("Update check is disabled because of init --analyze")
+				logrus.Debugf("Update check and survey prompt when running `init --analyze`")
 			default:
 				go func() {
 					if err := updateCheck(updateMsg, opts.GlobalConfig); err != nil {
 						logrus.Infof("update check failed: %s", err)
 					}
+					surveyPrompt <- config.ShouldDisplayPrompt(opts.GlobalConfig)
 				}()
 			}
 			return nil
@@ -102,6 +105,16 @@ func NewSkaffoldCommand(out, err io.Writer) *cobra.Command {
 				fmt.Fprintf(cmd.OutOrStdout(), "%s\n", msg)
 			default:
 			}
+			// check if survey prompt needs to be displayed
+			select {
+			case shouldDisplay := <-surveyPrompt:
+				if shouldDisplay {
+					if err := survey.New(opts.GlobalConfig).DisplaySurveyPrompt(cmd.OutOrStdout()); err != nil {
+						fmt.Fprintf(cmd.OutOrStderr(), "%v\n", err)
+					}
+				}
+			default:
+			}
 
 			if shutdownAPIServer != nil {
 				shutdownAPIServer()

pkg/skaffold/config/util.go

@@ -58,6 +58,8 @@ var (
 
 	// update global config with the time the survey was last taken
 	updateLastTaken = "skaffold config set --survey --global last-taken %s"
+	// update global config with the time the survey was last prompted
+	updateLastPrompted = "skaffold config set --survey --global last-prompted %s"
 )
 
 // readConfigFileCached reads the specified file and returns the contents
@@ -281,7 +283,7 @@ func isSurveyPromptDisabled(configfile string) (*ContextConfig, bool) {
 	if err != nil {
 		return nil, false
 	}
-	return cfg, cfg != nil && cfg.Survey != nil && *cfg.Survey.DisablePrompt
+	return cfg, cfg != nil && cfg.Survey != nil && cfg.Survey.DisablePrompt != nil && *cfg.Survey.DisablePrompt
 }
 
 func recentlyPromptedOrTaken(cfg *ContextConfig) bool {
@@ -328,6 +330,34 @@ func UpdateGlobalSurveyTaken(configFile string) error {
 	return err
 }
 
+func UpdateGlobalSurveyPrompted(configFile string) error {
+	// Today's date
+	today := current().Format(time.RFC3339)
+	ai := fmt.Sprintf(updateLastPrompted, today)
+	aiErr := fmt.Errorf("could not automatically update the survey prompted timestamp - please run `%s`", ai)
+
+	configFile, err := ResolveConfigFile(configFile)
+	if err != nil {
+		return aiErr
+	}
+	fullConfig, err := ReadConfigFile(configFile)
+	if err != nil {
+		return aiErr
+	}
+	if fullConfig.Global == nil {
+		fullConfig.Global = &ContextConfig{}
+	}
+	if fullConfig.Global.Survey == nil {
+		fullConfig.Global.Survey = &SurveyConfig{}
+	}
+	fullConfig.Global.Survey.LastPrompted = today
+	err = WriteFullConfig(configFile, fullConfig)
+	if err != nil {
+		return aiErr
+	}
+	return err
+}
+
 func WriteFullConfig(configFile string, cfg *GlobalConfig) error {
 	contents, err := yaml.Marshal(cfg)
 	if err != nil {

pkg/skaffold/config/util_test.go

@@ -411,6 +411,10 @@ func TestIsSurveyPromptDisabled(t *testing.T) {
 			description: "config disable-prompt is false",
 			cfg:         &ContextConfig{Survey: &SurveyConfig{DisablePrompt: util.BoolPtr(false)}},
 		},
+		{
+			description: "disable prompt is nil",
+			cfg:         &ContextConfig{Survey: &SurveyConfig{}},
+		},
 		{
 			description: "config is nil",
 			cfg:         nil,
@@ -640,3 +644,63 @@ kubeContexts: []`,
 		})
 	}
 }
+
+func TestUpdateGlobalSurveyPrompted(t *testing.T) {
+	tests := []struct {
+		description string
+		cfg         string
+		expectedCfg *GlobalConfig
+	}{
+		{
+			description: "update global context when context is empty",
+			expectedCfg: &GlobalConfig{
+				Global:         &ContextConfig{Survey: &SurveyConfig{}},
+				ContextConfigs: []*ContextConfig{},
+			},
+		},
+		{
+			description: "update global context when survey config is not nil",
+			cfg: `
+global:
+  survey:
+    last-taken: "some date"
+kubeContexts: []`,
+			expectedCfg: &GlobalConfig{
+				Global:         &ContextConfig{Survey: &SurveyConfig{LastTaken: "some date"}},
+				ContextConfigs: []*ContextConfig{},
+			},
+		},
+		{
+			description: "update global context when survey config last prompted is in past",
+			cfg: `
+global:
+  survey:
+    last-prompted: "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 := UpdateGlobalSurveyPrompted(cfg)
+			t.CheckNoError(err)
+
+			actualConfig, cfgErr := ReadConfigFile(cfg)
+			t.CheckNoError(cfgErr)
+			// update time in expected cfg.
+			test.expectedCfg.Global.Survey.LastPrompted = testTime.Format(time.RFC3339)
+			t.CheckDeepEqual(test.expectedCfg, actualConfig)
+		})
+	}
+}

pkg/skaffold/survey/survey.go

@@ -43,8 +43,9 @@ To permanently disable the survey prompt, run:
    skaffold config set --survey --global disable-prompt true`, URL)
 
 	// for testing
-	isStdOut = stdOut
-	open     = browser.OpenURL
+	isStdOut     = stdOut
+	open         = browser.OpenURL
+	updateConfig = config.UpdateGlobalSurveyPrompted
 )
 
 type Runner struct {
@@ -57,10 +58,11 @@ func New(configFile string) *Runner {
 	}
 }
 
-func DisplaySurveyPrompt(out io.Writer) {
+func (s *Runner) DisplaySurveyPrompt(out io.Writer) error {
 	if isStdOut(out) {
 		fmt.Fprintln(out, Prompt)
 	}
+	return updateConfig(s.configFile)
 }
 
 func (s *Runner) OpenSurveyForm(_ context.Context, out io.Writer) error {

pkg/skaffold/survey/survey_test.go

@@ -46,8 +46,9 @@ func TestDisplaySurveyForm(t *testing.T) {
 			t.Override(&isStdOut, mock)
 			mockOpen := func(string) error { return nil }
 			t.Override(&open, mockOpen)
+			t.Override(&updateConfig, func(_ string) error { return nil })
 			var buf bytes.Buffer
-			DisplaySurveyPrompt(&buf)
+			New("test").DisplaySurveyPrompt(&buf)
 			t.CheckDeepEqual(test.expected, buf.String())
 		})
 	}