Skip to content

Copy Events from tmp PVCs during Clone #3933

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Copy Events from tmp PVCs during Clone #3933

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
95fca75
move CopyEvents to common package so it can be called from other cont…
Dsanatar Oct 6, 2025
5832cc4
copy events from tmp clone pvcs to their target pvc
Dsanatar Oct 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions pkg/controller/clone/csi-clone.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,12 @@ func (p *CSIClonePhase) Reconcile(ctx context.Context) (*reconcile.Result, error
}
}

targetPvc, err := cc.GetAnnotatedEventSource(ctx, p.Client, pvc)
if err != nil {
return nil, err
}
cc.CopyEvents(pvc, targetPvc, p.Client, p.Recorder)

done, err := isClaimBoundOrWFFC(ctx, p.Client, pvc)
if err != nil {
return nil, err
Expand Down Expand Up @@ -108,6 +114,9 @@ func (p *CSIClonePhase) createClaim(ctx context.Context) (*corev1.PersistentVolu
desiredClaim.Spec.Resources.Requests[corev1.ResourceStorage] = sourceSize

cc.AddAnnotation(desiredClaim, cc.AnnPopulatorKind, cdiv1.VolumeCloneSourceRef)
cc.AddAnnotation(desiredClaim, cc.AnnEventSourceKind, p.Owner.GetObjectKind().GroupVersionKind().Kind)
cc.AddAnnotation(desiredClaim, cc.AnnEventSource, fmt.Sprintf("%s/%s", p.Owner.GetNamespace(), p.Owner.GetName()))

if p.OwnershipLabel != "" {
AddOwnershipLabel(p.OwnershipLabel, desiredClaim, p.Owner)
}
Expand Down
6 changes: 6 additions & 0 deletions pkg/controller/clone/host-clone.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,12 @@ func (p *HostClonePhase) Reconcile(ctx context.Context) (*reconcile.Result, erro
}
}

targetPvc, err := cc.GetAnnotatedEventSource(ctx, p.Client, actualClaim)
if err != nil {
return nil, err
}
cc.CopyEvents(actualClaim, targetPvc, p.Client, p.Recorder)

if !p.hostCloneComplete(actualClaim) {
// requeue to update status
return &reconcile.Result{RequeueAfter: 3 * time.Second}, nil
Expand Down
8 changes: 8 additions & 0 deletions pkg/controller/clone/snap-clone.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,12 @@ func (p *SnapshotClonePhase) Reconcile(ctx context.Context) (*reconcile.Result,
}
}

targetPvc, err := cc.GetAnnotatedEventSource(ctx, p.Client, pvc)
if err != nil {
return nil, err
}
cc.CopyEvents(pvc, targetPvc, p.Client, p.Recorder)

done, err := isClaimBoundOrWFFC(ctx, p.Client, pvc)
if err != nil {
return nil, err
Expand Down Expand Up @@ -101,6 +107,8 @@ func (p *SnapshotClonePhase) createClaim(ctx context.Context, snapshot *snapshot
claim.Spec.Resources.Requests[corev1.ResourceStorage] = *rs
}

cc.AddAnnotation(claim, cc.AnnEventSourceKind, p.Owner.GetObjectKind().GroupVersionKind().Kind)
cc.AddAnnotation(claim, cc.AnnEventSource, fmt.Sprintf("%s/%s", p.Owner.GetNamespace(), p.Owner.GetName()))
cc.AddAnnotation(claim, cc.AnnPopulatorKind, cdiv1.VolumeCloneSourceRef)
if p.OwnershipLabel != "" {
AddOwnershipLabel(p.OwnershipLabel, claim, p.Owner)
Expand Down
50 changes: 50 additions & 0 deletions pkg/controller/common/util.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2219,3 +2219,53 @@ func UpdatePVCBoundContionFromEvents(pvc *corev1.PersistentVolumeClaim, c client

return nil
}

// CopyEvents gets srcPvc events and re-emits them on the target PVC with the src name prefix
func CopyEvents(srcPVC, targetPVC client.Object, c client.Client, recorder record.EventRecorder) {
srcPrefixMsg := fmt.Sprintf("[%s] : ", srcPVC.GetName())

newEvents := &corev1.EventList{}
err := c.List(context.TODO(), newEvents,
client.InNamespace(srcPVC.GetNamespace()),
client.MatchingFields{"involvedObject.name": srcPVC.GetName(),
"involvedObject.uid": string(srcPVC.GetUID())},
)

if err != nil {
klog.Error(err, "Could not retrieve srcPVC list of Events")
}

currEvents := &corev1.EventList{}
err = c.List(context.TODO(), currEvents,
client.InNamespace(targetPVC.GetNamespace()),
client.MatchingFields{"involvedObject.name": targetPVC.GetName(),
"involvedObject.uid": string(targetPVC.GetUID())},
)

if err != nil {
klog.Error(err, "Could not retrieve targetPVC list of Events")
}

// use this to hash each message for quick lookup, value is unused
eventMap := map[string]struct{}{}

for _, event := range currEvents.Items {
eventMap[event.Message] = struct{}{}
}

for _, newEvent := range newEvents.Items {
msg := newEvent.Message

// check if target PVC already has this equivalent event
if _, exists := eventMap[msg]; exists {
continue
}
Comment on lines +2259 to +2262
Copy link
Collaborator

@akalenyu akalenyu Oct 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do we feel about using the message as a sign of existence?
I can totally imagine some part of the message being dynamic

Copy link
Collaborator

@akalenyu akalenyu Oct 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Depending on your answer (maybe this is somehow a non issue) we can come up with other ideas like a combo of reason and something else

Copy link
Collaborator Author

@Dsanatar Dsanatar Oct 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can't hurt to prepend the reason for a more precise comparison, will address

Copy link
Collaborator

@akalenyu akalenyu Oct 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I think there's a types.UID in the event object

Copy link
Collaborator

@akalenyu akalenyu Oct 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At first I wasn't sure we want to propagate duplicates but as long as our fired events will qualify for aggregation (.count increase instead of lots of CREATEs for every event) we should be good

Copy link
Collaborator Author

@Dsanatar Dsanatar Oct 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, any duplicate events that may get re-emitted just increment the event counter.

Copy link
Collaborator

@akalenyu akalenyu Oct 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm yeah as long as we don't do anything funky (reason/event source/etc are all the same) I guess we should be fine. If the message has a dynamic element, the coalescing should break, but, that's not on us - we just propagate existing events.


formattedMsg := srcPrefixMsg + msg
// check if we already emitted this event with the src prefix
if _, exists := eventMap[formattedMsg]; exists {
continue
}
recorder.Event(targetPVC, newEvent.Type, newEvent.Reason, formattedMsg)
}
}
2 changes: 1 addition & 1 deletion pkg/controller/populators/import-populator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ func (r *ImportPopulatorReconciler) reconcileTargetPVC(pvc, pvcPrime *corev1.Per
}

// copy over any new events from pvcPrime to pvc
r.copyEvents(pvcPrime, pvcCopy)
cc.CopyEvents(pvcPrime, pvcCopy, r.client, r.recorder)

err = cc.UpdatePVCBoundContionFromEvents(pvcCopy, r.client, r.log)
if err != nil {
Expand Down
51 changes: 0 additions & 51 deletions pkg/controller/populators/populator-base.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package populators

import (
"context"
"fmt"
"reflect"

"github.com/go-logr/logr"
Expand Down Expand Up @@ -298,56 +297,6 @@ func (r *ReconcilerBase) updatePVCWithPVCPrimeLabels(pvc *corev1.PersistentVolum
return pvcCopy, nil
}

// CopyEvents gets primePVC events and re-emits them on the target PVC with the prime name prefix
func (r *ReconcilerBase) copyEvents(primePVC, targetPVC client.Object) {
primePrefixMsg := fmt.Sprintf("[%s] : ", primePVC.GetName())

newEvents := &corev1.EventList{}
err := r.client.List(context.TODO(), newEvents,
client.InNamespace(primePVC.GetNamespace()),
client.MatchingFields{"involvedObject.name": primePVC.GetName(),
"involvedObject.uid": string(primePVC.GetUID())},
)

if err != nil {
r.log.Error(err, "Could not retrieve primePVC list of Events")
}

currEvents := &corev1.EventList{}
err = r.client.List(context.TODO(), currEvents,
client.InNamespace(targetPVC.GetNamespace()),
client.MatchingFields{"involvedObject.name": targetPVC.GetName(),
"involvedObject.uid": string(targetPVC.GetUID())},
)

if err != nil {
r.log.Error(err, "Could not retrieve targetPVC list of Events")
}

// use this to hash each message for quick lookup, value is unused
eventMap := make(map[string]bool)

for _, event := range currEvents.Items {
eventMap[event.Message] = true
}

for _, newEvent := range newEvents.Items {
msg := newEvent.Message

// check if target PVC already has this equivalent event
if _, exists := eventMap[msg]; exists {
continue
}

formattedMsg := primePrefixMsg + msg
// check if we already emitted this event with the prime prefix
if _, exists := eventMap[formattedMsg]; exists {
continue
}
r.recorder.Event(targetPVC, newEvent.Type, newEvent.Reason, formattedMsg)
}
}

// reconcile functions

func (r *ReconcilerBase) reconcile(req reconcile.Request, populator populatorController, pvcNameLogger logr.Logger) (reconcile.Result, error) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/controller/populators/upload-populator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ func (r *UploadPopulatorReconciler) reconcileTargetPVC(pvc, pvcPrime *corev1.Per
}

// copy over any new events from pvcPrime to pvc
r.copyEvents(pvcPrime, pvcCopy)
cc.CopyEvents(pvcPrime, pvcCopy, r.client, r.recorder)

err := cc.UpdatePVCBoundContionFromEvents(pvcCopy, r.client, r.log)
if err != nil {
Expand Down