Skip to content

Commit

Permalink
Retry creation REST calls that fail
Browse files Browse the repository at this point in the history
This commit will introduce a simple retry mechanism that makes the
Kubernetes provider more tolerant to misordered resource creation, as is
particularly common in Helm and Kubernetes YAML. For example, if
Kubernetes YAML specifies to create a `Pod` before the `Namespace` it
resides in, it is likely this will cause `pulumi up` to fail. Another
example is creating `CustomResoruces` before an operator or controller has
the opportunity to register the relevant `CustomResourceDefinition`s.

We ameliorate this bug by retrying creation REST calls that fail, with
some backoff, and some maximum retry threshold. The effects of this are:

* Makes the Kubernetes provider somewhat tolerant to undeclared
  dependencies between resources.
* Makes the Kubernetes provider somewhat tolerant to under-specified
  CRDs, custom controllers, and custom operators, since the retry logic
  can stand in for all but the most complex of these (which would still
  require await logic).

This commmit represents major progress towards #240, and should (we
believe) fix #239.
  • Loading branch information
hausdorff committed Nov 5, 2018
1 parent fbb8a6c commit 209ad68
Show file tree
Hide file tree
Showing 8 changed files with 259 additions and 1 deletion.
13 changes: 12 additions & 1 deletion pkg/await/await.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,18 @@ func Creation(
}

// Issue create request.
outputs, err := clientForResource.Create(inputs)
var outputs *unstructured.Unstructured
err = sleepingRetry(
func(i uint) error {
if i > 0 {
_ = host.LogStatus(ctx, diag.Info, urn, fmt.Sprintf("Creation failed, retrying (%d)", i))
}
outputs, err = clientForResource.Create(inputs)
return err
}).
WithMaxRetries(5).
WithBackoffFactor(2).
Do()
if err != nil {
return nil, err
}
Expand Down
50 changes: 50 additions & 0 deletions pkg/await/retry.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package await

import (
"time"
)

type retrier struct {
try func(currTry uint) error
sleep func(time.Duration)
waitTime time.Duration
tries uint
maxRetries uint
backOffFactor uint
}

func sleepingRetry(try func(uint) error) *retrier {
return &retrier{
try: try,
sleep: time.Sleep,
waitTime: time.Second * 1,
tries: 0,
maxRetries: 5,
backOffFactor: 2,
}
}

func (r *retrier) WithMaxRetries(n uint) *retrier {
r.maxRetries = n
return r
}

func (r *retrier) WithBackoffFactor(t uint) *retrier {
r.backOffFactor = t
return r
}

func (r *retrier) Do() error {
var err error
for r.tries <= r.maxRetries {
err = r.try(r.tries)
r.tries++
if err != nil {
r.sleep(r.waitTime)
} else {
break
}
r.waitTime = r.waitTime * time.Duration(r.backOffFactor)
}
return err
}
73 changes: 73 additions & 0 deletions pkg/await/retry_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
package await

import (
"fmt"
"testing"
"time"

"github.com/stretchr/testify/assert"
)

func mockRetrier(try func(uint) error) *retrier {
return &retrier{
try: try,
sleep: func(time.Duration) {},
waitTime: time.Second * 1,
tries: 0,
maxRetries: 5,
backOffFactor: 3,
}
}

func Test_Retrier(t *testing.T) {
tests := []struct {
description string
retrier *retrier
err error
tries uint
finalWaitTime time.Duration
}{
{
description: "Should succeed after 1 retry if maxRetries == 0",
retrier: mockRetrier(func(uint) error { return nil }).WithMaxRetries(0),
tries: 1,
finalWaitTime: 1 * time.Second,
},
{
description: "Should succeed after one try if maxRetries > 0",
retrier: mockRetrier(func(uint) error { return nil }).WithMaxRetries(10),
tries: 1,
finalWaitTime: 1 * time.Second,
},
{
description: "Should back off if first request fails",
retrier: mockRetrier(
func(i uint) error {
if i == 0 {
return fmt.Errorf("Operation failed")
}
return nil
}).
WithMaxRetries(10).
WithBackoffFactor(5),
tries: 2,
finalWaitTime: 5 * time.Second,
},
{
description: "Should fail if retry budget exceeded",
retrier: mockRetrier(func(uint) error { return fmt.Errorf("Operation failed") }).
WithMaxRetries(3).
WithBackoffFactor(2),
err: fmt.Errorf("Operation failed"),
tries: 4,
finalWaitTime: 16 * time.Second,
},
}

for _, test := range tests {
err := test.retrier.Do()
assert.Equal(t, test.err, err, test.description)
assert.Equal(t, test.tries, test.retrier.tries)
assert.Equal(t, test.finalWaitTime, test.retrier.waitTime)
}
}
49 changes: 49 additions & 0 deletions tests/integration/retry/retry_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
// Copyright 2016-2018, Pulumi Corporation. All rights reserved.

