You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2020/09/30 23:01:58 UTC
[GitHub] [cloudstack] ravening opened a new pull request #4359: Failed to update host password if username/password is not saved in db
ravening opened a new pull request #4359:
URL: https://github.com/apache/cloudstack/pull/4359
## Description
<!--- Describe your changes in detail -->
If the entries for a host in host_details table are missing then we cant update the
password of the host using the cloudmonkey command
This can happen because host was removed earlier resulting in deletion
of entries in the database
This commit fixes the issue
(local) mgt01 > update hostpassword
hostid=50436ac8-b73a-4eaf-bbab-7adbd9a50870 username=root
password=cloudstack
{
"success": true
}
Now the entries should be added in host_details table as well
## Types of changes
<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] New feature (non-breaking change which adds functionality)
- [X] Bug fix (non-breaking change which fixes an issue)
- [ ] Enhancement (improves an existing feature and functionality)
- [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
## Screenshots (if appropriate):
## How Has This Been Tested?
<!-- Please describe in detail how you tested your changes. -->
<!-- Include details of your testing environment, and the tests you ran to -->
<!-- see how your change affects other areas of the code, etc. -->
Using cloudmonkey api updateHost
<!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/master/CONTRIBUTING.md) document -->
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4359: Failed to update host password if username/password is not saved in db
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4359:
URL: https://github.com/apache/cloudstack/pull/4359#issuecomment-709238639
@sureshanaparti are your concerns addressed?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4359: Failed to update host password if username/password is not saved in db
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4359:
URL: https://github.com/apache/cloudstack/pull/4359#discussion_r508199690
##########
File path: server/src/main/java/com/cloud/server/ManagementServerImpl.java
##########
@@ -3938,7 +3938,12 @@ public void doInTransactionWithoutResult(final TransactionStatus status) {
}
// update password for this host
final DetailVO nv = _detailsDao.findDetail(h.getId(), ApiConstants.USERNAME);
- if (nv.getValue().equals(command.getUsername())) {
+ if (nv == null) {
+ final DetailVO nvu = new DetailVO(h.getId(), ApiConstants.USERNAME, command.getUsername());
Review comment:
@ravening No point in saving password with username empty. Hope you got it. Wanted to confirm the same based on the entries persisted in _host_details_ table for your test. Can you share these? Thanks.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4359: Failed to update host password if username/password is not saved in db
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4359:
URL: https://github.com/apache/cloudstack/pull/4359#discussion_r503423570
##########
File path: server/src/main/java/com/cloud/server/ManagementServerImpl.java
##########
@@ -3994,7 +3999,12 @@ public void doInTransactionWithoutResult(final TransactionStatus status) {
}
// update password for this host
final DetailVO nv = _detailsDao.findDetail(host.getId(), ApiConstants.USERNAME);
- if (nv.getValue().equals(cmd.getUsername())) {
+ if (nv == null) {
+ final DetailVO nvu = new DetailVO(host.getId(), ApiConstants.USERNAME, cmd.getUsername());
Review comment:
`cmd.getUsername()` can be null or empty here, please 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:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4359: Failed to update host password if username/password is not saved in db
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4359:
URL: https://github.com/apache/cloudstack/pull/4359#discussion_r506175495
##########
File path: server/src/main/java/com/cloud/server/ManagementServerImpl.java
##########
@@ -3938,7 +3938,12 @@ public void doInTransactionWithoutResult(final TransactionStatus status) {
}
// update password for this host
final DetailVO nv = _detailsDao.findDetail(h.getId(), ApiConstants.USERNAME);
- if (nv.getValue().equals(command.getUsername())) {
+ if (nv == null) {
+ final DetailVO nvu = new DetailVO(h.getId(), ApiConstants.USERNAME, command.getUsername());
Review comment:
@ravening username is not null, but empty string, so can you add check for that ? Ex: "_StringUtils.isBlank(command.getUsername())_"
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on a change in pull request #4359: Failed to update host password if username/password is not saved in db
Posted by GitBox <gi...@apache.org>.
ravening commented on a change in pull request #4359:
URL: https://github.com/apache/cloudstack/pull/4359#discussion_r506128186
##########
File path: server/src/main/java/com/cloud/server/ManagementServerImpl.java
##########
@@ -3938,7 +3938,12 @@ public void doInTransactionWithoutResult(final TransactionStatus status) {
}
// update password for this host
final DetailVO nv = _detailsDao.findDetail(h.getId(), ApiConstants.USERNAME);
- if (nv.getValue().equals(command.getUsername())) {
+ if (nv == null) {
+ final DetailVO nvu = new DetailVO(h.getId(), ApiConstants.USERNAME, command.getUsername());
Review comment:
@sureshanaparti Even if I dont pass username, I dont get NPE
```
update hostpassword hostid=50436ac8-b73a-4eaf-bbab-7adbd9a50870 password=cloudstack username=
🙈 Error: (HTTP 431, error code 4350) The username is not same for the hosts..
```
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on pull request #4359: Failed to update host password if username/password is not saved in db
Posted by GitBox <gi...@apache.org>.
ravening commented on pull request #4359:
URL: https://github.com/apache/cloudstack/pull/4359#issuecomment-706969355
> @ravening this is a bug fix, which should be merged into 4.14 as well
rebased to 4.14
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4359: Failed to update host password if username/password is not saved in db
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4359:
URL: https://github.com/apache/cloudstack/pull/4359#discussion_r513343883
##########
File path: server/src/main/java/com/cloud/server/ManagementServerImpl.java
##########
@@ -3994,7 +4004,12 @@ public void doInTransactionWithoutResult(final TransactionStatus status) {
}
// update password for this host
final DetailVO nv = _detailsDao.findDetail(host.getId(), ApiConstants.USERNAME);
- if (nv.getValue().equals(cmd.getUsername())) {
+ if (nv == null) {
+ final DetailVO nvu = new DetailVO(host.getId(), ApiConstants.USERNAME, cmd.getUsername());
Review comment:
@ravening Username null or empty fix is missing here.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4359: Failed to update host password if username/password is not saved in db
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4359:
URL: https://github.com/apache/cloudstack/pull/4359#issuecomment-707626409
@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:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on a change in pull request #4359: Failed to update host password if username/password is not saved in db
Posted by GitBox <gi...@apache.org>.
ravening commented on a change in pull request #4359:
URL: https://github.com/apache/cloudstack/pull/4359#discussion_r503920494
##########
File path: server/src/main/java/com/cloud/server/ManagementServerImpl.java
##########
@@ -3938,7 +3938,12 @@ public void doInTransactionWithoutResult(final TransactionStatus status) {
}
// update password for this host
final DetailVO nv = _detailsDao.findDetail(h.getId(), ApiConstants.USERNAME);
- if (nv.getValue().equals(command.getUsername())) {
+ if (nv == null) {
+ final DetailVO nvu = new DetailVO(h.getId(), ApiConstants.USERNAME, command.getUsername());
Review comment:
> @ravening empty "" (or) " " username string ?
@sureshanaparti those are non null 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:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland removed a comment on pull request #4359: Failed to update host password if username/password is not saved in db
Posted by GitBox <gi...@apache.org>.
DaanHoogland removed a comment on pull request #4359:
URL: https://github.com/apache/cloudstack/pull/4359#issuecomment-707159514
@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:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on a change in pull request #4359: Failed to update host password if username/password is not saved in db
Posted by GitBox <gi...@apache.org>.
ravening commented on a change in pull request #4359:
URL: https://github.com/apache/cloudstack/pull/4359#discussion_r506616085
##########
File path: server/src/main/java/com/cloud/server/ManagementServerImpl.java
##########
@@ -3938,7 +3938,12 @@ public void doInTransactionWithoutResult(final TransactionStatus status) {
}
// update password for this host
final DetailVO nv = _detailsDao.findDetail(h.getId(), ApiConstants.USERNAME);
- if (nv.getValue().equals(command.getUsername())) {
+ if (nv == null) {
+ final DetailVO nvu = new DetailVO(h.getId(), ApiConstants.USERNAME, command.getUsername());
Review comment:
> can you share the entries in _host_details_ table of that test?
I think it's not going anywhere further. Can you let me know what exactly you are looking for?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on a change in pull request #4359: Failed to update host password if username/password is not saved in db
Posted by GitBox <gi...@apache.org>.
ravening commented on a change in pull request #4359:
URL: https://github.com/apache/cloudstack/pull/4359#discussion_r510143455
##########
File path: server/src/main/java/com/cloud/server/ManagementServerImpl.java
##########
@@ -3938,7 +3943,12 @@ public void doInTransactionWithoutResult(final TransactionStatus status) {
}
// update password for this host
final DetailVO nv = _detailsDao.findDetail(h.getId(), ApiConstants.USERNAME);
- if (nv.getValue().equals(command.getUsername())) {
+ if (nv == null) {
+ final DetailVO nvu = new DetailVO(h.getId(), ApiConstants.USERNAME, command.getUsername());
Review comment:
yes correct. probably i missed it. will change that
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on pull request #4359: Failed to update host password if username/password is not saved in db
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #4359:
URL: https://github.com/apache/cloudstack/pull/4359#issuecomment-712704717
> @sureshanaparti I need clear picture what you are looking for exactly. Once you clarify that I can look into it.
@ravening As per the PR changes, when there is no "username" detail in the _host_details_ table, the "username" and "password" are persisted from the updateHostPassword API input, which are not null, but can be empty. Persisting the password with username empty doesn't make sense. So, ensure that username is not empty, when persisting "username" and "password". Thanks.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan removed a comment on pull request #4359: Failed to update host password if username/password is not saved in db
Posted by GitBox <gi...@apache.org>.
blueorangutan removed a comment on pull request #4359:
URL: https://github.com/apache/cloudstack/pull/4359#issuecomment-707160355
@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:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4359: Failed to update host password if username/password is not saved in db
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4359:
URL: https://github.com/apache/cloudstack/pull/4359#discussion_r503918601
##########
File path: server/src/main/java/com/cloud/server/ManagementServerImpl.java
##########
@@ -3938,7 +3938,12 @@ public void doInTransactionWithoutResult(final TransactionStatus status) {
}
// update password for this host
final DetailVO nv = _detailsDao.findDetail(h.getId(), ApiConstants.USERNAME);
- if (nv.getValue().equals(command.getUsername())) {
+ if (nv == null) {
+ final DetailVO nvu = new DetailVO(h.getId(), ApiConstants.USERNAME, command.getUsername());
Review comment:
@ravening empty username string ?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on pull request #4359: Failed to update host password if username/password is not saved in db
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4359:
URL: https://github.com/apache/cloudstack/pull/4359#issuecomment-712713552
> > @sureshanaparti I need clear picture what you are looking for exactly. Once you clarify that I can look into it.
>
> @ravening As per the PR changes, when there is no "username" detail in the _host_details_ table, the "username" and "password" are persisted from the updateHostPassword API input, which are not null, but can be empty. Persisting the password with username empty doesn't make sense. So, ensure that username is not empty, when persisting "username" and "password". Thanks.
@sureshanaparti per my understanding, we should add a check before db transaction.
```
if (StringUtils.isBlank(cmd.getUserName()) {
throw InvalidParameterValueException("aaaaa");
}
```
is this what you mean ?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on pull request #4359: Failed to update host password if username/password is not saved in db
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4359:
URL: https://github.com/apache/cloudstack/pull/4359#issuecomment-702013195
@ravening this is a bug fix, which should be merged into 4.14 as well
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on pull request #4359: Failed to update host password if username/password is not saved in db
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #4359:
URL: https://github.com/apache/cloudstack/pull/4359#issuecomment-712715231
> > > @sureshanaparti I need clear picture what you are looking for exactly. Once you clarify that I can look into it.
> >
> >
> > @ravening As per the PR changes, when there is no "username" detail in the _host_details_ table, the "username" and "password" are persisted from the updateHostPassword API input, which are not null, but can be empty. Persisting the password with username empty doesn't make sense. So, ensure that username is not empty, when persisting "username" and "password". Thanks.
>
> @sureshanaparti per my understanding, we should add a check before db transaction.
>
> ```
> if (StringUtils.isBlank(cmd.getUserName()) {
> throw InvalidParameterValueException("aaaaa");
> }
> ```
>
> is this what you mean ?
@weizhouapache yes, you got it. thanks.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4359: Failed to update host password if username/password is not saved in db
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4359:
URL: https://github.com/apache/cloudstack/pull/4359#discussion_r506273334
##########
File path: server/src/main/java/com/cloud/server/ManagementServerImpl.java
##########
@@ -3938,7 +3938,12 @@ public void doInTransactionWithoutResult(final TransactionStatus status) {
}
// update password for this host
final DetailVO nv = _detailsDao.findDetail(h.getId(), ApiConstants.USERNAME);
- if (nv.getValue().equals(command.getUsername())) {
+ if (nv == null) {
+ final DetailVO nvu = new DetailVO(h.getId(), ApiConstants.USERNAME, command.getUsername());
Review comment:
can you share the entries in _host_details_ table of that 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:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4359: Failed to update host password if username/password is not saved in db
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4359:
URL: https://github.com/apache/cloudstack/pull/4359#issuecomment-707641742
Packaging result: ✔centos7 ✖centos8 ✔debian. JID-2167
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on pull request #4359: Failed to update host password if username/password is not saved in db
Posted by GitBox <gi...@apache.org>.
ravening commented on pull request #4359:
URL: https://github.com/apache/cloudstack/pull/4359#issuecomment-714313317
> > > > @sureshanaparti I need clear picture what you are looking for exactly. Once you clarify that I can look into it.
> > >
> > >
> > > @ravening As per the PR changes, when there is no "username" detail in the _host_details_ table, the "username" and "password" are persisted from the updateHostPassword API input, which are not null, but can be empty. Persisting the password with username empty doesn't make sense. So, ensure that username is not empty, when persisting "username" and "password". Thanks.
> >
> >
> > @sureshanaparti per my understanding, we should add a check before db transaction.
> > ```
> > if (StringUtils.isBlank(cmd.getUserName()) {
> > throw InvalidParameterValueException("aaaaa");
> > }
> > ```
> >
> >
> > is this what you mean ?
>
> @weizhouapache yes, you got it. thanks.
@sureshanaparti I have made the changes. Please review it again
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4359: Failed to update host password if username/password is not saved in db
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4359:
URL: https://github.com/apache/cloudstack/pull/4359#discussion_r506253106
##########
File path: server/src/main/java/com/cloud/server/ManagementServerImpl.java
##########
@@ -3938,7 +3938,12 @@ public void doInTransactionWithoutResult(final TransactionStatus status) {
}
// update password for this host
final DetailVO nv = _detailsDao.findDetail(h.getId(), ApiConstants.USERNAME);
- if (nv.getValue().equals(command.getUsername())) {
+ if (nv == null) {
+ final DetailVO nvu = new DetailVO(h.getId(), ApiConstants.USERNAME, command.getUsername());
Review comment:
@ravening can you test your changes when "_nv == null_" with above username?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4359: Failed to update host password if username/password is not saved in db
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4359:
URL: https://github.com/apache/cloudstack/pull/4359#discussion_r503423069
##########
File path: server/src/main/java/com/cloud/server/ManagementServerImpl.java
##########
@@ -3938,7 +3938,12 @@ public void doInTransactionWithoutResult(final TransactionStatus status) {
}
// update password for this host
final DetailVO nv = _detailsDao.findDetail(h.getId(), ApiConstants.USERNAME);
- if (nv.getValue().equals(command.getUsername())) {
+ if (nv == null) {
+ final DetailVO nvu = new DetailVO(h.getId(), ApiConstants.USERNAME, command.getUsername());
Review comment:
username from the command object can be null or empty ?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4359: Failed to update host password if username/password is not saved in db
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4359:
URL: https://github.com/apache/cloudstack/pull/4359#discussion_r506225465
##########
File path: server/src/main/java/com/cloud/server/ManagementServerImpl.java
##########
@@ -3938,7 +3938,12 @@ public void doInTransactionWithoutResult(final TransactionStatus status) {
}
// update password for this host
final DetailVO nv = _detailsDao.findDetail(h.getId(), ApiConstants.USERNAME);
- if (nv.getValue().equals(command.getUsername())) {
+ if (nv == null) {
+ final DetailVO nvu = new DetailVO(h.getId(), ApiConstants.USERNAME, command.getUsername());
Review comment:
@ravening if you pass username, then it shouldn't be empty, password can be empty. can you confirm if username is persisted with empty value in the _host_details_ table?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4359: Failed to update host password if username/password is not saved in db
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4359:
URL: https://github.com/apache/cloudstack/pull/4359#issuecomment-718004666
@DaanHoogland 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:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4359: Failed to update host password if username/password is not saved in db
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4359:
URL: https://github.com/apache/cloudstack/pull/4359#discussion_r513341028
##########
File path: server/src/main/java/com/cloud/server/ManagementServerImpl.java
##########
@@ -3994,7 +3999,12 @@ public void doInTransactionWithoutResult(final TransactionStatus status) {
}
// update password for this host
final DetailVO nv = _detailsDao.findDetail(host.getId(), ApiConstants.USERNAME);
- if (nv.getValue().equals(cmd.getUsername())) {
+ if (nv == null) {
+ final DetailVO nvu = new DetailVO(host.getId(), ApiConstants.USERNAME, cmd.getUsername());
Review comment:
@ravening Username null or empty fix is not addressed here. Please 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:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland merged pull request #4359: Failed to update host password if username/password is not saved in db
Posted by GitBox <gi...@apache.org>.
DaanHoogland merged pull request #4359:
URL: https://github.com/apache/cloudstack/pull/4359
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4359: Failed to update host password if username/password is not saved in db
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4359:
URL: https://github.com/apache/cloudstack/pull/4359#issuecomment-717927589
@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:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4359: Failed to update host password if username/password is not saved in db
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4359:
URL: https://github.com/apache/cloudstack/pull/4359#issuecomment-707627588
@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:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on a change in pull request #4359: Failed to update host password if username/password is not saved in db
Posted by GitBox <gi...@apache.org>.
ravening commented on a change in pull request #4359:
URL: https://github.com/apache/cloudstack/pull/4359#discussion_r513375557
##########
File path: server/src/main/java/com/cloud/server/ManagementServerImpl.java
##########
@@ -3994,7 +4004,12 @@ public void doInTransactionWithoutResult(final TransactionStatus status) {
}
// update password for this host
final DetailVO nv = _detailsDao.findDetail(host.getId(), ApiConstants.USERNAME);
- if (nv.getValue().equals(cmd.getUsername())) {
+ if (nv == null) {
+ final DetailVO nvu = new DetailVO(host.getId(), ApiConstants.USERNAME, cmd.getUsername());
Review comment:
@sureshanaparti fixed 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:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland removed a comment on pull request #4359: Failed to update host password if username/password is not saved in db
Posted by GitBox <gi...@apache.org>.
DaanHoogland removed a comment on pull request #4359:
URL: https://github.com/apache/cloudstack/pull/4359#issuecomment-707626409
@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:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on a change in pull request #4359: Failed to update host password if username/password is not saved in db
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on a change in pull request #4359:
URL: https://github.com/apache/cloudstack/pull/4359#discussion_r510111780
##########
File path: server/src/main/java/com/cloud/server/ManagementServerImpl.java
##########
@@ -3938,7 +3943,12 @@ public void doInTransactionWithoutResult(final TransactionStatus status) {
}
// update password for this host
final DetailVO nv = _detailsDao.findDetail(h.getId(), ApiConstants.USERNAME);
- if (nv.getValue().equals(command.getUsername())) {
+ if (nv == null) {
+ final DetailVO nvu = new DetailVO(h.getId(), ApiConstants.USERNAME, command.getUsername());
Review comment:
@ravening should we use 'userNameWithoutSpaces' instead of cmd.getUserName() ?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on a change in pull request #4359: Failed to update host password if username/password is not saved in db
Posted by GitBox <gi...@apache.org>.
ravening commented on a change in pull request #4359:
URL: https://github.com/apache/cloudstack/pull/4359#discussion_r506268448
##########
File path: server/src/main/java/com/cloud/server/ManagementServerImpl.java
##########
@@ -3938,7 +3938,12 @@ public void doInTransactionWithoutResult(final TransactionStatus status) {
}
// update password for this host
final DetailVO nv = _detailsDao.findDetail(h.getId(), ApiConstants.USERNAME);
- if (nv.getValue().equals(command.getUsername())) {
+ if (nv == null) {
+ final DetailVO nvu = new DetailVO(h.getId(), ApiConstants.USERNAME, command.getUsername());
Review comment:
@sureshanaparti
```
(local) 🐱 > update hostpassword hostid=f16d5515-8c53-4873-b395-dc439f5c67d6 password=cloudstack username=" "
{
"success": true
}
```
this is when nv is null
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4359: Failed to update host password if username/password is not saved in db
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4359:
URL: https://github.com/apache/cloudstack/pull/4359#discussion_r503918601
##########
File path: server/src/main/java/com/cloud/server/ManagementServerImpl.java
##########
@@ -3938,7 +3938,12 @@ public void doInTransactionWithoutResult(final TransactionStatus status) {
}
// update password for this host
final DetailVO nv = _detailsDao.findDetail(h.getId(), ApiConstants.USERNAME);
- if (nv.getValue().equals(command.getUsername())) {
+ if (nv == null) {
+ final DetailVO nvu = new DetailVO(h.getId(), ApiConstants.USERNAME, command.getUsername());
Review comment:
@ravening empty "" (or) " " username string ?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4359: Failed to update host password if username/password is not saved in db
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4359:
URL: https://github.com/apache/cloudstack/pull/4359#issuecomment-707159514
@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:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4359: Failed to update host password if username/password is not saved in db
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4359:
URL: https://github.com/apache/cloudstack/pull/4359#issuecomment-707160355
@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:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4359: Failed to update host password if username/password is not saved in db
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4359:
URL: https://github.com/apache/cloudstack/pull/4359#issuecomment-718003762
@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:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on a change in pull request #4359: Failed to update host password if username/password is not saved in db
Posted by GitBox <gi...@apache.org>.
ravening commented on a change in pull request #4359:
URL: https://github.com/apache/cloudstack/pull/4359#discussion_r506231678
##########
File path: server/src/main/java/com/cloud/server/ManagementServerImpl.java
##########
@@ -3938,7 +3938,12 @@ public void doInTransactionWithoutResult(final TransactionStatus status) {
}
// update password for this host
final DetailVO nv = _detailsDao.findDetail(h.getId(), ApiConstants.USERNAME);
- if (nv.getValue().equals(command.getUsername())) {
+ if (nv == null) {
+ final DetailVO nvu = new DetailVO(h.getId(), ApiConstants.USERNAME, command.getUsername());
Review comment:
@sureshanaparti
```
(local) 🐱 > update hostpassword hostid=50436ac8-b73a-4eaf-bbab-7adbd9a50870 password=cloudstack username=" "
🙈 Error: (HTTP 431, error code 4350) The username is not same for the hosts..
(local) 🐱 >
(local) 🐱 > update hostpassword hostid=50436ac8-b73a-4eaf-bbab-7adbd9a50870 password=cloudstack username=""
🙈 Error: (HTTP 431, error code 4350) The username is not same for the hosts..
(local) 🐱 >
```
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on a change in pull request #4359: Failed to update host password if username/password is not saved in db
Posted by GitBox <gi...@apache.org>.
ravening commented on a change in pull request #4359:
URL: https://github.com/apache/cloudstack/pull/4359#discussion_r506628121
##########
File path: server/src/main/java/com/cloud/server/ManagementServerImpl.java
##########
@@ -3938,7 +3938,12 @@ public void doInTransactionWithoutResult(final TransactionStatus status) {
}
// update password for this host
final DetailVO nv = _detailsDao.findDetail(h.getId(), ApiConstants.USERNAME);
- if (nv.getValue().equals(command.getUsername())) {
+ if (nv == null) {
+ final DetailVO nvu = new DetailVO(h.getId(), ApiConstants.USERNAME, command.getUsername());
Review comment:
> can you share the entries in _host_details_ table of that test?
@sureshanaparti the last result I pasted was tested when host_details entry were removed for that host
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4359: Failed to update host password if username/password is not saved in db
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4359:
URL: https://github.com/apache/cloudstack/pull/4359#issuecomment-708598653
<b>Trillian test result (tid-2942)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 30106 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4359-t2942-kvm-centos7.zip
Smoke tests completed. 83 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:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on a change in pull request #4359: Failed to update host password if username/password is not saved in db
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on a change in pull request #4359:
URL: https://github.com/apache/cloudstack/pull/4359#discussion_r508235074
##########
File path: server/src/main/java/com/cloud/server/ManagementServerImpl.java
##########
@@ -3938,7 +3938,12 @@ public void doInTransactionWithoutResult(final TransactionStatus status) {
}
// update password for this host
final DetailVO nv = _detailsDao.findDetail(h.getId(), ApiConstants.USERNAME);
- if (nv.getValue().equals(command.getUsername())) {
+ if (nv == null) {
+ final DetailVO nvu = new DetailVO(h.getId(), ApiConstants.USERNAME, command.getUsername());
Review comment:
@sureshanaparti @ravening we had the issue in the past that Removed kvm host is added back to cloudstack when cloudstack-agent is started. in that case, there is no username/password saved in host_details for the host (because the entries are removed from db when host is removed, but not added back).
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4359: Failed to update host password if username/password is not saved in db
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4359:
URL: https://github.com/apache/cloudstack/pull/4359#issuecomment-708302477
@rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4359: Failed to update host password if username/password is not saved in db
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4359:
URL: https://github.com/apache/cloudstack/pull/4359#issuecomment-707626994
@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:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on pull request #4359: Failed to update host password if username/password is not saved in db
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #4359:
URL: https://github.com/apache/cloudstack/pull/4359#issuecomment-710191041
> @sureshanaparti are your concerns addressed?
@DaanHoogland not yet, waiting for update from @ravening
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on a change in pull request #4359: Failed to update host password if username/password is not saved in db
Posted by GitBox <gi...@apache.org>.
ravening commented on a change in pull request #4359:
URL: https://github.com/apache/cloudstack/pull/4359#discussion_r506177567
##########
File path: server/src/main/java/com/cloud/server/ManagementServerImpl.java
##########
@@ -3938,7 +3938,12 @@ public void doInTransactionWithoutResult(final TransactionStatus status) {
}
// update password for this host
final DetailVO nv = _detailsDao.findDetail(h.getId(), ApiConstants.USERNAME);
- if (nv.getValue().equals(command.getUsername())) {
+ if (nv == null) {
+ final DetailVO nvu = new DetailVO(h.getId(), ApiConstants.USERNAME, command.getUsername());
Review comment:
> @ravening username is not null, but empty string, so can you add check for that ? Ex: "_StringUtils.isBlank(command.getUsername())_"
@sureshanaparti but why? For some hosts, username and password can be empty string
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on pull request #4359: Failed to update host password if username/password is not saved in db
Posted by GitBox <gi...@apache.org>.
ravening commented on pull request #4359:
URL: https://github.com/apache/cloudstack/pull/4359#issuecomment-710268005
@sureshanaparti I need clear picture what you are looking for exactly. Once you clarify that I can look into 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:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4359: Failed to update host password if username/password is not saved in db
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4359:
URL: https://github.com/apache/cloudstack/pull/4359#issuecomment-717927125
@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:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4359: Failed to update host password if username/password is not saved in db
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4359:
URL: https://github.com/apache/cloudstack/pull/4359#issuecomment-717974192
Packaging result: ✔centos7 ✖centos8 ✖debian. JID-2298
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on a change in pull request #4359: Failed to update host password if username/password is not saved in db
Posted by GitBox <gi...@apache.org>.
ravening commented on a change in pull request #4359:
URL: https://github.com/apache/cloudstack/pull/4359#discussion_r503750115
##########
File path: server/src/main/java/com/cloud/server/ManagementServerImpl.java
##########
@@ -3938,7 +3938,12 @@ public void doInTransactionWithoutResult(final TransactionStatus status) {
}
// update password for this host
final DetailVO nv = _detailsDao.findDetail(h.getId(), ApiConstants.USERNAME);
- if (nv.getValue().equals(command.getUsername())) {
+ if (nv == null) {
+ final DetailVO nvu = new DetailVO(h.getId(), ApiConstants.USERNAME, command.getUsername());
Review comment:
@sureshanaparti username and password are mandatory parameters.. They cant be null
```
(local) 🐱 > update hostpassword hostid=50c8-b73a-4eaf-bbab-7a70
💩 Missing required parameters: password, username
```
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #4359: Failed to update host password if username/password is not saved in db
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4359:
URL: https://github.com/apache/cloudstack/pull/4359#issuecomment-708301892
@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:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4359: Failed to update host password if username/password is not saved in db
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4359:
URL: https://github.com/apache/cloudstack/pull/4359#issuecomment-718277589
<b>Trillian test result (tid-3099)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 29814 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4359-t3099-kvm-centos7.zip
Smoke tests completed. 83 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:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4359: Failed to update host password if username/password is not saved in db
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4359:
URL: https://github.com/apache/cloudstack/pull/4359#discussion_r506253106
##########
File path: server/src/main/java/com/cloud/server/ManagementServerImpl.java
##########
@@ -3938,7 +3938,12 @@ public void doInTransactionWithoutResult(final TransactionStatus status) {
}
// update password for this host
final DetailVO nv = _detailsDao.findDetail(h.getId(), ApiConstants.USERNAME);
- if (nv.getValue().equals(command.getUsername())) {
+ if (nv == null) {
+ final DetailVO nvu = new DetailVO(h.getId(), ApiConstants.USERNAME, command.getUsername());
Review comment:
@ravening can you test your changes when "_nv == null_" with above username and share the persisted records in the _host_details_ table?
----------------------------------------------------------------
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:
users@infra.apache.org