Backroll Plugin - #8251
Conversation
|
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)
|
|
@blueorangutan package |
|
@weizhouapache 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. |
Codecov Report✅ All modified and coverable lines are covered by tests.
Additional details and impacted files@@ Coverage Diff @@
## main #8251 +/- ##
============================================
- Coverage 17.92% 3.66% -14.26%
============================================
Files 5939 454 -5485
Lines 533181 38540 -494641
Branches 65237 7126 -58111
============================================
- Hits 95585 1414 -94171
+ Misses 426856 36938 -389918
+ Partials 10740 188 -10552
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 7806 |
|
@PeterBackroll , nice to see this coming in. Will you add a documentation PR as well? I think you'll have a lot of reviews ;) |
|
@blueorangutan test |
|
@DaanHoogland a [SL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
|
[SF] Trillian test result (tid-8383)
|
|
Hi @DaanHoogland Yes I will add a documentation PR as soon as possible :) |
DaanHoogland
left a comment
DaanHoogland
left a comment
There was a problem hiding this comment.
some comments @PeterBackroll .
I haven't gotten to review the Provider and Client yet, the core of the plugin.
I'll do that next week ;)
|
@blueorangutan package |
|
@rohityadavcloud 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. |
|
Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 7834 |
|
@PeterBackroll can you please have a look at the GH actions failures |
DaanHoogland
left a comment
DaanHoogland
left a comment
There was a problem hiding this comment.
some comments @PeterBackroll .
I haven't gotten to review the Provider and Client yet, the core of the plugin.
Thatś up next ;)
DaanHoogland
left a comment
DaanHoogland
left a comment
There was a problem hiding this comment.
looks generally good, @PeterBackroll .
some changes will have to be done though (before merging)
|
|
||
| public ConfigKey<String> BackrollPasswordConfigKey = new ConfigKey<>("Advanced", String.class, | ||
| "backup.plugin.backroll.config.password", | ||
| "VviX8dALauSyYJMqVYJqf3UyZOpO3joS", |
There was a problem hiding this comment.
can we make this more clear, this string is just as insecure but less inviting to change
| "VviX8dALauSyYJMqVYJqf3UyZOpO3joS", | |
| "password", |
or
| "VviX8dALauSyYJMqVYJqf3UyZOpO3joS", | |
| "C'est vraiment secret", |
| public boolean deleteBackup(Backup backup, boolean forced) { | ||
| s_logger.info("backroll delete backup id: " + backup.getExternalId()); | ||
| if(backup.getStatus().equals(Backup.Status.BackingUp)) { | ||
| throw new CloudRuntimeException("You can't delete a backup while it still BackingUp"); |
There was a problem hiding this comment.
| throw new CloudRuntimeException("You can't delete a backup while it still BackingUp"); | |
| throw new CloudRuntimeException("You can't delete a backup while it's still BackingUp"); |
|
@PeterBackroll do we have any docs on how to configure the plugin, what kind of storage is supported, any limitations, etc? |
|
@PeterBackroll Since this is for the 4.22.1 release, could you retarget the PR to the 4.22 branch? |
|
@PeterBackroll , is work on this still progressing? |
Co-authored-by: Harikrishna <harikrishna.patnala@gmail.com>
|
Hello there, we are working on experimental changes on other branches and then we merge on the PR’s branch. While I was testing the plugin, I faced an error. Before starting to dig into the issue, I thought that it could be better to be up-to-date with the main branch. So I updated the main branch and merged it into my working branch but it was no longer compiling. Then I tried to compile the main branch with To understand better what was happening, I had a look at these pages : So I tried I ended up with two questions :
|
|
@m-dhellin , we keep phony PRs like #12198 to regularly test release branches (main, 4.22, 4.20) . internally as a company we have a daily build as well, but that is not shared. Last night’s passed on all branches. Did you check java versions? |
|
Thank you @DaanHoogland, I reviewed and updated my development environment (java + maven) and it’s compiling again. |
|
Hello, I’m no longer able to test the plugin because of this error :
I run |
|
@m-dhellin , this sounds like some bean cannot load (maybe because the DB-scheme doesn’t match its current definition??) Do or did your beans use ‘locations’ and did that get changed or did an update of the upstream repository contain such a change? |
|
Thank you @DaanHoogland for your quick reply, I reset the DB with |
|
I’ve been able to fix the issue by adding a dependency link with a temporary absolute path and recompiling : core/src/main/resources/META-INF/cloudstack/bootstrap/spring-bootstrap-context-inheritable.xml Added this line : <import resource="file:/opt/cloudstack/framework/spring/module/src/main/resources/org/apache/cloudstack/spring/module/model/impl/defaults-context.xml"/>The resulting XML : <beans xmlns="http://www.springframework.org/schema/beans"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns:context="http://www.springframework.org/schema/context"
xmlns:aop="http://www.springframework.org/schema/aop"
xsi:schemaLocation="http://www.springframework.org/schema/beans
http://www.springframework.org/schema/beans/spring-beans.xsd
http://www.springframework.org/schema/aop http://www.springframework.org/schema/aop/spring-aop.xsd
http://www.springframework.org/schema/context
http://www.springframework.org/schema/context/spring-context.xsd"
>
<context:annotation-config />
<import resource="file:/opt/cloudstack/framework/spring/module/src/main/resources/org/apache/cloudstack/spring/module/model/impl/defaults-context.xml"/>
<bean class="org.springframework.beans.factory.config.PropertyPlaceholderConfigurer">
<property name="order" value="100" />
<property name="ignoreResourceNotFound" value="true" />
<property name="ignoreUnresolvablePlaceholders" value="true" />
<property name="locations" ref="DefaultConfigResources" />
</bean>
</beans>But know that’s the bean « registryRegistry » which is not found. I may try this for every « ref » attribute I find in beans definitions if I don’t find a better solution. |
|
@m-dhellin , I never heard of this being needed before. If it works for you it work, but I am really curious about the root cause and whether this is specific to your component/project/plugin/change??? |
|
Hi @PeterBackroll @m-dhellin is this PR ready for review, or are you still working on it? |
DaanHoogland
moved this from Backlog
to conflict/waiting for author
in CloudStack Testing
19 participants
Description
This is the pull request for Backroll Plugin (a backup & restore plugin).
This plugin allows users to use Backroll as backup provider inside Cloudstack.
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity