Implement placement shim allocations via hv1 bookings#787
Draft
PhilippMatthes wants to merge 3 commits intomainfrom
Draft
Implement placement shim allocations via hv1 bookings#787PhilippMatthes wants to merge 3 commits intomainfrom
PhilippMatthes wants to merge 3 commits intomainfrom
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
- Reorganize file so lowercase handlers sit directly below their corresponding exported Handle* functions - Remove section divider comments between function groups - Add doc comments to all unexported handler functions - Fix listAllocationsHybrid to bubble up non-OK upstream responses instead of falling through to CRD queries - Fix deleteAllocationsHybrid to check CRD existence first and use statusRecorder consistently with other hybrid handlers - Replace //nolint:errcheck with proper error handling - Use hv1.GroupVersion for GroupResource instead of hardcoded literal - Fix lint issues: variable shadowing in e2e closures, unused imports, constant parameters (e2ePollUntil, testHypervisorWithBooking) - Add info logging at key decision points in hybrid/crd handlers
PhilippMatthes
force-pushed
the
allocations
branch
from
236f00e to
4e42d6f
Compare
… API The rebase onto main upgraded the operator dependency to v1.2.0, which does not yet include spec.bookings (PR #296 is still open). Restore the pre-release pseudo-version. Also add the third `hasBackingConfig` argument to featureModeFromConfOrHeader calls introduced on main.
Contributor
Test Coverage ReportTest Coverage 📊: 68.7% |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The Cortex Placement Shim needs to serve the Placement API's allocations endpoints (
GET/PUT/DELETE /allocations/{consumer_uuid}andPOST /allocations) directly from Hypervisor CRDs, without always forwarding to upstream Placement. This PR implements those handlers by reading and writingspec.bookingson Hypervisor CRs — the field introduced in cobaltcore-dev/openstack-hypervisor-operator#296.Three operating modes are supported per request, configured globally or overridden via request header:
status.hypervisorId) are written to CRD bookings; all others are forwarded upstream. GET merges both sources; DELETE removes from both.Key implementation details:
spec.bookings.consumer.uuid) enables fast lookup of all Hypervisors that hold bookings for a given consumer UUID, including the multi-host case during live migrations.VCPU,MEMORY_MB) and CRD quantities (cpu,memoryas Kubernetesresource.Quantity) is handled byplacementToHVResources/hvToPlacementResources.consumer_generationconcurrency control is enforced: null generation means new consumer, non-null must match the stored generation, mismatches return 409 Conflict.POST /allocations(batch manage, microversion 1.13+) supports atomic multi-consumer writes — the primary mechanism for live-migration move operations.statusRecordercaptures upstream responses in hybrid mode so the shim can inspect the status code before deciding whether to proceed with CRD writes.The
go.moddependency is bumped to pull in thespec.bookingstypes and helper functions (GetConsumers,GetConsumer,HasConsumer) from the operator.Tests cover: unit tests for resource translation, per-mode unit tests for all four HTTP verbs, hybrid-mode merge and upstream-failure scenarios, and a full e2e test suite exercising both passthrough (with a dynamically created RP and custom resource class) and CRD paths against a real cluster.
See: cobaltcore-dev/openstack-hypervisor-operator#296