Upgrade tests silently ignore failed provider upgrades

[schlingemann]

The ApplyProvider function called by UpgradeProvider in the upgrade package calls InstallCrossplaneProvider, which will simply wait for a provider with the name of the provider to be upgraded to become healthy. The issue is that, during the upgrade tests, the provider name will not change, only the version will.

For this reason, if the new provider version cannot be pulled (ErrImageNeverPull), the upgrade testing framework will silently ignore this issue because there's still a provider with the expected name that is healthy. The upgrade tests will still run fine, except that they're no longer testing a provider upgrade.

This still happens when using a mustPullImage function like in the example, for example when testing a provider with double image builds if the upgrade options are misconfigured.

Possible solutions:

• Modifying the provider name
• Checking for a field on the provider to match the expected version

[christophrj]

Hi @schlingemann thanks for bringing this up, I'll look into it

[christophrj]

@schlingemann I wasn't able to reproduce what you are describing. I tried the following two scenarios:

1. With a non-pullable toProvider image, the upgrade test fails as expected with error log upgrade.go:175: apply provider xpkg.crossplane.io/crossplane-contrib/provider-nop:non-existent failed: Error response from daemon: No such image: [xpkg.crossplane.io/crossplane-contrib/provider-nop:non-existent](http://xpkg.crossplane.io/crossplane-contrib/provider-nop:non-existent)
2. With a toProvider image that fails to install, the upgrade test fails as expected with error log UnhealthyPackageRevision upgrade.go:175: apply provider xpkg.crossplane.io/crossplane-contrib/provider-nop:error failed: client rate limiter Wait returned an error: context deadline exceeded

If this happens with a publicly available test, let me know where i can find the test.

I suppose the proper fix in your case would be to adjust the health check to look at the ProviderRevisioninstead of the Provider directly. But it sounds a little odd so I would like to reproduce this first.