Merge pull request #105343 from jonyhy96/fix-patch-node-once

kubeadm: fix some retry logic in PatchNodeOnce
This commit is contained in:
Kubernetes Prow Robot 2021-10-17 09:49:49 -07:00 committed by GitHub
commit 9804a83d8f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 20 additions and 13 deletions

View File

@ -19,7 +19,6 @@ package apiclient
import (
"context"
"encoding/json"
"fmt"
"github.com/pkg/errors"
@ -278,13 +277,13 @@ func CreateOrUpdateClusterRoleBinding(client clientset.Interface, clusterRoleBin
// This is a condition function meant to be used with wait.Poll. false, nil
// implies it is safe to try again, an error indicates no more tries should be
// made and true indicates success.
func PatchNodeOnce(client clientset.Interface, nodeName string, patchFn func(*v1.Node)) func() (bool, error) {
func PatchNodeOnce(client clientset.Interface, nodeName string, patchFn func(*v1.Node), lastError *error) func() (bool, error) {
return func() (bool, error) {
// First get the node object
n, err := client.CoreV1().Nodes().Get(context.TODO(), nodeName, metav1.GetOptions{})
if err != nil {
// TODO this should only be for timeouts
return false, nil
*lastError = err
return false, nil // retry on any error
}
// The node may appear to have no labels at first,
@ -295,7 +294,8 @@ func PatchNodeOnce(client clientset.Interface, nodeName string, patchFn func(*v1
oldData, err := json.Marshal(n)
if err != nil {
return false, errors.Wrapf(err, "failed to marshal unmodified node %q into JSON", n.Name)
*lastError = errors.Wrapf(err, "failed to marshal unmodified node %q into JSON", n.Name)
return false, *lastError
}
// Execute the mutating function
@ -303,21 +303,22 @@ func PatchNodeOnce(client clientset.Interface, nodeName string, patchFn func(*v1
newData, err := json.Marshal(n)
if err != nil {
return false, errors.Wrapf(err, "failed to marshal modified node %q into JSON", n.Name)
*lastError = errors.Wrapf(err, "failed to marshal modified node %q into JSON", n.Name)
return false, *lastError
}
patchBytes, err := strategicpatch.CreateTwoWayMergePatch(oldData, newData, v1.Node{})
if err != nil {
return false, errors.Wrap(err, "failed to create two way merge patch")
*lastError = errors.Wrap(err, "failed to create two way merge patch")
return false, *lastError
}
if _, err := client.CoreV1().Nodes().Patch(context.TODO(), n.Name, types.StrategicMergePatchType, patchBytes, metav1.PatchOptions{}); err != nil {
// TODO also check for timeouts
if apierrors.IsConflict(err) {
fmt.Println("Temporarily unable to update node metadata due to conflict (will retry)")
*lastError = errors.Wrapf(err, "error patching node %q through apiserver", n.Name)
if apierrors.IsTimeout(err) || apierrors.IsConflict(err) {
return false, nil
}
return false, errors.Wrapf(err, "error patching node %q through apiserver", n.Name)
return false, *lastError
}
return true, nil
@ -327,10 +328,15 @@ func PatchNodeOnce(client clientset.Interface, nodeName string, patchFn func(*v1
// PatchNode tries to patch a node using patchFn for the actual mutating logic.
// Retries are provided by the wait package.
func PatchNode(client clientset.Interface, nodeName string, patchFn func(*v1.Node)) error {
var lastError error
// wait.Poll will rerun the condition function every interval function if
// the function returns false. If the condition function returns an error
// then the retries end and the error is returned.
return wait.Poll(constants.APICallRetryInterval, constants.PatchNodeTimeout, PatchNodeOnce(client, nodeName, patchFn))
err := wait.Poll(constants.APICallRetryInterval, constants.PatchNodeTimeout, PatchNodeOnce(client, nodeName, patchFn, &lastError))
if err == nil {
return nil
}
return lastError
}
// GetConfigMapWithRetry tries to retrieve a ConfigMap using the given client,

View File

@ -74,11 +74,12 @@ func TestPatchNodeNonErrorCases(t *testing.T) {
if err != nil {
t.Fatalf("failed to create node to fake client: %v", err)
}
var lastError error
conditionFunction := PatchNodeOnce(client, tc.lookupName, func(node *v1.Node) {
node.Annotations = map[string]string{
"updatedBy": "test",
}
})
}, &lastError)
success, err := conditionFunction()
if err != nil {
t.Fatalf("did not expect error: %v", err)