From 3c5e45646450f5d7044e61ea6697e3fa4e75513d Mon Sep 17 00:00:00 2001 From: Ashwin Venkatesh Date: Thu, 25 Mar 2021 15:28:19 -0400 Subject: [PATCH] Make lastSyncedTime a pointer to all nil value (#466) - Reconciling last synced time fails when the sync fails as it tries to set time to a null value. By updating the type of LastSyncedTime to a pointer, it gets assined nil on failure, which can reconcile successfully. --- api/common/configentry.go | 2 +- api/common/configentry_webhook_test.go | 2 +- api/v1alpha1/ingressgateway_types.go | 2 +- api/v1alpha1/ingressgateway_types_test.go | 4 ++-- api/v1alpha1/proxydefaults_types.go | 2 +- api/v1alpha1/proxydefaults_types_test.go | 4 ++-- api/v1alpha1/servicedefaults_types.go | 2 +- api/v1alpha1/servicedefaults_types_test.go | 4 ++-- api/v1alpha1/serviceintentions_types.go | 2 +- api/v1alpha1/serviceintentions_types_test.go | 4 ++-- api/v1alpha1/serviceresolver_types.go | 2 +- api/v1alpha1/serviceresolver_types_test.go | 4 ++-- api/v1alpha1/servicerouter_types.go | 2 +- api/v1alpha1/servicerouter_types_test.go | 4 ++-- api/v1alpha1/servicesplitter_types.go | 2 +- api/v1alpha1/servicesplitter_types_test.go | 4 ++-- api/v1alpha1/status.go | 2 +- api/v1alpha1/terminatinggateway_types.go | 2 +- api/v1alpha1/terminatinggateway_types_test.go | 4 ++-- api/v1alpha1/zz_generated.deepcopy.go | 5 ++++- controller/configentry_controller.go | 3 ++- 21 files changed, 33 insertions(+), 29 deletions(-) diff --git a/api/common/configentry.go b/api/common/configentry.go index ad3abcd68c..eefd52635f 100644 --- a/api/common/configentry.go +++ b/api/common/configentry.go @@ -41,7 +41,7 @@ type ConfigEntryResource interface { // SetSyncedCondition updates the synced condition. SetSyncedCondition(status corev1.ConditionStatus, reason, message string) // SetLastSyncedTime updates the last synced time. - SetLastSyncedTime(time metav1.Time) + SetLastSyncedTime(time *metav1.Time) // SyncedCondition gets the synced condition. SyncedCondition() (status corev1.ConditionStatus, reason, message string) // SyncedConditionStatus returns the status of the synced condition. diff --git a/api/common/configentry_webhook_test.go b/api/common/configentry_webhook_test.go index 264c3a3d01..258f4886de 100644 --- a/api/common/configentry_webhook_test.go +++ b/api/common/configentry_webhook_test.go @@ -306,7 +306,7 @@ func (in *mockConfigEntry) ConsulName() string { func (in *mockConfigEntry) SetSyncedCondition(_ corev1.ConditionStatus, _ string, _ string) {} -func (in *mockConfigEntry) SetLastSyncedTime(_ metav1.Time) {} +func (in *mockConfigEntry) SetLastSyncedTime(_ *metav1.Time) {} func (in *mockConfigEntry) SyncedCondition() (status corev1.ConditionStatus, reason string, message string) { return corev1.ConditionTrue, "", "" diff --git a/api/v1alpha1/ingressgateway_types.go b/api/v1alpha1/ingressgateway_types.go index 20a6f04927..ae1d07d82a 100644 --- a/api/v1alpha1/ingressgateway_types.go +++ b/api/v1alpha1/ingressgateway_types.go @@ -170,7 +170,7 @@ func (in *IngressGateway) SetSyncedCondition(status corev1.ConditionStatus, reas } } -func (in *IngressGateway) SetLastSyncedTime(time metav1.Time) { +func (in *IngressGateway) SetLastSyncedTime(time *metav1.Time) { in.Status.LastSyncedTime = time } diff --git a/api/v1alpha1/ingressgateway_types_test.go b/api/v1alpha1/ingressgateway_types_test.go index fa5ad853f0..985418210f 100644 --- a/api/v1alpha1/ingressgateway_types_test.go +++ b/api/v1alpha1/ingressgateway_types_test.go @@ -583,9 +583,9 @@ func TestIngressGateway_SetSyncedCondition(t *testing.T) { func TestIngressGateway_SetLastSyncedTime(t *testing.T) { ingressGateway := &IngressGateway{} syncedTime := metav1.NewTime(time.Now()) - ingressGateway.SetLastSyncedTime(syncedTime) + ingressGateway.SetLastSyncedTime(&syncedTime) - require.Equal(t, syncedTime, ingressGateway.Status.LastSyncedTime) + require.Equal(t, &syncedTime, ingressGateway.Status.LastSyncedTime) } func TestIngressGateway_GetSyncedConditionStatus(t *testing.T) { diff --git a/api/v1alpha1/proxydefaults_types.go b/api/v1alpha1/proxydefaults_types.go index 37a75d7a5b..fcfc15d0bd 100644 --- a/api/v1alpha1/proxydefaults_types.go +++ b/api/v1alpha1/proxydefaults_types.go @@ -138,7 +138,7 @@ func (in *ProxyDefaults) SetSyncedCondition(status corev1.ConditionStatus, reaso } } -func (in *ProxyDefaults) SetLastSyncedTime(time metav1.Time) { +func (in *ProxyDefaults) SetLastSyncedTime(time *metav1.Time) { in.Status.LastSyncedTime = time } diff --git a/api/v1alpha1/proxydefaults_types_test.go b/api/v1alpha1/proxydefaults_types_test.go index d531441106..e84d3a25f3 100644 --- a/api/v1alpha1/proxydefaults_types_test.go +++ b/api/v1alpha1/proxydefaults_types_test.go @@ -290,9 +290,9 @@ func TestProxyDefaults_SetSyncedCondition(t *testing.T) { func TestProxyDefaults_SetLastSyncedTime(t *testing.T) { proxyDefaults := &ProxyDefaults{} syncedTime := metav1.NewTime(time.Now()) - proxyDefaults.SetLastSyncedTime(syncedTime) + proxyDefaults.SetLastSyncedTime(&syncedTime) - require.Equal(t, syncedTime, proxyDefaults.Status.LastSyncedTime) + require.Equal(t, &syncedTime, proxyDefaults.Status.LastSyncedTime) } func TestProxyDefaults_GetSyncedConditionStatus(t *testing.T) { diff --git a/api/v1alpha1/servicedefaults_types.go b/api/v1alpha1/servicedefaults_types.go index 338666a42c..0b72a1eacd 100644 --- a/api/v1alpha1/servicedefaults_types.go +++ b/api/v1alpha1/servicedefaults_types.go @@ -137,7 +137,7 @@ func (in *ServiceDefaults) SetSyncedCondition(status corev1.ConditionStatus, rea } } -func (in *ServiceDefaults) SetLastSyncedTime(time metav1.Time) { +func (in *ServiceDefaults) SetLastSyncedTime(time *metav1.Time) { in.Status.LastSyncedTime = time } diff --git a/api/v1alpha1/servicedefaults_types_test.go b/api/v1alpha1/servicedefaults_types_test.go index 8d84352505..d7d8415672 100644 --- a/api/v1alpha1/servicedefaults_types_test.go +++ b/api/v1alpha1/servicedefaults_types_test.go @@ -353,9 +353,9 @@ func TestServiceDefaults_SetSyncedCondition(t *testing.T) { func TestServiceDefaults_SetLastSyncedTime(t *testing.T) { serviceDefaults := &ServiceDefaults{} syncedTime := metav1.NewTime(time.Now()) - serviceDefaults.SetLastSyncedTime(syncedTime) + serviceDefaults.SetLastSyncedTime(&syncedTime) - require.Equal(t, syncedTime, serviceDefaults.Status.LastSyncedTime) + require.Equal(t, &syncedTime, serviceDefaults.Status.LastSyncedTime) } func TestServiceDefaults_GetSyncedConditionStatus(t *testing.T) { diff --git a/api/v1alpha1/serviceintentions_types.go b/api/v1alpha1/serviceintentions_types.go index e076d69b63..cbd7bf26d4 100644 --- a/api/v1alpha1/serviceintentions_types.go +++ b/api/v1alpha1/serviceintentions_types.go @@ -189,7 +189,7 @@ func (in *ServiceIntentions) SetSyncedCondition(status corev1.ConditionStatus, r } } -func (in *ServiceIntentions) SetLastSyncedTime(time metav1.Time) { +func (in *ServiceIntentions) SetLastSyncedTime(time *metav1.Time) { in.Status.LastSyncedTime = time } diff --git a/api/v1alpha1/serviceintentions_types_test.go b/api/v1alpha1/serviceintentions_types_test.go index 4f57b69ca4..657fc62697 100644 --- a/api/v1alpha1/serviceintentions_types_test.go +++ b/api/v1alpha1/serviceintentions_types_test.go @@ -386,9 +386,9 @@ func TestServiceIntentions_SetSyncedCondition(t *testing.T) { func TestServiceIntentions_SetLastSyncedTime(t *testing.T) { serviceIntentions := &ServiceIntentions{} syncedTime := metav1.NewTime(time.Now()) - serviceIntentions.SetLastSyncedTime(syncedTime) + serviceIntentions.SetLastSyncedTime(&syncedTime) - require.Equal(t, syncedTime, serviceIntentions.Status.LastSyncedTime) + require.Equal(t, &syncedTime, serviceIntentions.Status.LastSyncedTime) } func TestServiceIntentions_GetSyncedConditionStatus(t *testing.T) { diff --git a/api/v1alpha1/serviceresolver_types.go b/api/v1alpha1/serviceresolver_types.go index 77c84875ed..a3bd4e6c1e 100644 --- a/api/v1alpha1/serviceresolver_types.go +++ b/api/v1alpha1/serviceresolver_types.go @@ -241,7 +241,7 @@ func (in *ServiceResolver) SetSyncedCondition(status corev1.ConditionStatus, rea } } -func (in *ServiceResolver) SetLastSyncedTime(time metav1.Time) { +func (in *ServiceResolver) SetLastSyncedTime(time *metav1.Time) { in.Status.LastSyncedTime = time } diff --git a/api/v1alpha1/serviceresolver_types_test.go b/api/v1alpha1/serviceresolver_types_test.go index e2db6f2187..3d494fb505 100644 --- a/api/v1alpha1/serviceresolver_types_test.go +++ b/api/v1alpha1/serviceresolver_types_test.go @@ -374,9 +374,9 @@ func TestServiceResolver_SetSyncedCondition(t *testing.T) { func TestServiceResolver_SetLastSyncedTime(t *testing.T) { serviceResolver := &ServiceResolver{} syncedTime := metav1.NewTime(time.Now()) - serviceResolver.SetLastSyncedTime(syncedTime) + serviceResolver.SetLastSyncedTime(&syncedTime) - require.Equal(t, syncedTime, serviceResolver.Status.LastSyncedTime) + require.Equal(t, &syncedTime, serviceResolver.Status.LastSyncedTime) } func TestServiceResolver_GetSyncedConditionStatus(t *testing.T) { diff --git a/api/v1alpha1/servicerouter_types.go b/api/v1alpha1/servicerouter_types.go index ad5262975c..cbe83f0f14 100644 --- a/api/v1alpha1/servicerouter_types.go +++ b/api/v1alpha1/servicerouter_types.go @@ -198,7 +198,7 @@ func (in *ServiceRouter) SetSyncedCondition(status corev1.ConditionStatus, reaso } } -func (in *ServiceRouter) SetLastSyncedTime(time metav1.Time) { +func (in *ServiceRouter) SetLastSyncedTime(time *metav1.Time) { in.Status.LastSyncedTime = time } diff --git a/api/v1alpha1/servicerouter_types_test.go b/api/v1alpha1/servicerouter_types_test.go index 4a9f5567f1..e8436d34cb 100644 --- a/api/v1alpha1/servicerouter_types_test.go +++ b/api/v1alpha1/servicerouter_types_test.go @@ -320,9 +320,9 @@ func TestServiceRouter_SetSyncedCondition(t *testing.T) { func TestServiceRouter_SetLastSyncedTime(t *testing.T) { serviceRouter := &ServiceRouter{} syncedTime := metav1.NewTime(time.Now()) - serviceRouter.SetLastSyncedTime(syncedTime) + serviceRouter.SetLastSyncedTime(&syncedTime) - require.Equal(t, syncedTime, serviceRouter.Status.LastSyncedTime) + require.Equal(t, &syncedTime, serviceRouter.Status.LastSyncedTime) } func TestServiceRouter_GetSyncedConditionStatus(t *testing.T) { diff --git a/api/v1alpha1/servicesplitter_types.go b/api/v1alpha1/servicesplitter_types.go index c476e7d024..fb03d0dee3 100644 --- a/api/v1alpha1/servicesplitter_types.go +++ b/api/v1alpha1/servicesplitter_types.go @@ -116,7 +116,7 @@ func (in *ServiceSplitter) SetSyncedCondition(status corev1.ConditionStatus, rea } } -func (in *ServiceSplitter) SetLastSyncedTime(time metav1.Time) { +func (in *ServiceSplitter) SetLastSyncedTime(time *metav1.Time) { in.Status.LastSyncedTime = time } diff --git a/api/v1alpha1/servicesplitter_types_test.go b/api/v1alpha1/servicesplitter_types_test.go index 64402d6c26..1f07569c3b 100644 --- a/api/v1alpha1/servicesplitter_types_test.go +++ b/api/v1alpha1/servicesplitter_types_test.go @@ -187,9 +187,9 @@ func TestServiceSplitter_SetSyncedCondition(t *testing.T) { func TestServiceSplitter_SetLastSyncedTime(t *testing.T) { serviceSplitter := &ServiceSplitter{} syncedTime := metav1.NewTime(time.Now()) - serviceSplitter.SetLastSyncedTime(syncedTime) + serviceSplitter.SetLastSyncedTime(&syncedTime) - require.Equal(t, syncedTime, serviceSplitter.Status.LastSyncedTime) + require.Equal(t, &syncedTime, serviceSplitter.Status.LastSyncedTime) } func TestServiceSplitter_GetSyncedConditionStatus(t *testing.T) { diff --git a/api/v1alpha1/status.go b/api/v1alpha1/status.go index ef43a67f9a..1dd19241f2 100644 --- a/api/v1alpha1/status.go +++ b/api/v1alpha1/status.go @@ -77,7 +77,7 @@ type Status struct { // LastSyncedTime is the last time the resource successfully synced with Consul. // +optional - LastSyncedTime metav1.Time `json:"lastSyncedTime,omitempty" description:"last time the condition transitioned from one status to another"` + LastSyncedTime *metav1.Time `json:"lastSyncedTime,omitempty" description:"last time the condition transitioned from one status to another"` } func (s *Status) GetCondition(t ConditionType) *Condition { diff --git a/api/v1alpha1/terminatinggateway_types.go b/api/v1alpha1/terminatinggateway_types.go index be067e42f3..b55ad642ac 100644 --- a/api/v1alpha1/terminatinggateway_types.go +++ b/api/v1alpha1/terminatinggateway_types.go @@ -134,7 +134,7 @@ func (in *TerminatingGateway) SetSyncedCondition(status corev1.ConditionStatus, } } -func (in *TerminatingGateway) SetLastSyncedTime(time metav1.Time) { +func (in *TerminatingGateway) SetLastSyncedTime(time *metav1.Time) { in.Status.LastSyncedTime = time } diff --git a/api/v1alpha1/terminatinggateway_types_test.go b/api/v1alpha1/terminatinggateway_types_test.go index f11d0c9100..d8446850a3 100644 --- a/api/v1alpha1/terminatinggateway_types_test.go +++ b/api/v1alpha1/terminatinggateway_types_test.go @@ -393,9 +393,9 @@ func TestTerminatingGateway_SetSyncedCondition(t *testing.T) { func TestTerminatingGateway_SetLastSyncedTime(t *testing.T) { terminatingGateway := &TerminatingGateway{} syncedTime := metav1.NewTime(time.Now()) - terminatingGateway.SetLastSyncedTime(syncedTime) + terminatingGateway.SetLastSyncedTime(&syncedTime) - require.Equal(t, syncedTime, terminatingGateway.Status.LastSyncedTime) + require.Equal(t, &syncedTime, terminatingGateway.Status.LastSyncedTime) } func TestTerminatingGateway_GetSyncedConditionStatus(t *testing.T) { diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index aeec9d3775..f709259905 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -1279,7 +1279,10 @@ func (in *Status) DeepCopyInto(out *Status) { (*in)[i].DeepCopyInto(&(*out)[i]) } } - in.LastSyncedTime.DeepCopyInto(&out.LastSyncedTime) + if in.LastSyncedTime != nil { + in, out := &in.LastSyncedTime, &out.LastSyncedTime + *out = (*in).DeepCopy() + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Status. diff --git a/controller/configentry_controller.go b/controller/configentry_controller.go index ab6d8d59a9..e75b4bf4a8 100644 --- a/controller/configentry_controller.go +++ b/controller/configentry_controller.go @@ -279,7 +279,8 @@ func (r *ConfigEntryController) syncFailed(ctx context.Context, logger logr.Logg func (r *ConfigEntryController) syncSuccessful(ctx context.Context, updater Controller, configEntry common.ConfigEntryResource) (ctrl.Result, error) { configEntry.SetSyncedCondition(corev1.ConditionTrue, "", "") - configEntry.SetLastSyncedTime(metav1.NewTime(time.Now())) + timeNow := metav1.NewTime(time.Now()) + configEntry.SetLastSyncedTime(&timeNow) return ctrl.Result{}, updater.UpdateStatus(ctx, configEntry) }