cherrypicker plugin does not process all cherrypick requests if one fails.

[VannTen]

When typing a comment such as

/cherrypick release-X
/cherrypick release-Y
/cherrypick release-Z

Prow will stop doing anything if the cherrypick on release-Z fails (because apparently the cherrypick are processed in inverse order).

Example of the behavior : kubernetes-sigs/kubespray#12682 (comment)

[petr-muller]

/area plugins
/kind bug

[petr-muller]

I think this is handled differently when the cherrypick comment is issued on an unmerged and merged PR. In the former case, the cherrypicker eventually reacts on merge event and seems to correctly try all cherrypicks, iterating over all and accumulating errors:

prow/cmd/external-plugins/cherrypicker/server.go

Lines 452 to 455 in 98a0c9d

	err := s.handle(branchLog, requester, ic, org, repo, targetBranch, baseBranch, targetBranchToChainBranches[targetBranch], title, body, num)
	if err != nil {
	errs = append(errs, fmt.Errorf("failed to create cherrypick: %w", err))
	}

In the latter case, like in kubernetes-sigs/kubespray#12682 (comment), it starts cherrypicking directly on the comment event and bails out on the first error:

prow/cmd/external-plugins/cherrypicker/server.go

Lines 304 to 306 in 98a0c9d

	if err := s.handle(branchLog, ic.Comment.User.Login, &ic.Comment, org, repo, targetBranch, baseBranch, commands[targetBranch], title, body, num); err != nil {
	return log, fmt.Errorf("failed to handle cherrypick for %s: %w", targetBranch, err)
	}

/label help-wanted

[petr-muller]

/label good-first-issue

I think #544 (comment) may be sufficient for a new contributor to be able to pick this up

[k8s-ci-robot]

@petr-muller: The label(s) /label good-first-issue cannot be applied. These labels are supported: api-review, tide/merge-method-merge, tide/merge-method-rebase, tide/merge-method-squash, team/katacoda, refactor, ci-short, ci-extended, ci-full. Is this label configured under labels -> additional_labels or labels -> restricted_labels in plugin.yaml?

In response to this:

/label good-first-issue

I think #544 (comment) may be sufficient for a new contributor to be able to pick this up

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[valen-mascarenhas14]

/assign @valen-mascarenhas14

[valen-mascarenhas14]

I've added the fix in the following PR
Changes worked fine locally, cherrypick was able to proceed with the remaining targets even after encountering a failed targed.

Here's the key code change

var errs []error

for _, targetBranch := range sets.List(sets.KeySet(commands)) {
    if err := s.handle(...); err != nil {
        errs = append(errs, err)
        continue // do not abort remaining cherry-picks
    }
}

if len(errs) > 0 {
    return log, utilerrors.NewAggregate(errs)
}

Sharing the snapshot with the successful build for reference

[valen-mascarenhas14]

Prow will stop doing anything if the cherrypick on release-Z fails (because apparently the cherrypick are processed in inverse order).

@VannTen Looks like the cherry-picks aren’t processed in inverse order.

prow/cmd/external-plugins/cherrypicker/server.go

Line 297 in 476eedb

for _, targetBranch := range sets.List(sets.KeySet(commands)) {

From the code, it returns a sorted list of branches, so the processing order would be alphabetical by branch name and not based on the order in the comment.