Skip to content

Demo changes to crossplane/runtime for deterministic external name#329

Closed
nolancon wants to merge 3 commits intomainfrom
patched-runtime
Closed

Demo changes to crossplane/runtime for deterministic external name#329
nolancon wants to merge 3 commits intomainfrom
patched-runtime

Conversation

@nolancon
Copy link
Copy Markdown
Collaborator

@nolancon nolancon commented May 28, 2025

Description of your changes

This PR is a demo of how [insert PR] is used by a downstream crossplane provider.
Changes in this PR:

  • Use patched nolancon/crossplane-runtime
  • Remove existing workaround - this workaround is a best-effort attempt to remove the create-pending annotation after it has been applied. See detailed workaround description here.
  • Update existing chainsaw test case by applying the new external-name-is-deterministic annotation to the edge-case bucket. This test case was specifically written to exercise the above workaround - it is not always guaranteed to pass as the workaround is "best-effort" as described. See full description of this test case here.

By removing the above workaround, and instead, making use of the new annotation in the patched version of crossplane-runtime - the desired effect is achieved and it is no longer on a "best-effort" basis. As demonstrated by passing Chainsaw tests under the new circumstances.

I have:

  • Run make reviewable to ensure this PR is ready for review.
  • Run make ceph-chainsaw to 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. See docs/TESTING.md for information on how to run tests against a Ceph cluster.
  • Added backport release-x.y labels to auto-backport this PR if necessary.

How has this code been tested

@codecov-commenter
Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 38.62%. Comparing base (72eb3c2) to head (67a6bd6).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #329   +/-   ##
=======================================
  Coverage   38.62%   38.62%           
=======================================
  Files          61       61           
  Lines        5629     5618   -11     
=======================================
- Hits         2174     2170    -4     
+ Misses       3325     3319    -6     
+ Partials      130      129    -1     
Flag Coverage Δ
unittests 38.62% <ø> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants