You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2021/11/10 10:42:38 UTC
[GitHub] [cloudstack] weizhouapache opened a new pull request #5675: server bug fix: remove network details when network is removed
weizhouapache opened a new pull request #5675:
URL: https://github.com/apache/cloudstack/pull/5675
### Description
This PR removes network details when network is removed.
<!--- Describe your changes in DETAIL - And how has behaviour functionally changed. -->
<!-- For new features, provide link to FS, dev ML discussion etc. -->
<!-- In case of bug fix, the expected and actual behaviours, steps to reproduce. -->
<!-- When "Fixes: #<id>" is specified, the issue/PR will automatically be closed when this PR gets merged -->
<!-- For addressing multiple issues/PRs, use multiple "Fixes: #<id>" -->
<!-- Fixes: # -->
<!--- ********************************************************************************* -->
<!--- NOTE: AUTOMATATION USES THE DESCRIPTIONS TO SET LABELS AND PRODUCE DOCUMENTATION. -->
<!--- PLEASE PUT AN 'X' in only **ONE** box -->
<!--- ********************************************************************************* -->
### Types of changes
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] New feature (non-breaking change which adds functionality)
- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] Enhancement (improves an existing feature and functionality)
- [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
### Feature/Enhancement Scale or Bug Severity
#### Feature/Enhancement Scale
- [ ] Major
- [ ] Minor
#### Bug Severity
- [ ] BLOCKER
- [ ] Critical
- [ ] Major
- [x] Minor
- [ ] Trivial
### Screenshots (if appropriate):
### How Has This Been Tested?
<!-- Please describe in detail how you tested your changes. -->
<!-- Include details of your testing environment, and the tests you ran to -->
<!-- see how your change affects other areas of the code, etc. -->
<!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/main/CONTRIBUTING.md) document -->
--
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-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on pull request #5675: server bug fix: remove network details when network is removed
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #5675:
URL: https://github.com/apache/cloudstack/pull/5675#issuecomment-977783212
@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-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #5675: server bug fix: remove network details when network is removed
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #5675:
URL: https://github.com/apache/cloudstack/pull/5675#issuecomment-976416871
@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-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on pull request #5675: server bug fix: remove network details when network is removed
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #5675:
URL: https://github.com/apache/cloudstack/pull/5675#issuecomment-972785560
@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-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #5675: server bug fix: remove network details when network is removed
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #5675:
URL: https://github.com/apache/cloudstack/pull/5675#discussion_r756014502
##########
File path: engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java
##########
@@ -3116,6 +3116,8 @@ public void doInTransactionWithoutResult(final TransactionStatus status) {
if (networkAccount != null) {
_networkAccountDao.remove(networkAccount.getId());
}
+
+ networkDetailsDao.removeDetails(networkFinal.getId());
Review comment:
The foreign key should serve the purpose here. But, when a network is destroyed, the removed date is set, and the record is not removed from the table (to keep/track the history at later stage when needed). The same is the case with most of the resources, where the foreign key is set with 'ON DELETE CASCADE' but unused. May be, a cleanup (enabled/disabled) for all such removed entires from the DB, have to be introduced at regular intervals (that can be set using global config).
```
CONSTRAINT `fk_network_details__network_id` FOREIGN KEY `fk_network_details__network_id`(`network_id`) REFERENCES `networks`(`id`) ON DELETE CASCADE
```
--
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-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #5675: server bug fix: remove network details when network is removed
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5675:
URL: https://github.com/apache/cloudstack/pull/5675#issuecomment-977817090
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 1777
--
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-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #5675: server bug fix: remove network details when network is removed
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #5675:
URL: https://github.com/apache/cloudstack/pull/5675#issuecomment-972637343
@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-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #5675: server bug fix: remove network details when network is removed
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5675:
URL: https://github.com/apache/cloudstack/pull/5675#issuecomment-976530710
Packaging result: :heavy_multiplication_x: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 1770
--
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-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache merged pull request #5675: server bug fix: remove network details when network is removed
Posted by GitBox <gi...@apache.org>.
weizhouapache merged pull request #5675:
URL: https://github.com/apache/cloudstack/pull/5675
--
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-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #5675: server bug fix: remove network details when network is removed
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5675:
URL: https://github.com/apache/cloudstack/pull/5675#issuecomment-978362893
<b>Trillian test result (tid-2580)</b>
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server 7
Total time taken: 35311 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5675-t2580-vmware-67u3.zip
Smoke tests completed. 91 look OK, 0 have errors
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
--
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-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #5675: server bug fix: remove network details when network is removed
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5675:
URL: https://github.com/apache/cloudstack/pull/5675#issuecomment-976417148
@rhtyd a Jenkins job has been kicked to build packages. 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-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #5675: server bug fix: remove network details when network is removed
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #5675:
URL: https://github.com/apache/cloudstack/pull/5675#discussion_r756014502
##########
File path: engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java
##########
@@ -3116,6 +3116,8 @@ public void doInTransactionWithoutResult(final TransactionStatus status) {
if (networkAccount != null) {
_networkAccountDao.remove(networkAccount.getId());
}
+
+ networkDetailsDao.removeDetails(networkFinal.getId());
Review comment:
The foreign key should serve the purpose here. But, when a network is destroyed, the removed date is set, and the record is not removed from the table (to keep/track the history at later stage when needed). The same is the case with most of the resources, where the foreign key is set with 'ON DELETE CASCADE' but unused. May be, a cleanup (enabled/disabled) for all such removed entires from the DB, have to be introduced at regular intervals (that can be set using global config).
`CONSTRAINT `fk_network_details__network_id` FOREIGN KEY `fk_network_details__network_id`(`network_id`) REFERENCES `networks`(`id`) ON DELETE CASCADE`
--
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-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache closed pull request #5675: server bug fix: remove network details when network is removed
Posted by GitBox <gi...@apache.org>.
weizhouapache closed pull request #5675:
URL: https://github.com/apache/cloudstack/pull/5675
--
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-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #5675: server bug fix: remove network details when network is removed
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5675:
URL: https://github.com/apache/cloudstack/pull/5675#issuecomment-972785826
@weizhouapache a Jenkins job has been kicked to build packages. 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-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #5675: server bug fix: remove network details when network is removed
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5675:
URL: https://github.com/apache/cloudstack/pull/5675#issuecomment-977784062
@sureshanaparti a Jenkins job has been kicked to build packages. 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-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #5675: server bug fix: remove network details when network is removed
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #5675:
URL: https://github.com/apache/cloudstack/pull/5675#discussion_r756014502
##########
File path: engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java
##########
@@ -3116,6 +3116,8 @@ public void doInTransactionWithoutResult(final TransactionStatus status) {
if (networkAccount != null) {
_networkAccountDao.remove(networkAccount.getId());
}
+
+ networkDetailsDao.removeDetails(networkFinal.getId());
Review comment:
The foreign key should serve the purpose here. But, when a network is destroyed, the removed date is set, and the record is not removed from the table (to keep/track the history at later stage when needed). The same is the case with most of the resources, where the foreign key is set with 'ON DELETE CASCADE' but unused. May be, a cleanup (enabled/disabled) for all such removed entires from the DB, have to be introduced at regular intervals (that can be set using global config).
`
CONSTRAINT `fk_network_details__network_id` FOREIGN KEY `fk_network_details__network_id`(`network_id`) REFERENCES `networks`(`id`) ON DELETE CASCADE
`
--
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-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on a change in pull request #5675: server bug fix: remove network details when network is removed
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on a change in pull request #5675:
URL: https://github.com/apache/cloudstack/pull/5675#discussion_r756083718
##########
File path: engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java
##########
@@ -3116,6 +3116,8 @@ public void doInTransactionWithoutResult(final TransactionStatus status) {
if (networkAccount != null) {
_networkAccountDao.remove(networkAccount.getId());
}
+
+ networkDetailsDao.removeDetails(networkFinal.getId());
Review comment:
>
@sureshanaparti
yes, you are absolutely right. `ON DELETE CASCADE` does not work as expected because cloudstack updates `removed` field instead deleting the record from DB.
--
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-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on pull request #5675: server bug fix: remove network details when network is removed
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #5675:
URL: https://github.com/apache/cloudstack/pull/5675#issuecomment-979452254
Merged based on 3 approvals and trillian test result.
--
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-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #5675: server bug fix: remove network details when network is removed
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5675:
URL: https://github.com/apache/cloudstack/pull/5675#issuecomment-972735910
Packaging result: :heavy_multiplication_x: el7 :heavy_multiplication_x: el8 :heavy_multiplication_x: debian :heavy_check_mark: suse15. SL-JID 1724
--
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-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #5675: server bug fix: remove network details when network is removed
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5675:
URL: https://github.com/apache/cloudstack/pull/5675#issuecomment-972637956
@rhtyd a Jenkins job has been kicked to build packages. 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-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #5675: server bug fix: remove network details when network is removed
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #5675:
URL: https://github.com/apache/cloudstack/pull/5675#discussion_r751993563
##########
File path: engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java
##########
@@ -3116,6 +3116,8 @@ public void doInTransactionWithoutResult(final TransactionStatus status) {
if (networkAccount != null) {
_networkAccountDao.remove(networkAccount.getId());
}
+ // Remove network details
Review comment:
I don't think this comment is needed, given the name of the method invoked
--
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-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #5675: server bug fix: remove network details when network is removed
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #5675:
URL: https://github.com/apache/cloudstack/pull/5675#issuecomment-977827526
@blueorangutan test centos7 vmware-67u3
--
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-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on pull request #5675: server bug fix: remove network details when network is removed
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #5675:
URL: https://github.com/apache/cloudstack/pull/5675#issuecomment-977888600
travis failures are not related to this pr
```
Tests ha enable/disable feature at cluster and zone level ... === TestName: test_ha_configure_enabledisable_across_clusterzones | Status : EXCEPTION ===
ERROR
```
and
```
Tests that an user is unable to remove, modify tags created by admin but should access ... === TestName: test_31_user_cant_remove_update_admin_tags | Status : EXCEPTION ===
ERROR
```
--
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-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache closed pull request #5675: server bug fix: remove network details when network is removed
Posted by GitBox <gi...@apache.org>.
weizhouapache closed pull request #5675:
URL: https://github.com/apache/cloudstack/pull/5675
--
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-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #5675: server bug fix: remove network details when network is removed
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5675:
URL: https://github.com/apache/cloudstack/pull/5675#issuecomment-977828715
@rhtyd a Trillian-Jenkins test job (centos7 mgmt + vmware-67u3) has been kicked to run smoke tests
--
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-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #5675: server bug fix: remove network details when network is removed
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5675:
URL: https://github.com/apache/cloudstack/pull/5675#issuecomment-972882338
Packaging result: :heavy_check_mark: el7 :heavy_multiplication_x: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 1735
--
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-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org