Re: [PR] Change the check for the existence of `cryptsetup` command [cloudstack]
weizhouapache commented on PR #8482: URL: https://github.com/apache/cloudstack/pull/8482#issuecomment-1921321296 > > @DaanHoogland I think this is a very small fix without any risk. I have cherry-picked the commit to 4.18 branch and pushed to the github repository. It will be in 4.18.2.0/4.19.1.0/4.20.0.0 cc @BryanMLima @lucas-a-martins @shwstppr @nvazquez > > Thanks, @weizhouapache! Sorry for any inconvenience guys, my mistake. I will pay more attention in the future. no worries @BryanMLima -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Change the check for the existence of `cryptsetup` command [cloudstack]
BryanMLima commented on PR #8482: URL: https://github.com/apache/cloudstack/pull/8482#issuecomment-1921267911 > @DaanHoogland I think this is a very small fix without any risk. I have cherry-picked the commit to 4.18 branch and pushed to the github repository. It will be in 4.18.2.0/4.19.1.0/4.20.0.0 cc @BryanMLima @lucas-a-martins @shwstppr @nvazquez Thanks, @weizhouapache! Sorry for any inconvenience guys, my mistake. I will pay more attention in the future. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Change the check for the existence of `cryptsetup` command [cloudstack]
weizhouapache commented on PR #8482: URL: https://github.com/apache/cloudstack/pull/8482#issuecomment-1920885944 > @BryanMLima @lucas-a-martins , as @nvazquez said and hopefully it will be on 4.20 ;) but if you need it in a mainstream release before, please backport. > A revert will prevent merge forward conflicts if you do so, but is not strictly necessary. cc @shwstppr @JoaoJandre . > either way, no major harm done, just an inconvenience. @DaanHoogland I think this is a very small fix without any risk. I have cherry-picked the commit to 4.18 branch and pushed to the github repository. It will be in 4.18.2.0/4.19.1.0/4.20.0.0 cc @BryanMLima @lucas-a-martins @shwstppr @nvazquez -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Change the check for the existence of `cryptsetup` command [cloudstack]
DaanHoogland commented on PR #8482: URL: https://github.com/apache/cloudstack/pull/8482#issuecomment-1920835536 @BryanMLima @lucas-a-martins , as @nvazquez said and hopefully it will be on 4.20 ;) but if you need it in a mainstream release before, please backport. A revert will prevent merge forward conflicts if you do so, but is not strictly necessary. cc @shwstppr @JoaoJandre . either way, no major harm done, just an inconvenience. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Change the check for the existence of `cryptsetup` command [cloudstack]
weizhouapache commented on PR #8482: URL: https://github.com/apache/cloudstack/pull/8482#issuecomment-1920587867 We can backport to 4.18, then merge forward to 4.19 and main @shwstppr @nvazquez @DaanHoogland @JoaoJandre -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Change the check for the existence of `cryptsetup` command [cloudstack]
shwstppr commented on PR #8482: URL: https://github.com/apache/cloudstack/pull/8482#issuecomment-1920457497 Good catch @nvazquez. If we want it in 4.19.1 I guess either we need to revert and merge again in 4.19 branch once 4.19 release is done or we would need to backport the commit. Any other suggestions @BryanMLima @DaanHoogland @weizhouapache ? If there will be another RC then all good -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Change the check for the existence of `cryptsetup` command [cloudstack]
nvazquez commented on PR #8482: URL: https://github.com/apache/cloudstack/pull/8482#issuecomment-1920408519 Hi guys, I think in case 4.19.0 RC4 makes it to be the final release this fix won't be on 4.19.1 as the milestone states, but will be present only on the future 4.20. @BryanMLima @shwstppr in case you are reverting this fix please target it to the 4.19 branch -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Change the check for the existence of `cryptsetup` command [cloudstack]
weizhouapache commented on PR #8482: URL: https://github.com/apache/cloudstack/pull/8482#issuecomment-1919123029 > @DaanHoogland @shwstppr, my mistake, I can revert the merge commit if it is required. No need to revert, I think @BryanMLima -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Change the check for the existence of `cryptsetup` command [cloudstack]
BryanMLima commented on PR #8482: URL: https://github.com/apache/cloudstack/pull/8482#issuecomment-1919115376 @DaanHoogland @shwstppr, my mistake, I can revert the merge commit if it is required. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Change the check for the existence of `cryptsetup` command [cloudstack]
DaanHoogland commented on PR #8482: URL: https://github.com/apache/cloudstack/pull/8482#issuecomment-1919102863 Hm @BryanMLima , we were still in freeze. don't think it will be a problem though. cc @shwstppr . -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Change the check for the existence of `cryptsetup` command [cloudstack]
boring-cyborg[bot] commented on PR #8482: URL: https://github.com/apache/cloudstack/pull/8482#issuecomment-1919097357 Awesome work, congrats on your first merged pull request! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Change the check for the existence of `cryptsetup` command [cloudstack]
BryanMLima merged PR #8482: URL: https://github.com/apache/cloudstack/pull/8482 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Change the check for the existence of `cryptsetup` command [cloudstack]
BryanMLima commented on PR #8482: URL: https://github.com/apache/cloudstack/pull/8482#issuecomment-1919097043 Merging this based on four approvals and manual test. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Change the check for the existence of `cryptsetup` command [cloudstack]
DaanHoogland commented on PR #8482: URL: https://github.com/apache/cloudstack/pull/8482#issuecomment-1885023273 > > clgtm @lucas-a-martins why did you close #8480 in favour of this PR? they seem to do the same. > > @DaanHoogland thanks for your review. About the #8480, it was my first PR. I made some mistakes and didn't know how to fix them, so I thought it would be better just closing it and creating a new one, but now I know it was not a good decision. I will be more careful. no problem @lucas-a-martins and no harm done. Just wondering what happened. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Change the check for the existence of `cryptsetup` command [cloudstack]
lucas-a-martins commented on PR #8482: URL: https://github.com/apache/cloudstack/pull/8482#issuecomment-1885020025 > clgtm @lucas-a-martins why did you close #8480 in favour of this PR? they seem to do the same. @DaanHoogland thanks for your review. About the #8480, it was my first PR. I made some mistakes and didn't know how to fix them, so I thought it would be better just closing it and creating a new one, but now I know it was not a good decision. I will be more careful. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Change the check for the existence of `cryptsetup` command [cloudstack]
blueorangutan commented on PR #8482: URL: https://github.com/apache/cloudstack/pull/8482#issuecomment-1884413659 Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 8263 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Change the check for the existence of `cryptsetup` command [cloudstack]
blueorangutan commented on PR #8482: URL: https://github.com/apache/cloudstack/pull/8482#issuecomment-1884310497 @sureshanaparti a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Change the check for the existence of `cryptsetup` command [cloudstack]
sureshanaparti commented on PR #8482: URL: https://github.com/apache/cloudstack/pull/8482#issuecomment-1884310040 @blueorangutan package -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Change the check for the existence of `cryptsetup` command [cloudstack]
codecov[bot] commented on PR #8482: URL: https://github.com/apache/cloudstack/pull/8482#issuecomment-1884275484 ## [Codecov](https://app.codecov.io/gh/apache/cloudstack/pull/8482?src=pr=h1_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache) Report All modified and coverable lines are covered by tests :white_check_mark: > Comparison is base [(`d6ac91f`)](https://app.codecov.io/gh/apache/cloudstack/commit/d6ac91f2df0903737028f882f121edc8fe19b215?el=desc_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache) 30.74% compared to head [(`0f08216`)](https://app.codecov.io/gh/apache/cloudstack/pull/8482?src=pr=desc_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache) 4.39%. > Report is 1 commits behind head on main. Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #8482 +/- ## - Coverage 30.74% 4.39% -26.36% Files 5341 361 -4980 Lines374918 28622 -346296 Branches 545344993-49541 - Hits 1152861258 -114028 + Misses 244374 27225 -217149 + Partials 15258 139-15119 ``` | [Flag](https://app.codecov.io/gh/apache/cloudstack/pull/8482/flags?src=pr=flags_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache) | Coverage Δ | | |---|---|---| | [simulator-marvin-tests](https://app.codecov.io/gh/apache/cloudstack/pull/8482/flags?src=pr=flag_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache) | `?` | | | [uitests](https://app.codecov.io/gh/apache/cloudstack/pull/8482/flags?src=pr=flag_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache) | `4.39% <ø> (ø)` | | | [unit-tests](https://app.codecov.io/gh/apache/cloudstack/pull/8482/flags?src=pr=flag_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache) | `?` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more. [:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/cloudstack/pull/8482?src=pr=continue_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache). :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] Change the check for the existence of `cryptsetup` command [cloudstack]
lucas-a-martins opened a new pull request, #8482:
URL: https://github.com/apache/cloudstack/pull/8482
### Description
Currently, when starting the Agent, ACS checks if the `cryptsetup` command
can be used by using the command's `help`; the output of the command is printed
in the logs. However, this output is too verbose and can be confusing.
This PR addresses this issue by using the utility version to check the
command, instead of `help`, which is much less verbose.
### Types of changes
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] Enhancement (improves an existing feature and functionality)
- [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
- [ ] build/CI
### Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
- [ ] Major
- [x] Minor
Bug Severity
- [ ] BLOCKER
- [ ] Critical
- [ ] Major
- [ ] Minor
- [ ] Trivial
### Screenshots (if appropriate):
- Before changes:
- After changes:
### How Has This Been Tested?
I checked the logs before and after changes. As shown in the screenshots
above, the logs became more intuitive.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Change the check for the existence of `cryptsetup` command [cloudstack]
lucas-a-martins closed pull request #8480: Change the check for the existence of `cryptsetup` command URL: https://github.com/apache/cloudstack/pull/8480 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Change the check for the existence of `cryptsetup` command [cloudstack]
boring-cyborg[bot] commented on PR #8480: URL: https://github.com/apache/cloudstack/pull/8480#issuecomment-1883695160 Congratulations on your first Pull Request and welcome to the Apache CloudStack community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/cloudstack/blob/main/CONTRIBUTING.md) Here are some useful points: - In case of a new feature add useful documentation (raise doc PR at https://github.com/apache/cloudstack-documentation) - Be patient and persistent. It might take some time to get a review or get the final approval from the committers. - Pay attention to the quality of your code, ensure tests are passing and your PR doesn't have conflicts. - Please follow [ASF Code of Conduct](https://github.com/apache/.github/blob/main/.github/CODE_OF_CONDUCT.md) for all communication including (but not limited to) comments on Pull Requests, Issues, Mailing list and Slack. - Be sure to read the [CloudStack Coding Conventions](https://cwiki.apache.org/confluence/display/CLOUDSTACK/Coding+conventions). Apache CloudStack is a community-driven project and together we are making it better . In case of doubts contact the developers at: Mailing List: d...@cloudstack.apache.org (https://cloudstack.apache.org/mailing-lists.html) Slack: https://apachecloudstack.slack.com/ -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] Change the check for the existence of `cryptsetup` command [cloudstack]
lucas-a-martins opened a new pull request, #8480:
URL: https://github.com/apache/cloudstack/pull/8480
### Description
Currently, when starting the Agent, ACS checks if the `cryptsetup` command
can be used by using the command's `help`; the output of the command is printed
in the logs. However, this output is too verbose and can be confusing.
This PR addresses this issue by using the utility version to check the
command, instead of `help`, which is much less verbose.
### Types of changes
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] Enhancement (improves an existing feature and functionality)
- [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
- [ ] build/CI
### Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
- [ ] Major
- [x] Minor
Bug Severity
- [ ] BLOCKER
- [ ] Critical
- [ ] Major
- [ ] Minor
- [ ] Trivial
### Screenshots (if appropriate):
- Before changes:
- After changes:
### How Has This Been Tested?
I checked the logs before and after changes. As shown in the screenshots
above, the logs became more intuitive.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
