Update DataSource LastTransitionTime when populated PVC is updated

[arnongilboa]

Signed-off-by: Arnon Gilboa agilboa@redhat.com

What this PR does / why we need it:
The DataSourceReady condition LastTransitionTime does not show a timestamp of the last source PVC update. It only shows a timestamp for its initial population, although the PVC name is updated.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

Special notes for your reviewer:

Release note:

Update DataSource LastTransitionTime when populated source PVC is updated

[awels]

So it looks like there is no good way of detecting that the current PVC has changed, which in turn leads us to doing what you did. Would it make sense to add a currentSourcePVC that contains the DataVolumeSourcePVC of the current source. Then if the spec changes, we can detect this change because the status doesn't match the spec, we can then clear out the ready condition and everything else takes care of itself.

[arnongilboa]

So it looks like there is no good way of detecting that the current PVC has changed, which in turn leads us to doing what you did. Would it make sense to add a currentSourcePVC that contains the DataVolumeSourcePVC of the current source. Then if the spec changes, we can detect this change because the status doesn't match the spec, we can then clear out the ready condition and everything else takes care of itself.

so why not add a mutating webhook to reset the Ready condition whenever source PVC is updated?

[mhenriks]

I think a new status field is a better solution. If the resource never transitioned from not ready to ready, the transition time should not change.

[arnongilboa]

I think a new status field is a better solution. If the resource never transitioned from not ready to ready, the transition time should not change.

added

[arnongilboa]

/retest

[arnongilboa]

/retest

[awels]

/lgtm

[awels]

/approve

[kubevirt-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: awels

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [awels]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[akrejcir]

Making this field required in the JSON, breaks API backward compatibility. Can you consider marking it as omitempty?

[arnongilboa]

Sure, sorry I missed it.

[arnongilboa]

@akrejcir #2382