(catsrc) add custom unmarshaller for registry poll interval (openshif…

…t#169)

This PR implements a custom unmarshaller for the CatalogSourceSpec.UpdateStrategy.
When a value that cannot be unmarshalled to *metav1.Duration was being passed to
UpdateStrategy.RegistryPoll.Interval, the catalogSource sync loops in the catalog
operator was stuck in an infinite loop instead of backing off and performing other
syncs. The custom unmarshaller tries to unmarshall the input to the field to
*metav1.Duration, and if it fails, it sets a default value of 15m for the field.
Otherwise it accepts the value passed to the field.

Upstream-repository: api
Upstream-commit: 7c97612f258921a973da23fb55dcd77892401be4
Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>

operator-lifecycle-manager.cherrypick

@@ -0,0 +1 @@
+7c97612f258921a973da23fb55dcd77892401be4

staging/api/pkg/operators/v1alpha1/catalogsource_types.go

@@ -1,6 +1,7 @@
 package v1alpha1
 
 import (
+	"encoding/json"
 	"fmt"
 	"github.com/sirupsen/logrus"
 	corev1 "k8s.io/api/core/v1"
@@ -10,8 +11,9 @@ import (
 )
 
 const (
-	CatalogSourceCRDAPIVersion = GroupName + "/" + GroupVersion
-	CatalogSourceKind          = "CatalogSource"
+	CatalogSourceCRDAPIVersion  = GroupName + "/" + GroupVersion
+	CatalogSourceKind           = "CatalogSource"
+	DefaultRegistryPollDuration = 15 * time.Minute
 )
 
 // SourceType indicates the type of backing store for a CatalogSource
@@ -36,6 +38,8 @@ const (
 	CatalogSourceConfigMapError ConditionReason = "ConfigMapError"
 	// CatalogSourceRegistryServerError denotes when there is an issue querying the specified registry server.
 	CatalogSourceRegistryServerError ConditionReason = "RegistryServerError"
+	// CatalogSourceIntervalInvalidError denotes if the registry polling interval is invalid.
+	CatalogSourceIntervalInvalidError ConditionReason = "InvalidIntervalError"
 )
 
 type CatalogSourceSpec struct {
@@ -119,7 +123,32 @@ type RegistryPoll struct {
 	// Interval is used to determine the time interval between checks of the latest catalog source version.
 	// The catalog operator polls to see if a new version of the catalog source is available.
 	// If available, the latest image is pulled and gRPC traffic is directed to the latest catalog source.
-	Interval *metav1.Duration `json:"interval,omitempty"`
+	RawInterval  string           `json:"interval,omitempty"`
+	Interval     *metav1.Duration `json:"-"`
+	ParsingError string           `json:"-"`
+}
+
+// UnmarshalJSON implements the encoding/json.Unmarshaler interface.
+func (u *UpdateStrategy) UnmarshalJSON(data []byte) (err error) {
+	type alias struct {
+		*RegistryPoll `json:"registryPoll,omitempty"`
+	}
+	us := alias{}
+	if err = json.Unmarshal(data, &us); err != nil {
+		return err
+	}
+	registryPoll := &RegistryPoll{
+		RawInterval: us.RegistryPoll.RawInterval,
+	}
+	duration, err := time.ParseDuration(registryPoll.RawInterval)
+	if err != nil {
+		registryPoll.ParsingError = fmt.Sprintf("error parsing spec.updateStrategy.registryPoll.interval. Using the default value of %s instead. Error: %s", DefaultRegistryPollDuration, err)
+		registryPoll.Interval = &metav1.Duration{Duration: DefaultRegistryPollDuration}
+	} else {
+		registryPoll.Interval = &metav1.Duration{Duration: duration}
+	}
+	u.RegistryPoll = registryPoll
+	return nil
 }
 
 type RegistryServiceStatus struct {

staging/api/pkg/operators/v1alpha1/types_test.go

@@ -1,6 +1,8 @@
 package v1alpha1
 
 import (
+	"encoding/json"
+	"fmt"
 	"sort"
 	"testing"
 	"time"
@@ -208,3 +210,72 @@ func TestCatalogSource_Poll(t *testing.T) {
 		require.Equal(t, tt.result, table[i].catsrc.Poll(), table[i].description)
 	}
 }
+
+func TestUpdateStrategyUnmarshal(t *testing.T) {
+	type TestStruct struct {
+		UpdateStrategy UpdateStrategy `json:"updateStrategy,omitempty"`
+	}
+	validDuration, err := time.ParseDuration("45m")
+	if err != nil {
+		panic(fmt.Errorf("error parsing duration: %s", err))
+	}
+	defaultDuration, err := time.ParseDuration("15m")
+	if err != nil {
+		panic(fmt.Errorf("error parsing duration: %s", err))
+	}
+	tests := []struct {
+		name string
+		in   []byte
+		out  TestStruct
+		err  error
+	}{
+		{
+			name: "valid",
+			in:   []byte(`{"UpdateStrategy": {"registryPoll":{"interval":"45m"}}}`),
+			out: TestStruct{
+				UpdateStrategy{
+					&RegistryPoll{
+						RawInterval:  "45m",
+						Interval:     &metav1.Duration{Duration: validDuration},
+						ParsingError: "",
+					},
+				},
+			},
+		},
+		{
+			name: "invalid",
+			in:   []byte(`{"UpdateStrategy": {"registryPoll":{"interval":"19mError Code"}}}`),
+			out: TestStruct{
+				UpdateStrategy{
+					&RegistryPoll{
+						RawInterval:  "19mError Code",
+						Interval:     &metav1.Duration{Duration: defaultDuration},
+						ParsingError: "error parsing spec.updateStrategy.registryPoll.interval. Using the default value of 15m0s instead. Error: time: unknown unit \"mError Code\" in duration \"19mError Code\"",
+					},
+				},
+			},
+		},
+		{
+			name: "empty",
+			in:   []byte(`{"UpdateStrategy": {"registryPoll":{"interval":""}}}`),
+			out: TestStruct{
+				UpdateStrategy{
+					&RegistryPoll{
+						Interval:     &metav1.Duration{Duration: defaultDuration},
+						ParsingError: "error parsing spec.updateStrategy.registryPoll.interval. Using the default value of 15m0s instead. Error: time: invalid duration \"\"",
+					},
+				},
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			s := TestStruct{}
+			err := json.Unmarshal(tt.in, &s)
+			require.Equal(t, tt.out.UpdateStrategy.RawInterval, s.UpdateStrategy.RawInterval)
+			require.Equal(t, tt.out.UpdateStrategy.Interval, s.UpdateStrategy.Interval)
+			require.Equal(t, tt.out.UpdateStrategy.ParsingError, s.UpdateStrategy.ParsingError)
+			require.Equal(t, tt.err, err)
+		})
+	}
+}