Separate API into Volume, Image and Snapshot#821
Conversation
WalkthroughRestructures image/snapshot APIs and controller flows and introduces a new Volume API and VolumeReconciler. ImageSpec now uses Changes
Sequence Diagram(s)sequenceDiagram
participant VR as VolumeReconciler
participant ES as EventSource
participant Store as VolumeStore
participant RADOS as RADOS
participant RBD as RBD
ES->>VR: Snapshot event
VR->>VR: Enqueue volume ID
VR->>VR: processNextWorkItem()
VR->>Store: Get Volume by ID
Store-->>VR: Volume
alt Volume marked for deletion
VR->>VR: deleteVolume() (finalizer handling)
VR->>Store: Update Volume (remove finalizer)
else Reconcile volume
VR->>RADOS: Open IOContext
RADOS-->>VR: IOContext
alt Source == OSVolume
VR->>RBD: Create/prepare image from OSVolume
RBD-->>VR: Image ready
else Source == SnapshotSource
VR->>RBD: Restore image from snapshot (ImageRef)
RBD-->>VR: Restore complete
else Empty volume
VR->>VR: reconcileEmptyVolume()
end
VR->>Store: Update Volume status/state
end
sequenceDiagram
participant IC as ImageController
participant SnapshotStore as SnapshotStore
participant SC as SnapshotController
participant RBD as RBD
IC->>IC: Detect `SnapshotSource` on ImageSpec
IC->>SnapshotStore: Fetch Snapshot by SnapshotSource
SnapshotStore-->>IC: Snapshot (Spec.ImageRef)
SC->>SC: Reconcile Snapshot (check Spec.Protection)
alt Protection == "protected"
SC->>RBD: Protect RBD snapshot
RBD-->>SC: Protected
else Protection == "none"
SC->>RBD: Unprotect or allow deletion
RBD-->>SC: Unprotected
end
SC->>SnapshotStore: Update Snapshot status
IC->>IC: Pull/reference image from snapshot
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (7)
✏️ Tip: You can disable this entire section by setting 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 |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai help |
ChatThere are 3 ways to chat with CodeRabbit:
CodeRabbit commands
Other keywords and placeholders
CodeRabbit configuration file (
|
|
@coderabbitai summary |
✅ Actions performedSummary regeneration triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
internal/controllers/image_controller.go (1)
220-223: EmptydeleteImagewill cause resource leaks and orphaned finalizers.The function body was removed but the finalizer (
ImageFinalizer) is still being set on images (line 332). When an image is deleted:
- The finalizer prevents garbage collection
deleteImageis called but does nothing- The finalizer is never removed
- RBD images are never cleaned up in Ceph
This will cause images to be stuck in deletion and leak Ceph storage.
If this is intentional scaffolding for the draft PR, consider adding a
// TODO: implement deletion logiccomment and either:
- Remove the finalizer addition temporarily, or
- Implement at minimum the finalizer removal to unblock deletion
🔧 Minimal fix to unblock deletion (if scaffold)
func (r *ImageReconciler) deleteImage(ctx context.Context, log logr.Logger, ioCtx *rados.IOContext, image *providerapi.Image) error { - + // TODO: implement RBD image cleanup + image.Finalizers = slices.DeleteFunc(image.Finalizers, func(f string) bool { + return f == ImageFinalizer + }) + if _, err := r.images.Update(ctx, image); err != nil { + return fmt.Errorf("failed to remove finalizer: %w", err) + } return nil }api/snapshot.go (1)
43-45: RemoveSourcefield assignment and fix field mismatch inSnapshotSourcestruct.The
Snapshotstruct does not have aSourcefield, but code ininternal/volumeserver/volumesnapshot_create.go:36andinternal/volumeserver/snapshot.go:40attempts to accesssnapshot.Source.VolumeImageID. Additionally, theSnapshotSourcestruct only definesIronCoreImage, notVolumeImageID. Either the struct definitions are incomplete or the field references are incorrect—this will cause compilation/runtime failures.internal/controllers/common.go (1)
30-42:getSnapshotSourceDetailsreferences fields that no longer exist onSnapshot.The function at lines 30-42 attempts to access
snapshot.Source.IronCoreImageandsnapshot.Source.VolumeImageID, but:
- The
Snapshotstruct containsSpec SnapshotSpecandStatus SnapshotStatus—there is noSourcefield- The
SnapshotSourcestruct only containsIronCoreImageand does not haveVolumeImageIDThe function will fail to compile as written.
internal/controllers/snapshot_controller.go (1)
236-255: Critical: Error fromGetSize()is checked after using its result.The error returned by
rbdImg.GetSize()at line 236 is not checked until line 244, but thesizevalue is already used at line 240 and 242. IfGetSize()fails, this uses an undefined/zero value for the size calculations.Additionally, the
if err != nilblock at line 244 appears to be orphaned refactoring remnants—it wraps error handling that referenceserrbut no failing operation directly precedes it.Proposed fix
// Get image size size, err := rbdImg.GetSize() if err != nil { + snapshot.Status.State = providerapi.SnapshotStateFailed + if _, updateErr := r.store.Update(ctx, snapshot); updateErr != nil { + return errors.Join(err, fmt.Errorf("failed to update snapshot state: %w", updateErr)) + } return fmt.Errorf("failed to get snapshot image size %s: %w", snapshotName, err) } roundedSize := round.OffBytes(size) - snapshot.Status.Size = int64(roundedSize) - - if err != nil { - snapshot.Status.State = providerapi.SnapshotStateFailed - if _, updateErr := r.store.Update(ctx, snapshot); updateErr != nil { - return errors.Join(err, fmt.Errorf("failed to update snapshot state: %w", updateErr)) - } - return fmt.Errorf("failed to reconcile snapshot: %w", err) - } - snapshot.Status.State = providerapi.SnapshotStateReady if _, err = r.store.Update(ctx, snapshot); err != nil { return fmt.Errorf("failed to update snapshot: %w", err) }
🤖 Fix all issues with AI agents
In `@internal/controllers/snapshot_controller.go`:
- Around line 218-228: The protection handling is wrong: remove the stale TODO,
stop blindly calling imgSnap.Unprotect() on a new snapshot (check
snapshot.Spec.Protection and the current protection state before calling
Unprotect/Protect), fix the error messages to reflect the actual operation
(e.g., "unable to unprotect snapshot" vs "unable to protect snapshot"), and add
a default case for snapshot.Spec.Protection to validate unexpected values and
return a clear error; locate this logic around snapshot.Spec.Protection and the
calls to imgSnap.Protect()/imgSnap.Unprotect() and update accordingly.
In `@internal/controllers/volume_controller.go`:
- Around line 207-210: The log currently says "Snapshot already populated"
inside the volume reconciler when checking if volume.Status.State ==
providerapi.VolumeStateAvailable; update the log message at the
log.V(1).Info(...) call to accurately reference the volume (e.g., "Volume
already available" or similar), so the message reflects volume.Status.State and
the providerapi.VolumeStateAvailable check.
- Around line 26-30: Rename the misspelled type SnapshoVolumelerOptions to
VolumeReconcilerOptions and update all references (including the constructor
parameter type used by NewVolumeReconciler) to the new name; change the type
declaration "type SnapshoVolumelerOptions struct { Pool string;
PopulatorBufferSize int64; WorkerSize int }" to "type VolumeReconcilerOptions
struct { ... }", update the NewVolumeReconciler signature and any variables,
function parameters, or tests that reference SnapshoVolumelerOptions to
VolumeReconcilerOptions, and run `go build`/tests to ensure no remaining
references.
- Around line 229-234: The code currently swallows the reconciliation error by
leaving the todo and returning nil; replace the TODO with returning the captured
err (preferably wrapped with context) so failures propagate and trigger
controller retries—for example, change the block that checks "if err != nil" to
"return fmt.Errorf(\"reconcile failed: %w\", err)" or simply "return err" inside
the same function (the one containing the reconciliation switch and the err
variable) so the controller’s rate-limited requeue behavior can act on real
errors.
🧹 Nitpick comments (4)
api/image.go (1)
30-37: Potential naming confusion:SnapshotSourcefield vsSnapshotSourcestruct type.The field
SnapshotSource *stringinImageSpecshares the same name as theSnapshotSourcestruct type defined inapi/snapshot.go. This may cause confusion for developers navigating the codebase.Consider renaming the field to
SnapshotReforSnapshotIDto clarify that it's a string reference rather than the struct type.💡 Suggested rename for clarity
type ImageSpec struct { Size uint64 `json:"size"` WWN string `json:"wwn"` Limits Limits `json:"limits"` Encryption EncryptionSpec `json:"encryption"` - SnapshotSource *string `json:"snapshotSource"` + SnapshotRef *string `json:"snapshotRef"` }api/volume.go (1)
56-59: Consider consolidating duplicate types withapi/image.go.
VolumeEncryptionSpecandVolumeAccessare nearly identical toEncryptionSpecandImageAccessinapi/image.go. If these semantically represent the same concepts, consider:
- Reusing the existing types, or
- Extracting common types to reduce duplication
This is a minor concern given the PR's scaffolding nature.
Also applies to: 68-74
internal/controllers/volume_controller.go (2)
264-278: Parameterctx2is misleadingly named.The parameter is a
*rados.IOContext, not acontext.Context. Rename toioCtxfor consistency withreconcileVolumeanddeleteVolume.Proposed fix
-func (r *VolumeReconciler) reconcileEmptyVolume(ctx context.Context, log logr.Logger, ctx2 *rados.IOContext, volume *providerapi.Volume) error { +func (r *VolumeReconciler) reconcileEmptyVolume(ctx context.Context, log logr.Logger, ioCtx *rados.IOContext, volume *providerapi.Volume) error { //Case 1: Empty Volume -> create img return nil } -func (r *VolumeReconciler) reconcileOSVolume(ctx context.Context, log logr.Logger, ctx2 *rados.IOContext, volume *providerapi.Volume) error { +func (r *VolumeReconciler) reconcileOSVolume(ctx context.Context, log logr.Logger, ioCtx *rados.IOContext, volume *providerapi.Volume) error { //Case 2: OS Volume -> create img, dump os on img, snapshot, create img with snap ref // r.populateImage return nil } -func (r *VolumeReconciler) reconcileRestoredVolume(ctx context.Context, log logr.Logger, ctx2 *rados.IOContext, volume *providerapi.Volume) error { +func (r *VolumeReconciler) reconcileRestoredVolume(ctx context.Context, log logr.Logger, ioCtx *rados.IOContext, volume *providerapi.Volume) error { //Case 3: Restore volume -> create img with snap ref return nil }
168-182: Deletion logic is a stub—finalizer removed without actual resource cleanup.The TODO at line 174 indicates this is intentional scaffold code. Ensure actual RBD image deletion is implemented before merging.
Would you like me to help generate the RBD image deletion logic based on patterns from the snapshot or image controllers?
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
api/image.goapi/snapshot.goapi/volume.gointernal/controllers/common.gointernal/controllers/image_controller.gointernal/controllers/snapshot_controller.gointernal/controllers/volume_controller.go
🧰 Additional context used
🧬 Code graph analysis (5)
internal/controllers/image_controller.go (1)
api/snapshot.go (1)
SnapshotSource(43-45)
api/image.go (1)
api/snapshot.go (1)
SnapshotSource(43-45)
api/volume.go (1)
api/image.go (1)
Limits(66-66)
internal/controllers/volume_controller.go (5)
api/snapshot.go (1)
Snapshot(10-15)api/image.go (1)
Image(10-15)api/volume.go (1)
Volume(10-15)internal/utils/utils.go (1)
DeleteSliceElement(11-18)internal/rater/rater.go (1)
NewRater(12-16)
internal/controllers/snapshot_controller.go (1)
api/snapshot.go (4)
SnapshotProtectionNone(20-20)SnapshotProtectionProtected(21-21)SnapshotStateFailed(34-34)SnapshotStateReady(33-33)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: lint
- GitHub Check: run
🔇 Additional comments (7)
internal/controllers/common.go (1)
21-24: LGTM — TODO noted for future cleanup.The TODO comment appropriately flags the function for potential removal during the refactoring effort.
api/snapshot.go (1)
13-27: LGTM — Clean API design withSnapshotSpecandSnapshotProtection.The new
SnapshotSpecstructure withImageRefandProtectionfields provides clear separation of concerns. The protection semantics (none/protected) align well with RBD snapshot protection concepts.api/volume.go (1)
10-42: LGTM — Volume API structure is well-organized.The new
Volumeresource model establishes a clean 1:1 mapping as described in the PR objectives. TheVolumeSourcewithOSVolumeandSnapshotSourceoptions provides flexibility for different volume provisioning paths.internal/controllers/image_controller.go (3)
164-166: LGTM — Event handler updated to useSnapshotSource.The change from
SnapshotReftoSnapshotSourceis consistent with the API changes inapi/image.go.
361-370: LGTM — Reconcile logic updated to useSnapshotSource.The switch case correctly uses
img.Spec.SnapshotSourceconsistent with the updatedImageSpec.
229-230: TODO noted — unique credentials per image.This is a valid enhancement tracked for future implementation.
internal/controllers/snapshot_controller.go (1)
198-204: LGTM!The switch to
snapshot.Spec.ImageRefaligns correctly with the new API structure.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| //TODO implement protection | ||
| switch snapshot.Spec.Protection { | ||
| case providerapi.SnapshotProtectionNone: | ||
| if err := imgSnap.Unprotect(); err != nil { | ||
| return fmt.Errorf("unable to protect snapshot: %w", err) | ||
| } | ||
| case providerapi.SnapshotProtectionProtected: | ||
| if err := imgSnap.Protect(); err != nil { | ||
| return fmt.Errorf("unable to protect snapshot: %w", err) | ||
| } | ||
| } |
There was a problem hiding this comment.
Error message mismatch and missing validation in protection handling.
- Line 222: The error message says "unable to protect" but the operation is
Unprotect(). - Line 218: The TODO comment is stale since protection is implemented below.
- Calling
Unprotect()on a newly created snapshot (which is unprotected by default) will likely return an error from RBD. - No default case handles unexpected
Protectionvalues.
Proposed fix
- //TODO implement protection
- switch snapshot.Spec.Protection {
- case providerapi.SnapshotProtectionNone:
- if err := imgSnap.Unprotect(); err != nil {
- return fmt.Errorf("unable to protect snapshot: %w", err)
- }
- case providerapi.SnapshotProtectionProtected:
- if err := imgSnap.Protect(); err != nil {
- return fmt.Errorf("unable to protect snapshot: %w", err)
- }
- }
+ switch snapshot.Spec.Protection {
+ case providerapi.SnapshotProtectionNone:
+ // Newly created snapshots are unprotected by default; only unprotect if already protected
+ isProtected, err := imgSnap.IsProtected()
+ if err != nil {
+ return fmt.Errorf("failed to check snapshot protection status: %w", err)
+ }
+ if isProtected {
+ if err := imgSnap.Unprotect(); err != nil {
+ return fmt.Errorf("unable to unprotect snapshot: %w", err)
+ }
+ }
+ case providerapi.SnapshotProtectionProtected:
+ if err := imgSnap.Protect(); err != nil {
+ return fmt.Errorf("unable to protect snapshot: %w", err)
+ }
+ default:
+ return fmt.Errorf("unknown snapshot protection value: %s", snapshot.Spec.Protection)
+ }🤖 Prompt for AI Agents
In `@internal/controllers/snapshot_controller.go` around lines 218 - 228, The
protection handling is wrong: remove the stale TODO, stop blindly calling
imgSnap.Unprotect() on a new snapshot (check snapshot.Spec.Protection and the
current protection state before calling Unprotect/Protect), fix the error
messages to reflect the actual operation (e.g., "unable to unprotect snapshot"
vs "unable to protect snapshot"), and add a default case for
snapshot.Spec.Protection to validate unexpected values and return a clear error;
locate this logic around snapshot.Spec.Protection and the calls to
imgSnap.Protect()/imgSnap.Unprotect() and update accordingly.
| type SnapshoVolumelerOptions struct { | ||
| Pool string | ||
| PopulatorBufferSize int64 | ||
| WorkerSize int | ||
| } |
There was a problem hiding this comment.
Typo in struct name: SnapshoVolumelerOptions.
This appears to be a copy-paste artifact. Rename to VolumeReconcilerOptions for consistency with VolumeReconciler.
Proposed fix
-type SnapshoVolumelerOptions struct {
+type VolumeReconcilerOptions struct {
Pool string
PopulatorBufferSize int64
WorkerSize int
}Also update the constructor parameter type at line 40.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| type SnapshoVolumelerOptions struct { | |
| Pool string | |
| PopulatorBufferSize int64 | |
| WorkerSize int | |
| } | |
| type VolumeReconcilerOptions struct { | |
| Pool string | |
| PopulatorBufferSize int64 | |
| WorkerSize int | |
| } |
🤖 Prompt for AI Agents
In `@internal/controllers/volume_controller.go` around lines 26 - 30, Rename the
misspelled type SnapshoVolumelerOptions to VolumeReconcilerOptions and update
all references (including the constructor parameter type used by
NewVolumeReconciler) to the new name; change the type declaration "type
SnapshoVolumelerOptions struct { Pool string; PopulatorBufferSize int64;
WorkerSize int }" to "type VolumeReconcilerOptions struct { ... }", update the
NewVolumeReconciler signature and any variables, function parameters, or tests
that reference SnapshoVolumelerOptions to VolumeReconcilerOptions, and run `go
build`/tests to ensure no remaining references.
| events event.Source[*providerapi.Snapshot] | ||
|
|
||
| pool string | ||
| populatorBufferSize int64 | ||
|
|
||
| workerSize int | ||
| } | ||
|
|
||
| func (r *VolumeReconciler) Start(ctx context.Context) error { | ||
| log := r.log | ||
|
|
||
| reg, err := r.events.AddHandler(event.HandlerFunc[*providerapi.Snapshot](func(event event.Event[*providerapi.Snapshot]) { | ||
| r.queue.Add(event.Object.ID) | ||
| })) |
There was a problem hiding this comment.
Event source mismatch: subscribing to Snapshot events but reconciling Volumes.
The VolumeReconciler subscribes to event.Source[*providerapi.Snapshot] and adds snapshot IDs to the queue, but reconcileVolume fetches from volumeStore using that ID. This will cause lookups to fail or return incorrect resources.
Proposed fix
Either change the event source to Volume events:
- events event.Source[*providerapi.Snapshot]
+ events event.Source[*providerapi.Volume]And update the handler accordingly:
- reg, err := r.events.AddHandler(event.HandlerFunc[*providerapi.Snapshot](func(event event.Event[*providerapi.Snapshot]) {
+ reg, err := r.events.AddHandler(event.HandlerFunc[*providerapi.Volume](func(event event.Event[*providerapi.Volume]) {
r.queue.Add(event.Object.ID)
}))Or if the design intent is to react to snapshot events to reconcile related volumes, the handler should map snapshot to volume ID before enqueueing.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| events event.Source[*providerapi.Snapshot] | |
| pool string | |
| populatorBufferSize int64 | |
| workerSize int | |
| } | |
| func (r *VolumeReconciler) Start(ctx context.Context) error { | |
| log := r.log | |
| reg, err := r.events.AddHandler(event.HandlerFunc[*providerapi.Snapshot](func(event event.Event[*providerapi.Snapshot]) { | |
| r.queue.Add(event.Object.ID) | |
| })) | |
| events event.Source[*providerapi.Volume] | |
| pool string | |
| populatorBufferSize int64 | |
| workerSize int | |
| } | |
| func (r *VolumeReconciler) Start(ctx context.Context) error { | |
| log := r.log | |
| reg, err := r.events.AddHandler(event.HandlerFunc[*providerapi.Volume](func(event event.Event[*providerapi.Volume]) { | |
| r.queue.Add(event.Object.ID) | |
| })) |
| if volume.Status.State == providerapi.VolumeStateAvailable { | ||
| log.V(1).Info("Snapshot already populated") | ||
| return nil | ||
| } |
There was a problem hiding this comment.
Log message refers to "Snapshot" but this is the Volume reconciler.
Update the log message to accurately describe the volume state.
Proposed fix
if volume.Status.State == providerapi.VolumeStateAvailable {
- log.V(1).Info("Snapshot already populated")
+ log.V(1).Info("Volume already available")
return nil
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if volume.Status.State == providerapi.VolumeStateAvailable { | |
| log.V(1).Info("Snapshot already populated") | |
| return nil | |
| } | |
| if volume.Status.State == providerapi.VolumeStateAvailable { | |
| log.V(1).Info("Volume already available") | |
| return nil | |
| } |
🤖 Prompt for AI Agents
In `@internal/controllers/volume_controller.go` around lines 207 - 210, The log
currently says "Snapshot already populated" inside the volume reconciler when
checking if volume.Status.State == providerapi.VolumeStateAvailable; update the
log message at the log.V(1).Info(...) call to accurately reference the volume
(e.g., "Volume already available" or similar), so the message reflects
volume.Status.State and the providerapi.VolumeStateAvailable check.
| if err != nil { | ||
| //TODO | ||
| } | ||
|
|
||
| return nil | ||
| } |
There was a problem hiding this comment.
Errors from reconciliation methods are silently discarded.
The err from the reconciliation switch is never propagated, causing failures to be lost and preventing rate-limited retries.
Proposed fix
if err != nil {
- //TODO
+ volume.Status.State = providerapi.VolumeStateFailed
+ if _, updateErr := r.volumeStore.Update(ctx, volume); updateErr != nil {
+ return errors.Join(err, fmt.Errorf("failed to update volume state: %w", updateErr))
+ }
+ return fmt.Errorf("failed to reconcile volume: %w", err)
}
- return nil
+ volume.Status.State = providerapi.VolumeStateAvailable
+ if _, err = r.volumeStore.Update(ctx, volume); err != nil {
+ return fmt.Errorf("failed to update volume: %w", err)
+ }
+ return nil
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if err != nil { | |
| //TODO | |
| } | |
| return nil | |
| } | |
| if err != nil { | |
| volume.Status.State = providerapi.VolumeStateFailed | |
| if _, updateErr := r.volumeStore.Update(ctx, volume); updateErr != nil { | |
| return errors.Join(err, fmt.Errorf("failed to update volume state: %w", updateErr)) | |
| } | |
| return fmt.Errorf("failed to reconcile volume: %w", err) | |
| } | |
| volume.Status.State = providerapi.VolumeStateAvailable | |
| if _, err = r.volumeStore.Update(ctx, volume); err != nil { | |
| return fmt.Errorf("failed to update volume: %w", err) | |
| } | |
| return nil | |
| } |
🤖 Prompt for AI Agents
In `@internal/controllers/volume_controller.go` around lines 229 - 234, The code
currently swallows the reconciliation error by leaving the todo and returning
nil; replace the TODO with returning the captured err (preferably wrapped with
context) so failures propagate and trigger controller retries—for example,
change the block that checks "if err != nil" to "return fmt.Errorf(\"reconcile
failed: %w\", err)" or simply "return err" inside the same function (the one
containing the reconciliation switch and the err variable) so the controller’s
rate-limited requeue behavior can act on real errors.
…e orchestration is done in a `Volume` resource to have a 1:1 mapping with `Image` and `Snapshot` to the ceph core resources.
volume controller will not directly interact with Ceph. Ceph interaction is done by image and snapshot controller.
gonzolino
force-pushed
the
idea/refactoring
branch
from
ef1d728 to
49a70bc
Compare
4 participants
Scaffold to illustrate a simpler and maintainable flow on how to realize snapshots. The orchestration is done in a
Volumeresource to have a 1:1 mapping withImageandSnapshotto the ceph core resources.