CNV-18014: Update Delete action for Bootable volumes #1057
Conversation
Update the Delete action for volumes in Bootable volumes list to delete description and both labels: - instancetype.kubevirt.io/default-preference - instancetype.kubevirt.io/default-instancetype Fixes https://issues.redhat.com/browse/CNV-18014
|
@hstastna: This pull request references CNV-18014 which is a valid jira issue. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
@metalice @pcbailey @upalatucci @vojtechszocs please review |
| bodyText={ | ||
| <Trans t={t}> | ||
| Please note that only the label data will be deleted and that the bootable volume will | ||
| remain. Are you sure you want to delete the label for bootable volume{' '} | ||
| Please note that only the labels data will be deleted and that the bootable volume will |
There was a problem hiding this comment.
If I understand correctly, this action will delete both the description annotation and the default labels. Shouldn't the message indicate that the description annotation will be deleted as well?
There was a problem hiding this comment.
This is how it was designed:
https://docs.google.com/document/d/15Cv54978AZKeukkurvkFN1YEx_kBsm_3XcmLey_cgZY/edit#heading=h.2mig9v8s38t
Note that, unfortunately, not everything is up to date in that design doc. I asked Foday to update it but it seems it takes time. This is the message I got on 9th of February from him:
"...The design doc is not updated yet but I'll let you know as soon it is. Most likely I'll get to it today."
It seems we cannot do anything with the fact that the message is not 100% appropriate. Also I think that "...and that the bootable volume will remain" is not quite correct, too, as the volume/DataSource resource will remain, but it won't be "bootable" anymore, and it will disappear from the list of bootable volumes.
And yes, we do want to delete both labels and also the description (got the info from Ronen).
There was a problem hiding this comment.
Understood. Let's merge it for now and we can readdress it when the design gets updated.
|
@hstastna: This pull request references CNV-18014 which is a valid jira issue. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
hstastna
requested review from
pcbailey
and removed request for
vojtechszocs and
metalice
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: hstastna, pcbailey 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:
Approvers can indicate their approval by writing |
📝 Description
Update the Delete action for volumes in Bootable volumes list to delete description annotation (metadata.annotations.description) and both labels related to bootable volumes:
This PR is related to the feature: https://issues.redhat.com/browse/CNV-18014
The delete action:
🎥 Demo
Before:
Only instancetype.kubevirt.io/default-preference deleted from the DataSource resource, the rest still there (yaml):
After:
Both labels and the description removed from the DataSource: