[GitHub] [cloudstack] blueorangutan commented on issue #3601: JDK11, Debian 10 support
blueorangutan commented on issue #3601: JDK11, Debian 10 support URL: https://github.com/apache/cloudstack/pull/3601#issuecomment-566894836 Trillian test result (tid-613) Environment: vmware-65u2 (x2), Advanced Networking with Mgmt server 7 Total time taken: 497916 seconds Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr3601-t613-vmware-65u2.zip Intermittent failure detected: /marvin/tests/smoke/test_accounts.py Intermittent failure detected: /marvin/tests/smoke/test_affinity_groups_projects.py Intermittent failure detected: /marvin/tests/smoke/test_affinity_groups.py Intermittent failure detected: /marvin/tests/smoke/test_async_job.py Intermittent failure detected: /marvin/tests/smoke/test_certauthority_root.py Intermittent failure detected: /marvin/tests/smoke/test_deploy_vgpu_enabled_vm.py Intermittent failure detected: /marvin/tests/smoke/test_deploy_vm_iso.py Intermittent failure detected: /marvin/tests/smoke/test_deploy_vm_root_resize.py Intermittent failure detected: /marvin/tests/smoke/test_deploy_vms_with_varied_deploymentplanners.py Intermittent failure detected: /marvin/tests/smoke/test_deploy_vm_with_userdata.py Intermittent failure detected: /marvin/tests/smoke/test_diagnostics.py Intermittent failure detected: /marvin/tests/smoke/test_domain_disk_offerings.py Intermittent failure detected: /marvin/tests/smoke/test_domain_network_offerings.py Intermittent failure detected: /marvin/tests/smoke/test_domain_service_offerings.py Intermittent failure detected: /marvin/tests/smoke/test_domain_vpc_offerings.py Intermittent failure detected: /marvin/tests/smoke/test_internal_lb.py Intermittent failure detected: /marvin/tests/smoke/test_iso.py Intermittent failure detected: /marvin/tests/smoke/test_list_ids_parameter.py Intermittent failure detected: /marvin/tests/smoke/test_loadbalance.py Intermittent failure detected: /marvin/tests/smoke/test_metrics_api.py Intermittent failure detected: /marvin/tests/smoke/test_migration.py Intermittent failure detected: /marvin/tests/smoke/test_multipleips_per_nic.py Intermittent failure detected: /marvin/tests/smoke/test_nested_virtualization.py Intermittent failure detected: /marvin/tests/smoke/test_network_acl.py Intermittent failure detected: /marvin/tests/smoke/test_network.py Intermittent failure detected: /marvin/tests/smoke/test_nic.py Intermittent failure detected: /marvin/tests/smoke/test_password_server.py Intermittent failure detected: /marvin/tests/smoke/test_portforwardingrules.py Intermittent failure detected: /marvin/tests/smoke/test_primary_storage.py Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py Intermittent failure detected: /marvin/tests/smoke/test_projects.py Intermittent failure detected: /marvin/tests/smoke/test_resource_accounting.py Intermittent failure detected: /marvin/tests/smoke/test_scale_vm.py Intermittent failure detected: /marvin/tests/smoke/test_snapshots.py Intermittent failure detected: /marvin/tests/smoke/test_templates.py Intermittent failure detected: /marvin/tests/smoke/test_usage.py Intermittent failure detected: /marvin/tests/smoke/test_vm_deployment_planner.py Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py Intermittent failure detected: /marvin/tests/smoke/test_vm_snapshots.py Intermittent failure detected: /marvin/tests/smoke/test_volumes.py Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py Intermittent failure detected: /marvin/tests/smoke/test_vpc_router_nics.py Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py Intermittent failure detected: /marvin/tests/smoke/test_host_maintenance.py Smoke tests completed. 36 look OK, 41 have error(s) 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #3425: [WIP DO NOT MERGE] Better tracking host maintanence success and failure
blueorangutan commented on issue #3425: [WIP DO NOT MERGE] Better tracking host maintanence success and failure URL: https://github.com/apache/cloudstack/pull/3425#issuecomment-566863714 @anuragaw a Trillian-Jenkins matrix job (centos7 mgmt + xs71, centos7 mgmt + vmware65, centos7 mgmt + kvmcentos7) 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack] anuragaw commented on issue #3425: [WIP DO NOT MERGE] Better tracking host maintanence success and failure
anuragaw commented on issue #3425: [WIP DO NOT MERGE] Better tracking host maintanence success and failure URL: https://github.com/apache/cloudstack/pull/3425#issuecomment-566863597 @blueorangutan test matrix 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #3425: [WIP DO NOT MERGE] Better tracking host maintanence success and failure
blueorangutan commented on issue #3425: [WIP DO NOT MERGE] Better tracking host maintanence success and failure URL: https://github.com/apache/cloudstack/pull/3425#issuecomment-566863308 Packaging result: ✖centos6 ✔centos7 ✔debian. JID-483 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack] anuragaw commented on issue #3425: [WIP DO NOT MERGE] Better tracking host maintanence success and failure
anuragaw commented on issue #3425: [WIP DO NOT MERGE] Better tracking host maintanence success and failure URL: https://github.com/apache/cloudstack/pull/3425#issuecomment-566860015 @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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #3425: [WIP DO NOT MERGE] Better tracking host maintanence success and failure
blueorangutan commented on issue #3425: [WIP DO NOT MERGE] Better tracking host maintanence success and failure URL: https://github.com/apache/cloudstack/pull/3425#issuecomment-566860030 @anuragaw 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack] anuragaw removed a comment on issue #3425: [WIP DO NOT MERGE] Better tracking host maintanence success and failure
anuragaw removed a comment on issue #3425: [WIP DO NOT MERGE] Better tracking host maintanence success and failure URL: https://github.com/apache/cloudstack/pull/3425#issuecomment-566847434 @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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack] anuragaw commented on issue #3425: [WIP DO NOT MERGE] Better tracking host maintanence success and failure
anuragaw commented on issue #3425: [WIP DO NOT MERGE] Better tracking host maintanence success and failure URL: https://github.com/apache/cloudstack/pull/3425#issuecomment-566847414 Rebased against master. ping for final review @rhtyd , @nvazquez , @DaanHoogland , @borisstoyanov , @andrijapanicsb , @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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack] anuragaw commented on issue #3425: [WIP DO NOT MERGE] Better tracking host maintanence success and failure
anuragaw commented on issue #3425: [WIP DO NOT MERGE] Better tracking host maintanence success and failure URL: https://github.com/apache/cloudstack/pull/3425#issuecomment-566847434 @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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack] syed commented on issue #3772: Revert of the "Revert "Fix virtual template size for managed storage for KVM / refactor cloud-install-sys-tmplt""
syed commented on issue #3772: Revert of the "Revert "Fix virtual template size for managed storage for KVM / refactor cloud-install-sys-tmplt"" URL: https://github.com/apache/cloudstack/pull/3772#issuecomment-566806910 LGTM :+1: 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #3772: Revert of the "Revert "Fix virtual template size for managed storage for KVM / refactor cloud-install-sys-tmplt""
blueorangutan commented on issue #3772: Revert of the "Revert "Fix virtual template size for managed storage for KVM / refactor cloud-install-sys-tmplt"" URL: https://github.com/apache/cloudstack/pull/3772#issuecomment-566788904 @andrijapanicsb 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack] andrijapanicsb commented on issue #3772: Revert of the "Revert "Fix virtual template size for managed storage for KVM / refactor cloud-install-sys-tmplt""
andrijapanicsb commented on issue #3772: Revert of the "Revert "Fix virtual template size for managed storage for KVM / refactor cloud-install-sys-tmplt"" URL: https://github.com/apache/cloudstack/pull/3772#issuecomment-566788689 @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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #3772: Revert of the "Revert "Fix virtual template size for managed storage for KVM / refactor cloud-install-sys-tmplt""
blueorangutan commented on issue #3772: Revert of the "Revert "Fix virtual template size for managed storage for KVM / refactor cloud-install-sys-tmplt"" URL: https://github.com/apache/cloudstack/pull/3772#issuecomment-566787992 Packaging result: ✖centos6 ✔centos7 ✔debian. JID-482 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack] andrijapanicsb commented on issue #3772: Revert of the "Revert "Fix virtual template size for managed storage for KVM / refactor cloud-install-sys-tmplt""
andrijapanicsb commented on issue #3772: Revert of the "Revert "Fix virtual template size for managed storage for KVM / refactor cloud-install-sys-tmplt"" URL: https://github.com/apache/cloudstack/pull/3772#issuecomment-566783160 @svenvogel @DennisKonrad please review this PR (revert of the revert = same PR as the original one #3371 that you guys created. I'm doing another round of automated tests on this one and will be ready for merging based on all LGTMs on the original PR @weizhouapache may I kindly ask you for the opinion if PR looks good (equivalent to original PR 3371) - I've done checks myslef, but prefer another opinion please. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #3772: Revert of the "Revert "Fix virtual template size for managed storage for KVM / refactor cloud-install-sys-tmplt""
blueorangutan commented on issue #3772: Revert of the "Revert "Fix virtual template size for managed storage for KVM / refactor cloud-install-sys-tmplt"" URL: https://github.com/apache/cloudstack/pull/3772#issuecomment-566782193 @andrijapanicsb 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack] andrijapanicsb commented on issue #3772: Revert of the "Revert "Fix virtual template size for managed storage for KVM / refactor cloud-install-sys-tmplt""
andrijapanicsb commented on issue #3772: Revert of the "Revert "Fix virtual template size for managed storage for KVM / refactor cloud-install-sys-tmplt"" URL: https://github.com/apache/cloudstack/pull/3772#issuecomment-566782120 @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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack] andrijapanicsb opened a new pull request #3772: Revert of the "Revert "Fix virtual template size for managed storage for KVM / refactor cloud-install-sys-tmplt""
andrijapanicsb opened a new pull request #3772: Revert of the "Revert "Fix virtual template size for managed storage for KVM / refactor cloud-install-sys-tmplt"" URL: https://github.com/apache/cloudstack/pull/3772 The original PR #3371 was merged without Trillian tests being done, so I have reverted the merge by creating (via GitHub) the PR #3771 Then, tests were done on the original PR 3371, showing NO regression (77 tests fine, 0 failed), so it is (with all the other LGTMs) safe to merge it after all. I'm thus creating this PR which is the revert of the revert (3771) of the original PR 3371 - i.e. same PR as the original one. Will run tests again and ask for LGTMs before finally merging it. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #3371: Fix virtual template size for managed storage for KVM / refactor cloud-install-sys-tmplt
blueorangutan commented on issue #3371: Fix virtual template size for managed storage for KVM / refactor cloud-install-sys-tmplt URL: https://github.com/apache/cloudstack/pull/3371#issuecomment-566776218 Trillian test result (tid-633) Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7 Total time taken: 26613 seconds Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr3371-t633-kvm-centos7.zip Smoke tests completed. 77 look OK, 0 have error(s) 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack] nvazquez commented on a change in pull request #3694: Ldap fixes
nvazquez commented on a change in pull request #3694: Ldap fixes
URL: https://github.com/apache/cloudstack/pull/3694#discussion_r358937338
##
File path:
plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/api/command/LdapListUsersCmd.java
##
@@ -104,20 +241,251 @@ public long getEntityOwnerId() {
return Account.ACCOUNT_ID_SYSTEM;
}
-private String getListType() {
+String getListTypeString() {
return listType == null ? "all" : listType;
}
-private boolean isACloudstackUser(final LdapUser ldapUser) {
-final ListResponse response =
_queryService.searchForUsers(new ListUsersCmd());
-final List cloudstackUsers = response.getResponses();
+String getUserFilterString() {
+return userFilter == null ? getListTypeString() == null ? "NoFilter" :
getListTypeString().equals("all") ? "NoFilter" : "AnyDomain" : userFilter;
+}
+
+UserFilter getUserFilter() {
+return UserFilter.fromString(getUserFilterString());
+}
+
+boolean isACloudstackUser(final LdapUser ldapUser) {
+boolean rc = false;
+final List cloudstackUsers = getCloudstackUsers();
+if (cloudstackUsers != null) {
+for (final UserResponse cloudstackUser : cloudstackUsers) {
+if
(ldapUser.getUsername().equals(cloudstackUser.getUsername())) {
+if(s_logger.isTraceEnabled()) {
+s_logger.trace(String.format("found user %s in
cloudstack", ldapUser.getUsername()));
+}
+
+rc = true;
+} else {
+if(s_logger.isTraceEnabled()) {
+s_logger.trace(String.format("ldap user %s does not
match cloudstack user", ldapUser.getUsername(), cloudstackUser.getUsername()));
+}
+}
+}
+}
+return rc;
+}
+
+boolean isACloudstackUser(final LdapUserResponse ldapUser) {
+if(s_logger.isTraceEnabled()) {
+s_logger.trace("checking response : " + ldapUser.toString());
+}
+final List cloudstackUsers = getCloudstackUsers();
if (cloudstackUsers != null && cloudstackUsers.size() != 0) {
-for (final UserResponse cloudstackUser : response.getResponses()) {
+for (final UserResponse cloudstackUser : cloudstackUsers) {
if
(ldapUser.getUsername().equals(cloudstackUser.getUsername())) {
+if(s_logger.isTraceEnabled()) {
+s_logger.trace(String.format("found user %s in
cloudstack", ldapUser.getUsername()));
+}
return true;
+} else {
+if(s_logger.isTraceEnabled()) {
+s_logger.trace(String.format("ldap user %s does not
match cloudstack user", ldapUser.getUsername(), cloudstackUser.getUsername()));
+}
}
}
}
return false;
}
+/**
+ * typecheck for userfilter values
+ */
+enum UserFilter {
+NO_FILTER("NoFilter"),
+LOCAL_DOMAIN("LocalDomain"),
+ANY_DOMAIN("AnyDomain"),
+POTENTIAL_IMPORT("PotentialImport");
+
+private final String value;
+
+UserFilter(String val) {
+this.value = val;
+}
+
+static UserFilter fromString(String val) {
+if(NO_FILTER.toString().equalsIgnoreCase(val)) {
+return NO_FILTER;
+} else if (LOCAL_DOMAIN.toString().equalsIgnoreCase(val)) {
+return LOCAL_DOMAIN;
+} else if(ANY_DOMAIN.toString().equalsIgnoreCase(val)) {
+return ANY_DOMAIN;
+} else if(POTENTIAL_IMPORT.toString().equalsIgnoreCase(val)) {
+return POTENTIAL_IMPORT;
+} else {
+throw new IllegalArgumentException(String.format("%s is not a
legal 'UserFilter' value", val));
+}
+}
+
+@Override public String toString() {
+return value;
+}
+}
+
+/**
+ * no filtering but improve with annotation of source for existing ACS
users
+ * @param input ldap response list of users
+ * @return unfiltered list of the input list of ldap users
+ */
+public List filterNoFilter(List input)
{
+if(s_logger.isTraceEnabled()) {
+s_logger.trace("returning unfiltered list of ldap users");
+}
+annotateUserListWithSources(input);
+return input;
+}
+
+/**
+ * filter the list of ldap users. no users visible to the caller should be
in the returned list
+ * @param input ldap response list of users
+ * @return a list of ldap users not already in ACS
+ */
+public List filterAnyDomain(List
input) {
+if(s_logger.isTraceEnabled()) {
+s_logger.trace("filtering existing users");
+
[GitHub] [cloudstack] nvazquez commented on a change in pull request #3694: Ldap fixes
nvazquez commented on a change in pull request #3694: Ldap fixes
URL: https://github.com/apache/cloudstack/pull/3694#discussion_r358926572
##
File path:
plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/api/command/LdapListUsersCmd.java
##
@@ -66,34 +122,115 @@ public LdapListUsersCmd(final LdapManager ldapManager,
final QueryService queryS
_queryService = queryService;
}
+/**
+ * (as a check for isACloudstackUser is done) only non cloudstack users
should be shown
+ * @param users a list of {@code LdapUser}s
+ * @return a (filtered?) list of user response objects
+ */
private List createLdapUserResponse(final List
users) {
final List ldapResponses = new
ArrayList();
for (final LdapUser user : users) {
-if (getListType().equals("all") || !isACloudstackUser(user)) {
-final LdapUserResponse ldapResponse =
_ldapManager.createLdapUserResponse(user);
-ldapResponse.setObjectName("LdapUser");
-ldapResponses.add(ldapResponse);
-}
+final LdapUserResponse ldapResponse =
_ldapManager.createLdapUserResponse(user);
+ldapResponse.setObjectName("LdapUser");
+ldapResponses.add(ldapResponse);
}
return ldapResponses;
}
+private List cloudstackUsers = null;
+
@Override
public void execute() throws ServerApiException {
-List ldapResponses = null;
+cloudstackUsers = null;
+List ldapResponses = new
ArrayList();
final ListResponse response = new
ListResponse();
try {
-final List users = _ldapManager.getUsers(null);
+final List users = _ldapManager.getUsers(domainId);
ldapResponses = createLdapUserResponse(users);
+//now filter and annotate
+ldapResponses = applyUserFilter(ldapResponses);
} catch (final NoLdapUserMatchingQueryException ex) {
-ldapResponses = new ArrayList();
+// ok, we'll make do with the empty list ldapResponses = new
ArrayList();
} finally {
response.setResponses(ldapResponses);
response.setResponseName(getCommandName());
setResponseObject(response);
}
}
+private List getCloudstackUsers() {
+// get a list of relevant cloudstack users, meaning
+// if we are filtering for local domain, only get users for the
current domain
+// if we are filtering for any domain, get recursive all users for the
root domain
+// if we are filtering for potential imports,
+//we are only looking for users in the linked domains/accounts,
+//which is only relevant if we ask ldap users for this domain.
+//So we are asking for all users in the current domain as well
+// in case of no filter we should find all users in the current domain
for annotation.
+if (cloudstackUsers == null) {
+ListResponse cloudstackUsersresponse;
+switch (getUserFilter()) {
+case ANY_DOMAIN:
Review comment:
Minor one - can these cases be indented with a tab?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] nvazquez commented on a change in pull request #3694: Ldap fixes
nvazquez commented on a change in pull request #3694: Ldap fixes
URL: https://github.com/apache/cloudstack/pull/3694#discussion_r358932516
##
File path:
plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/api/command/LdapListUsersCmd.java
##
@@ -104,20 +241,251 @@ public long getEntityOwnerId() {
return Account.ACCOUNT_ID_SYSTEM;
}
-private String getListType() {
+String getListTypeString() {
return listType == null ? "all" : listType;
}
-private boolean isACloudstackUser(final LdapUser ldapUser) {
-final ListResponse response =
_queryService.searchForUsers(new ListUsersCmd());
-final List cloudstackUsers = response.getResponses();
+String getUserFilterString() {
+return userFilter == null ? getListTypeString() == null ? "NoFilter" :
getListTypeString().equals("all") ? "NoFilter" : "AnyDomain" : userFilter;
+}
+
+UserFilter getUserFilter() {
+return UserFilter.fromString(getUserFilterString());
+}
+
+boolean isACloudstackUser(final LdapUser ldapUser) {
+boolean rc = false;
+final List cloudstackUsers = getCloudstackUsers();
+if (cloudstackUsers != null) {
+for (final UserResponse cloudstackUser : cloudstackUsers) {
+if
(ldapUser.getUsername().equals(cloudstackUser.getUsername())) {
Review comment:
Can you also add StringUtils.isNotBlank() check?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] nvazquez commented on a change in pull request #3694: Ldap fixes
nvazquez commented on a change in pull request #3694: Ldap fixes
URL: https://github.com/apache/cloudstack/pull/3694#discussion_r358930053
##
File path:
plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/api/command/LdapListUsersCmd.java
##
@@ -66,34 +122,115 @@ public LdapListUsersCmd(final LdapManager ldapManager,
final QueryService queryS
_queryService = queryService;
}
+/**
+ * (as a check for isACloudstackUser is done) only non cloudstack users
should be shown
+ * @param users a list of {@code LdapUser}s
+ * @return a (filtered?) list of user response objects
+ */
private List createLdapUserResponse(final List
users) {
final List ldapResponses = new
ArrayList();
for (final LdapUser user : users) {
-if (getListType().equals("all") || !isACloudstackUser(user)) {
-final LdapUserResponse ldapResponse =
_ldapManager.createLdapUserResponse(user);
-ldapResponse.setObjectName("LdapUser");
-ldapResponses.add(ldapResponse);
-}
+final LdapUserResponse ldapResponse =
_ldapManager.createLdapUserResponse(user);
+ldapResponse.setObjectName("LdapUser");
+ldapResponses.add(ldapResponse);
}
return ldapResponses;
}
+private List cloudstackUsers = null;
+
@Override
public void execute() throws ServerApiException {
-List ldapResponses = null;
+cloudstackUsers = null;
+List ldapResponses = new
ArrayList();
final ListResponse response = new
ListResponse();
try {
-final List users = _ldapManager.getUsers(null);
+final List users = _ldapManager.getUsers(domainId);
ldapResponses = createLdapUserResponse(users);
+//now filter and annotate
+ldapResponses = applyUserFilter(ldapResponses);
} catch (final NoLdapUserMatchingQueryException ex) {
-ldapResponses = new ArrayList();
+// ok, we'll make do with the empty list ldapResponses = new
ArrayList();
} finally {
response.setResponses(ldapResponses);
response.setResponseName(getCommandName());
setResponseObject(response);
}
}
+private List getCloudstackUsers() {
+// get a list of relevant cloudstack users, meaning
+// if we are filtering for local domain, only get users for the
current domain
+// if we are filtering for any domain, get recursive all users for the
root domain
+// if we are filtering for potential imports,
+//we are only looking for users in the linked domains/accounts,
+//which is only relevant if we ask ldap users for this domain.
+//So we are asking for all users in the current domain as well
+// in case of no filter we should find all users in the current domain
for annotation.
+if (cloudstackUsers == null) {
+ListResponse cloudstackUsersresponse;
+switch (getUserFilter()) {
+case ANY_DOMAIN:
+// get the user domain so if the calling user is a root admin
+cloudstackUsersresponse =
_queryService.searchForUsers(CallContext.current().getCallingAccount().getDomainId(),
true);
+break;
+case NO_FILTER:
+cloudstackUsersresponse =
_queryService.searchForUsers(this.domainId,true);
+break;
+case POTENTIAL_IMPORT:
+case LOCAL_DOMAIN:
+cloudstackUsersresponse =
_queryService.searchForUsers(this.domainId,false);
+break;
+default:
+throw new CloudRuntimeException("error in program login; we
are not filtering but still querying users to filter???");
+}
+cloudstackUsers = cloudstackUsersresponse.getResponses();
+if(s_logger.isTraceEnabled()) {
+StringBuilder users = new StringBuilder();
+for (UserResponse user : cloudstackUsers) {
+if (users.length()> 0) {
+users.append(", ");
+}
+users.append(user.getUsername());
+}
+
+s_logger.trace(String.format("checking against %d
cloudstackusers: %s.", this.cloudstackUsers.size(), users.toString()));
+}
+}
+return cloudstackUsers;
+}
+
+private List applyUserFilter(List
ldapResponses) {
+if(s_logger.isTraceEnabled()) {
+s_logger.trace(String.format("applying filter: %s or %s.",
this.getListTypeString(), this.getUserFilter()));
+}
+Method filterMethod = getFilterMethod();
+List responseList = ldapResponses;
+if (filterMethod != null) {
+if(s_logger.isTraceEnabled()) {
+
[GitHub] [cloudstack] nvazquez commented on a change in pull request #3694: Ldap fixes
nvazquez commented on a change in pull request #3694: Ldap fixes URL: https://github.com/apache/cloudstack/pull/3694#discussion_r358907873 ## File path: plugins/user-authenticators/ldap/pom.xml ## @@ -81,38 +86,126 @@ **/*Spec.groovy **/*Test.java + +META-INF/*.SF +META-INF/*.DSA +META-INF/*.RSA + com.btmatthews.maven.plugins ldap-maven-plugin -1.1.0 +1.1.3 11389 ldap false dc=cloudstack,dc=org 10389 -test/resources/cloudstack.org.ldif +src/test/resources/cloudstack.org.ldif -test +src/test/java + +com.btmatthews.ldapunit +ldapunit +1.1.3 Review comment: Following the same pattern for dependencies, can version be moved to the properties section? This one and the ones below 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack] nvazquez commented on a change in pull request #3694: Ldap fixes
nvazquez commented on a change in pull request #3694: Ldap fixes URL: https://github.com/apache/cloudstack/pull/3694#discussion_r358917787 ## File path: plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/api/command/LdapListUsersCmd.java ## @@ -38,24 +48,70 @@ import com.cloud.user.Account; -@APICommand(name = "listLdapUsers", responseObject = LdapUserResponse.class, description = "Lists all LDAP Users", since = "4.2.0", -requestHasSensitiveInfo = false, responseHasSensitiveInfo = false) +/** + * @startuml + * start + * :list ldap users request; + * :get ldap binding; + * if (domain == null) then (true) + * :get global trust domain; + * else (false) + * :get trustdomain for domain; + * endif + * :get ldap users\n using trust domain; + * if (filter == 'NoFilter') then (pass as is) + * elseif (filter == 'AnyDomain') then (anydomain) + * :filterList = all\n\t\tcloudstack\n\t\tusers; + * elseif (filter == 'LocalDomain') + * :filterList = local users\n\t\tfor domain; + * elseif (filter == 'PotentialImport') then (address account\nsynchronisation\nconfigurations) + * :query\n the account\n bindings; + * :check and markup\n ldap users\n for bound OUs\n with usersource; + * else ( unknown value for filter ) + * :throw invalid parameter; + * stop + * endif + * :remove users in filterList\nfrom ldap users list; + * :return remaining; + * stop + * @enduml + */ Review comment: +1 can you add a short comment on what is needed to "see" the UML? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack] nvazquez commented on a change in pull request #3694: Ldap fixes
nvazquez commented on a change in pull request #3694: Ldap fixes
URL: https://github.com/apache/cloudstack/pull/3694#discussion_r358918976
##
File path:
plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/api/command/LdapListUsersCmd.java
##
@@ -38,24 +48,70 @@
import com.cloud.user.Account;
-@APICommand(name = "listLdapUsers", responseObject = LdapUserResponse.class,
description = "Lists all LDAP Users", since = "4.2.0",
-requestHasSensitiveInfo = false, responseHasSensitiveInfo = false)
+/**
+ * @startuml
+ * start
+ * :list ldap users request;
+ * :get ldap binding;
+ * if (domain == null) then (true)
+ * :get global trust domain;
+ * else (false)
+ * :get trustdomain for domain;
+ * endif
+ * :get ldap users\n using trust domain;
+ * if (filter == 'NoFilter') then (pass as is)
+ * elseif (filter == 'AnyDomain') then (anydomain)
+ * :filterList = all\n\t\tcloudstack\n\t\tusers;
+ * elseif (filter == 'LocalDomain')
+ * :filterList = local users\n\t\tfor domain;
+ * elseif (filter == 'PotentialImport') then (address
account\nsynchronisation\nconfigurations)
+ * :query\n the account\n bindings;
+ * :check and markup\n ldap users\n for bound OUs\n with usersource;
+ * else ( unknown value for filter )
+ * :throw invalid parameter;
+ * stop
+ * endif
+ * :remove users in filterList\nfrom ldap users list;
+ * :return remaining;
+ * stop
+ * @enduml
+ */
+@APICommand(name = "listLdapUsers", responseObject = LdapUserResponse.class,
description = "Lists LDAP Users according to the specifications from the user
request.", since = "4.2.0",
+requestHasSensitiveInfo = false, responseHasSensitiveInfo = false,
authorized = {RoleType.Admin,RoleType.DomainAdmin})
public class LdapListUsersCmd extends BaseListCmd {
public static final Logger s_logger =
Logger.getLogger(LdapListUsersCmd.class.getName());
private static final String s_name = "ldapuserresponse";
@Inject
private LdapManager _ldapManager;
+// TODO queryservice is already injected oin basecmd remove it here
@Inject
private QueryService _queryService;
Review comment:
Can we remove it as the comment says?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] nvazquez commented on a change in pull request #3694: Ldap fixes
nvazquez commented on a change in pull request #3694: Ldap fixes
URL: https://github.com/apache/cloudstack/pull/3694#discussion_r358933052
##
File path:
plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/api/command/LdapListUsersCmd.java
##
@@ -104,20 +241,251 @@ public long getEntityOwnerId() {
return Account.ACCOUNT_ID_SYSTEM;
}
-private String getListType() {
+String getListTypeString() {
return listType == null ? "all" : listType;
}
-private boolean isACloudstackUser(final LdapUser ldapUser) {
-final ListResponse response =
_queryService.searchForUsers(new ListUsersCmd());
-final List cloudstackUsers = response.getResponses();
+String getUserFilterString() {
+return userFilter == null ? getListTypeString() == null ? "NoFilter" :
getListTypeString().equals("all") ? "NoFilter" : "AnyDomain" : userFilter;
+}
+
+UserFilter getUserFilter() {
+return UserFilter.fromString(getUserFilterString());
+}
+
+boolean isACloudstackUser(final LdapUser ldapUser) {
+boolean rc = false;
+final List cloudstackUsers = getCloudstackUsers();
+if (cloudstackUsers != null) {
+for (final UserResponse cloudstackUser : cloudstackUsers) {
+if
(ldapUser.getUsername().equals(cloudstackUser.getUsername())) {
+if(s_logger.isTraceEnabled()) {
+s_logger.trace(String.format("found user %s in
cloudstack", ldapUser.getUsername()));
+}
+
+rc = true;
+} else {
+if(s_logger.isTraceEnabled()) {
+s_logger.trace(String.format("ldap user %s does not
match cloudstack user", ldapUser.getUsername(), cloudstackUser.getUsername()));
+}
+}
+}
+}
+return rc;
+}
+
+boolean isACloudstackUser(final LdapUserResponse ldapUser) {
+if(s_logger.isTraceEnabled()) {
+s_logger.trace("checking response : " + ldapUser.toString());
+}
+final List cloudstackUsers = getCloudstackUsers();
if (cloudstackUsers != null && cloudstackUsers.size() != 0) {
-for (final UserResponse cloudstackUser : response.getResponses()) {
+for (final UserResponse cloudstackUser : cloudstackUsers) {
if
(ldapUser.getUsername().equals(cloudstackUser.getUsername())) {
+if(s_logger.isTraceEnabled()) {
+s_logger.trace(String.format("found user %s in
cloudstack", ldapUser.getUsername()));
+}
return true;
+} else {
+if(s_logger.isTraceEnabled()) {
+s_logger.trace(String.format("ldap user %s does not
match cloudstack user", ldapUser.getUsername(), cloudstackUser.getUsername()));
+}
}
}
}
return false;
}
+/**
+ * typecheck for userfilter values
+ */
+enum UserFilter {
+NO_FILTER("NoFilter"),
+LOCAL_DOMAIN("LocalDomain"),
+ANY_DOMAIN("AnyDomain"),
+POTENTIAL_IMPORT("PotentialImport");
+
+private final String value;
+
+UserFilter(String val) {
+this.value = val;
+}
+
+static UserFilter fromString(String val) {
+if(NO_FILTER.toString().equalsIgnoreCase(val)) {
+return NO_FILTER;
+} else if (LOCAL_DOMAIN.toString().equalsIgnoreCase(val)) {
+return LOCAL_DOMAIN;
+} else if(ANY_DOMAIN.toString().equalsIgnoreCase(val)) {
+return ANY_DOMAIN;
+} else if(POTENTIAL_IMPORT.toString().equalsIgnoreCase(val)) {
+return POTENTIAL_IMPORT;
+} else {
+throw new IllegalArgumentException(String.format("%s is not a
legal 'UserFilter' value", val));
+}
+}
+
+@Override public String toString() {
+return value;
+}
+}
+
+/**
+ * no filtering but improve with annotation of source for existing ACS
users
+ * @param input ldap response list of users
+ * @return unfiltered list of the input list of ldap users
+ */
+public List filterNoFilter(List input)
{
+if(s_logger.isTraceEnabled()) {
+s_logger.trace("returning unfiltered list of ldap users");
+}
+annotateUserListWithSources(input);
+return input;
+}
+
+/**
+ * filter the list of ldap users. no users visible to the caller should be
in the returned list
+ * @param input ldap response list of users
+ * @return a list of ldap users not already in ACS
+ */
+public List filterAnyDomain(List
input) {
+if(s_logger.isTraceEnabled()) {
+s_logger.trace("filtering existing users");
+
[GitHub] [cloudstack] nvazquez commented on a change in pull request #3694: Ldap fixes
nvazquez commented on a change in pull request #3694: Ldap fixes
URL: https://github.com/apache/cloudstack/pull/3694#discussion_r358927359
##
File path:
plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/api/command/LdapListUsersCmd.java
##
@@ -66,34 +122,115 @@ public LdapListUsersCmd(final LdapManager ldapManager,
final QueryService queryS
_queryService = queryService;
}
+/**
+ * (as a check for isACloudstackUser is done) only non cloudstack users
should be shown
+ * @param users a list of {@code LdapUser}s
+ * @return a (filtered?) list of user response objects
+ */
private List createLdapUserResponse(final List
users) {
final List ldapResponses = new
ArrayList();
for (final LdapUser user : users) {
-if (getListType().equals("all") || !isACloudstackUser(user)) {
-final LdapUserResponse ldapResponse =
_ldapManager.createLdapUserResponse(user);
-ldapResponse.setObjectName("LdapUser");
-ldapResponses.add(ldapResponse);
-}
+final LdapUserResponse ldapResponse =
_ldapManager.createLdapUserResponse(user);
+ldapResponse.setObjectName("LdapUser");
+ldapResponses.add(ldapResponse);
}
return ldapResponses;
}
+private List cloudstackUsers = null;
+
@Override
public void execute() throws ServerApiException {
-List ldapResponses = null;
+cloudstackUsers = null;
+List ldapResponses = new
ArrayList();
final ListResponse response = new
ListResponse();
try {
-final List users = _ldapManager.getUsers(null);
+final List users = _ldapManager.getUsers(domainId);
ldapResponses = createLdapUserResponse(users);
+//now filter and annotate
+ldapResponses = applyUserFilter(ldapResponses);
} catch (final NoLdapUserMatchingQueryException ex) {
-ldapResponses = new ArrayList();
+// ok, we'll make do with the empty list ldapResponses = new
ArrayList();
} finally {
response.setResponses(ldapResponses);
response.setResponseName(getCommandName());
setResponseObject(response);
}
}
+private List getCloudstackUsers() {
+// get a list of relevant cloudstack users, meaning
+// if we are filtering for local domain, only get users for the
current domain
+// if we are filtering for any domain, get recursive all users for the
root domain
+// if we are filtering for potential imports,
+//we are only looking for users in the linked domains/accounts,
+//which is only relevant if we ask ldap users for this domain.
+//So we are asking for all users in the current domain as well
+// in case of no filter we should find all users in the current domain
for annotation.
+if (cloudstackUsers == null) {
+ListResponse cloudstackUsersresponse;
+switch (getUserFilter()) {
+case ANY_DOMAIN:
+// get the user domain so if the calling user is a root admin
+cloudstackUsersresponse =
_queryService.searchForUsers(CallContext.current().getCallingAccount().getDomainId(),
true);
+break;
+case NO_FILTER:
+cloudstackUsersresponse =
_queryService.searchForUsers(this.domainId,true);
+break;
+case POTENTIAL_IMPORT:
+case LOCAL_DOMAIN:
+cloudstackUsersresponse =
_queryService.searchForUsers(this.domainId,false);
+break;
+default:
+throw new CloudRuntimeException("error in program login; we
are not filtering but still querying users to filter???");
+}
+cloudstackUsers = cloudstackUsersresponse.getResponses();
Review comment:
Null check?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] nvazquez commented on a change in pull request #3694: Ldap fixes
nvazquez commented on a change in pull request #3694: Ldap fixes
URL: https://github.com/apache/cloudstack/pull/3694#discussion_r358940815
##
File path:
plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapManagerImpl.java
##
@@ -142,12 +142,15 @@ private LdapConfigurationResponse
addConfigurationInternal(final String hostname
public boolean canAuthenticate(final String principal, final String
password, final Long domainId) {
try {
// TODO return the right account for this user
-final LdapContext context =
_ldapContextFactory.createUserContext(principal, password,domainId);
+final LdapContext context =
_ldapContextFactory.createUserContext(principal, password, domainId);
closeContext(context);
+if(LOGGER.isTraceEnabled()) {
+LOGGER.trace(String.format("User(%s) authenticated for
domain(%s)", principal, domainId));
+}
return true;
-} catch (NamingException | IOException e) {
-s_logger.debug("Exception while doing an LDAP bind for user "+"
"+principal, e);
-s_logger.info("Failed to authenticate user: " + principal + ".
incorrect password.");
+} catch (NamingException /* | AuthenticationException */ | IOException
e) {
Review comment:
Can we remove this comment?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] nvazquez commented on a change in pull request #3694: Ldap fixes
nvazquez commented on a change in pull request #3694: Ldap fixes
URL: https://github.com/apache/cloudstack/pull/3694#discussion_r358933564
##
File path:
plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/api/command/LdapListUsersCmd.java
##
@@ -104,20 +241,251 @@ public long getEntityOwnerId() {
return Account.ACCOUNT_ID_SYSTEM;
}
-private String getListType() {
+String getListTypeString() {
return listType == null ? "all" : listType;
}
-private boolean isACloudstackUser(final LdapUser ldapUser) {
-final ListResponse response =
_queryService.searchForUsers(new ListUsersCmd());
-final List cloudstackUsers = response.getResponses();
+String getUserFilterString() {
+return userFilter == null ? getListTypeString() == null ? "NoFilter" :
getListTypeString().equals("all") ? "NoFilter" : "AnyDomain" : userFilter;
+}
+
+UserFilter getUserFilter() {
+return UserFilter.fromString(getUserFilterString());
+}
+
+boolean isACloudstackUser(final LdapUser ldapUser) {
+boolean rc = false;
+final List cloudstackUsers = getCloudstackUsers();
+if (cloudstackUsers != null) {
+for (final UserResponse cloudstackUser : cloudstackUsers) {
+if
(ldapUser.getUsername().equals(cloudstackUser.getUsername())) {
+if(s_logger.isTraceEnabled()) {
+s_logger.trace(String.format("found user %s in
cloudstack", ldapUser.getUsername()));
+}
+
+rc = true;
+} else {
+if(s_logger.isTraceEnabled()) {
+s_logger.trace(String.format("ldap user %s does not
match cloudstack user", ldapUser.getUsername(), cloudstackUser.getUsername()));
+}
+}
+}
+}
+return rc;
+}
+
+boolean isACloudstackUser(final LdapUserResponse ldapUser) {
+if(s_logger.isTraceEnabled()) {
+s_logger.trace("checking response : " + ldapUser.toString());
+}
+final List cloudstackUsers = getCloudstackUsers();
if (cloudstackUsers != null && cloudstackUsers.size() != 0) {
-for (final UserResponse cloudstackUser : response.getResponses()) {
+for (final UserResponse cloudstackUser : cloudstackUsers) {
if
(ldapUser.getUsername().equals(cloudstackUser.getUsername())) {
+if(s_logger.isTraceEnabled()) {
+s_logger.trace(String.format("found user %s in
cloudstack", ldapUser.getUsername()));
+}
return true;
+} else {
+if(s_logger.isTraceEnabled()) {
+s_logger.trace(String.format("ldap user %s does not
match cloudstack user", ldapUser.getUsername(), cloudstackUser.getUsername()));
+}
}
}
}
return false;
}
+/**
+ * typecheck for userfilter values
+ */
+enum UserFilter {
+NO_FILTER("NoFilter"),
+LOCAL_DOMAIN("LocalDomain"),
+ANY_DOMAIN("AnyDomain"),
+POTENTIAL_IMPORT("PotentialImport");
+
+private final String value;
+
+UserFilter(String val) {
+this.value = val;
+}
+
+static UserFilter fromString(String val) {
+if(NO_FILTER.toString().equalsIgnoreCase(val)) {
+return NO_FILTER;
+} else if (LOCAL_DOMAIN.toString().equalsIgnoreCase(val)) {
+return LOCAL_DOMAIN;
+} else if(ANY_DOMAIN.toString().equalsIgnoreCase(val)) {
+return ANY_DOMAIN;
+} else if(POTENTIAL_IMPORT.toString().equalsIgnoreCase(val)) {
+return POTENTIAL_IMPORT;
+} else {
+throw new IllegalArgumentException(String.format("%s is not a
legal 'UserFilter' value", val));
+}
+}
+
+@Override public String toString() {
+return value;
+}
+}
+
+/**
+ * no filtering but improve with annotation of source for existing ACS
users
+ * @param input ldap response list of users
+ * @return unfiltered list of the input list of ldap users
+ */
+public List filterNoFilter(List input)
{
+if(s_logger.isTraceEnabled()) {
+s_logger.trace("returning unfiltered list of ldap users");
+}
+annotateUserListWithSources(input);
+return input;
+}
+
+/**
+ * filter the list of ldap users. no users visible to the caller should be
in the returned list
+ * @param input ldap response list of users
+ * @return a list of ldap users not already in ACS
+ */
+public List filterAnyDomain(List
input) {
+if(s_logger.isTraceEnabled()) {
+s_logger.trace("filtering existing users");
+
[GitHub] [cloudstack] nvazquez commented on a change in pull request #3694: Ldap fixes
nvazquez commented on a change in pull request #3694: Ldap fixes URL: https://github.com/apache/cloudstack/pull/3694#discussion_r358907249 ## File path: plugins/user-authenticators/ldap/pom.xml ## @@ -27,6 +27,11 @@ 4.14.0.0-SNAPSHOT ../../pom.xml + + +2.0.0.AM26-SNAPSHOT Review comment: Can't we use a release version instead of snapshot? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #3575: [WIP DO NOT MERGE] Health check feature for virtual router
blueorangutan commented on issue #3575: [WIP DO NOT MERGE] Health check feature for virtual router URL: https://github.com/apache/cloudstack/pull/3575#issuecomment-566676023 Packaging result: ✖centos6 ✔centos7 ✔debian. JID-481 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #3575: [WIP DO NOT MERGE] Health check feature for virtual router
blueorangutan commented on issue #3575: [WIP DO NOT MERGE] Health check feature for virtual router URL: https://github.com/apache/cloudstack/pull/3575#issuecomment-58584 @anuragaw 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack] anuragaw commented on issue #3575: [WIP DO NOT MERGE] Health check feature for virtual router
anuragaw commented on issue #3575: [WIP DO NOT MERGE] Health check feature for virtual router URL: https://github.com/apache/cloudstack/pull/3575#issuecomment-58340 @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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack] anuragaw commented on a change in pull request #3575: [WIP DO NOT MERGE] Health check feature for virtual router
anuragaw commented on a change in pull request #3575: [WIP DO NOT MERGE] Health check feature for virtual router URL: https://github.com/apache/cloudstack/pull/3575#discussion_r358922388 ## File path: systemvm/debian/root/health_checks/memory_usage_check.py ## @@ -0,0 +1,55 @@ +#!/usr/bin/python +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +from os import sys, path, statvfs +from subprocess import * Review comment: It does. I've seen it use at many places too. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack] anuragaw commented on a change in pull request #3575: [WIP DO NOT MERGE] Health check feature for virtual router
anuragaw commented on a change in pull request #3575: [WIP DO NOT MERGE] Health
check feature for virtual router
URL: https://github.com/apache/cloudstack/pull/3575#discussion_r358922246
##
File path: systemvm/debian/opt/cloud/bin/getRouterMonitorResults.sh
##
@@ -0,0 +1,48 @@
+#!/usr/bin/env bash
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# getRouterMonitorResults.sh --- Send the monitor results to Management Server
+
+if [ "$1" == "true" ]
+then
+python /root/monitorServices.py > /dev/null
+fi
+
+if [ -f /root/failing_health_checks ]
+then
+printf "FAILING CHECKS:\n"
+echo `cat /root/failing_health_checks`
+fi
+
+printf "MONITOR RESULTS:\n"
+echo "{\"basic\":"
+if [ -f /root/basic_monitor_results.json ]
+then
+echo `cat /root/basic_monitor_results.json`
+else
+echo "{}"
+fi
+echo ",\"advance\":"
+if [ -f /root/advance_monitor_results.json ]
+then
+echo `cat /root/advance_monitor_results.json`
+else
+echo "{}"
+fi
Review comment:
Yes, that's correct. The Health checks are scheduled on the VR and is self
contained. Health checks are executed at admin defined intervals and kept in
files within the VR. This script fetches the results of the last run. The
checks can be run on demand with fresh checks by getRouterHealthCheckResults
and performFreshRun=true. Which first updates the health check config, then
runs the checks, and finally prints the most recent results.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] anuragaw commented on a change in pull request #3575: [WIP DO NOT MERGE] Health check feature for virtual router
anuragaw commented on a change in pull request #3575: [WIP DO NOT MERGE] Health
check feature for virtual router
URL: https://github.com/apache/cloudstack/pull/3575#discussion_r358921139
##
File path: systemvm/debian/root/health_checks/router_version_check.py
##
@@ -0,0 +1,83 @@
+#!/usr/bin/python
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from os import sys, path, statvfs
+from utility import getHealthChecksData
+
+
+def getFirstLine(file=None):
+if file is not None and path.isfile(file):
+ret = None
+with open(file, 'r') as oFile:
+lines = oFile.readlines()
+if len(lines) > 0:
+ret = lines[0].strip()
+oFile.close()
+
+return ret
+else:
+return None
+
+
+def main():
+entries = getHealthChecksData("routerVersion")
+data = {}
+if len(entries) == 1:
+data = entries[0]
+
+if len(data) == 0:
+print "Missing routerVersion in health_checks_data, skipping"
+exit(0)
+
+templateVersionMatches = True
+scriptVersionMatches = True
+
+if "templateVersion" in data:
+expected = data["templateVersion"].strip()
+releaseFile = "/etc/cloudstack-release"
+found = getFirstLine(releaseFile)
+if found is None:
+print "Release version not yet setup at " + releaseFile +\
+ ", skipping."
+elif expected != found:
+print "Template Version mismatch. Expected: " + \
+ expected + ", found: " + found
+templateVersionMatches = False
+
+if "scriptsVersion" in data:
+expected = data["scriptsVersion"].strip()
+sigFile = "/var/cache/cloud/cloud-scripts-signature"
+found = getFirstLine(sigFile)
+if found is None:
Review comment:
It is possible that it occurs so this check handles intermittent failures
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] anuragaw commented on a change in pull request #3575: [WIP DO NOT MERGE] Health check feature for virtual router
anuragaw commented on a change in pull request #3575: [WIP DO NOT MERGE] Health
check feature for virtual router
URL: https://github.com/apache/cloudstack/pull/3575#discussion_r358915230
##
File path: systemvm/debian/root/health_checks/router_version_check.py
##
@@ -0,0 +1,83 @@
+#!/usr/bin/python
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from os import sys, path, statvfs
+from utility import getHealthChecksData
+
+
+def getFirstLine(file=None):
+if file is not None and path.isfile(file):
+ret = None
+with open(file, 'r') as oFile:
+lines = oFile.readlines()
+if len(lines) > 0:
+ret = lines[0].strip()
+oFile.close()
+
+return ret
+else:
+return None
+
+
+def main():
+entries = getHealthChecksData("routerVersion")
+data = {}
+if len(entries) == 1:
+data = entries[0]
+
+if len(data) == 0:
+print "Missing routerVersion in health_checks_data, skipping"
+exit(0)
+
+templateVersionMatches = True
+scriptVersionMatches = True
+
+if "templateVersion" in data:
+expected = data["templateVersion"].strip()
+releaseFile = "/etc/cloudstack-release"
+found = getFirstLine(releaseFile)
+if found is None:
Review comment:
This would happen when the checks are started by the cron job before the
router is patched up with the version value.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] anuragaw commented on a change in pull request #3575: [WIP DO NOT MERGE] Health check feature for virtual router
anuragaw commented on a change in pull request #3575: [WIP DO NOT MERGE] Health
check feature for virtual router
URL: https://github.com/apache/cloudstack/pull/3575#discussion_r358914570
##
File path: systemvm/debian/opt/cloud/bin/cs/CsMonitor.py
##
@@ -17,27 +17,55 @@
import logging
from cs.CsDatabag import CsDataBag
from CsFile import CsFile
+import json
MON_CONFIG = "/etc/monitor.conf"
+HC_CONFIG = "/root/health_checks_data.json"
class CsMonitor(CsDataBag):
""" Manage dhcp entries """
def process(self):
-if "config" not in self.dbag:
-return
-procs = [x.strip() for x in self.dbag['config'].split(',')]
-file = CsFile(MON_CONFIG)
-for proc in procs:
-bits = [x for x in proc.split(':')]
-if len(bits) < 5:
-continue
-for i in range(0, 4):
-file.add(bits[i], -1)
-file.commit()
+if "config" in self.dbag:
+procs = [x.strip() for x in self.dbag['config'].split(',')]
+file = CsFile(MON_CONFIG)
+for proc in procs:
+bits = [x for x in proc.split(':')]
+if len(bits) < 5:
+continue
+for i in range(0, 4):
+file.add(bits[i], -1)
+file.commit()
+
+hc_data = {}
+
+cron_rep_basic = self.dbag["health_checks_basic_run_interval"] if
"health_checks_basic_run_interval" in self.dbag else 3
+cron_rep_advance = self.dbag["health_checks_advance_run_interval"] if
"health_checks_advance_run_interval" in self.dbag else 0
+hc_data["health_checks_basic_run_interval"] = cron_rep_basic
+hc_data["health_checks_advance_run_interval"] = cron_rep_advance
+
+hc_data["health_checks_enabled"] = self.dbag["health_checks_enabled"]
if "health_checks_enabled" in self.dbag else False
+
+if "excluded_health_checks" in self.dbag:
+excluded_checks = self.dbag["excluded_health_checks"]
+hc_data["excluded_health_checks"] = [ch.strip() for ch in
excluded_checks.split(",")] if len(excluded_checks) > 0 else []
+else:
+hc_data["excluded_health_checks"] = []
+if "additional_data" in self.dbag:
+hc_data["additional_data"] = self.dbag["additional_data"]
+else:
+hc_data["additional_data"] = {}
+
cron = CsFile("/etc/cron.d/process")
+cron.deleteLine("root /usr/bin/python /root/monitorServices.py")
cron.add("SHELL=/bin/bash", 0)
cron.add("PATH=/usr/local/sbin:/usr/local/bin:/sbin:/bin:/usr/sbin:/usr/bin", 1)
-cron.add("*/3 * * * * root /usr/bin/python /root/monitorServices.py",
-1)
+if cron_rep_basic > 0:
+cron.add("*/" + str(cron_rep_basic) + " * * * * root
/usr/bin/python /root/monitorServices.py basic", -1)
+if cron_rep_advance > 0:
+cron.add("*/" + str(cron_rep_advance) + " * * * * root
/usr/bin/python /root/monitorServices.py advance", -1)
cron.commit()
+
+with open(HC_CONFIG, 'w') as f:
Review comment:
done
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] rhtyd commented on issue #3769: README: that time of the year!
rhtyd commented on issue #3769: README: that time of the year! URL: https://github.com/apache/cloudstack/pull/3769#issuecomment-566593270 That went in a little fast, hope it doesn't stop everyone to still continue exchanging greetings. Cheers all. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[cloudstack] 01/01: Revert "Revert "Fix virtual template size for managed storage for KVM / refactor cloud-install-sys-tmplt (#3371)" (#3771)"
This is an automated email from the ASF dual-hosted git repository.
andrijapanic pushed a commit to branch revert-3771-revert-3371-template_rewrite
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
commit cf493b60d2252edd7c2020ea2a4b7dd115834964
Author: Andrija Panic <45762285+andrijapani...@users.noreply.github.com>
AuthorDate: Tue Dec 17 16:00:37 2019 +0100
Revert "Revert "Fix virtual template size for managed storage for KVM /
refactor cloud-install-sys-tmplt (#3371)" (#3771)"
This reverts commit e319c8b8f0e742b4f6139603d7881974d0a02e8f.
---
debian/control| 2 +-
packaging/centos7/cloud.spec | 1 +
scripts/storage/secondary/cloud-install-sys-tmplt | 280 ++
scripts/storage/secondary/createtmplt.sh | 1 -
4 files changed, 131 insertions(+), 153 deletions(-)
diff --git a/debian/control b/debian/control
index 9b67c82..3fde8d6 100644
--- a/debian/control
+++ b/debian/control
@@ -15,7 +15,7 @@ Description: A common package which contains files which are
shared by several C
Package: cloudstack-management
Architecture: all
-Depends: ${python:Depends}, openjdk-8-jre-headless | java8-runtime-headless |
java8-runtime | openjdk-9-jre-headless, cloudstack-common (=
${source:Version}), sudo, python-mysql.connector, libmysql-java, augeas-tools,
mysql-client, adduser, bzip2, ipmitool, file, gawk, iproute2, lsb-release,
init-system-helpers (>= 1.14~)
+Depends: ${python:Depends}, openjdk-8-jre-headless | java8-runtime-headless |
java8-runtime | openjdk-9-jre-headless, cloudstack-common (=
${source:Version}), sudo, python-mysql.connector, libmysql-java, augeas-tools,
mysql-client, adduser, bzip2, ipmitool, file, gawk, iproute2, lsb-release,
init-system-helpers (>= 1.14~), qemu-utils
Conflicts: cloud-server, cloud-client, cloud-client-ui
Description: CloudStack server library
The CloudStack management server
diff --git a/packaging/centos7/cloud.spec b/packaging/centos7/cloud.spec
index aadf941..2dbc5ec 100644
--- a/packaging/centos7/cloud.spec
+++ b/packaging/centos7/cloud.spec
@@ -78,6 +78,7 @@ Requires: mysql-connector-python
Requires: ipmitool
Requires: %{name}-common = %{_ver}
Requires: iptables-services
+Requires: qemu-img
Group: System Environment/Libraries
%description management
The CloudStack management server is the central point of coordination,
diff --git a/scripts/storage/secondary/cloud-install-sys-tmplt
b/scripts/storage/secondary/cloud-install-sys-tmplt
index 91b3a7c..b900f8f 100755
--- a/scripts/storage/secondary/cloud-install-sys-tmplt
+++ b/scripts/storage/secondary/cloud-install-sys-tmplt
@@ -1,5 +1,4 @@
#!/bin/bash
-# $Id: installrtng.sh 11251 2010-07-23 23:40:44Z abhishek $ $HeadURL:
svn://svn.lab.vmops.com/repos/vmdev/java/scripts/storage/secondary/installrtng.sh
$
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
@@ -20,15 +19,30 @@
usage() {
- printf "Usage: %s: -m -f [-h ] [ -s ][-u ] [-F ] [-e ]
[-o ] [-r ] [-p &2
- printf "%s: -m -u [-h ] [ -s ]\n" $(basename $0) >&2
+ printf "\nUsage: %s:\n\t-m secondary storage mount point\n\t-f system vm
template file\n\t-h hypervisor name: kvm|vmware|xenserver|hyperv|ovm3\n\t-s
mgmt server secret key, if you specified any when running
cloudstack-setup-database, default is password\n\t-u Url to system vm
template\n\t-F clean up system templates of specified hypervisor\n\t-e Template
suffix, e.g vhd, ova, qcow2\n\t-o Database server hostname or ip, e.g
localhost\n\t-r Database user name, e.g root\n\t-p mysql databa [...]
+ printf "\tor\n"
+ printf "\nUsage: %s:\n\t-m secondary storage mount point\n\t-u http url for
system vm template\n\t-h hypervisor name:
kvm|vmware|xenserver|hyperv|ovm3\n\t-s mgmt server secret key\n\n" $(basename
$0) >&2
}
+# Usage: e.g. failed $? "this is an error"
failed() {
- echo "Installation failed"
- exit $1
+ local returnval=$1
+ local returnmsg=$2
+
+ # check for an message, if there is no one dont print anything
+ if [[ -z $returnmsg ]]; then
+:
+ else
+echo -e $returnmsg
+ fi
+ if [[ $returnval -eq 0 ]]; then
+return 0
+ else
+echo "Installation failed"
+exit $returnval
+ fi
}
+
#set -x
mflag=
fflag=
@@ -42,7 +56,15 @@ dbUser="root"
dbPassword=
dbPort=3306
jasypt='/usr/share/cloudstack-common/lib/jasypt-1.9.2.jar'
-while getopts 'm:h:f:u:Ft:e:s:o:r:d:p:'# OPTION
+
+# check if first parameter is not a dash (-) then print the usage block
+if [[ ! $@ =~ ^\-.+ ]]; then
+ usage
+ exit 0
+fi
+
+OPTERR=0
+while getopts 'm:h:f:u:Ft:e:Ms:o:r:d:p:'# OPTION
do
case $OPTION in
m)mflag=1
@@ -78,121 +100,94 @@ do
dbPort="$OPTARG"
;;
?)usage
-failed 2
+exit 0
+;;
+ *)usage
+exit 0
;;
esac
done
-if [[ "$mflag$fflag" != "11" && "$mflag$uflag" !=
[cloudstack] branch revert-3771-revert-3371-template_rewrite created (now cf493b6)
This is an automated email from the ASF dual-hosted git repository. andrijapanic pushed a change to branch revert-3771-revert-3371-template_rewrite in repository https://gitbox.apache.org/repos/asf/cloudstack.git. at cf493b6 Revert "Revert "Fix virtual template size for managed storage for KVM / refactor cloud-install-sys-tmplt (#3371)" (#3771)" This branch includes the following new commits: new cf493b6 Revert "Revert "Fix virtual template size for managed storage for KVM / refactor cloud-install-sys-tmplt (#3371)" (#3771)" The 1 revisions listed above as "new" are entirely new to this repository and will be described in separate emails. The revisions listed as "add" were already present in the repository and have only been added to this reference.
[GitHub] [cloudstack] svenvogel commented on issue #3186: Add possibility to set KVM MTU size for all NIC
svenvogel commented on issue #3186: Add possibility to set KVM MTU size for all NIC URL: https://github.com/apache/cloudstack/pull/3186#issuecomment-566575607 @GabrielBrascher @andrijapanicsb sure we can put it on-hold and work in progress. at the moment there are no resources to get the things done with the router scripts. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack] pavanaravapalli commented on issue #3638: UEFI Support on CloudStack
pavanaravapalli commented on issue #3638: UEFI Support on CloudStack URL: https://github.com/apache/cloudstack/pull/3638#issuecomment-566572740 @nathanejohnson @nvazquez @rhtyd Please review. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack] pavanaravapalli commented on a change in pull request #3638: UEFI Support on CloudStack
pavanaravapalli commented on a change in pull request #3638: UEFI Support on
CloudStack
URL: https://github.com/apache/cloudstack/pull/3638#discussion_r358830755
##
File path:
server/src/main/java/com/cloud/agent/manager/allocator/impl/FirstFitAllocator.java
##
@@ -112,6 +117,16 @@
VMTemplateVO template = (VMTemplateVO)vmProfile.getTemplate();
Account account = vmProfile.getOwner();
+boolean isVMDeployedWithUefi = false;
+UserVmDetailVO userVmDetailVO =
_userVmDetailsDao.findDetail(vmProfile.getId(), "UEFI");
+if(userVmDetailVO != null){
+if ("secure".equalsIgnoreCase(userVmDetailVO.getValue()) ||
"legacy".equalsIgnoreCase(userVmDetailVO.getValue())) {
Review comment:
changes done as suggested
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #3638: UEFI Support on CloudStack
sureshanaparti commented on a change in pull request #3638: UEFI Support on
CloudStack
URL: https://github.com/apache/cloudstack/pull/3638#discussion_r356214213
##
File path: server/src/main/java/com/cloud/hypervisor/HypervisorGuruBase.java
##
@@ -166,6 +166,11 @@ protected VirtualMachineTO
toVirtualMachineTO(VirtualMachineProfile vmProfile) {
offering.getRamSize() * 1024l * 1024l, null, null,
vm.isHaEnabled(), vm.limitCpuUse(), vm.getVncPassword());
to.setBootArgs(vmProfile.getBootArgs());
+String bootType = (String)vmProfile.getParameter(new
VirtualMachineProfile.Param("BootType"));
+if (bootType != null && !bootType.isEmpty()) {
Review comment:
can use StringUtils
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #3638: UEFI Support on CloudStack
sureshanaparti commented on a change in pull request #3638: UEFI Support on
CloudStack
URL: https://github.com/apache/cloudstack/pull/3638#discussion_r356211331
##
File path:
server/src/main/java/com/cloud/agent/manager/allocator/impl/FirstFitAllocator.java
##
@@ -112,6 +117,16 @@
VMTemplateVO template = (VMTemplateVO)vmProfile.getTemplate();
Account account = vmProfile.getOwner();
+boolean isVMDeployedWithUefi = false;
+UserVmDetailVO userVmDetailVO =
_userVmDetailsDao.findDetail(vmProfile.getId(), "UEFI");
+if(userVmDetailVO != null){
+if ("secure".equalsIgnoreCase(userVmDetailVO.getValue()) ||
"legacy".equalsIgnoreCase(userVmDetailVO.getValue())) {
Review comment:
define enum globally for boot types & modes (secure and legacy) and use it
across, whenever applicable.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #3638: UEFI Support on CloudStack
sureshanaparti commented on a change in pull request #3638: UEFI Support on
CloudStack
URL: https://github.com/apache/cloudstack/pull/3638#discussion_r356213475
##
File path: server/src/main/java/com/cloud/deploy/FirstFitPlanner.java
##
@@ -218,6 +230,20 @@ public int compare(Long o1, Long o2) {
});
}
+private Long getHostsByCapability(List hostList, String
hostCapability) {
+int totalHostswithCapability = 0;
+for (Long host : hostList) { //TODO: Fix this in single query instead
of polling request for each Host
+Map details = hostDetailsDao.findDetails(host);
+if (details.containsKey(Host.HOST_UEFI_ENABLE)) {
+if
(details.get(Host.HOST_UEFI_ENABLE).equalsIgnoreCase("Yes")) {
+totalHostswithCapability++;
Review comment:
Better return here when a host found with UEFI capability as that satisfies
the return cond.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #3638: UEFI Support on CloudStack
sureshanaparti commented on a change in pull request #3638: UEFI Support on
CloudStack
URL: https://github.com/apache/cloudstack/pull/3638#discussion_r356214635
##
File path: server/src/main/java/com/cloud/server/ManagementServerImpl.java
##
@@ -1183,6 +1187,16 @@ private HypervisorType getHypervisorType(VMInstanceVO
vm, StoragePool srcVolumeP
throw ex;
}
+UserVmDetailVO userVmDetailVO =
_UserVmDetailsDao.findDetail(vm.getId(), "UEFI");
+if (userVmDetailVO != null){
+s_logger.info(" Live Migration of UEFI enabled VM : " +
vm.getInstanceName()+ " is not supported");
+if("legacy".equalsIgnoreCase(userVmDetailVO.getValue()) ||
"secure".equalsIgnoreCase(userVmDetailVO.getValue())) {
Review comment:
enum for boot mode
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #3638: UEFI Support on CloudStack
sureshanaparti commented on a change in pull request #3638: UEFI Support on
CloudStack
URL: https://github.com/apache/cloudstack/pull/3638#discussion_r358759269
##
File path:
api/src/main/java/org/apache/cloudstack/api/command/user/vm/DeployVMCmd.java
##
@@ -245,6 +252,22 @@ public Long getDomainId() {
return domainId;
}
+private ApiConstants.BootType getBootType() {
+
+if (StringUtils.isNotBlank(bootType)) {
+try {
+String type = bootType.trim().toUpperCase();
+return ApiConstants.BootType.valueOf(type);
+} catch (IllegalArgumentException e) {
+String errMesg = "Invalid bootType " + bootType + "Specified
for vm " + getName()
++ " Valid values are: UEFI,BIOS ";
Review comment:
Use the enum for valid values.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #3638: UEFI Support on CloudStack
sureshanaparti commented on a change in pull request #3638: UEFI Support on
CloudStack
URL: https://github.com/apache/cloudstack/pull/3638#discussion_r358759670
##
File path:
api/src/main/java/org/apache/cloudstack/api/command/user/vm/DeployVMCmd.java
##
@@ -257,12 +280,35 @@ public Long getDomainId() {
}
}
}
+if(getBootType() != null){ // export to get
+if(getBootType() == ApiConstants.BootType.UEFI) {
+customparameterMap.put(getBootType().toString(),
getBootMode().toString());
+}
+}
+
if (rootdisksize != null &&
!customparameterMap.containsKey("rootdisksize")) {
customparameterMap.put("rootdisksize", rootdisksize.toString());
}
return customparameterMap;
}
+
+public ApiConstants.BootMode getBootMode() {
+if (StringUtils.isNotBlank(bootMode)) {
+try {
+String mode = bootMode.trim().toUpperCase();
+return ApiConstants.BootMode.valueOf(mode);
+} catch (IllegalArgumentException e) {
+String errMesg = "Invalid bootMode " + bootMode + "Specified
for vm " + getName()
++ " Valid values are: LEGACY,SECURE ";
Review comment:
Use the enum for valid values.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] pavanaravapalli commented on issue #3638: UEFI Support on CloudStack
pavanaravapalli commented on issue #3638: UEFI Support on CloudStack URL: https://github.com/apache/cloudstack/pull/3638#issuecomment-566568818 @sureshanaparti, review comments addressed as suggested 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #3371: Fix virtual template size for managed storage for KVM / refactor cloud-install-sys-tmplt
blueorangutan commented on issue #3371: Fix virtual template size for managed storage for KVM / refactor cloud-install-sys-tmplt URL: https://github.com/apache/cloudstack/pull/3371#issuecomment-566567277 @andrijapanicsb 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack] andrijapanicsb commented on issue #3371: Fix virtual template size for managed storage for KVM / refactor cloud-install-sys-tmplt
andrijapanicsb commented on issue #3371: Fix virtual template size for managed storage for KVM / refactor cloud-install-sys-tmplt URL: https://github.com/apache/cloudstack/pull/3371#issuecomment-566566974 @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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack] andrijapanicsb commented on issue #3371: Fix virtual template size for managed storage for KVM / refactor cloud-install-sys-tmplt
andrijapanicsb commented on issue #3371: Fix virtual template size for managed storage for KVM / refactor cloud-install-sys-tmplt URL: https://github.com/apache/cloudstack/pull/3371#issuecomment-566565399 Apologies @svenvogel , I have reverted this PR/merge - since tests have NOT complete (there is no output) - let me kick the tests again, and if all fine, you can merge it after that, please. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[cloudstack] branch master updated (a0efbf9 -> e319c8b)
This is an automated email from the ASF dual-hosted git repository. andrijapanic pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/cloudstack.git. from a0efbf9 Fix virtual template size for managed storage for KVM / refactor cloud-install-sys-tmplt (#3371) add e319c8b Revert "Fix virtual template size for managed storage for KVM / refactor cloud-install-sys-tmplt (#3371)" (#3771) No new revisions were added by this update. Summary of changes: debian/control| 2 +- packaging/centos7/cloud.spec | 1 - scripts/storage/secondary/cloud-install-sys-tmplt | 280 -- scripts/storage/secondary/createtmplt.sh | 1 + 4 files changed, 153 insertions(+), 131 deletions(-)
[GitHub] [cloudstack] andrijapanicsb merged pull request #3771: Revert "Fix virtual template size for managed storage for KVM / refactor cloud-install-sys-tmplt"
andrijapanicsb merged pull request #3771: Revert "Fix virtual template size for managed storage for KVM / refactor cloud-install-sys-tmplt" URL: https://github.com/apache/cloudstack/pull/3771 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[cloudstack] 01/01: Revert "Fix virtual template size for managed storage for KVM / refactor cloud-install-sys-tmplt (#3371)"
This is an automated email from the ASF dual-hosted git repository.
andrijapanic pushed a commit to branch revert-3371-template_rewrite
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
commit 08be29a753cbdadac17d23b4fa86e3c011b0c21a
Author: Andrija Panic <45762285+andrijapani...@users.noreply.github.com>
AuthorDate: Tue Dec 17 15:28:47 2019 +0100
Revert "Fix virtual template size for managed storage for KVM / refactor
cloud-install-sys-tmplt (#3371)"
This reverts commit a0efbf9d09e57a907cd599d28a1def02686240f1.
---
debian/control| 2 +-
packaging/centos7/cloud.spec | 1 -
scripts/storage/secondary/cloud-install-sys-tmplt | 280 --
scripts/storage/secondary/createtmplt.sh | 1 +
4 files changed, 153 insertions(+), 131 deletions(-)
diff --git a/debian/control b/debian/control
index 3fde8d6..9b67c82 100644
--- a/debian/control
+++ b/debian/control
@@ -15,7 +15,7 @@ Description: A common package which contains files which are
shared by several C
Package: cloudstack-management
Architecture: all
-Depends: ${python:Depends}, openjdk-8-jre-headless | java8-runtime-headless |
java8-runtime | openjdk-9-jre-headless, cloudstack-common (=
${source:Version}), sudo, python-mysql.connector, libmysql-java, augeas-tools,
mysql-client, adduser, bzip2, ipmitool, file, gawk, iproute2, lsb-release,
init-system-helpers (>= 1.14~), qemu-utils
+Depends: ${python:Depends}, openjdk-8-jre-headless | java8-runtime-headless |
java8-runtime | openjdk-9-jre-headless, cloudstack-common (=
${source:Version}), sudo, python-mysql.connector, libmysql-java, augeas-tools,
mysql-client, adduser, bzip2, ipmitool, file, gawk, iproute2, lsb-release,
init-system-helpers (>= 1.14~)
Conflicts: cloud-server, cloud-client, cloud-client-ui
Description: CloudStack server library
The CloudStack management server
diff --git a/packaging/centos7/cloud.spec b/packaging/centos7/cloud.spec
index 2dbc5ec..aadf941 100644
--- a/packaging/centos7/cloud.spec
+++ b/packaging/centos7/cloud.spec
@@ -78,7 +78,6 @@ Requires: mysql-connector-python
Requires: ipmitool
Requires: %{name}-common = %{_ver}
Requires: iptables-services
-Requires: qemu-img
Group: System Environment/Libraries
%description management
The CloudStack management server is the central point of coordination,
diff --git a/scripts/storage/secondary/cloud-install-sys-tmplt
b/scripts/storage/secondary/cloud-install-sys-tmplt
index b900f8f..91b3a7c 100755
--- a/scripts/storage/secondary/cloud-install-sys-tmplt
+++ b/scripts/storage/secondary/cloud-install-sys-tmplt
@@ -1,4 +1,5 @@
#!/bin/bash
+# $Id: installrtng.sh 11251 2010-07-23 23:40:44Z abhishek $ $HeadURL:
svn://svn.lab.vmops.com/repos/vmdev/java/scripts/storage/secondary/installrtng.sh
$
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
@@ -19,30 +20,15 @@
usage() {
- printf "\nUsage: %s:\n\t-m secondary storage mount point\n\t-f system vm
template file\n\t-h hypervisor name: kvm|vmware|xenserver|hyperv|ovm3\n\t-s
mgmt server secret key, if you specified any when running
cloudstack-setup-database, default is password\n\t-u Url to system vm
template\n\t-F clean up system templates of specified hypervisor\n\t-e Template
suffix, e.g vhd, ova, qcow2\n\t-o Database server hostname or ip, e.g
localhost\n\t-r Database user name, e.g root\n\t-p mysql databa [...]
- printf "\tor\n"
- printf "\nUsage: %s:\n\t-m secondary storage mount point\n\t-u http url for
system vm template\n\t-h hypervisor name:
kvm|vmware|xenserver|hyperv|ovm3\n\t-s mgmt server secret key\n\n" $(basename
$0) >&2
+ printf "Usage: %s: -m -f [-h ] [ -s ][-u ] [-F ] [-e ]
[-o ] [-r ] [-p &2
+ printf "%s: -m -u [-h ] [ -s ]\n" $(basename $0) >&2
}
-# Usage: e.g. failed $? "this is an error"
failed() {
- local returnval=$1
- local returnmsg=$2
-
- # check for an message, if there is no one dont print anything
- if [[ -z $returnmsg ]]; then
-:
- else
-echo -e $returnmsg
- fi
- if [[ $returnval -eq 0 ]]; then
-return 0
- else
-echo "Installation failed"
-exit $returnval
- fi
+ echo "Installation failed"
+ exit $1
}
-
#set -x
mflag=
fflag=
@@ -56,15 +42,7 @@ dbUser="root"
dbPassword=
dbPort=3306
jasypt='/usr/share/cloudstack-common/lib/jasypt-1.9.2.jar'
-
-# check if first parameter is not a dash (-) then print the usage block
-if [[ ! $@ =~ ^\-.+ ]]; then
- usage
- exit 0
-fi
-
-OPTERR=0
-while getopts 'm:h:f:u:Ft:e:Ms:o:r:d:p:'# OPTION
+while getopts 'm:h:f:u:Ft:e:s:o:r:d:p:'# OPTION
do
case $OPTION in
m)mflag=1
@@ -100,94 +78,121 @@ do
dbPort="$OPTARG"
;;
?)usage
-exit 0
-;;
- *)usage
-exit 0
+failed 2
;;
esac
done
-if [[ "$mflag$fflag" != "11" && "$mflag$uflag" != "11" ]]; then
- failed 2
[GitHub] [cloudstack] andrijapanicsb commented on issue #3771: Revert "Fix virtual template size for managed storage for KVM / refactor cloud-install-sys-tmplt"
andrijapanicsb commented on issue #3771: Revert "Fix virtual template size for managed storage for KVM / refactor cloud-install-sys-tmplt" URL: https://github.com/apache/cloudstack/pull/3771#issuecomment-566564830 Revert 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[cloudstack] branch revert-3371-template_rewrite created (now 08be29a)
This is an automated email from the ASF dual-hosted git repository. andrijapanic pushed a change to branch revert-3371-template_rewrite in repository https://gitbox.apache.org/repos/asf/cloudstack.git. at 08be29a Revert "Fix virtual template size for managed storage for KVM / refactor cloud-install-sys-tmplt (#3371)" This branch includes the following new commits: new 08be29a Revert "Fix virtual template size for managed storage for KVM / refactor cloud-install-sys-tmplt (#3371)" The 1 revisions listed above as "new" are entirely new to this repository and will be described in separate emails. The revisions listed as "add" were already present in the repository and have only been added to this reference.
[GitHub] [cloudstack] andrijapanicsb opened a new pull request #3771: Revert "Fix virtual template size for managed storage for KVM / refactor cloud-install-sys-tmplt"
andrijapanicsb opened a new pull request #3771: Revert "Fix virtual template size for managed storage for KVM / refactor cloud-install-sys-tmplt" URL: https://github.com/apache/cloudstack/pull/3771 Reverts apache/cloudstack#3371 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[cloudstack] branch master updated (16527f1 -> a0efbf9)
This is an automated email from the ASF dual-hosted git repository. svogel pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/cloudstack.git. from 16527f1 Add missing HA config keys (#3737) add a0efbf9 Fix virtual template size for managed storage for KVM / refactor cloud-install-sys-tmplt (#3371) No new revisions were added by this update. Summary of changes: debian/control| 2 +- packaging/centos7/cloud.spec | 1 + scripts/storage/secondary/cloud-install-sys-tmplt | 280 ++ scripts/storage/secondary/createtmplt.sh | 1 - 4 files changed, 131 insertions(+), 153 deletions(-)
[GitHub] [cloudstack] svenvogel merged pull request #3371: Fix virtual template size for managed storage for KVM / refactor cloud-install-sys-tmplt
svenvogel merged pull request #3371: Fix virtual template size for managed storage for KVM / refactor cloud-install-sys-tmplt URL: https://github.com/apache/cloudstack/pull/3371 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack] svenvogel commented on issue #3371: Fix virtual template size for managed storage for KVM / refactor cloud-install-sys-tmplt
svenvogel commented on issue #3371: Fix virtual template size for managed storage for KVM / refactor cloud-install-sys-tmplt URL: https://github.com/apache/cloudstack/pull/3371#issuecomment-566563730 4 LGTM, checks passed tested in different environments CentOS and Ubuntu. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack] andrijapanicsb commented on issue #3186: Add possibility to set KVM MTU size for all NIC
andrijapanicsb commented on issue #3186: Add possibility to set KVM MTU size for all NIC URL: https://github.com/apache/cloudstack/pull/3186#issuecomment-566563338 I agree with @GabrielBrascher, I've seen some nasty issues with MTU not being in place correctly. I do want an opinion from @rhtyd as well on this one. That being said, I do need to back out from this PR, due to lack of time (instead of silently ignoring it) as it would mean considerable testing effort (I can't help much with code obviously). 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack] svenvogel merged pull request #3737: Add missing HA config keys
svenvogel merged pull request #3737: Add missing HA config keys URL: https://github.com/apache/cloudstack/pull/3737 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[cloudstack] branch master updated (2e8c069 -> 16527f1)
This is an automated email from the ASF dual-hosted git repository. svogel pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/cloudstack.git. from 2e8c069 Load Average for KVM (#3738) add 16527f1 Add missing HA config keys (#3737) No new revisions were added by this update. Summary of changes: .../java/com/cloud/ha/HighAvailabilityManager.java | 20 - .../com/cloud/ha/HighAvailabilityManagerImpl.java | 87 ++ 2 files changed, 72 insertions(+), 35 deletions(-)
[GitHub] [cloudstack] svenvogel commented on issue #3737: Add missing HA config keys
svenvogel commented on issue #3737: Add missing HA config keys URL: https://github.com/apache/cloudstack/pull/3737#issuecomment-566562566 3 LTGM and tested in our environment. Merge into Master. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack] anuragaw commented on a change in pull request #3575: [WIP DO NOT MERGE] Health check feature for virtual router
anuragaw commented on a change in pull request #3575: [WIP DO NOT MERGE] Health
check feature for virtual router
URL: https://github.com/apache/cloudstack/pull/3575#discussion_r358793792
##
File path:
server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java
##
@@ -1186,6 +1227,431 @@ protected void pushToUpdateQueue(final List
networks) throws Interrup
}
}
+protected class FetchRouterHealthChecksResultTask extends
ManagedContextRunnable {
+public FetchRouterHealthChecksResultTask() {
+}
+
+@Override
+protected void runInContext() {
+try {
+final List routers =
_routerDao.listByStateAndManagementServer(VirtualMachine.State.Running,
mgmtSrvrId);
+s_logger.debug("Found " + routers.size() + " running routers.
");
+
+for (final DomainRouterVO router : routers) {
+GetRouterMonitorResultsAnswer answer =
fetchAndUpdateRouterHealthChecks(router, false);
+String checkFailsToRestartVr =
RouterHealthChecksFailuresToRestartVr.valueIn(router.getDataCenterId());
+if (answer == null) {
+s_logger.warn("Unable to fetch monitor results for
router " + router);
+updateRouterConnectivityHealthCheck(router.getId(),
false, "Communication failed");
+} else if (!answer.getResult()) {
+s_logger.warn("Failed to fetch monitor results from
router " + router + " with details: " + answer.getDetails());
+updateRouterConnectivityHealthCheck(router.getId(),
false, "Failed to fetch results with details: " + answer.getDetails());
+} else {
+updateRouterConnectivityHealthCheck(router.getId(),
true, "Successfully fetched data");
+parseAndMergeDbHealthChecks(router.getId(),
answer.getMonitoringResults());
+
+// Check failing tests and restart if needed
+if (answer.getFailingChecks().size() > 0 &&
StringUtils.isNotBlank(checkFailsToRestartVr)) {
+s_logger.info("Checking failed health checks to
see if router needs reboot");
+for (String failedCheck :
answer.getFailingChecks()) {
+if
(checkFailsToRestartVr.contains(failedCheck)) {
+s_logger.info("Found failing health check
" + failedCheck + " so restarting router.");
+rebootRouter(router.getId(), true);
+}
+}
+}
+}
+}
+} catch (final Exception ex) {
+s_logger.error("Fail to complete the
FetchRouterHealthChecksResultTask! ", ex);
+ex.printStackTrace();
+}
+}
+}
+
+private Map>
getHealthChecksFromDb(long routerId) {
+List healthChecksList =
routerHealthCheckResultDao.getHealthCheckResults(routerId);
+Map> healthCheckResults
= new HashMap<>();
+if (healthChecksList.isEmpty()) {
+return healthCheckResults;
+}
+
+for (RouterHealthCheckResultVO healthCheck : healthChecksList) {
+if (!healthCheckResults.containsKey(healthCheck.getCheckType())) {
+healthCheckResults.put(healthCheck.getCheckType(), new
HashMap<>());
+}
+
healthCheckResults.get(healthCheck.getCheckType()).put(healthCheck.getCheckName(),
healthCheck);
+}
+
+return healthCheckResults;
+}
+
+private RouterHealthCheckResultVO
updateRouterConnectivityHealthCheck(final long routerId, boolean connected,
String message) {
+boolean newEntry = false;
+RouterHealthCheckResultVO connectivityVO =
routerHealthCheckResultDao.getRouterHealthCheckResult(routerId, "connectivity",
"basic");
+if (connectivityVO == null) {
+connectivityVO = new RouterHealthCheckResultVO(routerId,
"connectivity", "basic");
+newEntry = true;
+}
+
+connectivityVO.setCheckResult(connected);
+connectivityVO.setLastUpdateTime(new Date());
+connectivityVO.setCheckDetails(StringUtils.isNotEmpty(message) ?
message.getBytes(Charset.forName("US-ASCII")) : null);
+
+if (newEntry) {
+routerHealthCheckResultDao.persist(connectivityVO);
+} else {
+routerHealthCheckResultDao.update(connectivityVO.getId(),
connectivityVO);
+}
+
+return routerHealthCheckResultDao.getRouterHealthCheckResult(routerId,
"connectivity", "basic");
+}
+
+private List parseAndMergeDbHealthChecks(final
long routerId, final String monitoringResult) {
+if (StringUtils.isBlank(monitoringResult)) {
+
[GitHub] [cloudstack] anuragaw commented on a change in pull request #3575: [WIP DO NOT MERGE] Health check feature for virtual router
anuragaw commented on a change in pull request #3575: [WIP DO NOT MERGE] Health
check feature for virtual router
URL: https://github.com/apache/cloudstack/pull/3575#discussion_r358793537
##
File path:
server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java
##
@@ -1186,6 +1227,431 @@ protected void pushToUpdateQueue(final List
networks) throws Interrup
}
}
+protected class FetchRouterHealthChecksResultTask extends
ManagedContextRunnable {
+public FetchRouterHealthChecksResultTask() {
+}
+
+@Override
+protected void runInContext() {
+try {
+final List routers =
_routerDao.listByStateAndManagementServer(VirtualMachine.State.Running,
mgmtSrvrId);
+s_logger.debug("Found " + routers.size() + " running routers.
");
+
+for (final DomainRouterVO router : routers) {
+GetRouterMonitorResultsAnswer answer =
fetchAndUpdateRouterHealthChecks(router, false);
+String checkFailsToRestartVr =
RouterHealthChecksFailuresToRestartVr.valueIn(router.getDataCenterId());
+if (answer == null) {
+s_logger.warn("Unable to fetch monitor results for
router " + router);
+updateRouterConnectivityHealthCheck(router.getId(),
false, "Communication failed");
+} else if (!answer.getResult()) {
+s_logger.warn("Failed to fetch monitor results from
router " + router + " with details: " + answer.getDetails());
+updateRouterConnectivityHealthCheck(router.getId(),
false, "Failed to fetch results with details: " + answer.getDetails());
+} else {
+updateRouterConnectivityHealthCheck(router.getId(),
true, "Successfully fetched data");
+parseAndMergeDbHealthChecks(router.getId(),
answer.getMonitoringResults());
+
+// Check failing tests and restart if needed
+if (answer.getFailingChecks().size() > 0 &&
StringUtils.isNotBlank(checkFailsToRestartVr)) {
+s_logger.info("Checking failed health checks to
see if router needs reboot");
+for (String failedCheck :
answer.getFailingChecks()) {
+if
(checkFailsToRestartVr.contains(failedCheck)) {
+s_logger.info("Found failing health check
" + failedCheck + " so restarting router.");
+rebootRouter(router.getId(), true);
+}
+}
+}
+}
+}
+} catch (final Exception ex) {
+s_logger.error("Fail to complete the
FetchRouterHealthChecksResultTask! ", ex);
+ex.printStackTrace();
+}
+}
+}
+
+private Map>
getHealthChecksFromDb(long routerId) {
+List healthChecksList =
routerHealthCheckResultDao.getHealthCheckResults(routerId);
+Map> healthCheckResults
= new HashMap<>();
+if (healthChecksList.isEmpty()) {
+return healthCheckResults;
+}
+
+for (RouterHealthCheckResultVO healthCheck : healthChecksList) {
+if (!healthCheckResults.containsKey(healthCheck.getCheckType())) {
+healthCheckResults.put(healthCheck.getCheckType(), new
HashMap<>());
+}
+
healthCheckResults.get(healthCheck.getCheckType()).put(healthCheck.getCheckName(),
healthCheck);
+}
+
+return healthCheckResults;
+}
+
+private RouterHealthCheckResultVO
updateRouterConnectivityHealthCheck(final long routerId, boolean connected,
String message) {
+boolean newEntry = false;
+RouterHealthCheckResultVO connectivityVO =
routerHealthCheckResultDao.getRouterHealthCheckResult(routerId, "connectivity",
"basic");
+if (connectivityVO == null) {
+connectivityVO = new RouterHealthCheckResultVO(routerId,
"connectivity", "basic");
+newEntry = true;
+}
+
+connectivityVO.setCheckResult(connected);
+connectivityVO.setLastUpdateTime(new Date());
+connectivityVO.setCheckDetails(StringUtils.isNotEmpty(message) ?
message.getBytes(Charset.forName("US-ASCII")) : null);
+
+if (newEntry) {
+routerHealthCheckResultDao.persist(connectivityVO);
+} else {
+routerHealthCheckResultDao.update(connectivityVO.getId(),
connectivityVO);
+}
+
+return routerHealthCheckResultDao.getRouterHealthCheckResult(routerId,
"connectivity", "basic");
+}
+
+private List parseAndMergeDbHealthChecks(final
long routerId, final String monitoringResult) {
+if (StringUtils.isBlank(monitoringResult)) {
+
[GitHub] [cloudstack] anuragaw commented on a change in pull request #3575: [WIP DO NOT MERGE] Health check feature for virtual router
anuragaw commented on a change in pull request #3575: [WIP DO NOT MERGE] Health
check feature for virtual router
URL: https://github.com/apache/cloudstack/pull/3575#discussion_r358792853
##
File path:
server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java
##
@@ -1186,6 +1227,431 @@ protected void pushToUpdateQueue(final List
networks) throws Interrup
}
}
+protected class FetchRouterHealthChecksResultTask extends
ManagedContextRunnable {
+public FetchRouterHealthChecksResultTask() {
+}
+
+@Override
+protected void runInContext() {
+try {
+final List routers =
_routerDao.listByStateAndManagementServer(VirtualMachine.State.Running,
mgmtSrvrId);
+s_logger.debug("Found " + routers.size() + " running routers.
");
+
+for (final DomainRouterVO router : routers) {
+GetRouterMonitorResultsAnswer answer =
fetchAndUpdateRouterHealthChecks(router, false);
+String checkFailsToRestartVr =
RouterHealthChecksFailuresToRestartVr.valueIn(router.getDataCenterId());
+if (answer == null) {
+s_logger.warn("Unable to fetch monitor results for
router " + router);
+updateRouterConnectivityHealthCheck(router.getId(),
false, "Communication failed");
+} else if (!answer.getResult()) {
+s_logger.warn("Failed to fetch monitor results from
router " + router + " with details: " + answer.getDetails());
+updateRouterConnectivityHealthCheck(router.getId(),
false, "Failed to fetch results with details: " + answer.getDetails());
+} else {
+updateRouterConnectivityHealthCheck(router.getId(),
true, "Successfully fetched data");
+parseAndMergeDbHealthChecks(router.getId(),
answer.getMonitoringResults());
+
+// Check failing tests and restart if needed
+if (answer.getFailingChecks().size() > 0 &&
StringUtils.isNotBlank(checkFailsToRestartVr)) {
+s_logger.info("Checking failed health checks to
see if router needs reboot");
+for (String failedCheck :
answer.getFailingChecks()) {
+if
(checkFailsToRestartVr.contains(failedCheck)) {
+s_logger.info("Found failing health check
" + failedCheck + " so restarting router.");
+rebootRouter(router.getId(), true);
+}
+}
+}
+}
+}
+} catch (final Exception ex) {
+s_logger.error("Fail to complete the
FetchRouterHealthChecksResultTask! ", ex);
+ex.printStackTrace();
+}
+}
+}
+
+private Map>
getHealthChecksFromDb(long routerId) {
+List healthChecksList =
routerHealthCheckResultDao.getHealthCheckResults(routerId);
+Map> healthCheckResults
= new HashMap<>();
+if (healthChecksList.isEmpty()) {
+return healthCheckResults;
+}
+
+for (RouterHealthCheckResultVO healthCheck : healthChecksList) {
+if (!healthCheckResults.containsKey(healthCheck.getCheckType())) {
+healthCheckResults.put(healthCheck.getCheckType(), new
HashMap<>());
+}
+
healthCheckResults.get(healthCheck.getCheckType()).put(healthCheck.getCheckName(),
healthCheck);
+}
+
+return healthCheckResults;
+}
+
+private RouterHealthCheckResultVO
updateRouterConnectivityHealthCheck(final long routerId, boolean connected,
String message) {
+boolean newEntry = false;
+RouterHealthCheckResultVO connectivityVO =
routerHealthCheckResultDao.getRouterHealthCheckResult(routerId, "connectivity",
"basic");
+if (connectivityVO == null) {
+connectivityVO = new RouterHealthCheckResultVO(routerId,
"connectivity", "basic");
+newEntry = true;
+}
+
+connectivityVO.setCheckResult(connected);
+connectivityVO.setLastUpdateTime(new Date());
+connectivityVO.setCheckDetails(StringUtils.isNotEmpty(message) ?
message.getBytes(Charset.forName("US-ASCII")) : null);
+
+if (newEntry) {
+routerHealthCheckResultDao.persist(connectivityVO);
+} else {
+routerHealthCheckResultDao.update(connectivityVO.getId(),
connectivityVO);
+}
+
+return routerHealthCheckResultDao.getRouterHealthCheckResult(routerId,
"connectivity", "basic");
+}
+
+private List parseAndMergeDbHealthChecks(final
long routerId, final String monitoringResult) {
+if (StringUtils.isBlank(monitoringResult)) {
+
[GitHub] [cloudstack] blueorangutan commented on issue #3755: Added zone check for attach iso
blueorangutan commented on issue #3755: Added zone check for attach iso URL: https://github.com/apache/cloudstack/pull/3755#issuecomment-566542120 @borisstoyanov 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack] borisstoyanov commented on issue #3755: Added zone check for attach iso
borisstoyanov commented on issue #3755: Added zone check for attach iso URL: https://github.com/apache/cloudstack/pull/3755#issuecomment-566541963 @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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack] anuragaw commented on a change in pull request #3575: [WIP DO NOT MERGE] Health check feature for virtual router
anuragaw commented on a change in pull request #3575: [WIP DO NOT MERGE] Health
check feature for virtual router
URL: https://github.com/apache/cloudstack/pull/3575#discussion_r358788785
##
File path:
server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java
##
@@ -1186,6 +1227,431 @@ protected void pushToUpdateQueue(final List
networks) throws Interrup
}
}
+protected class FetchRouterHealthChecksResultTask extends
ManagedContextRunnable {
+public FetchRouterHealthChecksResultTask() {
+}
+
+@Override
+protected void runInContext() {
+try {
+final List routers =
_routerDao.listByStateAndManagementServer(VirtualMachine.State.Running,
mgmtSrvrId);
+s_logger.debug("Found " + routers.size() + " running routers.
");
+
+for (final DomainRouterVO router : routers) {
+GetRouterMonitorResultsAnswer answer =
fetchAndUpdateRouterHealthChecks(router, false);
+String checkFailsToRestartVr =
RouterHealthChecksFailuresToRestartVr.valueIn(router.getDataCenterId());
+if (answer == null) {
+s_logger.warn("Unable to fetch monitor results for
router " + router);
+updateRouterConnectivityHealthCheck(router.getId(),
false, "Communication failed");
+} else if (!answer.getResult()) {
+s_logger.warn("Failed to fetch monitor results from
router " + router + " with details: " + answer.getDetails());
+updateRouterConnectivityHealthCheck(router.getId(),
false, "Failed to fetch results with details: " + answer.getDetails());
+} else {
+updateRouterConnectivityHealthCheck(router.getId(),
true, "Successfully fetched data");
+parseAndMergeDbHealthChecks(router.getId(),
answer.getMonitoringResults());
+
+// Check failing tests and restart if needed
+if (answer.getFailingChecks().size() > 0 &&
StringUtils.isNotBlank(checkFailsToRestartVr)) {
+s_logger.info("Checking failed health checks to
see if router needs reboot");
+for (String failedCheck :
answer.getFailingChecks()) {
+if
(checkFailsToRestartVr.contains(failedCheck)) {
+s_logger.info("Found failing health check
" + failedCheck + " so restarting router.");
+rebootRouter(router.getId(), true);
+}
+}
+}
+}
+}
+} catch (final Exception ex) {
+s_logger.error("Fail to complete the
FetchRouterHealthChecksResultTask! ", ex);
+ex.printStackTrace();
+}
+}
+}
+
+private Map>
getHealthChecksFromDb(long routerId) {
+List healthChecksList =
routerHealthCheckResultDao.getHealthCheckResults(routerId);
+Map> healthCheckResults
= new HashMap<>();
+if (healthChecksList.isEmpty()) {
+return healthCheckResults;
+}
+
+for (RouterHealthCheckResultVO healthCheck : healthChecksList) {
+if (!healthCheckResults.containsKey(healthCheck.getCheckType())) {
+healthCheckResults.put(healthCheck.getCheckType(), new
HashMap<>());
+}
+
healthCheckResults.get(healthCheck.getCheckType()).put(healthCheck.getCheckName(),
healthCheck);
+}
+
+return healthCheckResults;
+}
+
+private RouterHealthCheckResultVO
updateRouterConnectivityHealthCheck(final long routerId, boolean connected,
String message) {
+boolean newEntry = false;
+RouterHealthCheckResultVO connectivityVO =
routerHealthCheckResultDao.getRouterHealthCheckResult(routerId, "connectivity",
"basic");
+if (connectivityVO == null) {
+connectivityVO = new RouterHealthCheckResultVO(routerId,
"connectivity", "basic");
+newEntry = true;
+}
+
+connectivityVO.setCheckResult(connected);
+connectivityVO.setLastUpdateTime(new Date());
+connectivityVO.setCheckDetails(StringUtils.isNotEmpty(message) ?
message.getBytes(Charset.forName("US-ASCII")) : null);
+
+if (newEntry) {
+routerHealthCheckResultDao.persist(connectivityVO);
+} else {
+routerHealthCheckResultDao.update(connectivityVO.getId(),
connectivityVO);
+}
+
+return routerHealthCheckResultDao.getRouterHealthCheckResult(routerId,
"connectivity", "basic");
+}
+
+private List parseAndMergeDbHealthChecks(final
long routerId, final String monitoringResult) {
+if (StringUtils.isBlank(monitoringResult)) {
+
[GitHub] [cloudstack] anuragaw commented on a change in pull request #3575: [WIP DO NOT MERGE] Health check feature for virtual router
anuragaw commented on a change in pull request #3575: [WIP DO NOT MERGE] Health
check feature for virtual router
URL: https://github.com/apache/cloudstack/pull/3575#discussion_r358788702
##
File path:
server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java
##
@@ -1186,6 +1227,431 @@ protected void pushToUpdateQueue(final List
networks) throws Interrup
}
}
+protected class FetchRouterHealthChecksResultTask extends
ManagedContextRunnable {
+public FetchRouterHealthChecksResultTask() {
+}
+
+@Override
+protected void runInContext() {
+try {
+final List routers =
_routerDao.listByStateAndManagementServer(VirtualMachine.State.Running,
mgmtSrvrId);
+s_logger.debug("Found " + routers.size() + " running routers.
");
+
+for (final DomainRouterVO router : routers) {
+GetRouterMonitorResultsAnswer answer =
fetchAndUpdateRouterHealthChecks(router, false);
+String checkFailsToRestartVr =
RouterHealthChecksFailuresToRestartVr.valueIn(router.getDataCenterId());
+if (answer == null) {
+s_logger.warn("Unable to fetch monitor results for
router " + router);
+updateRouterConnectivityHealthCheck(router.getId(),
false, "Communication failed");
+} else if (!answer.getResult()) {
+s_logger.warn("Failed to fetch monitor results from
router " + router + " with details: " + answer.getDetails());
+updateRouterConnectivityHealthCheck(router.getId(),
false, "Failed to fetch results with details: " + answer.getDetails());
+} else {
+updateRouterConnectivityHealthCheck(router.getId(),
true, "Successfully fetched data");
+parseAndMergeDbHealthChecks(router.getId(),
answer.getMonitoringResults());
+
+// Check failing tests and restart if needed
+if (answer.getFailingChecks().size() > 0 &&
StringUtils.isNotBlank(checkFailsToRestartVr)) {
+s_logger.info("Checking failed health checks to
see if router needs reboot");
+for (String failedCheck :
answer.getFailingChecks()) {
+if
(checkFailsToRestartVr.contains(failedCheck)) {
+s_logger.info("Found failing health check
" + failedCheck + " so restarting router.");
+rebootRouter(router.getId(), true);
+}
+}
+}
+}
+}
+} catch (final Exception ex) {
+s_logger.error("Fail to complete the
FetchRouterHealthChecksResultTask! ", ex);
+ex.printStackTrace();
+}
+}
+}
+
+private Map>
getHealthChecksFromDb(long routerId) {
+List healthChecksList =
routerHealthCheckResultDao.getHealthCheckResults(routerId);
+Map> healthCheckResults
= new HashMap<>();
+if (healthChecksList.isEmpty()) {
+return healthCheckResults;
+}
+
+for (RouterHealthCheckResultVO healthCheck : healthChecksList) {
+if (!healthCheckResults.containsKey(healthCheck.getCheckType())) {
+healthCheckResults.put(healthCheck.getCheckType(), new
HashMap<>());
+}
+
healthCheckResults.get(healthCheck.getCheckType()).put(healthCheck.getCheckName(),
healthCheck);
+}
+
+return healthCheckResults;
+}
+
+private RouterHealthCheckResultVO
updateRouterConnectivityHealthCheck(final long routerId, boolean connected,
String message) {
+boolean newEntry = false;
+RouterHealthCheckResultVO connectivityVO =
routerHealthCheckResultDao.getRouterHealthCheckResult(routerId, "connectivity",
"basic");
+if (connectivityVO == null) {
+connectivityVO = new RouterHealthCheckResultVO(routerId,
"connectivity", "basic");
+newEntry = true;
+}
+
+connectivityVO.setCheckResult(connected);
+connectivityVO.setLastUpdateTime(new Date());
+connectivityVO.setCheckDetails(StringUtils.isNotEmpty(message) ?
message.getBytes(Charset.forName("US-ASCII")) : null);
+
+if (newEntry) {
+routerHealthCheckResultDao.persist(connectivityVO);
+} else {
+routerHealthCheckResultDao.update(connectivityVO.getId(),
connectivityVO);
+}
+
+return routerHealthCheckResultDao.getRouterHealthCheckResult(routerId,
"connectivity", "basic");
+}
+
+private List parseAndMergeDbHealthChecks(final
long routerId, final String monitoringResult) {
+if (StringUtils.isBlank(monitoringResult)) {
+
[GitHub] [cloudstack] anuragaw commented on a change in pull request #3575: [WIP DO NOT MERGE] Health check feature for virtual router
anuragaw commented on a change in pull request #3575: [WIP DO NOT MERGE] Health
check feature for virtual router
URL: https://github.com/apache/cloudstack/pull/3575#discussion_r358783155
##
File path:
server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java
##
@@ -1186,6 +1227,431 @@ protected void pushToUpdateQueue(final List
networks) throws Interrup
}
}
+protected class FetchRouterHealthChecksResultTask extends
ManagedContextRunnable {
+public FetchRouterHealthChecksResultTask() {
+}
+
+@Override
+protected void runInContext() {
+try {
+final List routers =
_routerDao.listByStateAndManagementServer(VirtualMachine.State.Running,
mgmtSrvrId);
+s_logger.debug("Found " + routers.size() + " running routers.
");
+
+for (final DomainRouterVO router : routers) {
+GetRouterMonitorResultsAnswer answer =
fetchAndUpdateRouterHealthChecks(router, false);
+String checkFailsToRestartVr =
RouterHealthChecksFailuresToRestartVr.valueIn(router.getDataCenterId());
+if (answer == null) {
+s_logger.warn("Unable to fetch monitor results for
router " + router);
+updateRouterConnectivityHealthCheck(router.getId(),
false, "Communication failed");
+} else if (!answer.getResult()) {
+s_logger.warn("Failed to fetch monitor results from
router " + router + " with details: " + answer.getDetails());
+updateRouterConnectivityHealthCheck(router.getId(),
false, "Failed to fetch results with details: " + answer.getDetails());
+} else {
+updateRouterConnectivityHealthCheck(router.getId(),
true, "Successfully fetched data");
+parseAndMergeDbHealthChecks(router.getId(),
answer.getMonitoringResults());
+
+// Check failing tests and restart if needed
+if (answer.getFailingChecks().size() > 0 &&
StringUtils.isNotBlank(checkFailsToRestartVr)) {
+s_logger.info("Checking failed health checks to
see if router needs reboot");
+for (String failedCheck :
answer.getFailingChecks()) {
+if
(checkFailsToRestartVr.contains(failedCheck)) {
+s_logger.info("Found failing health check
" + failedCheck + " so restarting router.");
+rebootRouter(router.getId(), true);
+}
+}
+}
+}
+}
+} catch (final Exception ex) {
+s_logger.error("Fail to complete the
FetchRouterHealthChecksResultTask! ", ex);
+ex.printStackTrace();
+}
+}
+}
+
+private Map>
getHealthChecksFromDb(long routerId) {
+List healthChecksList =
routerHealthCheckResultDao.getHealthCheckResults(routerId);
+Map> healthCheckResults
= new HashMap<>();
+if (healthChecksList.isEmpty()) {
+return healthCheckResults;
+}
+
+for (RouterHealthCheckResultVO healthCheck : healthChecksList) {
+if (!healthCheckResults.containsKey(healthCheck.getCheckType())) {
+healthCheckResults.put(healthCheck.getCheckType(), new
HashMap<>());
+}
+
healthCheckResults.get(healthCheck.getCheckType()).put(healthCheck.getCheckName(),
healthCheck);
+}
+
+return healthCheckResults;
+}
+
+private RouterHealthCheckResultVO
updateRouterConnectivityHealthCheck(final long routerId, boolean connected,
String message) {
+boolean newEntry = false;
+RouterHealthCheckResultVO connectivityVO =
routerHealthCheckResultDao.getRouterHealthCheckResult(routerId, "connectivity",
"basic");
+if (connectivityVO == null) {
+connectivityVO = new RouterHealthCheckResultVO(routerId,
"connectivity", "basic");
+newEntry = true;
+}
+
+connectivityVO.setCheckResult(connected);
+connectivityVO.setLastUpdateTime(new Date());
+connectivityVO.setCheckDetails(StringUtils.isNotEmpty(message) ?
message.getBytes(Charset.forName("US-ASCII")) : null);
+
+if (newEntry) {
+routerHealthCheckResultDao.persist(connectivityVO);
+} else {
+routerHealthCheckResultDao.update(connectivityVO.getId(),
connectivityVO);
+}
+
+return routerHealthCheckResultDao.getRouterHealthCheckResult(routerId,
"connectivity", "basic");
+}
+
+private List parseAndMergeDbHealthChecks(final
long routerId, final String monitoringResult) {
Review comment:
done.
[GitHub] [cloudstack] pavanaravapalli commented on issue #3643: WIP: UEFI Support on CloudStack
pavanaravapalli commented on issue #3643: WIP: UEFI Support on CloudStack URL: https://github.com/apache/cloudstack/pull/3643#issuecomment-566515206 Closing this PR Since forked branch is been obsoleted, re-opened another PR #3638 for the same. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack] pavanaravapalli closed pull request #3643: WIP: UEFI Support on CloudStack
pavanaravapalli closed pull request #3643: WIP: UEFI Support on CloudStack URL: https://github.com/apache/cloudstack/pull/3643 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack] anuragaw commented on a change in pull request #3575: [WIP DO NOT MERGE] Health check feature for virtual router
anuragaw commented on a change in pull request #3575: [WIP DO NOT MERGE] Health
check feature for virtual router
URL: https://github.com/apache/cloudstack/pull/3575#discussion_r358742417
##
File path:
server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java
##
@@ -1186,6 +1227,431 @@ protected void pushToUpdateQueue(final List
networks) throws Interrup
}
}
+protected class FetchRouterHealthChecksResultTask extends
ManagedContextRunnable {
+public FetchRouterHealthChecksResultTask() {
+}
+
+@Override
+protected void runInContext() {
+try {
+final List routers =
_routerDao.listByStateAndManagementServer(VirtualMachine.State.Running,
mgmtSrvrId);
+s_logger.debug("Found " + routers.size() + " running routers.
");
+
+for (final DomainRouterVO router : routers) {
+GetRouterMonitorResultsAnswer answer =
fetchAndUpdateRouterHealthChecks(router, false);
+String checkFailsToRestartVr =
RouterHealthChecksFailuresToRestartVr.valueIn(router.getDataCenterId());
+if (answer == null) {
+s_logger.warn("Unable to fetch monitor results for
router " + router);
+updateRouterConnectivityHealthCheck(router.getId(),
false, "Communication failed");
+} else if (!answer.getResult()) {
+s_logger.warn("Failed to fetch monitor results from
router " + router + " with details: " + answer.getDetails());
+updateRouterConnectivityHealthCheck(router.getId(),
false, "Failed to fetch results with details: " + answer.getDetails());
+} else {
+updateRouterConnectivityHealthCheck(router.getId(),
true, "Successfully fetched data");
+parseAndMergeDbHealthChecks(router.getId(),
answer.getMonitoringResults());
+
+// Check failing tests and restart if needed
+if (answer.getFailingChecks().size() > 0 &&
StringUtils.isNotBlank(checkFailsToRestartVr)) {
+s_logger.info("Checking failed health checks to
see if router needs reboot");
+for (String failedCheck :
answer.getFailingChecks()) {
+if
(checkFailsToRestartVr.contains(failedCheck)) {
+s_logger.info("Found failing health check
" + failedCheck + " so restarting router.");
+rebootRouter(router.getId(), true);
+}
+}
+}
+}
+}
+} catch (final Exception ex) {
+s_logger.error("Fail to complete the
FetchRouterHealthChecksResultTask! ", ex);
+ex.printStackTrace();
+}
+}
+}
+
+private Map>
getHealthChecksFromDb(long routerId) {
+List healthChecksList =
routerHealthCheckResultDao.getHealthCheckResults(routerId);
+Map> healthCheckResults
= new HashMap<>();
+if (healthChecksList.isEmpty()) {
+return healthCheckResults;
+}
+
+for (RouterHealthCheckResultVO healthCheck : healthChecksList) {
+if (!healthCheckResults.containsKey(healthCheck.getCheckType())) {
+healthCheckResults.put(healthCheck.getCheckType(), new
HashMap<>());
+}
+
healthCheckResults.get(healthCheck.getCheckType()).put(healthCheck.getCheckName(),
healthCheck);
+}
+
+return healthCheckResults;
+}
+
+private RouterHealthCheckResultVO
updateRouterConnectivityHealthCheck(final long routerId, boolean connected,
String message) {
+boolean newEntry = false;
+RouterHealthCheckResultVO connectivityVO =
routerHealthCheckResultDao.getRouterHealthCheckResult(routerId, "connectivity",
"basic");
+if (connectivityVO == null) {
+connectivityVO = new RouterHealthCheckResultVO(routerId,
"connectivity", "basic");
+newEntry = true;
+}
+
+connectivityVO.setCheckResult(connected);
+connectivityVO.setLastUpdateTime(new Date());
+connectivityVO.setCheckDetails(StringUtils.isNotEmpty(message) ?
message.getBytes(Charset.forName("US-ASCII")) : null);
+
+if (newEntry) {
+routerHealthCheckResultDao.persist(connectivityVO);
+} else {
+routerHealthCheckResultDao.update(connectivityVO.getId(),
connectivityVO);
+}
+
+return routerHealthCheckResultDao.getRouterHealthCheckResult(routerId,
"connectivity", "basic");
+}
+
+private List parseAndMergeDbHealthChecks(final
long routerId, final String monitoringResult) {
+if (StringUtils.isBlank(monitoringResult)) {
+
[GitHub] [cloudstack] anuragaw commented on a change in pull request #3575: [WIP DO NOT MERGE] Health check feature for virtual router
anuragaw commented on a change in pull request #3575: [WIP DO NOT MERGE] Health
check feature for virtual router
URL: https://github.com/apache/cloudstack/pull/3575#discussion_r358742618
##
File path:
core/src/main/java/com/cloud/agent/resource/virtualnetwork/facade/SetMonitorServiceConfigItem.java
##
@@ -21,21 +21,50 @@
import java.util.List;
+import org.apache.log4j.Logger;
+
import com.cloud.agent.api.routing.NetworkElementCommand;
import com.cloud.agent.api.routing.SetMonitorServiceCommand;
import com.cloud.agent.resource.virtualnetwork.ConfigItem;
+import com.cloud.agent.resource.virtualnetwork.ScriptConfigItem;
import com.cloud.agent.resource.virtualnetwork.VRScripts;
import com.cloud.agent.resource.virtualnetwork.model.ConfigBase;
import com.cloud.agent.resource.virtualnetwork.model.MonitorService;
public class SetMonitorServiceConfigItem extends AbstractConfigItemFacade {
+private static final Logger s_logger =
Logger.getLogger(SetMonitorServiceConfigItem.class);
@Override
public List generateConfig(final NetworkElementCommand cmd) {
final SetMonitorServiceCommand command = (SetMonitorServiceCommand)
cmd;
-final MonitorService monitorService = new
MonitorService(command.getConfiguration(),
cmd.getAccessDetail(NetworkElementCommand.ROUTER_MONITORING_ENABLE));
-return generateConfigItems(monitorService);
+final MonitorService monitorService = new MonitorService(
+command.getConfiguration(),
+
cmd.getAccessDetail(SetMonitorServiceCommand.ROUTER_MONITORING_ENABLED),
+
cmd.getAccessDetail(SetMonitorServiceCommand.ROUTER_HEALTH_CHECKS_ENABLED));
+
+try {
+
monitorService.setHealthChecksBasicRunInterval(Integer.parseInt(cmd.getAccessDetail(SetMonitorServiceCommand.ROUTER_HEALTH_CHECKS_BASIC_INTERVAL)));
+} catch (NumberFormatException exception) {
+s_logger.error("Unexpected health check basic interval set" +
cmd.getAccessDetail(SetMonitorServiceCommand.ROUTER_HEALTH_CHECKS_BASIC_INTERVAL)
+
+". Exception: " + exception + "Will use default value");
+}
+
+try {
+
monitorService.setHealthChecksAdvanceRunInterval(Integer.parseInt(cmd.getAccessDetail(SetMonitorServiceCommand.ROUTER_HEALTH_CHECKS_ADVANCED_INTERVAL)));
+} catch (NumberFormatException exception) {
+s_logger.error("Unexpected health check advance interval set" +
cmd.getAccessDetail(SetMonitorServiceCommand.ROUTER_HEALTH_CHECKS_ADVANCED_INTERVAL)
+
+". Exception: " + exception + "Will use default value");
+}
+
+
monitorService.setExcludedHealthChecks(cmd.getAccessDetail(SetMonitorServiceCommand.ROUTER_HEALTH_CHECKS_EXCLUDED));
+monitorService.setAdditionalData(command.getAdditionalData());
+
monitorService.setDeleteFromProcessedCache(command.shouldDeleteFromProcessedCache());
+List configItems = generateConfigItems(monitorService);
+if (configItems != null && command.shouldReconfigureAfterUpdate()) {
+configItems.add(new ScriptConfigItem(VRScripts.CONFIGURE,
"monitor_service.json"));
+}
Review comment:
Done.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] anuragaw commented on a change in pull request #3575: [WIP DO NOT MERGE] Health check feature for virtual router
anuragaw commented on a change in pull request #3575: [WIP DO NOT MERGE] Health
check feature for virtual router
URL: https://github.com/apache/cloudstack/pull/3575#discussion_r358742018
##
File path:
server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java
##
@@ -1186,6 +1227,431 @@ protected void pushToUpdateQueue(final List
networks) throws Interrup
}
}
+protected class FetchRouterHealthChecksResultTask extends
ManagedContextRunnable {
+public FetchRouterHealthChecksResultTask() {
+}
+
+@Override
+protected void runInContext() {
+try {
+final List routers =
_routerDao.listByStateAndManagementServer(VirtualMachine.State.Running,
mgmtSrvrId);
+s_logger.debug("Found " + routers.size() + " running routers.
");
+
+for (final DomainRouterVO router : routers) {
+GetRouterMonitorResultsAnswer answer =
fetchAndUpdateRouterHealthChecks(router, false);
+String checkFailsToRestartVr =
RouterHealthChecksFailuresToRestartVr.valueIn(router.getDataCenterId());
+if (answer == null) {
+s_logger.warn("Unable to fetch monitor results for
router " + router);
+updateRouterConnectivityHealthCheck(router.getId(),
false, "Communication failed");
+} else if (!answer.getResult()) {
+s_logger.warn("Failed to fetch monitor results from
router " + router + " with details: " + answer.getDetails());
+updateRouterConnectivityHealthCheck(router.getId(),
false, "Failed to fetch results with details: " + answer.getDetails());
+} else {
+updateRouterConnectivityHealthCheck(router.getId(),
true, "Successfully fetched data");
+parseAndMergeDbHealthChecks(router.getId(),
answer.getMonitoringResults());
+
+// Check failing tests and restart if needed
+if (answer.getFailingChecks().size() > 0 &&
StringUtils.isNotBlank(checkFailsToRestartVr)) {
+s_logger.info("Checking failed health checks to
see if router needs reboot");
+for (String failedCheck :
answer.getFailingChecks()) {
+if
(checkFailsToRestartVr.contains(failedCheck)) {
+s_logger.info("Found failing health check
" + failedCheck + " so restarting router.");
Review comment:
done
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] anuragaw commented on a change in pull request #3575: [WIP DO NOT MERGE] Health check feature for virtual router
anuragaw commented on a change in pull request #3575: [WIP DO NOT MERGE] Health
check feature for virtual router
URL: https://github.com/apache/cloudstack/pull/3575#discussion_r358740910
##
File path:
server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java
##
@@ -658,7 +685,21 @@ public boolean start() {
if (routerAlertsCheckInterval > 0) {
_checkExecutor.scheduleAtFixedRate(new CheckRouterAlertsTask(),
routerAlertsCheckInterval, routerAlertsCheckInterval, TimeUnit.SECONDS);
} else {
-s_logger.debug("router.alerts.check.interval - " +
routerAlertsCheckInterval + " so not scheduling the router alerts checking
thread");
+s_logger.debug(RouterAlertsCheckIntervalCK + "=" +
routerAlertsCheckInterval + " so not scheduling the router alerts checking
thread");
+}
+
+final int routerHealthCheckDataRefreshInterval =
RouterHealthChecksDataRefreshInterval.value();
+if (routerHealthCheckDataRefreshInterval > 0) {
+_checkExecutor.scheduleAtFixedRate(new
UpdateRouterHealthChecksConfigDataTask(), routerHealthCheckDataRefreshInterval,
routerHealthCheckDataRefreshInterval, TimeUnit.MINUTES);
+} else {
+s_logger.debug(RouterHealthChecksDataRefreshIntervalCK + "=" +
routerAlertsCheckInterval + " so not scheduling the router health check data
thread");
+}
+
+final int routerHealthChecksFetchInterval =
RouterHealthChecksResultFetchInterval.value();
+if (routerHealthChecksFetchInterval > 0) {
+_checkExecutor.scheduleAtFixedRate(new
FetchRouterHealthChecksResultTask(), routerHealthChecksFetchInterval,
routerHealthChecksFetchInterval, TimeUnit.MINUTES);
+} else {
+s_logger.debug(RouterHealthChecksResultFetchIntervalCK + "=" +
routerAlertsCheckInterval + " so not scheduling the router checks fetching
thread");
Review comment:
Fixed.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] anuragaw commented on a change in pull request #3575: [WIP DO NOT MERGE] Health check feature for virtual router
anuragaw commented on a change in pull request #3575: [WIP DO NOT MERGE] Health
check feature for virtual router
URL: https://github.com/apache/cloudstack/pull/3575#discussion_r358732178
##
File path:
engine/schema/src/main/java/com/cloud/network/dao/RouterHealthCheckResultVO.java
##
@@ -0,0 +1,129 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package com.cloud.network.dao;
+
+import java.nio.charset.Charset;
+import java.util.Date;
+
+import javax.persistence.Column;
+import javax.persistence.Entity;
+import javax.persistence.GeneratedValue;
+import javax.persistence.GenerationType;
+import javax.persistence.Id;
+import javax.persistence.Table;
+import javax.persistence.Temporal;
+import javax.persistence.TemporalType;
+
+import com.cloud.network.RouterHealthCheckResult;
+
+@Entity
+@Table(name = "router_health_check")
+public class RouterHealthCheckResultVO implements RouterHealthCheckResult {
+@Id
+@GeneratedValue(strategy = GenerationType.IDENTITY)
+@Column(name = "id", updatable = false, nullable = false)
+private long id;
+
+@Column(name = "router_id", updatable = false, nullable = false)
+private long routerId;
+
+@Column(name = "check_name", updatable = false, nullable = false)
+private String checkName;
+
+@Column(name = "check_type", updatable = false, nullable = false)
+private String checkType;
+
+@Column(name = "check_result")
+private boolean checkResult;
+
+@Temporal(TemporalType.TIMESTAMP)
+@Column(name = "last_update", updatable = true, nullable = true)
+private Date lastUpdateTime;
+
+@Column(name = "check_details", updatable = true, nullable = true)
+private byte[] checkDetails;
+
+protected RouterHealthCheckResultVO() {
+}
+
+public RouterHealthCheckResultVO(long routerId, String checkName, String
checkType) {
+this.routerId = routerId;
+this.checkName = checkName;
+this.checkType = checkType;
+}
+
+public long getId() {
+return id;
+}
+
+@Override
+public long getRouterId() {
+return routerId;
+}
+
+@Override
+public String getCheckName() {
+return checkName;
+}
+
+@Override
+public String getCheckType() {
+return checkType;
+}
+
+@Override
+public boolean getCheckResult() {
+return checkResult;
+}
+
+@Override
+public Date getLastUpdateTime() {
+return lastUpdateTime;
+}
+
+@Override
+public String getParsedCheckDetails() {
+return checkDetails != null ? new String(checkDetails,
Charset.forName("US-ASCII")) : "";
Review comment:
done
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] anuragaw commented on a change in pull request #3575: [WIP DO NOT MERGE] Health check feature for virtual router
anuragaw commented on a change in pull request #3575: [WIP DO NOT MERGE] Health
check feature for virtual router
URL: https://github.com/apache/cloudstack/pull/3575#discussion_r358731768
##
File path:
engine/schema/src/main/java/com/cloud/network/dao/RouterHealthCheckResultDao.java
##
@@ -0,0 +1,43 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package com.cloud.network.dao;
+
+import java.util.List;
+
+import com.cloud.utils.db.GenericDao;
+
+public interface RouterHealthCheckResultDao extends
GenericDao {
+/**
+ * @param routerId
+ * @return Returns all the health checks in the database for the given
router id
+ */
+List getHealthCheckResults(long routerId);
+
+/**
+ * @param routerId
+ * @return true if there are checks that have been marked failed in the
database
+ */
+boolean hasFailingChecks(long routerId);
+
+/**
+ * @param routerId
+ * @param checkName
+ * @return returns the check result for the routerId, check type and the
check name.
+ */
+RouterHealthCheckResultVO getRouterHealthCheckResult(long routerId, String
checkName, String checkType);
Review comment:
done
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] anuragaw commented on a change in pull request #3575: [WIP DO NOT MERGE] Health check feature for virtual router
anuragaw commented on a change in pull request #3575: [WIP DO NOT MERGE] Health
check feature for virtual router
URL: https://github.com/apache/cloudstack/pull/3575#discussion_r358730435
##
File path:
core/src/main/java/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java
##
@@ -268,6 +267,49 @@ private CheckS2SVpnConnectionsAnswer
execute(CheckS2SVpnConnectionsCommand cmd)
return new CheckS2SVpnConnectionsAnswer(cmd, result.isSuccess(),
result.getDetails());
}
+private GetRouterMonitorResultsAnswer
execute(GetRouterMonitorResultsCommand cmd) {
+String routerIp = cmd.getAccessDetail(NetworkElementCommand.ROUTER_IP);
+String args = cmd.shouldPerformFreshChecks() ? "true" : "false";
+s_logger.debug("Fetching health check result for " + routerIp + " and
executing fresh checks: " + args);
+ExecutionResult result = _vrDeployer.executeInVR(routerIp,
VRScripts.ROUTER_MONITOR_RESULTS, args);
+
+if (!result.isSuccess()) {
+s_logger.warn("Result of " + cmd + " failed with details: " +
result.getDetails());
+return new GetRouterMonitorResultsAnswer(cmd, false, null,
result.getDetails());
+}
+
+if (result.getDetails().isEmpty()) {
+s_logger.warn("Result of " + cmd + " received no details.");
+return new GetRouterMonitorResultsAnswer(cmd, false, null, "No
results available.");
+}
+
+List failingChecks = new ArrayList<>();
+StringBuilder monitorResults = new StringBuilder();
+String[] lines = result.getDetails().trim().split("\n");
+boolean readingFailedChecks = false, readingMonitorResults = false;
+for (String line : lines) {
+line = line.trim();
+if (line.contains("FAILING CHECKS")) {
+readingFailedChecks = true;
+readingMonitorResults = false;
+} else if (line.contains("MONITOR RESULTS")) {
+readingFailedChecks = false;
+readingMonitorResults = true;
+} else if (readingFailedChecks && !readingMonitorResults) {
+for (String w : line.split(",")) {
+if (!w.trim().isEmpty()) {
+failingChecks.add(w.trim());
+}
+}
+} else if (!readingFailedChecks && readingMonitorResults) {
+monitorResults.append(line);
+} else {
+s_logger.info("Unexpected state of lines reached while parsing
response. Skipping line.");
+}
+}
Review comment:
done
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] anuragaw commented on a change in pull request #3575: [WIP DO NOT MERGE] Health check feature for virtual router
anuragaw commented on a change in pull request #3575: [WIP DO NOT MERGE] Health
check feature for virtual router
URL: https://github.com/apache/cloudstack/pull/3575#discussion_r358730520
##
File path:
core/src/main/java/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java
##
@@ -268,6 +267,49 @@ private CheckS2SVpnConnectionsAnswer
execute(CheckS2SVpnConnectionsCommand cmd)
return new CheckS2SVpnConnectionsAnswer(cmd, result.isSuccess(),
result.getDetails());
}
+private GetRouterMonitorResultsAnswer
execute(GetRouterMonitorResultsCommand cmd) {
+String routerIp = cmd.getAccessDetail(NetworkElementCommand.ROUTER_IP);
+String args = cmd.shouldPerformFreshChecks() ? "true" : "false";
+s_logger.debug("Fetching health check result for " + routerIp + " and
executing fresh checks: " + args);
+ExecutionResult result = _vrDeployer.executeInVR(routerIp,
VRScripts.ROUTER_MONITOR_RESULTS, args);
+
+if (!result.isSuccess()) {
+s_logger.warn("Result of " + cmd + " failed with details: " +
result.getDetails());
+return new GetRouterMonitorResultsAnswer(cmd, false, null,
result.getDetails());
+}
+
+if (result.getDetails().isEmpty()) {
+s_logger.warn("Result of " + cmd + " received no details.");
+return new GetRouterMonitorResultsAnswer(cmd, false, null, "No
results available.");
+}
+
+List failingChecks = new ArrayList<>();
+StringBuilder monitorResults = new StringBuilder();
+String[] lines = result.getDetails().trim().split("\n");
+boolean readingFailedChecks = false, readingMonitorResults = false;
+for (String line : lines) {
+line = line.trim();
+if (line.contains("FAILING CHECKS")) {
+readingFailedChecks = true;
+readingMonitorResults = false;
+} else if (line.contains("MONITOR RESULTS")) {
+readingFailedChecks = false;
+readingMonitorResults = true;
+} else if (readingFailedChecks && !readingMonitorResults) {
+for (String w : line.split(",")) {
+if (!w.trim().isEmpty()) {
+failingChecks.add(w.trim());
+}
+}
Review comment:
done
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] svenvogel commented on issue #3737: Add missing HA config keys
svenvogel commented on issue #3737: Add missing HA config keys URL: https://github.com/apache/cloudstack/pull/3737#issuecomment-566486833 @weizhouapache should be ready to merge? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #3765: Promiscuous vm reject
blueorangutan commented on issue #3765: Promiscuous vm reject URL: https://github.com/apache/cloudstack/pull/3765#issuecomment-566484784 Packaging result: ✖centos6 ✔centos7 ✔debian. JID-479 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #3755: Added zone check for attach iso
blueorangutan commented on issue #3755: Added zone check for attach iso URL: https://github.com/apache/cloudstack/pull/3755#issuecomment-566481831 Packaging result: ✖centos6 ✔centos7 ✔debian. JID-478 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #3350: Get Diagnostics: Download logs and diagnostics data from SSVM, CPVM, Router
blueorangutan commented on issue #3350: Get Diagnostics: Download logs and diagnostics data from SSVM, CPVM, Router URL: https://github.com/apache/cloudstack/pull/3350#issuecomment-566479761 Packaging result: ✖centos6 ✔centos7 ✔debian. JID-477 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack] anuragaw commented on a change in pull request #3575: [WIP DO NOT MERGE] Health check feature for virtual router
anuragaw commented on a change in pull request #3575: [WIP DO NOT MERGE] Health check feature for virtual router URL: https://github.com/apache/cloudstack/pull/3575#discussion_r358709278 ## File path: server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManager.java ## @@ -45,6 +45,17 @@ static final String SetServiceMonitorCK = "network.router.EnableServiceMonitoring"; static final String RouterAlertsCheckIntervalCK = "router.alerts.check.interval"; +static final String RouterHealthChecksEnabledCK = "router.health.checks.enabled"; +static final String RouterHealthChecksBasicIntervalCK = "router.health.checks.basic.interval"; +static final String RouterHealthChecksAdvancedIntervalCK = "router.health.checks.advanced.interval"; +static final String RouterHealthChecksDataRefreshIntervalCK = "router.health.checks.data.refresh.interval"; +static final String RouterHealthChecksResultFetchIntervalCK = "router.health.checks.results.fetch.interval"; +static final String RouterHealthChecksFailuresToRestartVrCK = "router.health.checks.failures.to.restart.vr"; +static final String RouterHealthChecksToExcludeCK = "router.health.checks.to.exclude"; +static final String RouterHealthChecksFreeDiskSpaceThresholdCK = "router.health.checks.free.disk.space.threshold"; +static final String RouterHealthChecksMaxCpuUsageThresholdCK = "router.health.checks.max.cpu.usage.threshold"; +static final String RouterHealthChecksMaxMemoryUsageThresholdCK = "router.health.checks.max.memory.usage.threshold"; + Review comment: I think I agree but I was following the convention other keys had followed in the class to maintain consistency. Makes is more easy to read within a file inmho. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack] anuragaw commented on a change in pull request #3575: [WIP DO NOT MERGE] Health check feature for virtual router
anuragaw commented on a change in pull request #3575: [WIP DO NOT MERGE] Health
check feature for virtual router
URL: https://github.com/apache/cloudstack/pull/3575#discussion_r358708732
##
File path:
server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java
##
@@ -658,7 +685,21 @@ public boolean start() {
if (routerAlertsCheckInterval > 0) {
_checkExecutor.scheduleAtFixedRate(new CheckRouterAlertsTask(),
routerAlertsCheckInterval, routerAlertsCheckInterval, TimeUnit.SECONDS);
} else {
-s_logger.debug("router.alerts.check.interval - " +
routerAlertsCheckInterval + " so not scheduling the router alerts checking
thread");
+s_logger.debug(RouterAlertsCheckIntervalCK + "=" +
routerAlertsCheckInterval + " so not scheduling the router alerts checking
thread");
+}
+
+final int routerHealthCheckDataRefreshInterval =
RouterHealthChecksDataRefreshInterval.value();
+if (routerHealthCheckDataRefreshInterval > 0) {
+_checkExecutor.scheduleAtFixedRate(new
UpdateRouterHealthChecksConfigDataTask(), routerHealthCheckDataRefreshInterval,
routerHealthCheckDataRefreshInterval, TimeUnit.MINUTES);
+} else {
+s_logger.debug(RouterHealthChecksDataRefreshIntervalCK + "=" +
routerAlertsCheckInterval + " so not scheduling the router health check data
thread");
Review comment:
Errr... mistake :disappointed: . fixing.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] anuragaw commented on a change in pull request #3575: [WIP DO NOT MERGE] Health check feature for virtual router
anuragaw commented on a change in pull request #3575: [WIP DO NOT MERGE] Health
check feature for virtual router
URL: https://github.com/apache/cloudstack/pull/3575#discussion_r358708732
##
File path:
server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java
##
@@ -658,7 +685,21 @@ public boolean start() {
if (routerAlertsCheckInterval > 0) {
_checkExecutor.scheduleAtFixedRate(new CheckRouterAlertsTask(),
routerAlertsCheckInterval, routerAlertsCheckInterval, TimeUnit.SECONDS);
} else {
-s_logger.debug("router.alerts.check.interval - " +
routerAlertsCheckInterval + " so not scheduling the router alerts checking
thread");
+s_logger.debug(RouterAlertsCheckIntervalCK + "=" +
routerAlertsCheckInterval + " so not scheduling the router alerts checking
thread");
+}
+
+final int routerHealthCheckDataRefreshInterval =
RouterHealthChecksDataRefreshInterval.value();
+if (routerHealthCheckDataRefreshInterval > 0) {
+_checkExecutor.scheduleAtFixedRate(new
UpdateRouterHealthChecksConfigDataTask(), routerHealthCheckDataRefreshInterval,
routerHealthCheckDataRefreshInterval, TimeUnit.MINUTES);
+} else {
+s_logger.debug(RouterHealthChecksDataRefreshIntervalCK + "=" +
routerAlertsCheckInterval + " so not scheduling the router health check data
thread");
Review comment:
Errr... mistake. fixing.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] anuragaw commented on a change in pull request #3575: [WIP DO NOT MERGE] Health check feature for virtual router
anuragaw commented on a change in pull request #3575: [WIP DO NOT MERGE] Health
check feature for virtual router
URL: https://github.com/apache/cloudstack/pull/3575#discussion_r358708486
##
File path:
core/src/main/java/com/cloud/agent/resource/virtualnetwork/facade/AbstractConfigItemFacade.java
##
@@ -127,7 +127,10 @@ private static String appendUuidToJsonFiles(final String
filename) {
final ConfigItem configFile = new
FileConfigItem(VRScripts.CONFIG_PERSIST_LOCATION, remoteFilename,
gson.toJson(configuration));
cfg.add(configFile);
-final ConfigItem updateCommand = new
ScriptConfigItem(VRScripts.UPDATE_CONFIG, remoteFilename);
+// By default keep files in processed cache on VR
+final String args = configuration.shouldDeleteFromProcessedCache() ?
remoteFilename + " false" : remoteFilename;
+
+final ConfigItem updateCommand = new
ScriptConfigItem(VRScripts.UPDATE_CONFIG, args);
Review comment:
This is implemented in a generic fashion. Any command can set the flag
(contained in ConfigBase.deleteFromProcessedCache) to true and the file won't
be saved to processed cache but instead delted AFTER processing. :)
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] bwsw commented on issue #3751: Add iscsi as secondary storage
bwsw commented on issue #3751: Add iscsi as secondary storage URL: https://github.com/apache/cloudstack/issues/3751#issuecomment-566477178 Why one needs that if one can use Ceph RBD or S3 (Minio) or even Gluster with minor hacks? I don't think block device is an easy way to use as a SS, because it's necessary to add certain kind of addressation inside (actual filesystem) and DLM. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #3765: Promiscuous vm reject
blueorangutan commented on issue #3765: Promiscuous vm reject URL: https://github.com/apache/cloudstack/pull/3765#issuecomment-566474947 @DaanHoogland 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack] DaanHoogland commented on issue #3765: Promiscuous vm reject
DaanHoogland commented on issue #3765: Promiscuous vm reject URL: https://github.com/apache/cloudstack/pull/3765#issuecomment-566474674 @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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack] Spaceman1984 commented on issue #3765: Promiscuous vm reject
Spaceman1984 commented on issue #3765: Promiscuous vm reject URL: https://github.com/apache/cloudstack/pull/3765#issuecomment-566471730 Rebased to 4.13 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #3755: Added zone check for attach iso
blueorangutan commented on issue #3755: Added zone check for attach iso URL: https://github.com/apache/cloudstack/pull/3755#issuecomment-566470433 @DaanHoogland 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack] DaanHoogland commented on issue #3755: Added zone check for attach iso
DaanHoogland commented on issue #3755: Added zone check for attach iso URL: https://github.com/apache/cloudstack/pull/3755#issuecomment-566470080 @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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack] anuragaw commented on issue #3350: Get Diagnostics: Download logs and diagnostics data from SSVM, CPVM, Router
anuragaw commented on issue #3350: Get Diagnostics: Download logs and diagnostics data from SSVM, CPVM, Router URL: https://github.com/apache/cloudstack/pull/3350#issuecomment-566469035 @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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
