From 3baee54831976384eeddd6da88f3e5dea7895999 Mon Sep 17 00:00:00 2001 From: Anik Bhattacharjee Date: Wed, 19 Jan 2022 12:53:02 -0500 Subject: [PATCH] (catsrc) add custom unmarshaller for registry poll interval (#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 --- operator-lifecycle-manager.cherrypick | 1 + .../operators/v1alpha1/catalogsource_types.go | 35 ++++++++- .../api/pkg/operators/v1alpha1/types_test.go | 71 +++++++++++++++++++ .../operators/v1alpha1/catalogsource_types.go | 35 ++++++++- 4 files changed, 136 insertions(+), 6 deletions(-) create mode 100644 operator-lifecycle-manager.cherrypick diff --git a/operator-lifecycle-manager.cherrypick b/operator-lifecycle-manager.cherrypick new file mode 100644 index 0000000000..c6f3f13390 --- /dev/null +++ b/operator-lifecycle-manager.cherrypick @@ -0,0 +1 @@ +7c97612f258921a973da23fb55dcd77892401be4 diff --git a/staging/api/pkg/operators/v1alpha1/catalogsource_types.go b/staging/api/pkg/operators/v1alpha1/catalogsource_types.go index e94618f9c7..726f8e1aa1 100644 --- a/staging/api/pkg/operators/v1alpha1/catalogsource_types.go +++ b/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 { diff --git a/staging/api/pkg/operators/v1alpha1/types_test.go b/staging/api/pkg/operators/v1alpha1/types_test.go index 93df990926..966ae1f182 100644 --- a/staging/api/pkg/operators/v1alpha1/types_test.go +++ b/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) + }) + } +} diff --git a/vendor/github.com/operator-framework/api/pkg/operators/v1alpha1/catalogsource_types.go b/vendor/github.com/operator-framework/api/pkg/operators/v1alpha1/catalogsource_types.go index e94618f9c7..726f8e1aa1 100644 --- a/vendor/github.com/operator-framework/api/pkg/operators/v1alpha1/catalogsource_types.go +++ b/vendor/github.com/operator-framework/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 {