Skip to content

Commit

Permalink
cluster: CreateWithRetry: make work for already existing object
Browse files Browse the repository at this point in the history
Intention of the function was to ensure that the object is created
and retry in case of webhook is temporary not available.

The original code lacks handling already existing object. Handling
with and without webhook configurations make the things more
complicated. Let's check Create() errors for different cases:

If webhook enabled:
  - no error (err == nil)
  - 500 InternalError likely if webhook is not available (yet)
  - 403 Forbidden if webhook blocks creation (check of existance)
  - some problem (real error)
else, if webhook disabled:
  - no error (err == nil)
  - 409 AlreadyExists if object exists
  - some problem (real error)

Check already existing object after Create() with a call to
Get(). It covers both with and without webhook configurations.

Reuse the same object for Get() to avoid fetching Gvk for it. It
doesn't make harm, the object structure is not used after creation.

500 InternalError is not 100% webhook problem, but consider it as a
reason for retry.

Fixes: e26100e ("upgrade: retry if default DSCI creation fails (opendatahub-io#1008)")

Signed-off-by: Yauheni Kaliuta <ykaliuta@redhat.com>
  • Loading branch information
ykaliuta committed Jul 30, 2024
1 parent 00af708 commit 69b6cb9
Showing 1 changed file with 29 additions and 5 deletions.
34 changes: 29 additions & 5 deletions pkg/cluster/resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -206,11 +206,35 @@ func CreateWithRetry(ctx context.Context, cli client.Client, obj client.Object,
timeout := time.Duration(timeoutMin) * time.Minute

return wait.PollUntilContextTimeout(ctx, interval, timeout, true, func(ctx context.Context) (bool, error) {
err := cli.Create(ctx, obj)
if err != nil {
fmt.Printf("Error creating object: %v. Retrying...\n", err)
return false, err
// Create can return:
// If webhook enabled:
// - no error (err == nil)
// - 500 InternalError likely if webhook is not available (yet)
// - 403 Forbidden if webhook blocks creation (check of existence)
// - some problem (real error)
// else, if webhook disabled:
// - no error (err == nil)
// - 409 AlreadyExists if object exists
// - some problem (real error)
errCreate := cli.Create(ctx, obj)
if errCreate == nil {
return true, nil
}
return true, nil

// check existence, success case for the function, covers 409 and 403
errGet := cli.Get(ctx, client.ObjectKeyFromObject(obj), obj)
if errGet == nil {
fmt.Printf("Resource %s already exists. It will not be updated with default values\n", obj.GetName())
return true, nil
}

// retry if 500, assume webhook is not available
if k8serr.IsInternalError(errCreate) {
fmt.Printf("Error creating object: %v. Retrying...\n", errCreate)
return false, nil
}

// some other error
return false, errCreate
})
}

0 comments on commit 69b6cb9

Please sign in to comment.