pulumi / upgrade-provider

A tool to automate provider upgrades on your local machine
6 stars 1 forks source link

Upgrade provider tool and/or make target should rebase patches via `make upgrade.rebase` #104

Open AaronFriel opened 1 year ago

AaronFriel commented 1 year ago

When running upgrade-provider --kind=provider on Pulumi GCP, we encounter this failure:

Logs:

  - update patched provider
 /usr/bin/git submodule update --force --init: done
 /usr/bin/git fetch --tags: done
 /usr/bin/git reset HEAD --hard: done
 /usr/bin/git checkout tags/v4.71.0: done
 /usr/bin/git add upstream: done
 /usr/bin/make upstream: exit status 2:
Applied patch to 'google-beta/provider.go' cleanly.
Applied patch to 'google-beta/utils.go' cleanly.
Error: error: google-beta/resource_sql_database_instance.go: does not exist in index
make: *** [Makefile:116: upstream] Error 1

If instead of running that sequence of steps (from git submodule update to make upstream) included in the logs, run this sequence:

FROM=HEAD TO=make upstream.rebase
make upstream.finalize
git add patches upstream
make upstream

It looks like I end up on a happy path - the patches are rebased using three-way merge.

Open questions:

aq17 commented 1 year ago

@iwahbe do you believe this should be automated?

iwahbe commented 1 year ago

This worked because of the coherent rebase, not because of 3 way merge. We actually apply patches with way merge as is:

https://github.com/pulumi/pulumi-gcp/blob/dfacd8a41f257a7011712346177aea9721e8e882/scripts/upstream.sh#L141

If make upstream.rebase is a more tolerant upgrade path, then I believe that we should prefer it.

In theory, we could run make upstream and when that fails do the rebase path, but I think that is unnecessary complexity. Let's just do FROM= TO= make upstream.rebase; make upstream.finalize; git add patches. If the rebase fails, it will exit with a non-zero code and will stop upgrade-provider, allowing the user to complete the rebase manually.

The cost is increased churn in the patch SHAs, but I don't think we care that much.