Caching registry responses to fix client.Timeout issue (#3681)

* debuging timeout issue

* use httpcaching (15min) for registry related requests

* update vendor

* add RegistryCacheTime preference option

* fix preference unit test

* add DownloadFileWithCache function

* address PR review comments

* address PR review comments

* errors related to caching should not be fatal

* cleaner cache logging

pkg/preference/preference.go

@@ -65,6 +65,9 @@ const (
 	// PushTargetDescription is human-readable description for the pushtarget setting
 	PushTargetDescription = "Set this value to 'kube' or 'docker' to tell odo where to push applications to. (Default: kube)"
 
+	// RegistryCacheTimeSetting is human-readable description for the registrycachetime setting
+	RegistryCacheTimeSetting = "RegistryCacheTime"
+
 	// Constants for PushTarget values
 
 	// DockerPushTarget represents the value of the push target when it's set to Docker
@@ -78,6 +81,9 @@ const (
 
 	// DefaultDevfileRegistryURL is the URL of default devfile registry
 	DefaultDevfileRegistryURL = "https://github.com/odo-devfiles/registry"
+
+	// DefaultRegistryCacheTime is time (in minutes) for how long odo will cache information from Devfile registry
+	DefaultRegistryCacheTime = 15
 )
 
 // TimeoutSettingDescription is human-readable description for the timeout setting
@@ -89,6 +95,9 @@ var PushTimeoutSettingDescription = fmt.Sprintf("PushTimeout (in seconds) for wa
 // BuildTimeoutSettingDescription adds a description for BuildTimeout
 var BuildTimeoutSettingDescription = fmt.Sprintf("BuildTimeout (in seconds) for waiting for a build of the git component to complete (Default: %d)", DefaultBuildTimeout)
 
+// RegistryCacheTimeDescription adds a description for RegistryCacheTime
+var RegistryCacheTimeDescription = fmt.Sprintf("For how long (in minutes) odo will cache information from Devfile registry (Default: %d)", DefaultRegistryCacheTime)
+
 // This value can be provided to set a seperate directory for users 'homedir' resolution
 // note for mocking purpose ONLY
 var customHomeDir = os.Getenv("CUSTOM_HOMEDIR")
@@ -103,6 +112,7 @@ var (
 		PushTimeoutSetting:        PushTimeoutSettingDescription,
 		ExperimentalSetting:       ExperimentalDescription,
 		PushTargetSetting:         PushTargetDescription,
+		RegistryCacheTimeSetting:  RegistryCacheTimeDescription,
 	}
 
 	// set-like map to quickly check if a parameter is supported
@@ -141,6 +151,9 @@ type OdoSettings struct {
 
 	// RegistryList for telling odo to connect to all the registries in the registry list
 	RegistryList *[]Registry `yaml:"RegistryList,omitempty"`
+
+	// RegistryCacheTime how long odo should cache information from registry
+	RegistryCacheTime *int `yaml:"RegistryCacheTime,omitempty"`
 }
 
 // Registry includes the registry metadata
@@ -367,6 +380,16 @@ func (c *PreferenceInfo) SetConfiguration(parameter string, value string) error
 			}
 			c.OdoSettings.PushTimeout = &typedval
 
+		case "registrycachetime":
+			typedval, err := strconv.Atoi(value)
+			if err != nil {
+				return errors.Wrapf(err, "unable to set %s to %s", parameter, value)
+			}
+			if typedval < 0 {
+				return errors.Errorf("cannot set timeout to less than 0")
+			}
+			c.OdoSettings.RegistryCacheTime = &typedval
+
 		case "updatenotification":
 			val, err := strconv.ParseBool(strings.ToLower(value))
 			if err != nil {
@@ -446,6 +469,11 @@ func (c *PreferenceInfo) GetPushTimeout() int {
 	return util.GetIntOrDefault(c.OdoSettings.PushTimeout, DefaultPushTimeout)
 }
 
+// GetRegistryCacheTime gets the value set by RegistryCacheTime
+func (c *PreferenceInfo) GetRegistryCacheTime() int {
+	return util.GetIntOrDefault(c.OdoSettings.RegistryCacheTime, DefaultRegistryCacheTime)
+}
+
 // GetUpdateNotification returns the value of UpdateNotification from preferences
 // and if absent then returns default
 func (c *PreferenceInfo) GetUpdateNotification() bool {

pkg/preference/preference_test.go

@@ -463,6 +463,20 @@ func TestSetConfiguration(t *testing.T) {
 			existingConfig: Preference{},
 			wantErr:        true,
 		},
+		{
+			name:           "Case 25: set RegistryCacheTime to 1",
+			parameter:      "RegistryCacheTime",
+			value:          "1",
+			existingConfig: Preference{},
+			wantErr:        false,
+		},
+		{
+			name:           "Case 26: set RegistryCacheTime to non int value",
+			parameter:      "RegistryCacheTime",
+			value:          "a",
+			existingConfig: Preference{},
+			wantErr:        true,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -494,6 +508,10 @@ func TestSetConfiguration(t *testing.T) {
 					if *cfg.OdoSettings.PushTarget != tt.want {
 						t.Errorf("unexpeced value after execution of SetConfiguration \ngot: %v \nexpected: %d\n", cfg.OdoSettings.PushTarget, tt.want)
 					}
+				case "registrycachetime":
+					if *cfg.OdoSettings.RegistryCacheTime != tt.want {
+						t.Errorf("unexpeced value after execution of SetConfiguration \ngot: %v \nexpected: %d\n", *cfg.OdoSettings.RegistryCacheTime, tt.want)
+					}
 				}
 			} else if tt.wantErr && err != nil {
 				// negative cases
@@ -693,6 +711,7 @@ Available Parameters:
 %s - %s
 %s - %s
 %s - %s
+%s - %s
 `
 	expected = fmt.Sprintf(expected,
 		BuildTimeoutSetting, BuildTimeoutSettingDescription,
@@ -700,8 +719,10 @@ Available Parameters:
 		NamePrefixSetting, NamePrefixSettingDescription,
 		PushTargetSetting, PushTargetDescription,
 		PushTimeoutSetting, PushTimeoutSettingDescription,
+		RegistryCacheTimeSetting, RegistryCacheTimeDescription,
 		TimeoutSetting, TimeoutSettingDescription,
-		UpdateNotificationSetting, UpdateNotificationSettingDescription)
+		UpdateNotificationSetting, UpdateNotificationSettingDescription,
+	)
 	actual := FormatSupportedParameters()
 	if expected != actual {
 		t.Errorf("expected '%s', got '%s'", expected, actual)
@@ -709,7 +730,16 @@ Available Parameters:
 }
 
 func TestLowerCaseParameters(t *testing.T) {
-	expected := map[string]bool{"nameprefix": true, "buildtimeout": true, "pushtimeout": true, "timeout": true, "updatenotification": true, "experimental": true, "pushtarget": true}
+	expected := map[string]bool{
+		"nameprefix":         true,
+		"buildtimeout":       true,
+		"pushtimeout":        true,
+		"registrycachetime":  true,
+		"timeout":            true,
+		"updatenotification": true,
+		"experimental":       true,
+		"pushtarget":         true,
+	}
 	actual := util.GetLowerCaseParameters(GetSupportedParameters())
 	if !reflect.DeepEqual(expected, actual) {
 		t.Errorf("expected '%v', got '%v'", expected, actual)