Skip to content

Commit

Permalink
make max attempts cofigurable
Browse files Browse the repository at this point in the history
Signed-off-by: Kent Rancourt <[email protected]>
  • Loading branch information
krancour committed Dec 16, 2024
1 parent ce3b000 commit 3d4ccb1
Show file tree
Hide file tree
Showing 6 changed files with 74 additions and 10 deletions.
20 changes: 19 additions & 1 deletion docs/docs/35-references/10-promotion-steps.md
Original file line number Diff line number Diff line change
Expand Up @@ -924,14 +924,32 @@ steps:
### `git-push`

`git-push` pushes the committed changes in a specified working tree to a
specified branch in the remote repository. This step typically follows a `git-commit` step and is often followed by a `git-open-pr` step.
specified branch in the remote repository. This step typically follows a
`git-commit` step and is often followed by a `git-open-pr` step.

This step also implements its own, internal retry logic. If a push fails, with
the cause determined to be the presence of new commits in the remote branch that
are not present in the local branch, the step will attempt to rebase before
retrying the push. Any merge conflict requiring manual resolution will
immediately halt further attempts.

:::info
This step's internal retry logic is helpful in scenarios when concurrent
Promotions to multiple Stages may all write to the same branch of the same
repository.

Because conflicts requiring manual resolution will halt further attempts, it is
recommended to design your Promotion processes such that Promotions to multiple
Stages that write to the same branch do not write to the same files.
:::

#### `git-push` Configuration

| Name | Type | Required | Description |
|------|------|----------|-------------|
| `path` | `string` | Y | Path to a Git working tree containing committed changes. |
| `targetBranch` | `string` | N | The branch to push to in the remote repository. Mutually exclusive with `generateTargetBranch=true`. If neither of these is provided, the target branch will be the same as the branch currently checked out in the working tree. |
| `maxAttempts` | `int32` | N | The maximum number of attempts to make when pushing to the remote repository. Default is 50. |
| `generateTargetBranch` | `boolean` | N | Whether to push to a remote branch named like `kargo/<project>/<stage>/promotion`. If such a branch does not already exist, it will be created. A value of 'true' is mutually exclusive with `targetBranch`. If neither of these is provided, the target branch will be the currently checked out branch. This option is useful when a subsequent promotion step will open a pull request against a Stage-specific branch. In such a case, the generated target branch pushed to by the `git-push` step can later be utilized as the source branch of the pull request. |

#### `git-push` Examples
Expand Down
27 changes: 18 additions & 9 deletions internal/directives/git_pusher.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,9 @@ package directives
import (
"context"
"fmt"
"time"

securejoin "github.com/cyphar/filepath-securejoin"
"github.com/xeipuuv/gojsonschema"
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/client-go/util/retry"

kargoapi "github.com/akuity/kargo/api/v1alpha1"
Expand Down Expand Up @@ -131,14 +129,25 @@ func (g *gitPushPusher) runPromotionStep(
}
}

backoff := retry.DefaultBackoff
if cfg.MaxAttempts != nil {
// Note, the docs for this field say:
//
// The remaining number of iterations in which the duration
// parameter may change...
//
// This is misleading, as it implies that the total number of attempts may
// exceed the value of Steps and that Steps only dictates the maximum number
// of adjustments to the interval between retries.
//
// Reading the implementation of retry.DefaultBackoff reveals that Steps
// is indeed the maximum number of attempts.
backoff.Steps = int(*cfg.MaxAttempts)
} else {
backoff.Steps = 50
}
if err = retry.OnError(
wait.Backoff{ // TODO(krancour): Make this at least partially configurable
Duration: 1 * time.Second,
Factor: 2,
Steps: 10,
Cap: 2 * time.Minute,
Jitter: 0.1,
},
backoff,
git.IsNonFastForward,
func() error {
return workTree.Push(pushOpts)
Expand Down
19 changes: 19 additions & 0 deletions internal/directives/git_pusher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package directives
import (
"context"
"fmt"
"math"
"net/http/httptest"
"os"
"path/filepath"
Expand Down Expand Up @@ -37,6 +38,24 @@ func Test_gitPusher_validate(t *testing.T) {
"path: String length must be greater than or equal to 1",
},
},
{
name: "maxAttempts < 1",
config: Config{
"maxAttempts": 0,
},
expectedProblems: []string{
"maxAttempts: Must be greater than or equal to 1",
},
},
{
name: fmt.Sprintf("maxAttempts > %d", math.MaxInt32),
config: Config{
"maxAttempts": math.MaxInt32 + 1,
},
expectedProblems: []string{
fmt.Sprintf("maxAttempts: Must be less than or equal to %.9e", float64(math.MaxInt32)),
},
},
{
name: "just generateTargetBranch is true",
config: Config{ // Should be completely valid
Expand Down
6 changes: 6 additions & 0 deletions internal/directives/schemas/git-push-config.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,12 @@
"type": "boolean",
"description": "Indicates whether to push to a new remote branch. A value of 'true' is mutually exclusive with 'targetBranch'. If neither of these is provided, the target branch will be the currently checked out branch."
},
"maxAttempts": {
"type": "integer",
"description": "This step implements its own internal retry logic for cases where a push is determined to have failed due to the remote branch having commits that that are not present locally. Each attempt, including the first, rebases prior to pushing. This field configures the maximum number of attempts to push to the remote repository. If not specified, the default is 50.",
"minimum": 1,
"maximum": 2147483647
},
"path": {
"type": "string",
"description": "The path to a working directory of a local repository.",
Expand Down
6 changes: 6 additions & 0 deletions internal/directives/zz_config_types.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions ui/src/gen/directives/git-push-config.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,12 @@
"type": "boolean",
"description": "Indicates whether to push to a new remote branch. A value of 'true' is mutually exclusive with 'targetBranch'. If neither of these is provided, the target branch will be the currently checked out branch."
},
"maxAttempts": {
"type": "integer",
"description": "This step implements its own internal retry logic for cases where a push is determined to have failed due to the remote branch having commits that that are not present locally. Each attempt, including the first, rebases prior to pushing. This field configures the maximum number of attempts to push to the remote repository. If not specified, the default is 50.",
"minimum": 1,
"maximum": 2147483647
},
"path": {
"type": "string",
"description": "The path to a working directory of a local repository.",
Expand Down

0 comments on commit 3d4ccb1

Please sign in to comment.