OCPBUGS-31570: Bump to latest preinstall utils for disk cleanup fixes #618
Conversation
|
@paul-maidment: This pull request references Jira Issue OCPBUGS-31570, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. 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 openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci
bot
added
the
needs-ok-to-test
|
Hi @paul-maidment. Thanks for your PR. I'm waiting for a openshift-kni 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. |
|
@paul-maidment: This pull request references Jira Issue OCPBUGS-31570, which is invalid:
Comment In response to this:
|
paul-maidment
force-pushed
the
OCPBUGS-31570
branch
from
3afd3ea
to
77554f0
Compare
|
/lgtm |
|
@eifrach: changing LGTM is restricted to collaborators 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. |
|
/ok-to-test |
|
@eifrach: Cannot trigger testing until a trusted user reviews the PR and leaves an 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. |
|
/approve |
|
/ok-to-test |
openshift-ci
bot
added
ok-to-test
|
/lgtm |
|
/retest |
paul-maidment
force-pushed
the
OCPBUGS-31570
branch
from
77554f0
to
ee602eb
Compare
|
New changes are detected. LGTM label has been removed. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: eifrach 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 |
|
/retest |
…h-ecosystem-edge/preinstall-utils@cf01558 This change is to ensure that in the case of thin provisioning we clean up all necessary disks in the correct order. This has been tested in assisted-test infra with a 5 node cluster and an artificially created thin provisioning, the cluster installs correctly where it would not have done so before this change. It was discovered that disk cleanup does not clean disks in every scenario. When thin provisioned disk mappings are present, the existing installer is not able to clean them up. this is because the `_tdata` and `_tmeta` mappings are the only mappings that will be returned when filtering by associated disk. The changed library code ensures that we search for a base DM and then order the DMs correctly so that deletion will be performed in the correct order. Certain customers were running into the scenario above when they have thin volume provisioning. For more context, please see the commit here We derive a name for the base DM using the function `getBaseDMDevicesForThinProvisioning` and we ensure proper delete order is adhered to using `sortDMDevicesForDeletion` This has been tested as part of an assisted-service deployment using assisted-test-infra to create a 5 node cluster. Some thin volumes were created and linked to the installation disk. These were cleaned correctly when they would not have been cleaned before. <!--- This is a personal checklist that should be applicable to most PRs. It's good to go over it in order to make sure you haven't missed anything. If you feel like some of these points are not relevant to your PR, feel free to keep them unchecked and if you want also explain why you think they're inapplicable. - [x] I performed a rough self-review of my changes - [x] I explained non-trivial motivation for my code using code-comments - [x] I made sure my code passes linting, tests, and builds correctly - [x] I have ran the code and made sure it works as intended, and doesn't introduce any obvious regressions - [x] I have not committed any irrelevant changes (if you did, please point them out and why, ideally separate them into a different PR) - [x] I added tests (or decided that tests aren't really necessary) Tests present in the updated library and also test have been performed on assisted-test-infra to ensure that this is working correctly.
paul-maidment
force-pushed
the
OCPBUGS-31570
branch
from
ee602eb
to
7f1d2c0
Compare
The purpose of this PR is to pick up a change to disk cleanup made in rh-ecosystem-edge/preinstall-utils@cf01558
This change is to ensure that in the case of thin provisioning we clean up all necessary disks in the correct order.
This has been tested in assisted-test infra with a 5 node cluster and an artificially created thin provisioning, the cluster installs correctly where it would not have done so before this change.
Background / Context
It was discovered that disk cleanup does not clean disks in every scenario.
When thin provisioned disk mappings are present, the existing installer is not able to clean them up. this is because the
_tdataand_tmetamappings are the only mappings that will be returned when filtering by associated disk.The changed library code ensures that we search for a base DM and then order the DMs correctly so that deletion will be performed in the correct order.
Issue / Requirement / Reason for change
Certain customers were running into the scenario above when they have thin volume provisioning.
Solution / Feature Overview
For more context, please see the commit here
Implementation Details
We derive a name for the base DM using the function
getBaseDMDevicesForThinProvisioningand we ensure proper delete order is adhered to usingsortDMDevicesForDeletionOther Information
This has been tested as part of an assisted-service deployment using assisted-test-infra to create a 5 node cluster. Some thin volumes were created and linked to the installation disk.
These were cleaned correctly when they would not have been cleaned before.