From 87a42b07762241e233a29ed886a533c267746721 Mon Sep 17 00:00:00 2001 From: Shachar Sharon Date: Thu, 11 May 2023 17:47:20 +0300 Subject: [PATCH] resources: refine get-create logic for openshift elems When doing reconcile logic of Get-or-Create for OpenShift's elements, treat AlreadyExists error as transient error and just Get the element once again from the API server. This may happen in cases where there is a skew between your apimachinery and client-go versions[1]. [1] https://github.com/kubernetes/client-go/issues/89 Signed-off-by: Shachar Sharon --- internal/resources/metrics.go | 29 +++++++++----- internal/resources/openshift.go | 71 +++++++++++++++++++++++---------- 2 files changed, 70 insertions(+), 30 deletions(-) diff --git a/internal/resources/metrics.go b/internal/resources/metrics.go index dc3fd6b1..e024ef79 100644 --- a/internal/resources/metrics.go +++ b/internal/resources/metrics.go @@ -124,13 +124,19 @@ func (m *SmbShareManager) getOrCreateMetricsService( } err = m.client.Create(ctx, srvWant, &rtclient.CreateOptions{}) if err != nil { - if errors.IsAlreadyExists(err) { - m.logger.Info("Metrics Service already exists", "key", srvKey) - } else { + if !errors.IsAlreadyExists(err) { m.logger.Error(err, "Failed to create metrics Service", "key", srvKey) + return nil, false, err } - return nil, false, err + m.logger.Info("Retry to get metrics Service", "key", srvKey) + srvCurr, srvKey, err = m.getMetricsServiceOf(ctx, pl, ns) + if err != nil { + m.logger.Error(err, "Failed to get existing metrics Service", + "key", srvKey) + return nil, false, err + } + return srvCurr, false, err } return srvWant, true, nil } @@ -201,14 +207,19 @@ func (m *SmbShareManager) getOrCreateMetricsServiceMonitor( } err = m.client.Create(ctx, smWant, &rtclient.CreateOptions{}) if err != nil { - if errors.IsAlreadyExists(err) { - m.logger.Info("Metrics ServiceMonitor already exists", - "key", smKey) - } else { + if !errors.IsAlreadyExists(err) { m.logger.Error(err, "Failed to create metrics ServiceMonitor", "key", smKey) + return nil, false, err } - return nil, false, err + m.logger.Info("Retry to get metrics ServiceMonitor", "key", smKey) + smCurr, smKey, err = m.getMetricsServiceMonitorOf(ctx, pl, ns) + if err != nil { + m.logger.Error(err, "Failed to get existing metrics ServiceMonitor", + "key", smKey) + return nil, false, err + } + return smCurr, false, nil } return smWant, true, nil } diff --git a/internal/resources/openshift.go b/internal/resources/openshift.go index a80e5343..80eb6205 100644 --- a/internal/resources/openshift.go +++ b/internal/resources/openshift.go @@ -325,13 +325,19 @@ func (m *SmbShareManager) getOrCreateServiceAccountOf( } err = m.client.Create(ctx, saWant, &rtclient.CreateOptions{}) if err != nil { - if errors.IsAlreadyExists(err) { - m.logger.Info("ServiceAccount already exists", "key", saKey) - } else { + if !errors.IsAlreadyExists(err) { m.logger.Error(err, "Failed to create ServiceAccount", "key", saKey) + return nil, false, err } - return nil, false, err + m.logger.Info("Retry to get ServiceAccount", "key", saKey) + saCurr, saKey, err = m.getServiceAccountOf(ctx, smbshare) + if err != nil { + m.logger.Error(err, "Failed to get existing ServiceAccount", + "key", saKey) + return nil, false, err + } + return saCurr, false, err } return saWant, true, nil } @@ -393,12 +399,18 @@ func (m *SmbShareManager) getOrCreateSCCRoleOf( } err = m.client.Create(ctx, roleWant, &rtclient.CreateOptions{}) if err != nil { - if errors.IsAlreadyExists(err) { - m.logger.Info("SCC Role already exists", "key", roleKey) - } else { + if !errors.IsAlreadyExists(err) { m.logger.Error(err, "Failed to create SCC Role", "key", roleKey) + return nil, false, err } - return nil, false, err + m.logger.Info("Retry to get SCC Role", "key", roleKey) + roleCurr, roleKey, err = m.getSCCRoleOf(ctx, smbshare) + if err != nil { + m.logger.Error(err, "Failed to get existing SCC Role", + "key", roleKey) + return nil, false, err + } + return roleCurr, false, err } return roleWant, true, nil } @@ -450,13 +462,19 @@ func (m *SmbShareManager) getOrCreateSCCRoleBindingOf( } err = m.client.Create(ctx, roleBindWant, &rtclient.CreateOptions{}) if err != nil { - if errors.IsAlreadyExists(err) { - m.logger.Info("SCC RoleBinding already exists", "key", roleBindKey) - } else { + if !errors.IsAlreadyExists(err) { m.logger.Error(err, "Failed to create RoleBinding", "key", roleBindKey) + return nil, false, err } - return nil, false, err + m.logger.Info("Retry to get RoleBinding", "key", roleBindKey) + roleBindCurr, roleBindKey, err = m.getSCCRoleBindingOf(ctx, smbshare) + if err != nil { + m.logger.Error(err, "Failed to get existing RoleBinding", + "key", roleBindKey) + return nil, false, err + } + return roleBindCurr, false, nil } return roleBindWant, true, nil } @@ -517,13 +535,19 @@ func (m *SmbShareManager) getOrCreateMetricsRoleOf( } err = m.client.Create(ctx, roleWant, &rtclient.CreateOptions{}) if err != nil { - if errors.IsAlreadyExists(err) { - m.logger.Info("Metrics Role already exists", "key", roleKey) - } else { + if !errors.IsAlreadyExists(err) { m.logger.Error(err, "Failed to create Metrics Role", "key", roleKey) + return nil, false, err } - return nil, false, err + m.logger.Info("Retry to get Metrics Role", "key", roleKey) + roleCurr, roleKey, err = m.getMetricsRoleOf(ctx, smbshare) + if err != nil { + m.logger.Error(err, "Failed to get existing Metrics Role", + "key", roleKey) + return nil, false, err + } + return roleCurr, false, err } return roleWant, true, nil } @@ -576,14 +600,19 @@ func (m *SmbShareManager) getOrCreateMetricsRoleBindingOf( } err = m.client.Create(ctx, roleBindWant, &rtclient.CreateOptions{}) if err != nil { - if errors.IsAlreadyExists(err) { - m.logger.Info("Metrics RoleBinding already exists", - "key", roleBindKey) - } else { + if !errors.IsAlreadyExists(err) { m.logger.Error(err, "Failed to create Metrics RoleBinding", "key", roleBindKey) + return nil, false, err } - return nil, false, err + m.logger.Info("Retry to get Metrics RoleBinding", "key", roleBindKey) + roleBindCurr, roleBindKey, err = m.getMetricsRoleBindingOf(ctx, smbshare) + if err != nil { + m.logger.Error(err, "Failed to get existing Metrics RoleBinding", + "key", roleBindKey) + return nil, false, err + } + return roleBindCurr, false, err } return roleBindWant, true, nil }