Add support for libvirt removable disks and bus types #2446
Conversation
Added support for test disks different bus types: usb, virtio, sata, scsi In case usb we set the disk to removable
openshift-ci
bot
added
the
size/M
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: bkopilov The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Hi @bkopilov. Thanks for your PR. I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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-sigs/prow repository. |
openshift-ci
bot
added
the
needs-ok-to-test
|
/ok-to-test |
|
/cc @eliorerz |
openshift-ci
bot
added
ok-to-test
|
@bkopilov we need to update the usb controller as well, right? |
yes , i think terraform |
|
@bkopilov: The following tests failed, say
Full PR test history. Your PR dashboard. 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-sigs/prow repository. I understand the commands that are listed here. |
|
Please change the title and commit message to contain the issue number or NO-ISSUE if there isn't any. |
| try: | ||
| result = next(candidate for candidate in string.ascii_lowercase if candidate not in identifiers_in_use) | ||
| except StopIteration as e: | ||
| raise ValueError(f"Couldn't find available scsi disk letter, all are taken: {identifiers_in_use}") from e | ||
|
|
||
| return result | ||
|
|
||
| def attach_test_disk(self, node_name, disk_size, bootable=False, persistent=False, with_wwn=False): | ||
| def attach_test_disk(self, node_name, disk_size, bus="scsi", bootable=False, persistent=False, with_wwn=False): |
There was a problem hiding this comment.
This whole method is relevant only to libvirt, not sure we want it here
There was a problem hiding this comment.
This method called from base_test .
Calling to self.node_controller.attach_test_disk which is exists in other controllers.
libvirt is the only that use it .
What do you suggest ?
@staticmethod
def attach_disk_flags(persistent):
modified_nodes = set()
def attach(node, disk_size, bus="scsi", bootable=False, with_wwn=False):
nonlocal modified_nodes
node.attach_test_disk(disk_size, bus=bus, bootable=bootable, persistent=persistent, with_wwn=with_wwn)
modified_nodes.add(node)
| dev = {"scsi": "sd", "sata": "sd", "usb": "sd", "virtio": "vd"} | ||
| target_dev = f"{dev[bus]}{self._get_available_identifier(node, None)}" | ||
| # Allow to create multiple devices when node shutdown required (sata) | ||
| disk_alias = f"{self.TEST_DISKS_PREFIX}{str(random.randint(0, 10000))}-{bus}-{target_dev}" |
There was a problem hiding this comment.
Please explain what is the random for
There was a problem hiding this comment.
There are two ways to add:
- VIR_DOMAIN_AFFECT_LIVE when support hotplugin (apply when execute on running node )
- VIR_DOMAIN_AFFECT_CURRENT - for SATA we must shutdown /start the node.
Not mandatory but if the change does not return new target_dev and we want to run the command multiple time it is going to prevent duplicate disk_alias which bubble libvirt exceptions...
for example:
attach_disk_persistent(master, ODF_DISK_SIZE, bus="sata") --> success
attach_disk_persistent(master, ODF_DISK_SIZE, bus="sata") --> fail without restart but indicate that target sdb already exists ......
|
Issues go stale after 90d of inactivity. Mark the issue as fresh by commenting If this issue is safe to close now please do so with /lifecycle stale |
openshift-ci
bot
added
the
lifecycle/stale
|
Stale issues rot after 30d of inactivity. Mark the issue as fresh by commenting If this issue is safe to close now please do so with /lifecycle rotten |
openshift-ci
bot
added
lifecycle/rotten
|
Rotten issues close after 30d of inactivity. Reopen the issue by commenting /close |
|
@openshift-bot: Closed this PR. 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-sigs/prow repository. |
Added support for test disks different bus types: usb, virtio, sata, scsi In case usb we set the disk to removable