Skip to content
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.

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

[ceph][luks] Fix ceph cephvolumescan for cephadm #1180

Closed
wants to merge 0 commits into from

Conversation

xbezdick
Copy link

@xbezdick xbezdick commented Feb 16, 2024

For cephadm the containers are named ceph--osd... while ceph-ansible still uses the ceph-osd-...

Other issue is that OSDs can have multiple volumes in them so filtering just for the first one is wrong and we need to check each volume for the encryption.

Resolves: rhbz#2264543
jira: RHEL-25838

Copy link

Thank you for contributing to the Leapp project!

Please note that every PR needs to comply with the Leapp Guidelines and must pass all tests in order to be mergeable.
If you want to request a review or rebuild a package in copr, you can use following commands as a comment:

  • review please @oamg/developers to notify leapp developers of the review request
  • /packit copr-build to submit a public copr build using packit

Packit will automatically schedule regression tests for this PR's build and latest upstream leapp build. If you need a different version of leapp from PR#42, use /packit test oamg/leapp#42

It is possible to schedule specific on-demand tests as well. Currently 2 test sets are supported, beaker-minimal and kernel-rt, both can be used to be run on all upgrade paths or just a couple of specific ones.
To launch on-demand tests with packit:

  • /packit test --labels kernel-rt to schedule kernel-rt tests set for all upgrade paths
  • /packit test --labels beaker-minimal-8.9to9.3,kernel-rt-8.9to9.3 to schedule kernel-rt and beaker-minimal test sets for 8.9->9.3 upgrade path

[Deprecated] To launch on-demand regression testing public members of oamg organization can leave the following comment:

  • /rerun to schedule basic regression tests using this pr build and latest upstream leapp build as artifacts
  • /rerun 42 to schedule basic regression tests using this pr build and leapp*PR42* as artifacts
  • /rerun-sst to schedule sst tests using this pr build and latest upstream leapp build as artifacts
  • /rerun-sst 42 to schedule sst tests using this pr build and leapp*PR42* as artifacts

Please open ticket in case you experience technical problem with the CI. (RH internal only)

Note: In case there are problems with tests not being triggered automatically on new PR/commit or pending for a long time, please contact leapp-infra.

@jbadiapa
Copy link

It looks good to me.

@pirat89
Copy link
Member

pirat89 commented Feb 20, 2024

@xbezdick rebase please against up-to-date master (fixes crashing tests for dropped upgrade paths)

sorry. too fast. wait with the rebase until PR #1176 .

@fernflower
Copy link
Member

Please make our linter happy, added a suggestion in the comment.
https://github.com/oamg/leapp-repository/actions/runs/7931869922/job/21763410499?pr=1180#step:4:1241

@xbezdick xbezdick force-pushed the master branch 3 times, most recently from 186cb35 to b9caf27 Compare February 20, 2024 18:29
Copy link
Member

@fernflower fernflower left a comment

Choose a reason for hiding this comment

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

LGTM. One thing - given that the behavior has changed and this actor already has unit tests - could you please consider adding a unit test?

@pirat89 pirat89 added the bug Something isn't working label Mar 1, 2024
@pirat89
Copy link
Member

pirat89 commented Mar 1, 2024

@fernflower Note this PR is bounded to RHEL ticket, so it cannot be merged before the ticket is approved for the release.

Copy link
Member

@pirat89 pirat89 left a comment

Choose a reason for hiding this comment

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

@xbezdick seems good to me. but I am rather setting req_changes to ensure the PR is not merged before it's approved to get to RHEL.

@holser
Copy link
Contributor

holser commented Apr 24, 2024

LGTM

@pirat89
Copy link
Member

pirat89 commented Apr 25, 2024

/packit build

@pirat89
Copy link
Member

pirat89 commented May 13, 2024

I am not able to reopen it. for some reason, I cannot see here the pushed commit and seems I have problem to push here from a different branch-name when master already exists on my system too. so moving to another pr instead of playing around.. #1230

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants