Skip to content

Commit

Permalink
kubeadm: apply retries to all API calls in idempotency.go
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
neolit123 committed Feb 18, 2024
1 parent 5bf2312 commit c29450e
Show file tree
Hide file tree
Showing 4 changed files with 1,110 additions and 399 deletions.
9 changes: 9 additions & 0 deletions cmd/kubeadm/app/phases/addons/proxy/proxy_test.go
Expand Up @@ -22,6 +22,7 @@ import (
"os"
"strings"
"testing"
"time"

"github.com/lithammer/dedent"

Expand Down Expand Up @@ -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
Expand Down
Expand Up @@ -21,6 +21,7 @@ import (
"os"
"testing"
"text/template"
"time"

rbac "k8s.io/api/rbac/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
Expand All @@ -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
Expand Down Expand Up @@ -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()
Expand Down

0 comments on commit c29450e

Please sign in to comment.