git-sync: adds flag to skip abort on first error #162
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
k8s-ci-robot
added
the
size/XS
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: If they are not already assigned, you can assign the PR to them by writing The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
thockin
left a comment
thockin
left a comment
There was a problem hiding this comment.
I thought about this logic the last time I was in here.
What if, instead of a new flag, we just simplified and make this the main-path behavior? I am not sure that being different at first sync is worthwhile. Do you want to take a run at this?
Works for me. I'll make a new PR tomorrow. I mostly didn't want to touch the code paths since I didn't understand how critical the current behavior is. Obligatory https://xkcd.com/1172 |
Adds the flag to allow git-sync to soldier on on first failure. This is useful if the git-sync lifecycle is managed by a controller that has its own ways of reporting sync failures. Fixes kubernetes#161.
|
I thought about this a bit. The benefit of aborting on first sync or after a number of failed retries is that we get a chance to recover from a botched configuration, such as Now, I don't know how much this matters in practice. I'll make another PR which just removes the special code path for the first sync. Feel free to choose either of the approaches. |
Old code used to exit at any error seen on first sync attempt. This didn't prove useful in practice, so removing that special case. This may make git-sync slower to recover after user fixes a non-retryable error, as now flMaxSyncFailures are needed before the pod fails. It may make sense in practice. Fixes kubernetes#161, in a different way than is proposed in PR kubernetes#162.
|
FYI added #163 |
|
Since #163 was approved, this is no longer needed. Closing. |
3 participants
Adds the flag to allow git-sync to soldier on on first failure. This
is useful if the git-sync lifecycle is managed by a controller that
has its own ways of reporting sync failures.
Fixes #161.