package ints

import (
"os"
"testing"

"github.com/pulumi/pulumi-kubernetes/pkg/openapi"
"github.com/pulumi/pulumi-kubernetes/tests"
"github.com/pulumi/pulumi/pkg/resource"
"github.com/pulumi/pulumi/pkg/resource/deploy/providers"
"github.com/pulumi/pulumi/pkg/testing/integration"
"github.com/stretchr/testify/assert"
)

func TestGet(t *testing.T) {
kubectx := os.Getenv("KUBERNETES_CONTEXT")

if kubectx == "" {
t.Skipf("Skipping test due to missing KUBERNETES_CONTEXT variable")
}

integration.ProgramTest(t, &integration.ProgramTestOptions{
Dir: "step1",
Dependencies: []string{"@pulumi/kubernetes"},
Quick: true,
ExtraRuntimeValidation: func(t *testing.T, stackInfo integration.RuntimeValidationStackInfo) {
assert.NotNil(t, stackInfo.Deployment)
assert.Equal(t, 3, len(stackInfo.Deployment.Resources))

tests.SortResourcesByURN(stackInfo)

stackRes := stackInfo.Deployment.Resources[2]
assert.Equal(t, resource.RootStackType, stackRes.URN.Type())

provRes := stackInfo.Deployment.Resources[1]
assert.True(t, providers.IsProviderType(provRes.URN.Type()))

// Assert the Pod was created
pod := stackInfo.Deployment.Resources[0]
assert.Equal(t, "nginx", string(pod.URN.Name()))
step1Name, _ := openapi.Pluck(pod.Outputs, "metadata", "name")
assert.Equal(t, "nginx", step1Name.(string))
step1Namespace, _ := openapi.Pluck(pod.Outputs, "metadata", "namespace")
assert.Equal(t, "test", step1Namespace.(string))
},
})
}
3 changes: 3 additions & 0 deletions tests/integration/retry/step1/Pulumi.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
name: retry-tests
description: Tests whether Pulumi successfully retries failed create operations.
runtime: nodejs
37 changes: 37 additions & 0 deletions tests/integration/retry/step1/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// Copyright 2016-2018, Pulumi Corporation. All rights reserved.

import * as pulumi from "@pulumi/pulumi";
import * as k8s from "@pulumi/kubernetes";

//
// Tests that if we force a `Pod` to be created before the `Namespace` it is supposed to exist in,
// it will retry until created.
//

new k8s.core.v1.Pod("nginx", {
metadata: { name: "nginx", namespace: "test" },
spec: {
containers: [
{
name: "nginx",
image: "nginx:1.7.9",
ports: [{ containerPort: 80 }]
}
]
}
});

new k8s.core.v1.Namespace("test", {
metadata: {
// Wait 10 seconds before creating the namespace, to make sure we retry the Pod creation.
annotations: {
timeout: new Promise(resolve => {
if (pulumi.runtime.isDryRun()) {
return resolve("<output>");
}
setTimeout(() => resolve("done"), 10000);
})
},
name: "test"
}
});
13 changes: 13 additions & 0 deletions tests/integration/retry/step1/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"name": "steps",
"version": "0.1.0",
"dependencies": {
"@pulumi/pulumi": "^0.15.1"
},
"devDependencies": {
"typescript": "^2.5.3"
},
"peerDependencies": {
"@pulumi/kubernetes": "latest"
}
}
22 changes: 22 additions & 0 deletions tests/integration/retry/step1/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
{
"compilerOptions": {
"outDir": "bin",
"target": "es6",
"module": "commonjs",
"moduleResolution": "node",
"declaration": true,
"sourceMap": true,
"stripInternal": true,
"experimentalDecorators": true,
"pretty": true,
"noFallthroughCasesInSwitch": true,
"noImplicitAny": true,
"noImplicitReturns": true,
"forceConsistentCasingInFileNames": true,
"strictNullChecks": true
},
"files": [
"index.ts"
]
}

0 comments on commit 209ad68

Please sign in to comment.