Ensure automated PRs are updated, not duplicated

[samuelallan72]

Previously there was the issue where a new PR to target repositories was created each time the terraform apply workflow runs, if there were outstanding changes to files. This caused a lot of noise.

By using a consistent branch name and some extra logic, we can ensure existing PRs are updated, rather than creating new ones.

Fixes: #63

[samuelallan72]

The method of simply hardcoding the branch for the PR does work, but the terraform resource to create the PR fails:

Error: POST https://api.github.com/repos/canonical/charm-nginx/pulls: 422 Validation Failed [{Resource:PullRequest Field: Code:custom Message:A pull request already exists for canonical:automation/update-managed-files.}]

So we need extra logic.

[chanchiwai-ray]

will it update the PR or just don't create new PR?

[samuelallan72]

will it update the PR or just don't create new PR?

@chanchiwai-ray if there is no existing PR, it will create it. If there is an existing PR, it will update it.

[samuelallan72]

What if we open two PR here and one is approved -> new PR for targeted repos is created and before such PR (in targeted repo) is approved and merge, another PR in this repo is merged. I think that this will cause silently not doing any new PR and missing those changes.

Sorry I can't follow what you're saying here.

If there's an already open PR in the target repo, that PR will be updated with new commits. So we won't miss the changes; we'll see them when reviewing the target repo automated PR.

[Pjack]

What if we open two PR here and one is approved -> new PR for targeted repos is created and before such PR (in targeted repo) is approved and merge, another PR in this repo is merged. I think that this will cause silently not doing any new PR and missing those changes.

Sorry I can't follow what you're saying here.

If there's an already open PR in the target repo, that PR will be updated with new commits. So we won't miss the changes; we'll see them when reviewing the target repo automated PR.

@rgildein I don't fully understand your wording, but I assume your main concern is that some changes might be merged without review. This could happen if the changes are made after the automated PR has been approved but not merged yet. We've enabled the 'Dismiss stale pull request approvals when new commits are pushed' setting in every repository, so this issue should no longer happen.

https://github.com/canonical/solutions-engineering-automation/blob/main/terraform-plans/modules/GitHub/settings/main.tf#L82

[rgildein]

What if we open two PR here and one is approved -> new PR for targeted repos is created and before such PR (in targeted repo) is approved and merge, another PR in this repo is merged. I think that this will cause silently not doing any new PR and missing those changes.

Sorry I can't follow what you're saying here.

If there's an already open PR in the target repo, that PR will be updated with new commits. So we won't miss the changes; we'll see them when reviewing the target repo automated PR.

I see, so one PR could have multiple changes from multiple automated commits? If yes, is this what we want? Since what I know, we are doing squash and merge, so multiple changes will be done by single PR <==> single commit. Maybe it's fine, since these changes are done by automation. Not Sure.

[samuelallan72]

@rgildein

I see, so one PR could have multiple changes from multiple automated commits? If yes, is this what we want?

Yep that's correct. :) It should be fine, because it's automated - the commit messages and description of the PR doesn't tell us much already; the real change history is in this automation repo.

[rgildein]

Yep that's correct. :) It should be fine, because it's automated - the commit messages and description of the PR doesn't tell us much already; the real change history is in this automation repo.

I agree :+1:, originally I thought that if PR already exists, new changes will not be applied. Thanks!