From 052eb7d9a59d96c0d94bcef13bf0fd54f20c3471 Mon Sep 17 00:00:00 2001 From: Xianglin Gao Date: Wed, 10 Jun 2020 00:12:25 +0800 Subject: [PATCH 1/3] Add retries for CreateOrUpdateRole Signed-off-by: Xianglin Gao --- cmd/kubeadm/app/constants/constants.go | 4 ++++ cmd/kubeadm/app/util/apiclient/idempotency.go | 18 ++++++++++-------- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/cmd/kubeadm/app/constants/constants.go b/cmd/kubeadm/app/constants/constants.go index 99aacc28b08..8051feed06b 100644 --- a/cmd/kubeadm/app/constants/constants.go +++ b/cmd/kubeadm/app/constants/constants.go @@ -185,6 +185,10 @@ const ( TLSBootstrapTimeout = 5 * time.Minute // TLSBootstrapRetryInterval specifies how long kubeadm should wait before retrying the TLS Bootstrap check TLSBootstrapRetryInterval = 5 * time.Second + // APICallWithWriteTimeout specifies how long kubeadm should wait for api calls with at least one write + APICallWithWriteTimeout = 40 * time.Second + // APICallWithReadTimeout specifies how long kubeadm should wait for api calls with only reads + APICallWithReadTimeout = 15 * time.Second // PullImageRetry specifies how many times ContainerRuntime retries when pulling image failed PullImageRetry = 5 diff --git a/cmd/kubeadm/app/util/apiclient/idempotency.go b/cmd/kubeadm/app/util/apiclient/idempotency.go index e8ffee8c06c..f893c352cec 100644 --- a/cmd/kubeadm/app/util/apiclient/idempotency.go +++ b/cmd/kubeadm/app/util/apiclient/idempotency.go @@ -205,16 +205,18 @@ func DeleteDeploymentForeground(client clientset.Interface, namespace, name stri // CreateOrUpdateRole creates a Role if the target resource doesn't exist. If the resource exists already, this function will update the resource instead. func CreateOrUpdateRole(client clientset.Interface, role *rbac.Role) error { - if _, err := client.RbacV1().Roles(role.ObjectMeta.Namespace).Create(context.TODO(), role, metav1.CreateOptions{}); err != nil { - if !apierrors.IsAlreadyExists(err) { - return errors.Wrap(err, "unable to create RBAC role") - } + return wait.PollImmediate(constants.APICallRetryInterval, constants.APICallWithWriteTimeout, func() (bool, error) { + if _, err := client.RbacV1().Roles(role.ObjectMeta.Namespace).Create(context.TODO(), role, metav1.CreateOptions{}); err != nil { + if !apierrors.IsAlreadyExists(err) { + return false, errors.Wrap(err, "unable to create RBAC role") + } - if _, err := client.RbacV1().Roles(role.ObjectMeta.Namespace).Update(context.TODO(), role, metav1.UpdateOptions{}); err != nil { - return errors.Wrap(err, "unable to update RBAC role") + if _, err := client.RbacV1().Roles(role.ObjectMeta.Namespace).Update(context.TODO(), role, metav1.UpdateOptions{}); err != nil { + return false, errors.Wrap(err, "unable to update RBAC role") + } } - } - return nil + return true, nil + }) } // CreateOrUpdateRoleBinding creates a RoleBinding if the target resource doesn't exist. If the resource exists already, this function will update the resource instead. From 6d572ea9b76b284ba3e864a08d09f8872e3bde65 Mon Sep 17 00:00:00 2001 From: Xianglin Gao Date: Wed, 10 Jun 2020 00:23:46 +0800 Subject: [PATCH 2/3] Add retries for CreateOrUpdateRoleBinding Signed-off-by: Xianglin Gao --- cmd/kubeadm/app/util/apiclient/idempotency.go | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/cmd/kubeadm/app/util/apiclient/idempotency.go b/cmd/kubeadm/app/util/apiclient/idempotency.go index f893c352cec..f47830bba6a 100644 --- a/cmd/kubeadm/app/util/apiclient/idempotency.go +++ b/cmd/kubeadm/app/util/apiclient/idempotency.go @@ -221,16 +221,18 @@ func CreateOrUpdateRole(client clientset.Interface, role *rbac.Role) error { // CreateOrUpdateRoleBinding creates a RoleBinding if the target resource doesn't exist. If the resource exists already, this function will update the resource instead. func CreateOrUpdateRoleBinding(client clientset.Interface, roleBinding *rbac.RoleBinding) error { - if _, err := client.RbacV1().RoleBindings(roleBinding.ObjectMeta.Namespace).Create(context.TODO(), roleBinding, metav1.CreateOptions{}); err != nil { - if !apierrors.IsAlreadyExists(err) { - return errors.Wrap(err, "unable to create RBAC rolebinding") - } + return wait.PollImmediate(constants.APICallRetryInterval, constants.APICallWithWriteTimeout, func() (bool, error) { + if _, err := client.RbacV1().RoleBindings(roleBinding.ObjectMeta.Namespace).Create(context.TODO(), roleBinding, metav1.CreateOptions{}); err != nil { + if !apierrors.IsAlreadyExists(err) { + return false, errors.Wrap(err, "unable to create RBAC rolebinding") + } - if _, err := client.RbacV1().RoleBindings(roleBinding.ObjectMeta.Namespace).Update(context.TODO(), roleBinding, metav1.UpdateOptions{}); err != nil { - return errors.Wrap(err, "unable to update RBAC rolebinding") + if _, err := client.RbacV1().RoleBindings(roleBinding.ObjectMeta.Namespace).Update(context.TODO(), roleBinding, metav1.UpdateOptions{}); err != nil { + return false, errors.Wrap(err, "unable to update RBAC rolebinding") + } } - } - return nil + return true, nil + }) } // CreateOrUpdateClusterRole creates a ClusterRole if the target resource doesn't exist. If the resource exists already, this function will update the resource instead. From 04ef3628e3b1fab1c07128f3b37cb3781ec9a85c Mon Sep 17 00:00:00 2001 From: Xianglin Gao Date: Wed, 10 Jun 2020 22:52:28 +0800 Subject: [PATCH 3/3] refact CreateOrMutateConfigMap and MutateConfigMap with PollImmediate Signed-off-by: Xianglin Gao --- cmd/kubeadm/app/util/apiclient/idempotency.go | 56 +++++++++++-------- 1 file changed, 33 insertions(+), 23 deletions(-) diff --git a/cmd/kubeadm/app/util/apiclient/idempotency.go b/cmd/kubeadm/app/util/apiclient/idempotency.go index f47830bba6a..208028ac7d1 100644 --- a/cmd/kubeadm/app/util/apiclient/idempotency.go +++ b/cmd/kubeadm/app/util/apiclient/idempotency.go @@ -20,7 +20,6 @@ import ( "context" "encoding/json" "fmt" - "time" "github.com/pkg/errors" @@ -62,12 +61,7 @@ func CreateOrUpdateConfigMap(client clientset.Interface, cm *v1.ConfigMap) error // taking place) func CreateOrMutateConfigMap(client clientset.Interface, cm *v1.ConfigMap, mutator ConfigMapMutator) error { var lastError error - err := wait.ExponentialBackoff(wait.Backoff{ - Steps: 20, - Duration: 500 * time.Millisecond, - Factor: 1.0, - Jitter: 0.1, - }, func() (bool, error) { + err := wait.PollImmediate(constants.APICallRetryInterval, constants.APICallWithWriteTimeout, func() (bool, error) { if _, err := client.CoreV1().ConfigMaps(cm.ObjectMeta.Namespace).Create(context.TODO(), cm, metav1.CreateOptions{}); err != nil { lastError = err if apierrors.IsAlreadyExists(err) { @@ -89,22 +83,24 @@ func CreateOrMutateConfigMap(client clientset.Interface, cm *v1.ConfigMap, mutat // to conflicts, and a retry will be issued if the ConfigMap was modified on the server between the refresh and the update (while the mutation was // taking place). func MutateConfigMap(client clientset.Interface, meta metav1.ObjectMeta, mutator ConfigMapMutator) error { - return clientsetretry.RetryOnConflict(wait.Backoff{ - Steps: 20, - Duration: 500 * time.Millisecond, - Factor: 1.0, - Jitter: 0.1, - }, func() error { + var lastError error + err := wait.PollImmediate(constants.APICallRetryInterval, constants.APICallWithWriteTimeout, func() (bool, error) { configMap, err := client.CoreV1().ConfigMaps(meta.Namespace).Get(context.TODO(), meta.Name, metav1.GetOptions{}) if err != nil { - return err + lastError = err + return false, nil } if err = mutator(configMap); err != nil { - return errors.Wrap(err, "unable to mutate ConfigMap") + lastError = errors.Wrap(err, "unable to mutate ConfigMap") + return false, nil } - _, err = client.CoreV1().ConfigMaps(configMap.ObjectMeta.Namespace).Update(context.TODO(), configMap, metav1.UpdateOptions{}) - return err + _, lastError = client.CoreV1().ConfigMaps(configMap.ObjectMeta.Namespace).Update(context.TODO(), configMap, metav1.UpdateOptions{}) + return lastError == nil, nil }) + if err == nil { + return nil + } + return lastError } // CreateOrRetainConfigMap creates a ConfigMap if the target resource doesn't exist. If the resource exists already, this function will retain the resource instead. @@ -205,34 +201,48 @@ func DeleteDeploymentForeground(client clientset.Interface, namespace, name stri // CreateOrUpdateRole creates a Role if the target resource doesn't exist. If the resource exists already, this function will update the resource instead. func CreateOrUpdateRole(client clientset.Interface, role *rbac.Role) error { - return wait.PollImmediate(constants.APICallRetryInterval, constants.APICallWithWriteTimeout, func() (bool, error) { + var lastError error + err := wait.PollImmediate(constants.APICallRetryInterval, constants.APICallWithWriteTimeout, func() (bool, error) { if _, err := client.RbacV1().Roles(role.ObjectMeta.Namespace).Create(context.TODO(), role, metav1.CreateOptions{}); err != nil { if !apierrors.IsAlreadyExists(err) { - return false, errors.Wrap(err, "unable to create RBAC role") + lastError = errors.Wrap(err, "unable to create RBAC role") + return false, nil } if _, err := client.RbacV1().Roles(role.ObjectMeta.Namespace).Update(context.TODO(), role, metav1.UpdateOptions{}); err != nil { - return false, errors.Wrap(err, "unable to update RBAC role") + lastError = errors.Wrap(err, "unable to update RBAC role") + return false, nil } } return true, nil }) + if err == nil { + return nil + } + return lastError } // CreateOrUpdateRoleBinding creates a RoleBinding if the target resource doesn't exist. If the resource exists already, this function will update the resource instead. func CreateOrUpdateRoleBinding(client clientset.Interface, roleBinding *rbac.RoleBinding) error { - return wait.PollImmediate(constants.APICallRetryInterval, constants.APICallWithWriteTimeout, func() (bool, error) { + var lastError error + err := wait.PollImmediate(constants.APICallRetryInterval, constants.APICallWithWriteTimeout, func() (bool, error) { if _, err := client.RbacV1().RoleBindings(roleBinding.ObjectMeta.Namespace).Create(context.TODO(), roleBinding, metav1.CreateOptions{}); err != nil { if !apierrors.IsAlreadyExists(err) { - return false, errors.Wrap(err, "unable to create RBAC rolebinding") + lastError = errors.Wrap(err, "unable to create RBAC rolebinding") + return false, nil } if _, err := client.RbacV1().RoleBindings(roleBinding.ObjectMeta.Namespace).Update(context.TODO(), roleBinding, metav1.UpdateOptions{}); err != nil { - return false, errors.Wrap(err, "unable to update RBAC rolebinding") + lastError = errors.Wrap(err, "unable to update RBAC rolebinding") + return false, nil } } return true, nil }) + if err == nil { + return nil + } + return lastError } // CreateOrUpdateClusterRole creates a ClusterRole if the target resource doesn't exist. If the resource exists already, this function will update the resource instead.