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
Removed state for removed accounts #7868
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good change! one remark, other entities have a state DESTROYED. maybe it would make sense to be in line with that?
Codecov Report
@@ Coverage Diff @@
## main #7868 +/- ##
=========================================
Coverage 14.40% 14.40%
Complexity 10109 10109
=========================================
Files 2748 2748
Lines 259390 259392 +2
Branches 40381 40381
=========================================
+ Hits 37354 37355 +1
- Misses 217203 217204 +1
Partials 4833 4833
... and 1 file with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
@blueorangutan package |
@weizhouapache a [SF] 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 6782 |
@hsato03 do you want this in 4.18.1? |
@DaanHoogland Yes, that's a good idea. But some entities have the
OK. |
223b7c2
to
91290ab
Compare
@DaanHoogland |
@weizhouapache REMOVED does make sense but having different terms with the same meaning is something we should prevent, and adding a new one should replace all occurences of the old ones. Not going to block this for that reason, but it will be confusing for new operators and developers alike. worth a discuss thread? |
@hsato03 |
I think this can be included in 4.18.1 as well, but do not mind either way |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - I'm not sure if there could be only possible side-effects; or it should also emit some kind of event for usage-server?
it is one of my concerns as well. |
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
91290ab
to
2110f09
Compare
@blueorangutan package |
@soreana a [SF] 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. |
I agree with @rohityadavcloud and @weizhouapache, REMOVED sounds better for accounts. @DaanHoogland Although your idea of standardizing the state of entities is good, there are already other entities that use the REMOVED state.
@rohityadavcloud ACS already emits an event when deleting an account. |
Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✖️ debian ✔️ suse15. SL-JID 6955 |
@blueorangutan package |
@soreana a [SF] 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 6973 |
@DaanHoogland @rohityadavcloud @weizhouapache I did some research to understand the definitions of Once an object is Destroyed in CloudStack, it remains visible to the root admin, who has the exclusive ability to either restore or remove it. Even when an object is destroyed, it can still be restored. But once an object is removed, it cannot be restored and will be lost permanently. As of now, it is not possible to restore the account that was removed. Therefore, using the term |
have you tested @soreana ? |
@DaanHoogland I tested that on fresh install of the CloudStack. I haven't tested the upgrade path from 4.18 to 4.19. I can check that as well later this week. |
Hi @soreana, Any updates on this one? |
I've tested the upgrade from 4.18.1 to 4.19 the process went smooth and the account have removed state now. Before upgrade:
After upgrade:
|
@soreana Thank's for testing. |
Description
When deleting an account, its state is maintained as
Enabled
even if it is removed.Accordingly the
Removed
state for accounts was created, which is always set when an account is deleted.Types of changes
Feature/Enhancement Scale or Bug Severity
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
I created and deleted an account. Then, I created a SELECT SQL query for the
account
table and the account state wasRemoved
.