Conversation
nolancon
force-pushed
the
crossplane-v2
branch
from
0cdead1 to
8a27e7b
Compare
| # as including the k8s_tools machinery prior to the xpkg machinery sets UP to | ||
| # point to tool cache. | ||
| build.init: $(UP) | ||
| build.init: $(CROSSPLANE_CLI) |
There was a problem hiding this comment.
Q: Is the comment still useful? I don't know the relationship between
There was a problem hiding this comment.
Good point, will remove the comment - basically the up CLI has been replaced by the crossplane CLI
| // Of course, this approach does not completely remove the possibility of us finding ourselves in | ||
| // the above scenario. It only mitigates it. As long as Crossplane persists with its existing logic | ||
| // then we can only make a "best-effort" to avoid it. | ||
| if err := c.updateBucketCR(ctx, bucket, func(bucketLatest *v1alpha1.Bucket) UpdateRequired { |
There was a problem hiding this comment.
Q: Does deleting them mean the issue was fully solved in v2 (and v1.2.0)?
There was a problem hiding this comment.
Yes, this PR fixed the issue upstream so we no longer need to attempt to mitigate it here 🥳 crossplane-runtime v2.0.0 is the first version to include this patch.
| managed.WithRecorder(event.NewAPIRecorder(mgr.GetEventRecorderFor(name))), | ||
| managed.WithConnectionPublishers(cps...), | ||
| managed.WithCreationGracePeriod(c.creationGracePeriod), | ||
| managed.WithDeterministicExternalName(true), |
There was a problem hiding this comment.
Q: Why are this change and removing ConnectionPublisher necessary?
There was a problem hiding this comment.
Similar to above, this PR allows a controller to designate a managed resource as having a deterministic name - this means crossplane will essentially "opt-out" of the previosuly problematic "pending annotation" behaviour (more details in the PR).
As for the ConnectionPublisher stuff - that has been removed upstream
2 participants
Description of your changes
Crossplane V2 has been released. This also means a new release (v2.0.0) of crossplane-runtime which includes crossplane/crossplane-runtime#841. This PR makes all the necessary changes to upgrade to crossplane-runtime V2 and updates the code to make use of the linked patch as this was a known issue with Provider Ceph and many other Crossplane Providers. This PR does not upgrade Provider Ceph to use Crossplane v2. For now, Crossplane v1.20 is kept in place (crossplane-runtime v2 is compatible). The main reason for this is that there are issues with Provider Ceph's current Chainsaw test workflow and Crossplane v2 - it seems we are no longer able to specify a Crossplane package as a ".gz" file which breaks the current
load-packagemake target. This needs to be investigated further and will require a separate PR. For now, we are only implementing crossplane-runtime v2 in order to make use of the linked patch.I have:
make reviewableto ensure this PR is ready for review.make ceph-chainsawto validate these changes against Ceph. This step is not always necessary. However, for changes related to S3 calls it is sensible to validate against an actual Ceph cluster. Localstack is used in our CI Chainsaw suite for convenience and there can be disparity in S3 behaviours betwee it and Ceph. Seedocs/TESTING.mdfor information on how to run tests against a Ceph cluster.backport release-x.ylabels to auto-backport this PR if necessary.How has this code been tested
All existing CI is passing locally and on GHA