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/09/23 09:35:41 UTC
[GitHub] [cloudstack] Pearl1594 opened a new pull request #5504: Fix permission issue during Diagnostic service garbage collection
Pearl1594 opened a new pull request #5504:
URL: https://github.com/apache/cloudstack/pull/5504
### Description
This PR attempts to fix the permission issue observed during unmounting of secondary store(s) during the diagnostic service run (on VMware) and the Diagnostic service garbage collector:
```
2021-09-23 05:17:58,206 DEBUG [o.a.c.s.NfsMountManager] (BackgroundTaskPollManager-1:ctx-d5fcff87) (logid:779879f6) Executing while with timeout : 10000
2021-09-23 05:17:58,263 DEBUG [o.a.c.s.NfsMountManager] (BackgroundTaskPollManager-1:ctx-d5fcff87) (logid:779879f6) Execution is successful.
2021-09-23 05:17:58,267 DEBUG [o.a.c.s.NfsMountManager] (BackgroundTaskPollManager-1:ctx-d5fcff87) (logid:779879f6) Executing: sudo chmod 1777 /var/cloudstack/mnt/32989157261914.7b4126ed
2021-09-23 05:17:58,275 DEBUG [o.a.c.s.NfsMountManager] (BackgroundTaskPollManager-1:ctx-d5fcff87) (logid:779879f6) Executing while with timeout : 10000
2021-09-23 05:17:58,292 DEBUG [o.a.c.s.NfsMountManager] (BackgroundTaskPollManager-1:ctx-d5fcff87) (logid:779879f6) Execution is successful.
2021-09-23 05:17:58,300 DEBUG [o.a.c.d.DiagnosticsHelper] (BackgroundTaskPollManager-1:ctx-d5fcff87) (logid:779879f6) Executing: /bin/bash -c umount /var/cloudstack/mnt/32989157261914.7b4126ed
2021-09-23 05:17:58,304 DEBUG [o.a.c.d.DiagnosticsHelper] (BackgroundTaskPollManager-1:ctx-d5fcff87) (logid:779879f6) Executing while with timeout : 3600000
2021-09-23 05:17:58,305 DEBUG [o.a.c.d.DiagnosticsHelper] (BackgroundTaskPollManager-1:ctx-d5fcff87) (logid:779879f6) Exit value is 1
2021-09-23 05:17:58,306 DEBUG [o.a.c.d.DiagnosticsHelper] (BackgroundTaskPollManager-1:ctx-d5fcff87) (logid:779879f6) umount: /var/cloudstack/mnt/32989157261914.7b4126ed: umount failed: Operation not permitted
```
This can be solved in one of 2 ways,
1. Do not unmount the secondary store across successive runs of the diagnostic service clean up job or collection of diagnostic data(on vmware)
2. Fix the issue of unmount (by running the umount command using sudo) and cleaning up the mount path so that during successive runs a new mount path can be created - https://github.com/shapeblue/cloudstack/commit/a9b460869b8f244f4b3db3cc3973f3419dffed83
Current solution follows Option 1 as it seems to be a simpler fix, and the storage unmount happens during Cloudstack Management service restarts
<!--- 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?
Testing on KVM and VMware env - reduced the GC interval for diagnostic service - 'diagnostics.data.gc.interval' and observed no more failures and on restart of MS, the store is unmounted and path is deleted
<!-- 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] nvazquez merged pull request #5504: Fix permission issue during Diagnostic service garbage collection
Posted by GitBox <gi...@apache.org>.
nvazquez merged pull request #5504:
URL: https://github.com/apache/cloudstack/pull/5504
--
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 #5504: Fix permission issue during Diagnostic service garbage collection
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #5504:
URL: https://github.com/apache/cloudstack/pull/5504#discussion_r717450314
##########
File path: server/src/main/java/org/apache/cloudstack/diagnostics/DiagnosticsServiceImpl.java
##########
@@ -481,17 +478,11 @@ public Long getDelay() {
private void cleanupOldDiagnosticFiles(DataStore store) {
String mountPoint = null;
- try {
- mountPoint = serviceImpl.mountManager.getMountPoint(store.getUri(), null);
- if (StringUtils.isNotBlank(mountPoint)) {
- File directory = new File(mountPoint + File.separator + DIAGNOSTICS_DIRECTORY);
- if (directory.isDirectory()) {
- deleteOldDiagnosticsFiles(directory, store.getName());
- }
- }
- } finally {
- if (StringUtils.isNotBlank(mountPoint)) {
- umountSecondaryStorage(mountPoint);
+ mountPoint = serviceImpl.mountManager.getMountPoint(store.getUri(), null);
+ if (StringUtils.isNotBlank(mountPoint)) {
+ File directory = new File(mountPoint + File.separator + DIAGNOSTICS_DIRECTORY);
+ if (directory.isDirectory()) {
+ deleteOldDiagnosticsFiles(directory, store.getName());
Review comment:
I would agree if this was on hosts @sureshanaparti , but it is on the MS and
1. I would hate using sudo once more in the system
2. I would prefer to keep as much uniformity over hypervisor types as possible, so rather not have a vmware specific solution.
what do you think?
--
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 #5504: Fix permission issue during Diagnostic service garbage collection
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #5504:
URL: https://github.com/apache/cloudstack/pull/5504#discussion_r717294026
##########
File path: server/src/main/java/org/apache/cloudstack/diagnostics/DiagnosticsServiceImpl.java
##########
@@ -481,17 +478,11 @@ public Long getDelay() {
private void cleanupOldDiagnosticFiles(DataStore store) {
String mountPoint = null;
- try {
- mountPoint = serviceImpl.mountManager.getMountPoint(store.getUri(), null);
- if (StringUtils.isNotBlank(mountPoint)) {
- File directory = new File(mountPoint + File.separator + DIAGNOSTICS_DIRECTORY);
- if (directory.isDirectory()) {
- deleteOldDiagnosticsFiles(directory, store.getName());
- }
- }
- } finally {
- if (StringUtils.isNotBlank(mountPoint)) {
- umountSecondaryStorage(mountPoint);
+ mountPoint = serviceImpl.mountManager.getMountPoint(store.getUri(), null);
+ if (StringUtils.isNotBlank(mountPoint)) {
+ File directory = new File(mountPoint + File.separator + DIAGNOSTICS_DIRECTORY);
+ if (directory.isDirectory()) {
+ deleteOldDiagnosticsFiles(directory, store.getName());
Review comment:
@Pearl1594 any update on this ^^^ , possible to fix this permission issue for VMware only?
--
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 #5504: Fix permission issue during Diagnostic service garbage collection
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5504:
URL: https://github.com/apache/cloudstack/pull/5504#issuecomment-925699521
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 1379
--
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 #5504: Fix permission issue during Diagnostic service garbage collection
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5504:
URL: https://github.com/apache/cloudstack/pull/5504#issuecomment-926435411
@rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) 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] rhtyd commented on pull request #5504: Fix permission issue during Diagnostic service garbage collection
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #5504:
URL: https://github.com/apache/cloudstack/pull/5504#issuecomment-926435147
@blueorangutan 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-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #5504: Fix permission issue during Diagnostic service garbage collection
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #5504:
URL: https://github.com/apache/cloudstack/pull/5504#issuecomment-929824945
--
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 #5504: Fix permission issue during Diagnostic service garbage collection
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #5504:
URL: https://github.com/apache/cloudstack/pull/5504#discussion_r717450314
##########
File path: server/src/main/java/org/apache/cloudstack/diagnostics/DiagnosticsServiceImpl.java
##########
@@ -481,17 +478,11 @@ public Long getDelay() {
private void cleanupOldDiagnosticFiles(DataStore store) {
String mountPoint = null;
- try {
- mountPoint = serviceImpl.mountManager.getMountPoint(store.getUri(), null);
- if (StringUtils.isNotBlank(mountPoint)) {
- File directory = new File(mountPoint + File.separator + DIAGNOSTICS_DIRECTORY);
- if (directory.isDirectory()) {
- deleteOldDiagnosticsFiles(directory, store.getName());
- }
- }
- } finally {
- if (StringUtils.isNotBlank(mountPoint)) {
- umountSecondaryStorage(mountPoint);
+ mountPoint = serviceImpl.mountManager.getMountPoint(store.getUri(), null);
+ if (StringUtils.isNotBlank(mountPoint)) {
+ File directory = new File(mountPoint + File.separator + DIAGNOSTICS_DIRECTORY);
+ if (directory.isDirectory()) {
+ deleteOldDiagnosticsFiles(directory, store.getName());
Review comment:
I would agree if this was on hosts @sureshanaparti , but it is on the MS and
1. I would hate using sudo once more in the system
2. I would prefer to keep as much uniformity over hypervisor types as possible, so rather not have a vmware specific solution.
what do you think?
--
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 #5504: Fix permission issue during Diagnostic service garbage collection
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5504:
URL: https://github.com/apache/cloudstack/pull/5504#issuecomment-929825292
@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] rhtyd commented on pull request #5504: Fix permission issue during Diagnostic service garbage collection
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #5504:
URL: https://github.com/apache/cloudstack/pull/5504#issuecomment-929853480
@blueorangutan 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-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 #5504: Fix permission issue during Diagnostic service garbage collection
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #5504:
URL: https://github.com/apache/cloudstack/pull/5504#discussion_r717327775
##########
File path: server/src/main/java/org/apache/cloudstack/diagnostics/DiagnosticsServiceImpl.java
##########
@@ -481,17 +478,11 @@ public Long getDelay() {
private void cleanupOldDiagnosticFiles(DataStore store) {
String mountPoint = null;
- try {
- mountPoint = serviceImpl.mountManager.getMountPoint(store.getUri(), null);
- if (StringUtils.isNotBlank(mountPoint)) {
- File directory = new File(mountPoint + File.separator + DIAGNOSTICS_DIRECTORY);
- if (directory.isDirectory()) {
- deleteOldDiagnosticsFiles(directory, store.getName());
- }
- }
- } finally {
- if (StringUtils.isNotBlank(mountPoint)) {
- umountSecondaryStorage(mountPoint);
+ mountPoint = serviceImpl.mountManager.getMountPoint(store.getUri(), null);
+ if (StringUtils.isNotBlank(mountPoint)) {
+ File directory = new File(mountPoint + File.separator + DIAGNOSTICS_DIRECTORY);
+ if (directory.isDirectory()) {
+ deleteOldDiagnosticsFiles(directory, store.getName());
Review comment:
OK @Pearl1594 , Got it. I think, keeping secondary store mounted always on all the hypervisors is also not good. Only SSVM should have the access to it always.
--
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 #5504: Fix permission issue during Diagnostic service garbage collection
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #5504:
URL: https://github.com/apache/cloudstack/pull/5504#discussion_r715556173
##########
File path: server/src/main/java/org/apache/cloudstack/diagnostics/DiagnosticsServiceImpl.java
##########
@@ -481,17 +478,11 @@ public Long getDelay() {
private void cleanupOldDiagnosticFiles(DataStore store) {
String mountPoint = null;
- try {
- mountPoint = serviceImpl.mountManager.getMountPoint(store.getUri(), null);
- if (StringUtils.isNotBlank(mountPoint)) {
- File directory = new File(mountPoint + File.separator + DIAGNOSTICS_DIRECTORY);
- if (directory.isDirectory()) {
- deleteOldDiagnosticsFiles(directory, store.getName());
- }
- }
- } finally {
- if (StringUtils.isNotBlank(mountPoint)) {
- umountSecondaryStorage(mountPoint);
+ mountPoint = serviceImpl.mountManager.getMountPoint(store.getUri(), null);
+ if (StringUtils.isNotBlank(mountPoint)) {
+ File directory = new File(mountPoint + File.separator + DIAGNOSTICS_DIRECTORY);
+ if (directory.isDirectory()) {
+ deleteOldDiagnosticsFiles(directory, store.getName());
Review comment:
Does the secondary storage stay mounted on hypervisors other than VMware, after cleanup of diagnostics files? If so, it is better to fix the permission issue (only for VMware), and unmount the secondary storage from mgmt server when not needed.
--
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 #5504: Fix permission issue during Diagnostic service garbage collection
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5504:
URL: https://github.com/apache/cloudstack/pull/5504#issuecomment-930679119
@nvazquez a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) 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] sureshanaparti commented on a change in pull request #5504: Fix permission issue during Diagnostic service garbage collection
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #5504:
URL: https://github.com/apache/cloudstack/pull/5504#discussion_r717294026
##########
File path: server/src/main/java/org/apache/cloudstack/diagnostics/DiagnosticsServiceImpl.java
##########
@@ -481,17 +478,11 @@ public Long getDelay() {
private void cleanupOldDiagnosticFiles(DataStore store) {
String mountPoint = null;
- try {
- mountPoint = serviceImpl.mountManager.getMountPoint(store.getUri(), null);
- if (StringUtils.isNotBlank(mountPoint)) {
- File directory = new File(mountPoint + File.separator + DIAGNOSTICS_DIRECTORY);
- if (directory.isDirectory()) {
- deleteOldDiagnosticsFiles(directory, store.getName());
- }
- }
- } finally {
- if (StringUtils.isNotBlank(mountPoint)) {
- umountSecondaryStorage(mountPoint);
+ mountPoint = serviceImpl.mountManager.getMountPoint(store.getUri(), null);
+ if (StringUtils.isNotBlank(mountPoint)) {
+ File directory = new File(mountPoint + File.separator + DIAGNOSTICS_DIRECTORY);
+ if (directory.isDirectory()) {
+ deleteOldDiagnosticsFiles(directory, store.getName());
Review comment:
@Pearl1594 any update on this ^^^ , possible to fix this permission issue for VMware only?
##########
File path: server/src/main/java/org/apache/cloudstack/diagnostics/DiagnosticsServiceImpl.java
##########
@@ -481,17 +478,11 @@ public Long getDelay() {
private void cleanupOldDiagnosticFiles(DataStore store) {
String mountPoint = null;
- try {
- mountPoint = serviceImpl.mountManager.getMountPoint(store.getUri(), null);
- if (StringUtils.isNotBlank(mountPoint)) {
- File directory = new File(mountPoint + File.separator + DIAGNOSTICS_DIRECTORY);
- if (directory.isDirectory()) {
- deleteOldDiagnosticsFiles(directory, store.getName());
- }
- }
- } finally {
- if (StringUtils.isNotBlank(mountPoint)) {
- umountSecondaryStorage(mountPoint);
+ mountPoint = serviceImpl.mountManager.getMountPoint(store.getUri(), null);
+ if (StringUtils.isNotBlank(mountPoint)) {
+ File directory = new File(mountPoint + File.separator + DIAGNOSTICS_DIRECTORY);
+ if (directory.isDirectory()) {
+ deleteOldDiagnosticsFiles(directory, store.getName());
Review comment:
OK @Pearl1594 , Got it. I think, keeping secondary store mounted always on all the hypervisors is also not good. Only SSVM should have the access to it always.
--
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 #5504: Fix permission issue during Diagnostic service garbage collection
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5504:
URL: https://github.com/apache/cloudstack/pull/5504#issuecomment-929850865
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 1434
--
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 #5504: Fix permission issue during Diagnostic service garbage collection
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5504:
URL: https://github.com/apache/cloudstack/pull/5504#issuecomment-930303786
<b>Trillian test result (tid-2231)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 35040 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5504-t2231-kvm-centos7.zip
Smoke tests completed. 88 look OK, 1 have errors
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_01_invalid_upgrade_kubernetes_cluster | `Failure` | 0.01 | test_kubernetes_clusters.py
test_02_deploy_and_upgrade_kubernetes_cluster | `Failure` | 0.01 | test_kubernetes_clusters.py
test_03_deploy_and_scale_kubernetes_cluster | `Failure` | 0.01 | test_kubernetes_clusters.py
test_04_basic_lifecycle_kubernetes_cluster | `Failure` | 0.01 | test_kubernetes_clusters.py
test_05_delete_kubernetes_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
test_06_deploy_invalid_kubernetes_ha_cluster | `Failure` | 0.01 | test_kubernetes_clusters.py
test_07_deploy_kubernetes_ha_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
test_08_deploy_and_upgrade_kubernetes_ha_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
test_09_delete_kubernetes_ha_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
--
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 #5504: Fix permission issue during Diagnostic service garbage collection
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #5504:
URL: https://github.com/apache/cloudstack/pull/5504#discussion_r715556173
##########
File path: server/src/main/java/org/apache/cloudstack/diagnostics/DiagnosticsServiceImpl.java
##########
@@ -481,17 +478,11 @@ public Long getDelay() {
private void cleanupOldDiagnosticFiles(DataStore store) {
String mountPoint = null;
- try {
- mountPoint = serviceImpl.mountManager.getMountPoint(store.getUri(), null);
- if (StringUtils.isNotBlank(mountPoint)) {
- File directory = new File(mountPoint + File.separator + DIAGNOSTICS_DIRECTORY);
- if (directory.isDirectory()) {
- deleteOldDiagnosticsFiles(directory, store.getName());
- }
- }
- } finally {
- if (StringUtils.isNotBlank(mountPoint)) {
- umountSecondaryStorage(mountPoint);
+ mountPoint = serviceImpl.mountManager.getMountPoint(store.getUri(), null);
+ if (StringUtils.isNotBlank(mountPoint)) {
+ File directory = new File(mountPoint + File.separator + DIAGNOSTICS_DIRECTORY);
+ if (directory.isDirectory()) {
+ deleteOldDiagnosticsFiles(directory, store.getName());
Review comment:
Does the secondary storage stay mounted on hypervisors other than VMware, after cleanup of diagnostics files? If so, it is better to fix the permission issue (on VMware), and unmount the secondary storage from mgmt server when not needed.
--
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 #5504: Fix permission issue during Diagnostic service garbage collection
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5504:
URL: https://github.com/apache/cloudstack/pull/5504#issuecomment-925671654
@Pearl1594 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 #5504: Fix permission issue during Diagnostic service garbage collection
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #5504:
URL: https://github.com/apache/cloudstack/pull/5504#discussion_r718157625
##########
File path: server/src/main/java/org/apache/cloudstack/diagnostics/DiagnosticsServiceImpl.java
##########
@@ -481,17 +478,11 @@ public Long getDelay() {
private void cleanupOldDiagnosticFiles(DataStore store) {
String mountPoint = null;
- try {
- mountPoint = serviceImpl.mountManager.getMountPoint(store.getUri(), null);
- if (StringUtils.isNotBlank(mountPoint)) {
- File directory = new File(mountPoint + File.separator + DIAGNOSTICS_DIRECTORY);
- if (directory.isDirectory()) {
- deleteOldDiagnosticsFiles(directory, store.getName());
- }
- }
- } finally {
- if (StringUtils.isNotBlank(mountPoint)) {
- umountSecondaryStorage(mountPoint);
+ mountPoint = serviceImpl.mountManager.getMountPoint(store.getUri(), null);
+ if (StringUtils.isNotBlank(mountPoint)) {
+ File directory = new File(mountPoint + File.separator + DIAGNOSTICS_DIRECTORY);
+ if (directory.isDirectory()) {
+ deleteOldDiagnosticsFiles(directory, store.getName());
Review comment:
> I would agree if this was on hosts @sureshanaparti , but it is on the MS and
>
> 1. I would hate using sudo once more in the system
> 2. I would prefer to keep as much uniformity over hypervisor types as possible, so rather not have a vmware specific solution.
> what do you think?
agree @DaanHoogland
--
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] Pearl1594 commented on a change in pull request #5504: Fix permission issue during Diagnostic service garbage collection
Posted by GitBox <gi...@apache.org>.
Pearl1594 commented on a change in pull request #5504:
URL: https://github.com/apache/cloudstack/pull/5504#discussion_r717303713
##########
File path: server/src/main/java/org/apache/cloudstack/diagnostics/DiagnosticsServiceImpl.java
##########
@@ -481,17 +478,11 @@ public Long getDelay() {
private void cleanupOldDiagnosticFiles(DataStore store) {
String mountPoint = null;
- try {
- mountPoint = serviceImpl.mountManager.getMountPoint(store.getUri(), null);
- if (StringUtils.isNotBlank(mountPoint)) {
- File directory = new File(mountPoint + File.separator + DIAGNOSTICS_DIRECTORY);
- if (directory.isDirectory()) {
- deleteOldDiagnosticsFiles(directory, store.getName());
- }
- }
- } finally {
- if (StringUtils.isNotBlank(mountPoint)) {
- umountSecondaryStorage(mountPoint);
+ mountPoint = serviceImpl.mountManager.getMountPoint(store.getUri(), null);
+ if (StringUtils.isNotBlank(mountPoint)) {
+ File directory = new File(mountPoint + File.separator + DIAGNOSTICS_DIRECTORY);
+ if (directory.isDirectory()) {
+ deleteOldDiagnosticsFiles(directory, store.getName());
Review comment:
Sorry @sureshanaparti I seemed to have missed your earlier comment. Currently what's happening is that due to the permission issue, we see the secondary stores remain mounted on all the hypervisors. We could fix the permission issue using this solution - https://github.com/shapeblue/cloudstack/commit/a9b460869b8f244f4b3db3cc3973f3419dffed83. However, I am of the opinion that we probably don't need to mount and un-mount every time the garbage collection thread runs. As fixing the permission issue wouldn't solve the issue entirely, we'd also need to cleanup the mount path (at /var/cloudstack/mnt) and remove that mount path from the static list of storageMounts maintained, so that in successive runs are successful.
--
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 #5504: Fix permission issue during Diagnostic service garbage collection
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #5504:
URL: https://github.com/apache/cloudstack/pull/5504#issuecomment-929824945
@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 #5504: Fix permission issue during Diagnostic service garbage collection
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5504:
URL: https://github.com/apache/cloudstack/pull/5504#issuecomment-926881701
<b>Trillian test result (tid-2199)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 40520 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5504-t2199-kvm-centos7.zip
Smoke tests completed. 88 look OK, 1 have errors
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_07_deploy_kubernetes_ha_cluster | `Failure` | 3617.82 | test_kubernetes_clusters.py
test_08_deploy_and_upgrade_kubernetes_ha_cluster | `Failure` | 0.08 | test_kubernetes_clusters.py
test_09_delete_kubernetes_ha_cluster | `Failure` | 0.05 | test_kubernetes_clusters.py
ContextSuite context=TestKubernetesCluster>:teardown | `Error` | 80.92 | test_kubernetes_clusters.py
--
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 #5504: Fix permission issue during Diagnostic service garbage collection
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5504:
URL: https://github.com/apache/cloudstack/pull/5504#issuecomment-929853774
@rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) 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 #5504: Fix permission issue during Diagnostic service garbage collection
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5504:
URL: https://github.com/apache/cloudstack/pull/5504#issuecomment-931218260
<b>Trillian test result (tid-2242)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 32574 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5504-t2242-kvm-centos7.zip
Smoke tests completed. 89 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] Pearl1594 commented on a change in pull request #5504: Fix permission issue during Diagnostic service garbage collection
Posted by GitBox <gi...@apache.org>.
Pearl1594 commented on a change in pull request #5504:
URL: https://github.com/apache/cloudstack/pull/5504#discussion_r717303713
##########
File path: server/src/main/java/org/apache/cloudstack/diagnostics/DiagnosticsServiceImpl.java
##########
@@ -481,17 +478,11 @@ public Long getDelay() {
private void cleanupOldDiagnosticFiles(DataStore store) {
String mountPoint = null;
- try {
- mountPoint = serviceImpl.mountManager.getMountPoint(store.getUri(), null);
- if (StringUtils.isNotBlank(mountPoint)) {
- File directory = new File(mountPoint + File.separator + DIAGNOSTICS_DIRECTORY);
- if (directory.isDirectory()) {
- deleteOldDiagnosticsFiles(directory, store.getName());
- }
- }
- } finally {
- if (StringUtils.isNotBlank(mountPoint)) {
- umountSecondaryStorage(mountPoint);
+ mountPoint = serviceImpl.mountManager.getMountPoint(store.getUri(), null);
+ if (StringUtils.isNotBlank(mountPoint)) {
+ File directory = new File(mountPoint + File.separator + DIAGNOSTICS_DIRECTORY);
+ if (directory.isDirectory()) {
+ deleteOldDiagnosticsFiles(directory, store.getName());
Review comment:
Sorry @sureshanaparti I seemed to have missed your earlier comment. Currently what's happening is that due to the permission issue, we see the secondary stores remain mounted on all the hypervisors. We could fix the permission issue using this solution - https://github.com/shapeblue/cloudstack/commit/a9b460869b8f244f4b3db3cc3973f3419dffed83. However, I am of the opinion that we probably don't need to mount and un-mount every time the garbage collection thread runs. As fixing the permission issue wouldn't solve the issue entirely, we'd also need to cleanup the mount path (at /var/cloudstack/mnt) and remove that mount path from the static list of storageMounts maintained, so that in successive runs are successful.
--
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 #5504: Fix permission issue during Diagnostic service garbage collection
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #5504:
URL: https://github.com/apache/cloudstack/pull/5504#discussion_r718157625
##########
File path: server/src/main/java/org/apache/cloudstack/diagnostics/DiagnosticsServiceImpl.java
##########
@@ -481,17 +478,11 @@ public Long getDelay() {
private void cleanupOldDiagnosticFiles(DataStore store) {
String mountPoint = null;
- try {
- mountPoint = serviceImpl.mountManager.getMountPoint(store.getUri(), null);
- if (StringUtils.isNotBlank(mountPoint)) {
- File directory = new File(mountPoint + File.separator + DIAGNOSTICS_DIRECTORY);
- if (directory.isDirectory()) {
- deleteOldDiagnosticsFiles(directory, store.getName());
- }
- }
- } finally {
- if (StringUtils.isNotBlank(mountPoint)) {
- umountSecondaryStorage(mountPoint);
+ mountPoint = serviceImpl.mountManager.getMountPoint(store.getUri(), null);
+ if (StringUtils.isNotBlank(mountPoint)) {
+ File directory = new File(mountPoint + File.separator + DIAGNOSTICS_DIRECTORY);
+ if (directory.isDirectory()) {
+ deleteOldDiagnosticsFiles(directory, store.getName());
Review comment:
> I would agree if this was on hosts @sureshanaparti , but it is on the MS and
>
> 1. I would hate using sudo once more in the system
> 2. I would prefer to keep as much uniformity over hypervisor types as possible, so rather not have a vmware specific solution.
> what do you think?
agree @DaanHoogland
--
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 #5504: Fix permission issue during Diagnostic service garbage collection
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5504:
URL: https://github.com/apache/cloudstack/pull/5504#issuecomment-929825292
--
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] nvazquez commented on pull request #5504: Fix permission issue during Diagnostic service garbage collection
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #5504:
URL: https://github.com/apache/cloudstack/pull/5504#issuecomment-930678712
@blueorangutan 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-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Pearl1594 commented on pull request #5504: Fix permission issue during Diagnostic service garbage collection
Posted by GitBox <gi...@apache.org>.
Pearl1594 commented on pull request #5504:
URL: https://github.com/apache/cloudstack/pull/5504#issuecomment-925670912
@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