From c29450eb0090e53db16c37a6468a57f6b916f827 Mon Sep 17 00:00:00 2001 From: "Lubomir I. Ivanov" Date: Tue, 13 Feb 2024 17:55:30 +0200 Subject: [PATCH] kubeadm: apply retries to all API calls in idempotency.go The idempotency.go (perhaps not so accurately named) contains API calls that kubeadm does against an API server using client-go. Some users seem to have unstable setups where for unknown reasons the API server can be unavailable or refuse to respond as expected. Use PollUntilContextTimeout in all exported functions to ensure such API calls are all retry-able. NOTE: The context passed to PollUntilContextTimeout is not propagated in the polled function. Instead the poll function creates it's own context 'ctx := context.Background()', this is to avoid breaking expectations on the side of the callers, that expect a certain type of error and not "context timeout" errors. Additional changes: - Make all context.TODO() -> context.Background() - Update all unit tests and make sure during testing the retry interval and timeout are short. Test coverage of idempotency.go is at ~97%. - Remove the TestMutateConfigMapWithConflict test. It does not contribute much, because conflict handling is done at the API, server side, not on the side of kubeadm. This simulating this is not needed. --- .../app/phases/addons/proxy/proxy_test.go | 9 + .../clusterinfo/clusterinfo_test.go | 11 + cmd/kubeadm/app/util/apiclient/idempotency.go | 329 +++-- .../app/util/apiclient/idempotency_test.go | 1174 +++++++++++++---- 4 files changed, 1117 insertions(+), 406 deletions(-) diff --git a/cmd/kubeadm/app/phases/addons/proxy/proxy_test.go b/cmd/kubeadm/app/phases/addons/proxy/proxy_test.go index 0f6ab130348..518e36e858f 100644 --- a/cmd/kubeadm/app/phases/addons/proxy/proxy_test.go +++ b/cmd/kubeadm/app/phases/addons/proxy/proxy_test.go @@ -22,6 +22,7 @@ import ( "os" "strings" "testing" + "time" "github.com/lithammer/dedent" @@ -114,6 +115,14 @@ func TestEnsureProxyAddon(t *testing.T) { }, } + // Override the default timeouts to be shorter + defaultTimeouts := kubeadmapi.GetActiveTimeouts() + defaultAPICallTimeout := defaultTimeouts.KubernetesAPICall + defaultTimeouts.KubernetesAPICall = &metav1.Duration{Duration: time.Microsecond * 500} + defer func() { + defaultTimeouts.KubernetesAPICall = defaultAPICallTimeout + }() + for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { // Create a fake client and set up default test configuration diff --git a/cmd/kubeadm/app/phases/bootstraptoken/clusterinfo/clusterinfo_test.go b/cmd/kubeadm/app/phases/bootstraptoken/clusterinfo/clusterinfo_test.go index 7bc4120a338..b352407d8af 100644 --- a/cmd/kubeadm/app/phases/bootstraptoken/clusterinfo/clusterinfo_test.go +++ b/cmd/kubeadm/app/phases/bootstraptoken/clusterinfo/clusterinfo_test.go @@ -21,6 +21,7 @@ import ( "os" "testing" "text/template" + "time" rbac "k8s.io/api/rbac/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -31,6 +32,8 @@ import ( clientsetfake "k8s.io/client-go/kubernetes/fake" core "k8s.io/client-go/testing" bootstrapapi "k8s.io/cluster-bootstrap/token/api" + + kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm" ) var testConfigTempl = template.Must(template.New("test").Parse(`apiVersion: v1 @@ -104,6 +107,14 @@ func TestCreateBootstrapConfigMapIfNotExists(t *testing.T) { t.Fatalf("could not close tempfile: %v", err) } + // Override the default timeouts to be shorter + defaultTimeouts := kubeadmapi.GetActiveTimeouts() + defaultAPICallTimeout := defaultTimeouts.KubernetesAPICall + defaultTimeouts.KubernetesAPICall = &metav1.Duration{Duration: time.Microsecond * 500} + defer func() { + defaultTimeouts.KubernetesAPICall = defaultAPICallTimeout + }() + for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { client := clientsetfake.NewSimpleClientset() diff --git a/cmd/kubeadm/app/util/apiclient/idempotency.go b/cmd/kubeadm/app/util/apiclient/idempotency.go index ebab533fe31..bbe9862fa2a 100644 --- a/cmd/kubeadm/app/util/apiclient/idempotency.go +++ b/cmd/kubeadm/app/util/apiclient/idempotency.go @@ -40,20 +40,34 @@ import ( // ConfigMapMutator is a function that mutates the given ConfigMap and optionally returns an error type ConfigMapMutator func(*v1.ConfigMap) error +// apiCallRetryInterval holds a local copy of apiCallRetryInterval for testing purposes +var apiCallRetryInterval = constants.KubernetesAPICallRetryInterval + // TODO: We should invent a dynamic mechanism for this using the dynamic client instead of hard-coding these functions per-type // CreateOrUpdateConfigMap creates a ConfigMap if the target resource doesn't exist. If the resource exists already, this function will update the resource instead. func CreateOrUpdateConfigMap(client clientset.Interface, cm *v1.ConfigMap) error { - if _, err := client.CoreV1().ConfigMaps(cm.ObjectMeta.Namespace).Create(context.TODO(), cm, metav1.CreateOptions{}); err != nil { - if !apierrors.IsAlreadyExists(err) { - return errors.Wrap(err, "unable to create ConfigMap") - } - - if _, err := client.CoreV1().ConfigMaps(cm.ObjectMeta.Namespace).Update(context.TODO(), cm, metav1.UpdateOptions{}); err != nil { - return errors.Wrap(err, "unable to update ConfigMap") - } + var lastError error + err := wait.PollUntilContextTimeout(context.Background(), + apiCallRetryInterval, kubeadmapi.GetActiveTimeouts().KubernetesAPICall.Duration, + true, func(_ context.Context) (bool, error) { + ctx := context.Background() + if _, err := client.CoreV1().ConfigMaps(cm.ObjectMeta.Namespace).Create(ctx, cm, metav1.CreateOptions{}); err != nil { + if !apierrors.IsAlreadyExists(err) { + lastError = errors.Wrap(err, "unable to create ConfigMap") + return false, nil + } + if _, err := client.CoreV1().ConfigMaps(cm.ObjectMeta.Namespace).Update(ctx, cm, metav1.UpdateOptions{}); err != nil { + lastError = errors.Wrap(err, "unable to update ConfigMap") + return false, nil + } + } + return true, nil + }) + if err == nil { + return nil } - return nil + return lastError } // CreateOrMutateConfigMap tries to create the ConfigMap provided as cm. If the resource exists already, the latest version will be fetched from @@ -63,12 +77,12 @@ func CreateOrUpdateConfigMap(client clientset.Interface, cm *v1.ConfigMap) error func CreateOrMutateConfigMap(client clientset.Interface, cm *v1.ConfigMap, mutator ConfigMapMutator) error { var lastError error err := wait.PollUntilContextTimeout(context.Background(), - constants.KubernetesAPICallRetryInterval, kubeadmapi.GetActiveTimeouts().KubernetesAPICall.Duration, + apiCallRetryInterval, kubeadmapi.GetActiveTimeouts().KubernetesAPICall.Duration, true, func(_ context.Context) (bool, error) { - if _, err := client.CoreV1().ConfigMaps(cm.ObjectMeta.Namespace).Create(context.TODO(), cm, metav1.CreateOptions{}); err != nil { + if _, err := client.CoreV1().ConfigMaps(cm.ObjectMeta.Namespace).Create(context.Background(), cm, metav1.CreateOptions{}); err != nil { lastError = err if apierrors.IsAlreadyExists(err) { - lastError = MutateConfigMap(client, metav1.ObjectMeta{Namespace: cm.ObjectMeta.Namespace, Name: cm.ObjectMeta.Name}, mutator) + lastError = mutateConfigMap(client, metav1.ObjectMeta{Namespace: cm.ObjectMeta.Namespace, Name: cm.ObjectMeta.Name}, mutator) return lastError == nil, nil } return false, nil @@ -81,26 +95,41 @@ func CreateOrMutateConfigMap(client clientset.Interface, cm *v1.ConfigMap, mutat return lastError } -// MutateConfigMap takes a ConfigMap Object Meta (namespace and name), retrieves the resource from the server and tries to mutate it +// mutateConfigMap takes a ConfigMap Object Meta (namespace and name), retrieves the resource from the server and tries to mutate it // by calling to the mutator callback, then an Update of the mutated ConfigMap will be performed. This function is resilient // 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 { +func mutateConfigMap(client clientset.Interface, meta metav1.ObjectMeta, mutator ConfigMapMutator) error { + ctx := context.Background() + configMap, err := client.CoreV1().ConfigMaps(meta.Namespace).Get(ctx, meta.Name, metav1.GetOptions{}) + if err != nil { + return errors.Wrap(err, "unable to get ConfigMap") + } + if err = mutator(configMap); err != nil { + return errors.Wrap(err, "unable to mutate ConfigMap") + } + _, err = client.CoreV1().ConfigMaps(configMap.ObjectMeta.Namespace).Update(ctx, configMap, metav1.UpdateOptions{}) + return err +} + +// CreateOrRetainConfigMap creates a ConfigMap if the target resource doesn't exist. If the resource exists already, this function will retain the resource instead. +func CreateOrRetainConfigMap(client clientset.Interface, cm *v1.ConfigMap, configMapName string) error { var lastError error err := wait.PollUntilContextTimeout(context.Background(), - constants.KubernetesAPICallRetryInterval, kubeadmapi.GetActiveTimeouts().KubernetesAPICall.Duration, + apiCallRetryInterval, kubeadmapi.GetActiveTimeouts().KubernetesAPICall.Duration, true, func(_ context.Context) (bool, error) { - configMap, err := client.CoreV1().ConfigMaps(meta.Namespace).Get(context.TODO(), meta.Name, metav1.GetOptions{}) - if err != nil { - lastError = err - return false, nil + ctx := context.Background() + if _, err := client.CoreV1().ConfigMaps(cm.ObjectMeta.Namespace).Get(ctx, configMapName, metav1.GetOptions{}); err != nil { + if !apierrors.IsNotFound(err) { + lastError = errors.Wrap(err, "unable to get ConfigMap") + return false, nil + } + if _, err := client.CoreV1().ConfigMaps(cm.ObjectMeta.Namespace).Create(ctx, cm, metav1.CreateOptions{}); err != nil { + lastError = errors.Wrap(err, "unable to create ConfigMap") + return false, nil + } } - if err = mutator(configMap); err != nil { - lastError = errors.Wrap(err, "unable to mutate ConfigMap") - return false, nil - } - _, lastError = client.CoreV1().ConfigMaps(configMap.ObjectMeta.Namespace).Update(context.TODO(), configMap, metav1.UpdateOptions{}) - return lastError == nil, nil + return true, nil }) if err == nil { return nil @@ -108,104 +137,147 @@ func MutateConfigMap(client clientset.Interface, meta metav1.ObjectMeta, mutator 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. -func CreateOrRetainConfigMap(client clientset.Interface, cm *v1.ConfigMap, configMapName string) error { - if _, err := client.CoreV1().ConfigMaps(cm.ObjectMeta.Namespace).Get(context.TODO(), configMapName, metav1.GetOptions{}); err != nil { - if !apierrors.IsNotFound(err) { - return nil - } - if _, err := client.CoreV1().ConfigMaps(cm.ObjectMeta.Namespace).Create(context.TODO(), cm, metav1.CreateOptions{}); err != nil { - if !apierrors.IsAlreadyExists(err) { - return errors.Wrap(err, "unable to create ConfigMap") - } - } - } - return nil -} - // CreateOrUpdateSecret creates a Secret if the target resource doesn't exist. If the resource exists already, this function will update the resource instead. func CreateOrUpdateSecret(client clientset.Interface, secret *v1.Secret) error { - if _, err := client.CoreV1().Secrets(secret.ObjectMeta.Namespace).Create(context.TODO(), secret, metav1.CreateOptions{}); err != nil { - if !apierrors.IsAlreadyExists(err) { - return errors.Wrap(err, "unable to create secret") - } - - if _, err := client.CoreV1().Secrets(secret.ObjectMeta.Namespace).Update(context.TODO(), secret, metav1.UpdateOptions{}); err != nil { - return errors.Wrap(err, "unable to update secret") - } + var lastError error + err := wait.PollUntilContextTimeout(context.Background(), + apiCallRetryInterval, kubeadmapi.GetActiveTimeouts().KubernetesAPICall.Duration, + true, func(_ context.Context) (bool, error) { + ctx := context.Background() + if _, err := client.CoreV1().Secrets(secret.ObjectMeta.Namespace).Create(ctx, secret, metav1.CreateOptions{}); err != nil { + if !apierrors.IsAlreadyExists(err) { + lastError = errors.Wrap(err, "unable to create Secret") + return false, nil + } + if _, err := client.CoreV1().Secrets(secret.ObjectMeta.Namespace).Update(ctx, secret, metav1.UpdateOptions{}); err != nil { + lastError = errors.Wrap(err, "unable to update Secret") + return false, nil + } + } + return true, nil + }) + if err == nil { + return nil } - return nil + return lastError } // CreateOrUpdateServiceAccount creates a ServiceAccount if the target resource doesn't exist. If the resource exists already, this function will update the resource instead. func CreateOrUpdateServiceAccount(client clientset.Interface, sa *v1.ServiceAccount) error { - if _, err := client.CoreV1().ServiceAccounts(sa.ObjectMeta.Namespace).Create(context.TODO(), sa, metav1.CreateOptions{}); err != nil { - // Note: We don't run .Update here afterwards as that's probably not required - // Only thing that could be updated is annotations/labels in .metadata, but we don't use that currently - if !apierrors.IsAlreadyExists(err) { - return errors.Wrap(err, "unable to create serviceaccount") - } + var lastError error + err := wait.PollUntilContextTimeout(context.Background(), + apiCallRetryInterval, kubeadmapi.GetActiveTimeouts().KubernetesAPICall.Duration, + true, func(_ context.Context) (bool, error) { + ctx := context.Background() + if _, err := client.CoreV1().ServiceAccounts(sa.ObjectMeta.Namespace).Create(ctx, sa, metav1.CreateOptions{}); err != nil { + if !apierrors.IsAlreadyExists(err) { + lastError = errors.Wrap(err, "unable to create ServicAccount") + return false, nil + } + if _, err := client.CoreV1().ServiceAccounts(sa.ObjectMeta.Namespace).Update(ctx, sa, metav1.UpdateOptions{}); err != nil { + lastError = errors.Wrap(err, "unable to update ServicAccount") + return false, nil + } + } + return true, nil + }) + if err == nil { + return nil } - return nil + return lastError } // CreateOrUpdateDeployment creates a Deployment if the target resource doesn't exist. If the resource exists already, this function will update the resource instead. func CreateOrUpdateDeployment(client clientset.Interface, deploy *apps.Deployment) error { - if _, err := client.AppsV1().Deployments(deploy.ObjectMeta.Namespace).Create(context.TODO(), deploy, metav1.CreateOptions{}); err != nil { - if !apierrors.IsAlreadyExists(err) { - return errors.Wrap(err, "unable to create deployment") - } - - if _, err := client.AppsV1().Deployments(deploy.ObjectMeta.Namespace).Update(context.TODO(), deploy, metav1.UpdateOptions{}); err != nil { - return errors.Wrap(err, "unable to update deployment") - } + var lastError error + err := wait.PollUntilContextTimeout(context.Background(), + apiCallRetryInterval, kubeadmapi.GetActiveTimeouts().KubernetesAPICall.Duration, + true, func(_ context.Context) (bool, error) { + ctx := context.Background() + if _, err := client.AppsV1().Deployments(deploy.ObjectMeta.Namespace).Create(ctx, deploy, metav1.CreateOptions{}); err != nil { + if !apierrors.IsAlreadyExists(err) { + lastError = errors.Wrap(err, "unable to create Deployment") + return false, nil + } + if _, err := client.AppsV1().Deployments(deploy.ObjectMeta.Namespace).Update(ctx, deploy, metav1.UpdateOptions{}); err != nil { + lastError = errors.Wrap(err, "unable to update Deployment") + return false, nil + } + } + return true, nil + }) + if err == nil { + return nil } - return nil + return lastError } // CreateOrRetainDeployment creates a Deployment if the target resource doesn't exist. If the resource exists already, this function will retain the resource instead. func CreateOrRetainDeployment(client clientset.Interface, deploy *apps.Deployment, deployName string) error { - if _, err := client.AppsV1().Deployments(deploy.ObjectMeta.Namespace).Get(context.TODO(), deployName, metav1.GetOptions{}); err != nil { - if !apierrors.IsNotFound(err) { - return nil - } - if _, err := client.AppsV1().Deployments(deploy.ObjectMeta.Namespace).Create(context.TODO(), deploy, metav1.CreateOptions{}); err != nil { - if !apierrors.IsAlreadyExists(err) { - return errors.Wrap(err, "unable to create deployment") + var lastError error + err := wait.PollUntilContextTimeout(context.Background(), + apiCallRetryInterval, kubeadmapi.GetActiveTimeouts().KubernetesAPICall.Duration, + true, func(_ context.Context) (bool, error) { + ctx := context.Background() + if _, err := client.AppsV1().Deployments(deploy.ObjectMeta.Namespace).Get(ctx, deployName, metav1.GetOptions{}); err != nil { + if !apierrors.IsNotFound(err) { + lastError = errors.Wrap(err, "unable to get Deployment") + return false, nil + } + if _, err := client.AppsV1().Deployments(deploy.ObjectMeta.Namespace).Create(ctx, deploy, metav1.CreateOptions{}); err != nil { + if !apierrors.IsAlreadyExists(err) { + lastError = errors.Wrap(err, "unable to create Deployment") + return false, nil + } + } } - } + return true, nil + }) + if err == nil { + return nil } - return nil + return lastError } // CreateOrUpdateDaemonSet creates a DaemonSet if the target resource doesn't exist. If the resource exists already, this function will update the resource instead. func CreateOrUpdateDaemonSet(client clientset.Interface, ds *apps.DaemonSet) error { - if _, err := client.AppsV1().DaemonSets(ds.ObjectMeta.Namespace).Create(context.TODO(), ds, metav1.CreateOptions{}); err != nil { - if !apierrors.IsAlreadyExists(err) { - return errors.Wrap(err, "unable to create daemonset") - } - - if _, err := client.AppsV1().DaemonSets(ds.ObjectMeta.Namespace).Update(context.TODO(), ds, metav1.UpdateOptions{}); err != nil { - return errors.Wrap(err, "unable to update daemonset") - } + var lastError error + err := wait.PollUntilContextTimeout(context.Background(), + apiCallRetryInterval, kubeadmapi.GetActiveTimeouts().KubernetesAPICall.Duration, + true, func(_ context.Context) (bool, error) { + ctx := context.Background() + if _, err := client.AppsV1().DaemonSets(ds.ObjectMeta.Namespace).Create(ctx, ds, metav1.CreateOptions{}); err != nil { + if !apierrors.IsAlreadyExists(err) { + lastError = errors.Wrap(err, "unable to create DaemonSet") + return false, nil + } + if _, err := client.AppsV1().DaemonSets(ds.ObjectMeta.Namespace).Update(ctx, ds, metav1.UpdateOptions{}); err != nil { + lastError = errors.Wrap(err, "unable to update DaemonSet") + return false, nil + } + } + return true, nil + }) + if err == nil { + return nil } - return nil + return lastError } // 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 { var lastError error err := wait.PollUntilContextTimeout(context.Background(), - constants.KubernetesAPICallRetryInterval, kubeadmapi.GetActiveTimeouts().KubernetesAPICall.Duration, + apiCallRetryInterval, kubeadmapi.GetActiveTimeouts().KubernetesAPICall.Duration, true, func(_ context.Context) (bool, error) { - if _, err := client.RbacV1().Roles(role.ObjectMeta.Namespace).Create(context.TODO(), role, metav1.CreateOptions{}); err != nil { + ctx := context.Background() + if _, err := client.RbacV1().Roles(role.ObjectMeta.Namespace).Create(ctx, role, metav1.CreateOptions{}); err != nil { if !apierrors.IsAlreadyExists(err) { - lastError = errors.Wrap(err, "unable to create RBAC role") + lastError = errors.Wrap(err, "unable to create Role") return false, nil } - - if _, err := client.RbacV1().Roles(role.ObjectMeta.Namespace).Update(context.TODO(), role, metav1.UpdateOptions{}); err != nil { - lastError = errors.Wrap(err, "unable to update RBAC role") + if _, err := client.RbacV1().Roles(role.ObjectMeta.Namespace).Update(ctx, role, metav1.UpdateOptions{}); err != nil { + lastError = errors.Wrap(err, "unable to update Role") return false, nil } } @@ -221,16 +293,16 @@ func CreateOrUpdateRole(client clientset.Interface, role *rbac.Role) error { func CreateOrUpdateRoleBinding(client clientset.Interface, roleBinding *rbac.RoleBinding) error { var lastError error err := wait.PollUntilContextTimeout(context.Background(), - constants.KubernetesAPICallRetryInterval, kubeadmapi.GetActiveTimeouts().KubernetesAPICall.Duration, + apiCallRetryInterval, kubeadmapi.GetActiveTimeouts().KubernetesAPICall.Duration, true, func(_ context.Context) (bool, error) { - if _, err := client.RbacV1().RoleBindings(roleBinding.ObjectMeta.Namespace).Create(context.TODO(), roleBinding, metav1.CreateOptions{}); err != nil { + ctx := context.Background() + if _, err := client.RbacV1().RoleBindings(roleBinding.ObjectMeta.Namespace).Create(ctx, roleBinding, metav1.CreateOptions{}); err != nil { if !apierrors.IsAlreadyExists(err) { - lastError = errors.Wrap(err, "unable to create RBAC rolebinding") + lastError = errors.Wrap(err, "unable to create RoleBinding") return false, nil } - - if _, err := client.RbacV1().RoleBindings(roleBinding.ObjectMeta.Namespace).Update(context.TODO(), roleBinding, metav1.UpdateOptions{}); err != nil { - lastError = errors.Wrap(err, "unable to update RBAC rolebinding") + if _, err := client.RbacV1().RoleBindings(roleBinding.ObjectMeta.Namespace).Update(ctx, roleBinding, metav1.UpdateOptions{}); err != nil { + lastError = errors.Wrap(err, "unable to update RoleBinding") return false, nil } } @@ -244,37 +316,60 @@ func CreateOrUpdateRoleBinding(client clientset.Interface, roleBinding *rbac.Rol // CreateOrUpdateClusterRole creates a ClusterRole if the target resource doesn't exist. If the resource exists already, this function will update the resource instead. func CreateOrUpdateClusterRole(client clientset.Interface, clusterRole *rbac.ClusterRole) error { - if _, err := client.RbacV1().ClusterRoles().Create(context.TODO(), clusterRole, metav1.CreateOptions{}); err != nil { - if !apierrors.IsAlreadyExists(err) { - return errors.Wrap(err, "unable to create RBAC clusterrole") - } - - if _, err := client.RbacV1().ClusterRoles().Update(context.TODO(), clusterRole, metav1.UpdateOptions{}); err != nil { - return errors.Wrap(err, "unable to update RBAC clusterrole") - } + var lastError error + err := wait.PollUntilContextTimeout(context.Background(), + apiCallRetryInterval, kubeadmapi.GetActiveTimeouts().KubernetesAPICall.Duration, + true, func(_ context.Context) (bool, error) { + ctx := context.Background() + if _, err := client.RbacV1().ClusterRoles().Create(ctx, clusterRole, metav1.CreateOptions{}); err != nil { + if !apierrors.IsAlreadyExists(err) { + lastError = errors.Wrap(err, "unable to create ClusterRole") + return false, nil + } + if _, err := client.RbacV1().ClusterRoles().Update(ctx, clusterRole, metav1.UpdateOptions{}); err != nil { + lastError = errors.Wrap(err, "unable to update ClusterRole") + return false, nil + } + } + return true, nil + }) + if err == nil { + return nil } - return nil + return lastError } // CreateOrUpdateClusterRoleBinding creates a ClusterRoleBinding if the target resource doesn't exist. If the resource exists already, this function will update the resource instead. func CreateOrUpdateClusterRoleBinding(client clientset.Interface, clusterRoleBinding *rbac.ClusterRoleBinding) error { - if _, err := client.RbacV1().ClusterRoleBindings().Create(context.TODO(), clusterRoleBinding, metav1.CreateOptions{}); err != nil { - if !apierrors.IsAlreadyExists(err) { - return errors.Wrap(err, "unable to create RBAC clusterrolebinding") - } - - if _, err := client.RbacV1().ClusterRoleBindings().Update(context.TODO(), clusterRoleBinding, metav1.UpdateOptions{}); err != nil { - return errors.Wrap(err, "unable to update RBAC clusterrolebinding") - } + var lastError error + err := wait.PollUntilContextTimeout(context.Background(), + apiCallRetryInterval, kubeadmapi.GetActiveTimeouts().KubernetesAPICall.Duration, + true, func(_ context.Context) (bool, error) { + ctx := context.Background() + if _, err := client.RbacV1().ClusterRoleBindings().Create(ctx, clusterRoleBinding, metav1.CreateOptions{}); err != nil { + if !apierrors.IsAlreadyExists(err) { + lastError = errors.Wrap(err, "unable to create ClusterRoleBinding") + return false, nil + } + if _, err := client.RbacV1().ClusterRoleBindings().Update(ctx, clusterRoleBinding, metav1.UpdateOptions{}); err != nil { + lastError = errors.Wrap(err, "unable to update ClusterRoleBinding") + return false, nil + } + } + return true, nil + }) + if err == nil { + return nil } - return nil + return lastError } // PatchNodeOnce executes patchFn on the node object found by the node name. func PatchNodeOnce(client clientset.Interface, nodeName string, patchFn func(*v1.Node), lastError *error) func(context.Context) (bool, error) { return func(_ context.Context) (bool, error) { // First get the node object - n, err := client.CoreV1().Nodes().Get(context.TODO(), nodeName, metav1.GetOptions{}) + ctx := context.Background() + n, err := client.CoreV1().Nodes().Get(ctx, nodeName, metav1.GetOptions{}) if err != nil { *lastError = err return false, nil // retry on any error @@ -307,8 +402,8 @@ func PatchNodeOnce(client clientset.Interface, nodeName string, patchFn func(*v1 return false, *lastError } - if _, err := client.CoreV1().Nodes().Patch(context.TODO(), n.Name, types.StrategicMergePatchType, patchBytes, metav1.PatchOptions{}); err != nil { - *lastError = errors.Wrapf(err, "error patching node %q through apiserver", n.Name) + if _, err := client.CoreV1().Nodes().Patch(ctx, n.Name, types.StrategicMergePatchType, patchBytes, metav1.PatchOptions{}); err != nil { + *lastError = errors.Wrapf(err, "error patching Node %q", n.Name) if apierrors.IsTimeout(err) || apierrors.IsConflict(err) || apierrors.IsServerTimeout(err) || apierrors.IsServiceUnavailable(err) { return false, nil } @@ -324,7 +419,7 @@ func PatchNodeOnce(client clientset.Interface, nodeName string, patchFn func(*v1 func PatchNode(client clientset.Interface, nodeName string, patchFn func(*v1.Node)) error { var lastError error err := wait.PollUntilContextTimeout(context.Background(), - constants.KubernetesAPICallRetryInterval, kubeadmapi.GetActiveTimeouts().KubernetesAPICall.Duration, + apiCallRetryInterval, kubeadmapi.GetActiveTimeouts().KubernetesAPICall.Duration, true, PatchNodeOnce(client, nodeName, patchFn, &lastError)) if err == nil { return nil diff --git a/cmd/kubeadm/app/util/apiclient/idempotency_test.go b/cmd/kubeadm/app/util/apiclient/idempotency_test.go index fcabe22b1fc..9acea1153a7 100644 --- a/cmd/kubeadm/app/util/apiclient/idempotency_test.go +++ b/cmd/kubeadm/app/util/apiclient/idempotency_test.go @@ -18,25 +18,813 @@ package apiclient import ( "context" - "reflect" + "os" "testing" + "time" + "github.com/google/go-cmp/cmp" "github.com/pkg/errors" + apps "k8s.io/api/apps/v1" v1 "k8s.io/api/core/v1" rbac "k8s.io/api/rbac/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" - clientset "k8s.io/client-go/kubernetes" - "k8s.io/client-go/kubernetes/fake" - core "k8s.io/client-go/testing" + clientsetfake "k8s.io/client-go/kubernetes/fake" + clientgotesting "k8s.io/client-go/testing" + + kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm" ) -const configMapName = "configmap" +func TestMain(m *testing.M) { + // Override the default interval and timeouts during tests + defaultRetryInterval := apiCallRetryInterval + apiCallRetryInterval = time.Millisecond * 50 -func TestPatchNode(t *testing.T) { + defaultTimeouts := kubeadmapi.GetActiveTimeouts() + defaultAPICallTimeout := defaultTimeouts.KubernetesAPICall + defaultTimeouts.KubernetesAPICall = &metav1.Duration{Duration: apiCallRetryInterval} + + exitVal := m.Run() + + // Restore the default interval and timeouts + apiCallRetryInterval = defaultRetryInterval + defaultTimeouts.KubernetesAPICall = defaultAPICallTimeout + + os.Exit(exitVal) +} + +func TestCreateOrUpdateConfigMap(t *testing.T) { + tests := []struct { + name string + setupClient func(*clientsetfake.Clientset) + expectedError bool + }{ + { + name: "create configmap success", + setupClient: func(client *clientsetfake.Clientset) { + client.PrependReactor("create", "configmaps", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, nil + }) + }, + expectedError: false, + }, + { + name: "create configmap returns error", + setupClient: func(client *clientsetfake.Clientset) { + client.PrependReactor("create", "configmaps", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, errors.New("unknown error") + }) + }, + expectedError: true, + }, + { + name: "configmap exists, update it", + setupClient: func(client *clientsetfake.Clientset) { + client.PrependReactor("create", "configmaps", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, apierrors.NewAlreadyExists(schema.GroupResource{}, "name") + }) + client.PrependReactor("update", "configmaps", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, nil + }) + }, + expectedError: false, + }, + { + name: "configmap exists, update error", + setupClient: func(client *clientsetfake.Clientset) { + client.PrependReactor("create", "configmaps", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, apierrors.NewAlreadyExists(schema.GroupResource{}, "name") + }) + client.PrependReactor("update", "configmaps", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, errors.New("") + }) + }, + expectedError: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + client := clientsetfake.NewSimpleClientset() + tc.setupClient(client) + err := CreateOrUpdateConfigMap(client, &v1.ConfigMap{}) + if (err != nil) != tc.expectedError { + t.Fatalf("expected error: %v, got %v, error: %v", tc.expectedError, err != nil, err) + } + }) + } +} + +func TestCreateOrMutateConfigMap(t *testing.T) { + tests := []struct { + name string + setupClient func(*clientsetfake.Clientset) + mutator func(*v1.ConfigMap) error + expectedError bool + }{ + { + name: "create configmap", + setupClient: func(client *clientsetfake.Clientset) { + client.PrependReactor("create", "configmaps", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, nil + }) + client.PrependReactor("get", "configmaps", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, nil + }) + client.PrependReactor("update", "configmaps", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, nil + }) + }, + expectedError: false, + }, + { + name: "create configmap error", + setupClient: func(client *clientsetfake.Clientset) { + client.PrependReactor("create", "configmaps", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, errors.New("") + }) + }, + expectedError: true, + }, + { + name: "configmap exists, mutate returns error", + setupClient: func(client *clientsetfake.Clientset) { + client.PrependReactor("create", "configmaps", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, apierrors.NewAlreadyExists(schema.GroupResource{}, "name") + }) + client.PrependReactor("get", "configmaps", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, &v1.ConfigMap{}, nil + }) + }, + mutator: func(*v1.ConfigMap) error { return errors.New("") }, + expectedError: true, + }, + { + name: "configmap exists, get returns error", + setupClient: func(client *clientsetfake.Clientset) { + client.PrependReactor("create", "configmaps", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, apierrors.NewAlreadyExists(schema.GroupResource{}, "name") + }) + client.PrependReactor("get", "configmaps", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, errors.New("") + }) + }, + expectedError: true, + }, + { + name: "configmap exists, mutate returns error", + setupClient: func(client *clientsetfake.Clientset) { + client.PrependReactor("create", "configmaps", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, apierrors.NewAlreadyExists(schema.GroupResource{}, "name") + }) + client.PrependReactor("get", "configmaps", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, &v1.ConfigMap{}, nil + }) + client.PrependReactor("update", "configmaps", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, errors.New("") + }) + }, + mutator: func(*v1.ConfigMap) error { return nil }, + expectedError: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + client := clientsetfake.NewSimpleClientset() + tc.setupClient(client) + err := CreateOrMutateConfigMap(client, &v1.ConfigMap{}, tc.mutator) + if (err != nil) != tc.expectedError { + t.Fatalf("expected error: %v, got %v, error: %v", tc.expectedError, err != nil, err) + } + }) + } +} + +func TestCreateOrRetainConfigMap(t *testing.T) { + tests := []struct { + name string + setupClient func(*clientsetfake.Clientset) + expectedError bool + }{ + { + name: "configmap exists", + setupClient: func(client *clientsetfake.Clientset) { + client.PrependReactor("get", "configmaps", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, &v1.ConfigMap{}, nil + }) + }, + expectedError: false, + }, + { + name: "configmap get returns an error", + setupClient: func(client *clientsetfake.Clientset) { + client.PrependReactor("get", "configmaps", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, errors.New("") + }) + }, + expectedError: true, + }, + { + name: "configmap is not found, create it", + setupClient: func(client *clientsetfake.Clientset) { + client.PrependReactor("get", "configmaps", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, apierrors.NewNotFound(schema.GroupResource{}, "name") + }) + client.PrependReactor("create", "configmaps", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, nil + }) + }, + expectedError: false, + }, + { + name: "configmap is not found, create returns an error", + setupClient: func(client *clientsetfake.Clientset) { + client.PrependReactor("get", "configmaps", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, apierrors.NewNotFound(schema.GroupResource{}, "name") + }) + client.PrependReactor("create", "configmaps", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, errors.New("") + }) + }, + expectedError: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + client := clientsetfake.NewSimpleClientset() + tc.setupClient(client) + err := CreateOrRetainConfigMap(client, &v1.ConfigMap{}, "some-cm") + if (err != nil) != tc.expectedError { + t.Fatalf("expected error: %v, got %v, error: %v", tc.expectedError, err != nil, err) + } + }) + } +} + +func TestCreateOrUpdateSecret(t *testing.T) { + tests := []struct { + name string + setupClient func(*clientsetfake.Clientset) + expectedError bool + }{ + { + name: "create secret success", + setupClient: func(client *clientsetfake.Clientset) { + client.PrependReactor("create", "secrets", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, nil + }) + }, + expectedError: false, + }, + { + name: "create secret returns error", + setupClient: func(client *clientsetfake.Clientset) { + client.PrependReactor("create", "secrets", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, errors.New("unknown error") + }) + }, + expectedError: true, + }, + { + name: "secret exists, update it", + setupClient: func(client *clientsetfake.Clientset) { + client.PrependReactor("create", "secrets", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, apierrors.NewAlreadyExists(schema.GroupResource{}, "name") + }) + client.PrependReactor("update", "secrets", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, nil + }) + }, + expectedError: false, + }, + { + name: "secret exists, update error", + setupClient: func(client *clientsetfake.Clientset) { + client.PrependReactor("create", "secrets", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, apierrors.NewAlreadyExists(schema.GroupResource{}, "name") + }) + client.PrependReactor("update", "secrets", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, errors.New("") + }) + }, + expectedError: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + client := clientsetfake.NewSimpleClientset() + tc.setupClient(client) + err := CreateOrUpdateSecret(client, &v1.Secret{}) + if (err != nil) != tc.expectedError { + t.Fatalf("expected error: %v, got %v, error: %v", tc.expectedError, err != nil, err) + } + }) + } +} + +func TestCreateOrUpdateServiceAccount(t *testing.T) { + tests := []struct { + name string + setupClient func(*clientsetfake.Clientset) + expectedError bool + }{ + { + name: "create serviceaccount success", + setupClient: func(client *clientsetfake.Clientset) { + client.PrependReactor("create", "serviceaccounts", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, nil + }) + }, + expectedError: false, + }, + { + name: "create serviceaccount returns error", + setupClient: func(client *clientsetfake.Clientset) { + client.PrependReactor("create", "serviceaccounts", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, errors.New("unknown error") + }) + }, + expectedError: true, + }, + { + name: "serviceaccount exists, update it", + setupClient: func(client *clientsetfake.Clientset) { + client.PrependReactor("create", "serviceaccounts", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, apierrors.NewAlreadyExists(schema.GroupResource{}, "name") + }) + client.PrependReactor("update", "serviceaccounts", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, nil + }) + }, + expectedError: false, + }, + { + name: "serviceaccount exists, update error", + setupClient: func(client *clientsetfake.Clientset) { + client.PrependReactor("create", "serviceaccounts", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, apierrors.NewAlreadyExists(schema.GroupResource{}, "name") + }) + client.PrependReactor("update", "serviceaccounts", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, errors.New("") + }) + }, + expectedError: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + client := clientsetfake.NewSimpleClientset() + tc.setupClient(client) + err := CreateOrUpdateServiceAccount(client, &v1.ServiceAccount{}) + if (err != nil) != tc.expectedError { + t.Fatalf("expected error: %v, got %v, error: %v", tc.expectedError, err != nil, err) + } + }) + } +} + +func TestCreateOrUpdateDeployment(t *testing.T) { + tests := []struct { + name string + setupClient func(*clientsetfake.Clientset) + expectedError bool + }{ + { + name: "create deployment success", + setupClient: func(client *clientsetfake.Clientset) { + client.PrependReactor("create", "deployments", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, nil + }) + }, + expectedError: false, + }, + { + name: "create deployment returns error", + setupClient: func(client *clientsetfake.Clientset) { + client.PrependReactor("create", "deployments", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, errors.New("unknown error") + }) + }, + expectedError: true, + }, + { + name: "deployment exists, update it", + setupClient: func(client *clientsetfake.Clientset) { + client.PrependReactor("create", "deployments", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, apierrors.NewAlreadyExists(schema.GroupResource{}, "name") + }) + client.PrependReactor("update", "deployments", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, nil + }) + }, + expectedError: false, + }, + { + name: "deployment exists, update error", + setupClient: func(client *clientsetfake.Clientset) { + client.PrependReactor("create", "deployments", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, apierrors.NewAlreadyExists(schema.GroupResource{}, "name") + }) + client.PrependReactor("update", "deployments", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, errors.New("") + }) + }, + expectedError: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + client := clientsetfake.NewSimpleClientset() + tc.setupClient(client) + err := CreateOrUpdateDeployment(client, &apps.Deployment{}) + if (err != nil) != tc.expectedError { + t.Fatalf("expected error: %v, got %v, error: %v", tc.expectedError, err != nil, err) + } + }) + } +} + +func TestCreateOrRetainDeployment(t *testing.T) { + tests := []struct { + name string + setupClient func(*clientsetfake.Clientset) + expectedError bool + }{ + { + name: "deployment exists", + setupClient: func(client *clientsetfake.Clientset) { + client.PrependReactor("get", "deployments", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, &apps.Deployment{}, nil + }) + }, + expectedError: false, + }, + { + name: "deployment get returns an error", + setupClient: func(client *clientsetfake.Clientset) { + client.PrependReactor("get", "deployments", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, errors.New("") + }) + }, + expectedError: true, + }, + { + name: "deployment is not found, create it", + setupClient: func(client *clientsetfake.Clientset) { + client.PrependReactor("get", "deployments", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, apierrors.NewNotFound(schema.GroupResource{}, "name") + }) + client.PrependReactor("create", "deployments", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, nil + }) + }, + expectedError: false, + }, + { + name: "deployment is not found, create returns an error", + setupClient: func(client *clientsetfake.Clientset) { + client.PrependReactor("get", "deployments", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, apierrors.NewNotFound(schema.GroupResource{}, "name") + }) + client.PrependReactor("create", "deployments", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, errors.New("") + }) + }, + expectedError: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + client := clientsetfake.NewSimpleClientset() + tc.setupClient(client) + err := CreateOrRetainDeployment(client, &apps.Deployment{}, "some-deployment") + if (err != nil) != tc.expectedError { + t.Fatalf("expected error: %v, got %v, error: %v", tc.expectedError, err != nil, err) + } + }) + } +} + +func TestCreateOrUpdateDaemonSet(t *testing.T) { + tests := []struct { + name string + setupClient func(*clientsetfake.Clientset) + expectedError bool + }{ + { + name: "create daemonset success", + setupClient: func(client *clientsetfake.Clientset) { + client.PrependReactor("create", "daemonsets", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, nil + }) + }, + expectedError: false, + }, + { + name: "create daemonset returns error", + setupClient: func(client *clientsetfake.Clientset) { + client.PrependReactor("create", "daemonsets", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, errors.New("unknown error") + }) + }, + expectedError: true, + }, + { + name: "daemonset exists, update it", + setupClient: func(client *clientsetfake.Clientset) { + client.PrependReactor("create", "daemonsets", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, apierrors.NewAlreadyExists(schema.GroupResource{}, "name") + }) + client.PrependReactor("update", "daemonsets", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, nil + }) + }, + expectedError: false, + }, + { + name: "daemonset exists, update error", + setupClient: func(client *clientsetfake.Clientset) { + client.PrependReactor("create", "daemonsets", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, apierrors.NewAlreadyExists(schema.GroupResource{}, "name") + }) + client.PrependReactor("update", "daemonsets", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, errors.New("") + }) + }, + expectedError: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + client := clientsetfake.NewSimpleClientset() + tc.setupClient(client) + err := CreateOrUpdateDaemonSet(client, &apps.DaemonSet{}) + if (err != nil) != tc.expectedError { + t.Fatalf("expected error: %v, got %v, error: %v", tc.expectedError, err != nil, err) + } + }) + } +} + +func TestCreateOrUpdateRole(t *testing.T) { + tests := []struct { + name string + setupClient func(*clientsetfake.Clientset) + expectedError bool + }{ + { + name: "create role success", + setupClient: func(client *clientsetfake.Clientset) { + client.PrependReactor("create", "roles", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, nil + }) + }, + expectedError: false, + }, + { + name: "create role returns error", + setupClient: func(client *clientsetfake.Clientset) { + client.PrependReactor("create", "roles", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, errors.New("unknown error") + }) + }, + expectedError: true, + }, + { + name: "role exists, update it", + setupClient: func(client *clientsetfake.Clientset) { + client.PrependReactor("create", "roles", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, apierrors.NewAlreadyExists(schema.GroupResource{}, "name") + }) + client.PrependReactor("update", "roles", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, nil + }) + }, + expectedError: false, + }, + { + name: "role exists, update error", + setupClient: func(client *clientsetfake.Clientset) { + client.PrependReactor("create", "roles", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, apierrors.NewAlreadyExists(schema.GroupResource{}, "name") + }) + client.PrependReactor("update", "roles", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, errors.New("") + }) + }, + expectedError: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + client := clientsetfake.NewSimpleClientset() + tc.setupClient(client) + err := CreateOrUpdateRole(client, &rbac.Role{}) + if (err != nil) != tc.expectedError { + t.Fatalf("expected error: %v, got %v, error: %v", tc.expectedError, err != nil, err) + } + }) + } +} + +func TestCreateOrUpdateRoleBindings(t *testing.T) { + tests := []struct { + name string + setupClient func(*clientsetfake.Clientset) + expectedError bool + }{ + { + name: "create rolebinding success", + setupClient: func(client *clientsetfake.Clientset) { + client.PrependReactor("create", "rolebindings", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, nil + }) + }, + expectedError: false, + }, + { + name: "create rolebinding returns error", + setupClient: func(client *clientsetfake.Clientset) { + client.PrependReactor("create", "rolebindings", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, errors.New("unknown error") + }) + }, + expectedError: true, + }, + { + name: "rolebinding exists, update it", + setupClient: func(client *clientsetfake.Clientset) { + client.PrependReactor("create", "rolebindings", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, apierrors.NewAlreadyExists(schema.GroupResource{}, "name") + }) + client.PrependReactor("update", "rolebindings", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, nil + }) + }, + expectedError: false, + }, + { + name: "rolebinding exists, update error", + setupClient: func(client *clientsetfake.Clientset) { + client.PrependReactor("create", "rolebindings", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, apierrors.NewAlreadyExists(schema.GroupResource{}, "name") + }) + client.PrependReactor("update", "rolebindings", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, errors.New("") + }) + }, + expectedError: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + client := clientsetfake.NewSimpleClientset() + tc.setupClient(client) + err := CreateOrUpdateRoleBinding(client, &rbac.RoleBinding{}) + if (err != nil) != tc.expectedError { + t.Fatalf("expected error: %v, got %v, error: %v", tc.expectedError, err != nil, err) + } + }) + } +} + +func TestCreateOrUpdateClusterRole(t *testing.T) { + tests := []struct { + name string + setupClient func(*clientsetfake.Clientset) + expectedError bool + }{ + { + name: "create clusterrole success", + setupClient: func(client *clientsetfake.Clientset) { + client.PrependReactor("create", "clusterroles", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, nil + }) + }, + expectedError: false, + }, + { + name: "create clusterrole returns error", + setupClient: func(client *clientsetfake.Clientset) { + client.PrependReactor("create", "clusterroles", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, errors.New("unknown error") + }) + }, + expectedError: true, + }, + { + name: "clusterrole exists, update it", + setupClient: func(client *clientsetfake.Clientset) { + client.PrependReactor("create", "clusterroles", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, apierrors.NewAlreadyExists(schema.GroupResource{}, "name") + }) + client.PrependReactor("update", "clusterroles", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, nil + }) + }, + expectedError: false, + }, + { + name: "clusterrole exists, update error", + setupClient: func(client *clientsetfake.Clientset) { + client.PrependReactor("create", "clusterroles", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, apierrors.NewAlreadyExists(schema.GroupResource{}, "name") + }) + client.PrependReactor("update", "clusterroles", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, errors.New("") + }) + }, + expectedError: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + client := clientsetfake.NewSimpleClientset() + tc.setupClient(client) + err := CreateOrUpdateClusterRole(client, &rbac.ClusterRole{}) + if (err != nil) != tc.expectedError { + t.Fatalf("expected error: %v, got %v, error: %v", tc.expectedError, err != nil, err) + } + }) + } +} + +func TestCreateOrUpdateClusterRoleBindings(t *testing.T) { + tests := []struct { + name string + setupClient func(*clientsetfake.Clientset) + expectedError bool + }{ + { + name: "create clusterrolebinding success", + setupClient: func(client *clientsetfake.Clientset) { + client.PrependReactor("create", "clusterrolebindings", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, nil + }) + }, + expectedError: false, + }, + { + name: "create clusterrolebinding returns error", + setupClient: func(client *clientsetfake.Clientset) { + client.PrependReactor("create", "clusterrolebindings", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, errors.New("unknown error") + }) + }, + expectedError: true, + }, + { + name: "clusterrolebinding exists, update it", + setupClient: func(client *clientsetfake.Clientset) { + client.PrependReactor("create", "clusterrolebindings", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, apierrors.NewAlreadyExists(schema.GroupResource{}, "name") + }) + client.PrependReactor("update", "clusterrolebindings", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, nil + }) + }, + expectedError: false, + }, + { + name: "clusterrolebinding exists, update error", + setupClient: func(client *clientsetfake.Clientset) { + client.PrependReactor("create", "clusterrolebindings", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, apierrors.NewAlreadyExists(schema.GroupResource{}, "name") + }) + client.PrependReactor("update", "clusterrolebindings", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, errors.New("") + }) + }, + expectedError: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + client := clientsetfake.NewSimpleClientset() + tc.setupClient(client) + err := CreateOrUpdateClusterRoleBinding(client, &rbac.ClusterRoleBinding{}) + if (err != nil) != tc.expectedError { + t.Fatalf("expected error: %v, got %v, error: %v", tc.expectedError, err != nil, err) + } + }) + } +} + +func TestPatchNodeOnce(t *testing.T) { testcases := []struct { name string lookupName string @@ -118,17 +906,29 @@ func TestPatchNode(t *testing.T) { success: false, fakeError: apierrors.NewServiceUnavailable("fake service unavailable"), }, + { + name: "patch node failed with unknown error", + lookupName: "testnode", + node: v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "testnode", + Labels: map[string]string{v1.LabelHostname: ""}, + }, + }, + success: false, + fakeError: errors.New("unknown error"), + }, } for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { - client := fake.NewSimpleClientset() - _, err := client.CoreV1().Nodes().Create(context.TODO(), &tc.node, metav1.CreateOptions{}) + client := clientsetfake.NewSimpleClientset() + _, err := client.CoreV1().Nodes().Create(context.Background(), &tc.node, metav1.CreateOptions{}) if err != nil { t.Fatalf("failed to create node to fake client: %v", err) } if tc.fakeError != nil { - client.PrependReactor("patch", "nodes", func(action core.Action) (handled bool, ret runtime.Object, err error) { + client.PrependReactor("patch", "nodes", func(action clientgotesting.Action) (handled bool, ret runtime.Object, err error) { return true, nil, tc.fakeError }) } @@ -139,7 +939,7 @@ func TestPatchNode(t *testing.T) { } }, &lastError) success, err := conditionFunction(context.Background()) - if err != nil { + if err != nil && tc.success { t.Fatalf("did not expect error: %v", err) } if success != tc.success { @@ -149,306 +949,102 @@ func TestPatchNode(t *testing.T) { } } -func TestCreateOrMutateConfigMap(t *testing.T) { - client := fake.NewSimpleClientset() - err := CreateOrMutateConfigMap(client, &v1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: configMapName, - Namespace: metav1.NamespaceSystem, +func TestPatchNode(t *testing.T) { + tests := []struct { + name string + setupClient func(*clientsetfake.Clientset) + expectedError bool + }{ + { + name: "success", + setupClient: func(client *clientsetfake.Clientset) { + client.PrependReactor("get", "nodes", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "some-node", + Labels: map[string]string{v1.LabelHostname: ""}, + }, + }, nil + }) + client.PrependReactor("patch", "nodes", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, nil + }) + }, + expectedError: false, }, - Data: map[string]string{ - "key": "some-value", + { + name: "error", + setupClient: func(client *clientsetfake.Clientset) { + client.PrependReactor("get", "nodes", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, errors.New("unknown error") + }) + }, + expectedError: true, }, - }, func(cm *v1.ConfigMap) error { - t.Fatal("mutate should not have been called, since the ConfigMap should have been created instead of mutated") - return nil - }) - if err != nil { - t.Fatalf("error creating ConfigMap: %v", err) - } - _, err = client.CoreV1().ConfigMaps(metav1.NamespaceSystem).Get(context.TODO(), configMapName, metav1.GetOptions{}) - if err != nil { - t.Fatalf("error retrieving ConfigMap: %v", err) - } -} - -func createClientAndConfigMap(t *testing.T) *fake.Clientset { - client := fake.NewSimpleClientset() - _, err := client.CoreV1().ConfigMaps(metav1.NamespaceSystem).Create(context.TODO(), &v1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: configMapName, - Namespace: metav1.NamespaceSystem, - }, - Data: map[string]string{ - "key": "some-value", - }, - }, metav1.CreateOptions{}) - if err != nil { - t.Fatalf("error creating ConfigMap: %v", err) - } - return client -} - -func TestMutateConfigMap(t *testing.T) { - client := createClientAndConfigMap(t) - - err := MutateConfigMap(client, metav1.ObjectMeta{ - Name: configMapName, - Namespace: metav1.NamespaceSystem, - }, func(cm *v1.ConfigMap) error { - cm.Data["key"] = "some-other-value" - return nil - }) - if err != nil { - t.Fatalf("error mutating regular ConfigMap: %v", err) } - cm, _ := client.CoreV1().ConfigMaps(metav1.NamespaceSystem).Get(context.TODO(), configMapName, metav1.GetOptions{}) - if cm.Data["key"] != "some-other-value" { - t.Fatalf("ConfigMap mutation was invalid, has: %q", cm.Data["key"]) - } -} - -func TestMutateConfigMapWithConflict(t *testing.T) { - client := createClientAndConfigMap(t) - - // Mimic that the first 5 updates of the ConfigMap returns a conflict, whereas the sixth update - // succeeds - conflict := 5 - client.PrependReactor("update", "configmaps", func(action core.Action) (bool, runtime.Object, error) { - update := action.(core.UpdateAction) - if conflict > 0 { - conflict-- - return true, update.GetObject(), apierrors.NewConflict(action.GetResource().GroupResource(), configMapName, errors.New("conflict")) - } - return false, update.GetObject(), nil - }) - - err := MutateConfigMap(client, metav1.ObjectMeta{ - Name: configMapName, - Namespace: metav1.NamespaceSystem, - }, func(cm *v1.ConfigMap) error { - cm.Data["key"] = "some-other-value" - return nil - }) - if err != nil { - t.Fatalf("error mutating conflicting ConfigMap: %v", err) - } - - cm, _ := client.CoreV1().ConfigMaps(metav1.NamespaceSystem).Get(context.TODO(), configMapName, metav1.GetOptions{}) - if cm.Data["key"] != "some-other-value" { - t.Fatalf("ConfigMap mutation with conflict was invalid, has: %q", cm.Data["key"]) + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + client := clientsetfake.NewSimpleClientset() + tc.setupClient(client) + patchFn := func(*v1.Node) {} + err := PatchNode(client, "some-node", patchFn) + if (err != nil) != tc.expectedError { + t.Fatalf("expected error: %v, got %v, error: %v", tc.expectedError, err != nil, err) + } + }) } } func TestGetConfigMapWithShortRetry(t *testing.T) { - type args struct { - client clientset.Interface - namespace string - name string + expectedConfigMap := &v1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "ns", + Name: "some-cm", + }, } tests := []struct { - name string - args args - want *v1.ConfigMap - wantErr bool + name string + setupClient func(*clientsetfake.Clientset) + expectedConfigMap *v1.ConfigMap + expectedError bool }{ { - name: "ConfigMap exists", - args: args{ - client: newMockClientForTest(t, "default", "foo", "ConfigMap"), - namespace: "default", - name: "foo", + name: "configmap exists", + setupClient: func(client *clientsetfake.Clientset) { + client.PrependReactor("get", "configmaps", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, expectedConfigMap, nil + }) }, - want: &v1.ConfigMap{ - TypeMeta: metav1.TypeMeta{ - Kind: configMapName, - APIVersion: "v1", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "foo", - Namespace: "default", - }, - }, - wantErr: false, + expectedConfigMap: expectedConfigMap, + expectedError: false, }, { - name: "ConfigMap does not exist", - args: args{ - client: fake.NewSimpleClientset(), - namespace: "default", - name: "foo", + name: "configmap get returns an error", + setupClient: func(client *clientsetfake.Clientset) { + client.PrependReactor("get", "configmaps", func(clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, errors.New("") + }) }, - want: nil, - wantErr: true, + expectedError: true, }, } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, err := GetConfigMapWithShortRetry(tt.args.client, tt.args.namespace, tt.args.name) - if (err != nil) != tt.wantErr { - t.Errorf("GetConfigMapWithShortRetry() error = %v, wantErr %v", err, tt.wantErr) + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + client := clientsetfake.NewSimpleClientset() + tc.setupClient(client) + actual, err := GetConfigMapWithShortRetry(client, "ns", "some-cm") + if (err != nil) != tc.expectedError { + t.Fatalf("expected error: %v, got %v, error: %v", tc.expectedError, err != nil, err) + } + if err != nil { return } - if !reflect.DeepEqual(got, tt.want) { - t.Errorf("GetConfigMapWithShortRetry() = %v, want %v", got, tt.want) + diff := cmp.Diff(tc.expectedConfigMap, actual) + if len(diff) > 0 { + t.Fatalf("got a diff with the expected config (-want,+got):\n%s", diff) } }) } } - -func TestCreateOrUpdateClusterRole(t *testing.T) { - testClusterRole := &rbac.ClusterRole{ - TypeMeta: metav1.TypeMeta{ - Kind: "ClusterRole", - APIVersion: "v1", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "foo", - }, - } - - type args struct { - client clientset.Interface - clusterRole *rbac.ClusterRole - } - tests := []struct { - name string - args args - wantErr bool - }{ - { - name: "ClusterRole does not exist", - args: args{ - client: fake.NewSimpleClientset(), - clusterRole: testClusterRole, - }, - wantErr: false, - }, - { - name: "ClusterRole exists", - args: args{ - client: newMockClientForTest(t, "", "foo", "ClusterRole"), - clusterRole: testClusterRole, - }, - wantErr: false, - }, - { - name: "ClusterRole is invalid", - args: args{ - client: fake.NewSimpleClientset(), - clusterRole: nil, - }, - wantErr: true, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if err := CreateOrUpdateClusterRole(tt.args.client, tt.args.clusterRole); (err != nil) != tt.wantErr { - t.Errorf("CreateOrUpdateClusterRole() error = %v, wantErr %v", err, tt.wantErr) - } - }) - } -} - -func TestCreateOrUpdateClusterRoleBinding(t *testing.T) { - testClusterRoleBinding := &rbac.ClusterRoleBinding{ - TypeMeta: metav1.TypeMeta{ - Kind: "ClusterRoleBinding", - APIVersion: "v1", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "foo", - }, - } - - type args struct { - client clientset.Interface - clusterRoleBinding *rbac.ClusterRoleBinding - } - tests := []struct { - name string - args args - wantErr bool - }{ - { - name: "ClusterRoleBinding does not exist", - args: args{ - client: fake.NewSimpleClientset(), - clusterRoleBinding: testClusterRoleBinding, - }, - wantErr: false, - }, - { - name: "ClusterRoleBinding exists", - args: args{ - client: newMockClientForTest(t, "", "foo", "ClusterRoleBinding"), - clusterRoleBinding: testClusterRoleBinding, - }, - wantErr: false, - }, - { - name: "ClusterRoleBinding is invalid", - args: args{ - client: fake.NewSimpleClientset(), - clusterRoleBinding: nil, - }, - wantErr: true, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if err := CreateOrUpdateClusterRoleBinding(tt.args.client, tt.args.clusterRoleBinding); (err != nil) != tt.wantErr { - t.Errorf("CreateOrUpdateClusterRoleBinding() error = %v, wantErr %v", err, tt.wantErr) - } - }) - } -} - -func newMockClientForTest(t *testing.T, namepsace string, name string, kind string) *fake.Clientset { - client := fake.NewSimpleClientset() - - switch kind { - case "ConfigMap": - _, err := client.CoreV1().ConfigMaps(namepsace).Create(context.Background(), &v1.ConfigMap{ - TypeMeta: metav1.TypeMeta{ - Kind: configMapName, - APIVersion: "v1", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: name, - Namespace: namepsace, - }, - }, metav1.CreateOptions{}) - if err != nil { - t.Fatalf("error creating ConfigMap: %v", err) - } - case "ClusterRole": - _, err := client.RbacV1().ClusterRoles().Create(context.Background(), &rbac.ClusterRole{ - TypeMeta: metav1.TypeMeta{ - Kind: "ClusterRole", - APIVersion: "v1", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: name, - }, - }, metav1.CreateOptions{}) - if err != nil { - t.Fatalf("error creating ClusterRole: %v", err) - } - case "ClusterRoleBinding": - _, err := client.RbacV1().ClusterRoleBindings().Create(context.Background(), &rbac.ClusterRoleBinding{ - TypeMeta: metav1.TypeMeta{ - Kind: "ClusterRoleBinding", - APIVersion: "v1", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: name, - }, - }, metav1.CreateOptions{}) - if err != nil { - t.Fatalf("error creating ClusterRoleBinding: %v", err) - } - } - return client -}