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 2022/12/09 10:59:06 UTC

[GitHub] [cloudstack] shwstppr opened a new pull request, #6851: api,server: fix listing vm metrics for infra resources

shwstppr opened a new pull request, #6851:
URL: https://github.com/apache/cloudstack/pull/6851

   ### Description
   
   Fixes #6786
   
   listVirtualMachinesMetrics does not support some of the params that are supported by admin API call for listVirtualMachines. These parameters are used in UI.
   
   <!--- ********************************************************************************* -->
   <!--- NOTE: AUTOMATATION USES THE DESCRIPTIONS TO SET LABELS AND PRODUCE DOCUMENTATION. -->
   <!--- PLEASE PUT AN 'X' in only **ONE** box -->
   <!--- ********************************************************************************* -->
   
   ### Types of changes
   
   - [ ] Breaking change (fix or feature that would cause existing functionality to change)
   - [ ] New feature (non-breaking change which adds functionality)
   - [x] Bug fix (non-breaking change which fixes an issue)
   - [ ] Enhancement (improves an existing feature and functionality)
   - [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
   
   ### Feature/Enhancement Scale or Bug Severity
   
   #### Feature/Enhancement Scale
   
   - [ ] Major
   - [ ] Minor
   
   #### Bug Severity
   
   - [ ] BLOCKER
   - [ ] Critical
   - [ ] Major
   - [ ] Minor
   - [ ] Trivial
   
   
   ### Screenshots (if appropriate):
   
   
   ### How Has This Been Tested?
   <!-- Please describe in detail how you tested your changes. -->
   <!-- Include details of your testing environment, and the tests you ran to -->
   <!-- see how your change affects other areas of the code, etc. -->
   
   
   <!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/main/CONTRIBUTING.md) document -->
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] blueorangutan commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1345925362

   @rohityadavcloud a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] DaanHoogland commented on a diff in pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on code in PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#discussion_r1044518315


##########
server/src/main/java/com/cloud/api/query/QueryManagerImpl.java:
##########
@@ -1035,7 +1035,7 @@ private Pair<List<UserVmJoinVO>, Integer> searchForUserVMsInternal(ListVMsCmd cm
         Object backupOfferingId = cmd.getBackupOfferingId();
         Object isHaEnabled = cmd.getHaEnabled();
         Object pod = null;
-        Long clusterId = null;
+        Object clusterId = null;
         Object hostId = null;
         Object storageId = null;
         if (_accountMgr.isRootAdmin(caller.getId())) {

Review Comment:
   ok, you are right, thanks. so it needs to be checking for both possibilities.
   
   You actually justified using reflection now, I didn´t see that before.
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] rohityadavcloud commented on a diff in pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
rohityadavcloud commented on code in PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#discussion_r1042976289


##########
plugins/metrics/src/main/java/org/apache/cloudstack/api/ListVMsMetricsCmdByAdmin.java:
##########
@@ -0,0 +1,74 @@
+// 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 org.apache.cloudstack.api;
+
+import org.apache.cloudstack.acl.RoleType;
+import org.apache.cloudstack.api.command.admin.AdminCmd;
+import org.apache.cloudstack.api.response.ClusterResponse;
+import org.apache.cloudstack.api.response.HostResponse;
+import org.apache.cloudstack.api.response.PodResponse;
+import org.apache.cloudstack.api.response.StoragePoolResponse;
+import org.apache.cloudstack.response.VmMetricsResponse;
+
+@APICommand(name = ListVMsMetricsCmd.APINAME, description = "Lists VM metrics", responseObject = VmMetricsResponse.class,
+        requestHasSensitiveInfo = false, responseHasSensitiveInfo = false,  responseView = ResponseObject.ResponseView.Full,
+        since = "4.9.3", authorized = {RoleType.Admin})
+@Deprecated(since = "4.17.0")

Review Comment:
   ```suggestion
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] rohityadavcloud commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
rohityadavcloud commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1344157121

   @DaanHoogland can you address your comments in code, fix this and ping me. I appreciate if we can get this merge for 4.17.2; Vladi has already tested this.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] DaanHoogland commented on a diff in pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on code in PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#discussion_r1037269465


##########
plugins/metrics/src/main/java/org/apache/cloudstack/api/ListVMsMetricsCmdByAdmin.java:
##########
@@ -0,0 +1,74 @@
+// 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 org.apache.cloudstack.api;
+
+import org.apache.cloudstack.acl.RoleType;
+import org.apache.cloudstack.api.command.admin.AdminCmd;
+import org.apache.cloudstack.api.response.ClusterResponse;
+import org.apache.cloudstack.api.response.HostResponse;
+import org.apache.cloudstack.api.response.PodResponse;
+import org.apache.cloudstack.api.response.StoragePoolResponse;
+import org.apache.cloudstack.response.VmMetricsResponse;
+
+@APICommand(name = ListVMsMetricsCmd.APINAME, description = "Lists VM metrics", responseObject = VmMetricsResponse.class,
+        requestHasSensitiveInfo = false, responseHasSensitiveInfo = false,  responseView = ResponseObject.ResponseView.Full,
+        since = "4.9.3", authorized = {RoleType.Admin})
+@Deprecated(since = "4.17.0")

Review Comment:
   @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.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] shwstppr commented on a diff in pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
shwstppr commented on code in PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#discussion_r1005837097


##########
server/src/main/java/com/cloud/api/query/QueryManagerImpl.java:
##########
@@ -1035,16 +1045,14 @@ private Pair<List<UserVmJoinVO>, Integer> searchForUserVMsInternal(ListVMsCmd cm
         Object backupOfferingId = cmd.getBackupOfferingId();
         Object isHaEnabled = cmd.getHaEnabled();
         Object pod = null;
-        Long clusterId = null;
+        Object clusterId = null;
         Object hostId = null;
         Object storageId = null;
         if (_accountMgr.isRootAdmin(caller.getId())) {
-            if (cmd instanceof ListVMsCmdByAdmin) {

Review Comment:
   `ListVMsMetricsCmd` is accessible to all users therefore new `ByAdmin` class added and new params not added in the original



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] DaanHoogland commented on a diff in pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on code in PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#discussion_r1005840991


##########
plugins/metrics/src/main/java/org/apache/cloudstack/api/ListVMsMetricsCmdByAdmin.java:
##########
@@ -0,0 +1,74 @@
+// 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 org.apache.cloudstack.api;
+
+import org.apache.cloudstack.acl.RoleType;
+import org.apache.cloudstack.api.command.admin.AdminCmd;
+import org.apache.cloudstack.api.response.ClusterResponse;
+import org.apache.cloudstack.api.response.HostResponse;
+import org.apache.cloudstack.api.response.PodResponse;
+import org.apache.cloudstack.api.response.StoragePoolResponse;
+import org.apache.cloudstack.response.VmMetricsResponse;
+
+@APICommand(name = ListVMsMetricsCmd.APINAME, description = "Lists VM metrics", responseObject = VmMetricsResponse.class,
+        requestHasSensitiveInfo = false, responseHasSensitiveInfo = false,  responseView = ResponseObject.ResponseView.Full,
+        since = "4.9.3", authorized = {RoleType.Admin})
+@Deprecated(since = "4.17.0")

Review Comment:
   I don´t know why it has/What the thought behind it was. But we are obviously intending to maintain it so it is not deprecated. Let's remove 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.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] DaanHoogland commented on a diff in pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on code in PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#discussion_r1005795069


##########
plugins/metrics/src/main/java/org/apache/cloudstack/api/ListVMsMetricsCmdByAdmin.java:
##########
@@ -0,0 +1,74 @@
+// 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 org.apache.cloudstack.api;
+
+import org.apache.cloudstack.acl.RoleType;
+import org.apache.cloudstack.api.command.admin.AdminCmd;
+import org.apache.cloudstack.api.response.ClusterResponse;
+import org.apache.cloudstack.api.response.HostResponse;
+import org.apache.cloudstack.api.response.PodResponse;
+import org.apache.cloudstack.api.response.StoragePoolResponse;
+import org.apache.cloudstack.response.VmMetricsResponse;
+
+@APICommand(name = ListVMsMetricsCmd.APINAME, description = "Lists VM metrics", responseObject = VmMetricsResponse.class,
+        requestHasSensitiveInfo = false, responseHasSensitiveInfo = false,  responseView = ResponseObject.ResponseView.Full,
+        since = "4.9.3", authorized = {RoleType.Admin})

Review Comment:
   ```suggestion
           since = "4.18.0", authorized = {RoleType.Admin})
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] rohityadavcloud commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
rohityadavcloud commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1342271099

   @blueorangutan package


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] rohityadavcloud commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
rohityadavcloud commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1342270873

   @DaanHoogland can you review/lgtm? 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] rohityadavcloud commented on a diff in pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
rohityadavcloud commented on code in PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#discussion_r1044563146


##########
server/src/main/java/com/cloud/api/query/QueryManagerImpl.java:
##########
@@ -971,6 +972,17 @@ public ListResponse<UserVmResponse> searchForUserVMs(ListVMsCmd cmd) {
         return response;
     }
 
+    private Object getObjectPossibleMethodValue(Object obj, String methodName) {
+        Object result = null;
+
+        try {
+            Method m = obj.getClass().getMethod(methodName);
+            result = m.invoke(obj);
+        } catch (NoSuchMethodException | InvocationTargetException | IllegalAccessException ignored) {}

Review Comment:
   Perhaps log here? @DaanHoogland or do you think we may never hit this?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] rohityadavcloud commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
rohityadavcloud commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1345881857

   @blueorangutan package 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] DaanHoogland commented on a diff in pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on code in PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#discussion_r1005796709


##########
plugins/metrics/src/main/java/org/apache/cloudstack/api/ListVMsMetricsCmd.java:
##########
@@ -42,9 +44,9 @@
  */
 @APICommand(name = ListVMsMetricsCmd.APINAME, description = "Lists VM metrics", responseObject = VmMetricsResponse.class,
         requestHasSensitiveInfo = false, responseHasSensitiveInfo = false,  responseView = ResponseObject.ResponseView.Full,
-        since = "4.9.3", authorized = {RoleType.Admin,  RoleType.ResourceAdmin, RoleType.DomainAdmin, RoleType.User})
+        since = "4.9.3", authorized = {RoleType.ResourceAdmin, RoleType.DomainAdmin, RoleType.User})
 @Deprecated(since = "4.17.0")

Review Comment:
   what was the idea of deprecation? should we leave this annotation in?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] shwstppr commented on a diff in pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
shwstppr commented on code in PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#discussion_r1005840103


##########
plugins/metrics/src/main/java/org/apache/cloudstack/api/ListVMsMetricsCmdByAdmin.java:
##########
@@ -0,0 +1,74 @@
+// 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 org.apache.cloudstack.api;
+
+import org.apache.cloudstack.acl.RoleType;
+import org.apache.cloudstack.api.command.admin.AdminCmd;
+import org.apache.cloudstack.api.response.ClusterResponse;
+import org.apache.cloudstack.api.response.HostResponse;
+import org.apache.cloudstack.api.response.PodResponse;
+import org.apache.cloudstack.api.response.StoragePoolResponse;
+import org.apache.cloudstack.response.VmMetricsResponse;
+
+@APICommand(name = ListVMsMetricsCmd.APINAME, description = "Lists VM metrics", responseObject = VmMetricsResponse.class,
+        requestHasSensitiveInfo = false, responseHasSensitiveInfo = false,  responseView = ResponseObject.ResponseView.Full,
+        since = "4.9.3", authorized = {RoleType.Admin})

Review Comment:
   Not sure if it makes sense to add 4.18 here as it is not a new API just an additional Admin class for adding some params for admin account



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] sonarcloud[bot] commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1292028783

   SonarCloud Quality Gate failed.&nbsp; &nbsp; [![Quality Gate failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/failed-16px.png 'Quality Gate failed')](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=6851)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6851&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6851&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6851&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6851&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6851&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6851&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6851&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6851&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6851&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6851&resolved=false&types=CODE_SMELL) [![D](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/D-16px.png 'D')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6851&resolved=false&types=CODE_SMELL) [8 Code Smells](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6851&resolved=false&types=CODE_SMELL)
   
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/0-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6851&metric=new_coverage&view=list) [0.0% Coverage](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6851&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6851&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6851&metric=new_duplicated_lines_density&view=list)
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] codecov[bot] commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1292122712

   # [Codecov](https://codecov.io/gh/apache/cloudstack/pull/6851?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#6851](https://codecov.io/gh/apache/cloudstack/pull/6851?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (1d4be98) into [4.17](https://codecov.io/gh/apache/cloudstack/commit/64316d513c88c4d5ba65b43ed8c85e974a767d81?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (64316d5) will **increase** coverage by `0.00%`.
   > The diff coverage is `0.00%`.
   
   ```diff
   @@            Coverage Diff            @@
   ##               4.17    #6851   +/-   ##
   =========================================
     Coverage     10.34%   10.34%           
   - Complexity     6610     6613    +3     
   =========================================
     Files          2451     2452    +1     
     Lines        242339   242350   +11     
     Branches      37923    37923           
   =========================================
   + Hits          25058    25069   +11     
   + Misses       214195   214193    -2     
   - Partials       3086     3088    +2     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/cloudstack/pull/6851?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...a/org/apache/cloudstack/api/ListVMsMetricsCmd.java](https://codecov.io/gh/apache/cloudstack/pull/6851/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGx1Z2lucy9tZXRyaWNzL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9jbG91ZHN0YWNrL2FwaS9MaXN0Vk1zTWV0cmljc0NtZC5qYXZh) | `0.00% <0.00%> (ø)` | |
   | [...pache/cloudstack/api/ListVMsMetricsCmdByAdmin.java](https://codecov.io/gh/apache/cloudstack/pull/6851/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGx1Z2lucy9tZXRyaWNzL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9jbG91ZHN0YWNrL2FwaS9MaXN0Vk1zTWV0cmljc0NtZEJ5QWRtaW4uamF2YQ==) | `0.00% <0.00%> (ø)` | |
   | [.../apache/cloudstack/metrics/MetricsServiceImpl.java](https://codecov.io/gh/apache/cloudstack/pull/6851/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGx1Z2lucy9tZXRyaWNzL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9jbG91ZHN0YWNrL21ldHJpY3MvTWV0cmljc1NlcnZpY2VJbXBsLmphdmE=) | `14.94% <0.00%> (-0.04%)` | :arrow_down: |
   | [...ain/java/com/cloud/api/query/QueryManagerImpl.java](https://codecov.io/gh/apache/cloudstack/pull/6851/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2VydmVyL3NyYy9tYWluL2phdmEvY29tL2Nsb3VkL2FwaS9xdWVyeS9RdWVyeU1hbmFnZXJJbXBsLmphdmE=) | `2.98% <0.00%> (-0.01%)` | :arrow_down: |
   | [...n/java/com/cloud/template/TemplateManagerImpl.java](https://codecov.io/gh/apache/cloudstack/pull/6851/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2VydmVyL3NyYy9tYWluL2phdmEvY29tL2Nsb3VkL3RlbXBsYXRlL1RlbXBsYXRlTWFuYWdlckltcGwuamF2YQ==) | `11.02% <0.00%> (-0.02%)` | :arrow_down: |
   | [...om/cloud/deploy/DeploymentPlanningManagerImpl.java](https://codecov.io/gh/apache/cloudstack/pull/6851/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2VydmVyL3NyYy9tYWluL2phdmEvY29tL2Nsb3VkL2RlcGxveS9EZXBsb3ltZW50UGxhbm5pbmdNYW5hZ2VySW1wbC5qYXZh) | `9.41% <0.00%> (+0.78%)` | :arrow_up: |
   | [...apache/cloudstack/syslog/AlertsSyslogAppender.java](https://codecov.io/gh/apache/cloudstack/pull/6851/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGx1Z2lucy9hbGVydC1oYW5kbGVycy9zeXNsb2ctYWxlcnRzL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9jbG91ZHN0YWNrL3N5c2xvZy9BbGVydHNTeXNsb2dBcHBlbmRlci5qYXZh) | `58.75% <0.00%> (+2.25%)` | :arrow_up: |
   
   :mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] blueorangutan commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1344418850

   @DaanHoogland a Jenkins job has been kicked to build packages. It will be bundled with  KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] rohityadavcloud commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
rohityadavcloud commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1345925498

   Unmanage VM test issues fixed in another commit. Merging this but will monitor tests on 4.17 branch. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] blueorangutan commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1340878589

   @rohityadavcloud a Jenkins job has been kicked to build packages. It will be bundled with
   
    SystemVM template(s). I'll keep you posted as I make progress.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] rohityadavcloud commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
rohityadavcloud commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1340877937

   
   @blueorangutan package
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] blueorangutan commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1343078569

   <b>Trillian test result (tid-5428)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 39074 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6851-t5428-kvm-centos7.zip
   Smoke tests completed. 100 look OK, 1 have errors, 0 did not run
   Only failed and skipped tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_08_upgrade_kubernetes_ha_cluster | `Failure` | 540.77 | test_kubernetes_clusters.py
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] rohityadavcloud commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
rohityadavcloud commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1344268353

   Will merge this after this round of pkging+smoketests.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] blueorangutan commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1344317531

   Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 4831


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] rohityadavcloud commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
rohityadavcloud commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1344500375

   @blueorangutan test


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] rohityadavcloud commented on a diff in pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
rohityadavcloud commented on code in PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#discussion_r1044562876


##########
server/src/main/java/com/cloud/api/query/QueryManagerImpl.java:
##########
@@ -971,6 +972,17 @@ public ListResponse<UserVmResponse> searchForUserVMs(ListVMsCmd cmd) {
         return response;
     }
 
+    private Object getObjectPossibleMethodValue(Object obj, String methodName) {
+        Object result = null;

Review Comment:
   @DaanHoogland could you add a unit test to cover this method?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] DaanHoogland commented on a diff in pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on code in PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#discussion_r1045029697


##########
server/src/main/java/com/cloud/api/query/QueryManagerImpl.java:
##########
@@ -971,6 +972,17 @@ public ListResponse<UserVmResponse> searchForUserVMs(ListVMsCmd cmd) {
         return response;
     }
 
+    private Object getObjectPossibleMethodValue(Object obj, String methodName) {
+        Object result = null;
+
+        try {
+            Method m = obj.getClass().getMethod(methodName);
+            result = m.invoke(obj);
+        } catch (NoSuchMethodException | InvocationTargetException | IllegalAccessException ignored) {}

Review Comment:
   i was thinking that as well, but didn´t want to change to much anymore. We will hit this for sure in the current state if ListVMsCmd is called in the API. It will try to add the attributes that don´t exist and we are supposed to ignore those.
   ```suggestion
           } catch (NoSuchMethodException | InvocationTargetException | IllegalAccessException ignored) {/*ignored when non-ByAdmin are calling this.*/}
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] blueorangutan commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1329388695

   <b>Trillian Build Failed (tid-5270)<b/>


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] shwstppr commented on a diff in pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
shwstppr commented on code in PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#discussion_r1005838326


##########
plugins/metrics/src/main/java/org/apache/cloudstack/api/ListVMsMetricsCmdByAdmin.java:
##########
@@ -0,0 +1,74 @@
+// 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 org.apache.cloudstack.api;
+
+import org.apache.cloudstack.acl.RoleType;
+import org.apache.cloudstack.api.command.admin.AdminCmd;
+import org.apache.cloudstack.api.response.ClusterResponse;
+import org.apache.cloudstack.api.response.HostResponse;
+import org.apache.cloudstack.api.response.PodResponse;
+import org.apache.cloudstack.api.response.StoragePoolResponse;
+import org.apache.cloudstack.response.VmMetricsResponse;
+
+@APICommand(name = ListVMsMetricsCmd.APINAME, description = "Lists VM metrics", responseObject = VmMetricsResponse.class,
+        requestHasSensitiveInfo = false, responseHasSensitiveInfo = false,  responseView = ResponseObject.ResponseView.Full,
+        since = "4.9.3", authorized = {RoleType.Admin})
+@Deprecated(since = "4.17.0")

Review Comment:
   I can remove the deprecated tag but the original api class has this deprecated tag



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] blueorangutan commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1292098360

   Packaging result: :heavy_check_mark: el7 :heavy_multiplication_x: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 4554


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] DaanHoogland commented on a diff in pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on code in PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#discussion_r1005794508


##########
plugins/metrics/src/main/java/org/apache/cloudstack/api/ListVMsMetricsCmdByAdmin.java:
##########
@@ -0,0 +1,74 @@
+// 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 org.apache.cloudstack.api;
+
+import org.apache.cloudstack.acl.RoleType;
+import org.apache.cloudstack.api.command.admin.AdminCmd;
+import org.apache.cloudstack.api.response.ClusterResponse;
+import org.apache.cloudstack.api.response.HostResponse;
+import org.apache.cloudstack.api.response.PodResponse;
+import org.apache.cloudstack.api.response.StoragePoolResponse;
+import org.apache.cloudstack.response.VmMetricsResponse;
+
+@APICommand(name = ListVMsMetricsCmd.APINAME, description = "Lists VM metrics", responseObject = VmMetricsResponse.class,
+        requestHasSensitiveInfo = false, responseHasSensitiveInfo = false,  responseView = ResponseObject.ResponseView.Full,
+        since = "4.9.3", authorized = {RoleType.Admin})
+@Deprecated(since = "4.17.0")

Review Comment:
   why create a deprecated class?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] shwstppr commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
shwstppr commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1291952010

   @blueorangutan package


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] nvazquez commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
nvazquez commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1297553992

   @blueorangutan package


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] DaanHoogland commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1336915131

   > I think we should keep the since @DaanHoogland as it's not a new API (may affect data in apidocs), for the Deprecated, can you check [16f2896](https://github.com/apache/cloudstack/commit/16f2896940cb6ae5557a51b6b439bd71917e1781) where this was introduced? I think too Deprecated should be removed.
   
   The since attribute is not incorporated with the ApiDocs, see for instance https://cloudstack.apache.org/api/apidocs-4.17/apis/addNetscalerLoadBalancer.html . it would only help in trouble shooting which is important enough. I think adding a anti-dated ssince attribute is misleading and implies something that is not true. I really think it should be removed on this class or set to 4.18/4.19


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] blueorangutan commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1338192913

   @vladimirpetrov a Jenkins job has been kicked to build packages. It will be bundled with  KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] DaanHoogland commented on a diff in pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on code in PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#discussion_r1044194391


##########
plugins/metrics/src/main/java/org/apache/cloudstack/api/ListVMsMetricsCmdByAdmin.java:
##########
@@ -0,0 +1,74 @@
+// 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 org.apache.cloudstack.api;
+
+import org.apache.cloudstack.acl.RoleType;
+import org.apache.cloudstack.api.command.admin.AdminCmd;
+import org.apache.cloudstack.api.response.ClusterResponse;
+import org.apache.cloudstack.api.response.HostResponse;
+import org.apache.cloudstack.api.response.PodResponse;
+import org.apache.cloudstack.api.response.StoragePoolResponse;
+import org.apache.cloudstack.response.VmMetricsResponse;
+
+@APICommand(name = ListVMsMetricsCmd.APINAME, description = "Lists VM metrics", responseObject = VmMetricsResponse.class,
+        requestHasSensitiveInfo = false, responseHasSensitiveInfo = false,  responseView = ResponseObject.ResponseView.Full,
+        since = "4.9.3", authorized = {RoleType.Admin})

Review Comment:
   ```suggestion
           authorized = {RoleType.Admin})
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] rohityadavcloud commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
rohityadavcloud commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1341223303

   
   @blueorangutan package
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] blueorangutan commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1344500848

   @rohityadavcloud a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] blueorangutan commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1345299864

   @rohityadavcloud a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] blueorangutan commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1330016938

   <b>Trillian test result (tid-5269)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 39124 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6851-t5269-kvm-centos7.zip
   Smoke tests completed. 101 look OK, 0 have errors, 0 did not run
   Only failed and skipped tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] rohityadavcloud commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
rohityadavcloud commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1333668891

   cc @nvazquez 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] rohityadavcloud commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
rohityadavcloud commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1334963724

   I think we should keep the since @DaanHoogland as it's not a new API (may affect data in apidocs), for the Deprecated, can you check 16f2896940cb6ae5557a51b6b439bd71917e1781 where this was introduced? I think too Deprecated should be removed.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] blueorangutan commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1291550657

   @acs-robot a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] blueorangutan commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1344990168

   @rohityadavcloud a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] DaanHoogland commented on a diff in pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on code in PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#discussion_r1044196915


##########
plugins/metrics/src/main/java/org/apache/cloudstack/api/ListVMsMetricsCmdByAdmin.java:
##########
@@ -0,0 +1,73 @@
+// 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 org.apache.cloudstack.api;
+
+import org.apache.cloudstack.acl.RoleType;
+import org.apache.cloudstack.api.command.admin.AdminCmd;
+import org.apache.cloudstack.api.response.ClusterResponse;
+import org.apache.cloudstack.api.response.HostResponse;
+import org.apache.cloudstack.api.response.PodResponse;
+import org.apache.cloudstack.api.response.StoragePoolResponse;
+import org.apache.cloudstack.response.VmMetricsResponse;
+
+@APICommand(name = ListVMsMetricsCmd.APINAME, description = "Lists VM metrics", responseObject = VmMetricsResponse.class,
+        requestHasSensitiveInfo = false, responseHasSensitiveInfo = false,  responseView = ResponseObject.ResponseView.Full,
+        since = "4.9.3", authorized = {RoleType.Admin})
+public class ListVMsMetricsCmdByAdmin extends ListVMsMetricsCmd implements AdminCmd {
+
+    /////////////////////////////////////////////////////
+    //////////////// API parameters /////////////////////
+    /////////////////////////////////////////////////////
+
+    @Parameter(name=ApiConstants.HOST_ID, type= BaseCmd.CommandType.UUID, entityType= HostResponse.class,
+            description="the host ID")
+    private Long hostId;
+
+    @Parameter(name=ApiConstants.POD_ID, type= BaseCmd.CommandType.UUID, entityType= PodResponse.class,
+            description="the pod ID")
+    private Long podId;
+
+    @Parameter(name=ApiConstants.STORAGE_ID, type= BaseCmd.CommandType.UUID, entityType= StoragePoolResponse.class,
+            description="the storage ID where vm's volumes belong to")
+    private Long storageId;
+
+    @Parameter(name = ApiConstants.CLUSTER_ID, type = BaseCmd.CommandType.UUID, entityType = ClusterResponse.class,
+            description = "the cluster ID", since = "4.16.0")

Review Comment:
   ```suggestion
               description = "the cluster ID")
   ```
   
   we should not add an antidated parameter.



##########
server/src/main/java/com/cloud/api/query/QueryManagerImpl.java:
##########
@@ -971,6 +972,15 @@ public ListResponse<UserVmResponse> searchForUserVMs(ListVMsCmd cmd) {
         return response;
     }
 
+    private Object getObjectPossibleMethodValue(Object obj, String methodName) {
+        Object result = null;
+        try {
+            Method m = obj.getClass().getMethod(methodName);
+            result = m.invoke(obj);
+        } catch (NoSuchMethodException | InvocationTargetException | IllegalAccessException ignored) {}

Review Comment:
   not even logged?



##########
server/src/main/java/com/cloud/api/query/QueryManagerImpl.java:
##########
@@ -1035,16 +1045,14 @@ private Pair<List<UserVmJoinVO>, Integer> searchForUserVMsInternal(ListVMsCmd cm
         Object backupOfferingId = cmd.getBackupOfferingId();
         Object isHaEnabled = cmd.getHaEnabled();
         Object pod = null;
-        Long clusterId = null;
+        Object clusterId = null;
         Object hostId = null;
         Object storageId = null;
         if (_accountMgr.isRootAdmin(caller.getId())) {
-            if (cmd instanceof ListVMsCmdByAdmin) {

Review Comment:
   But these use the `if (cmd instaceof <...>ByAdmin) {}` pattern. Or we changing this? It is a good improvement in some ways but would justify its own PR.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] blueorangutan commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1341961770

   @rohityadavcloud a Jenkins job has been kicked to build packages. It will be bundled with  SystemVM template(s). I'll keep you posted as I make progress.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] rohityadavcloud commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
rohityadavcloud commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1341960708

   @blueorangutan package 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] rohityadavcloud commented on a diff in pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
rohityadavcloud commented on code in PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#discussion_r1043066177


##########
server/src/main/java/com/cloud/api/query/QueryManagerImpl.java:
##########
@@ -1035,16 +1045,14 @@ private Pair<List<UserVmJoinVO>, Integer> searchForUserVMsInternal(ListVMsCmd cm
         Object backupOfferingId = cmd.getBackupOfferingId();
         Object isHaEnabled = cmd.getHaEnabled();
         Object pod = null;
-        Long clusterId = null;
+        Object clusterId = null;
         Object hostId = null;
         Object storageId = null;
         if (_accountMgr.isRootAdmin(caller.getId())) {
-            if (cmd instanceof ListVMsCmdByAdmin) {

Review Comment:
   I think is this a common pattern (unless we want to deprecate this) used across 54 APIs:
   
   
   ```
   > find . | grep ByAdmin.java
   ./api/src/main/java/org/apache/cloudstack/api/command/admin/vmsnapshot/RevertToVMSnapshotCmdByAdmin.java
   ./api/src/main/java/org/apache/cloudstack/api/command/admin/iso/ListIsosCmdByAdmin.java
   ./api/src/main/java/org/apache/cloudstack/api/command/admin/iso/UpdateIsoCmdByAdmin.java
   ./api/src/main/java/org/apache/cloudstack/api/command/admin/iso/DetachIsoCmdByAdmin.java
   ./api/src/main/java/org/apache/cloudstack/api/command/admin/iso/ListIsoPermissionsCmdByAdmin.java
   ./api/src/main/java/org/apache/cloudstack/api/command/admin/iso/AttachIsoCmdByAdmin.java
   ./api/src/main/java/org/apache/cloudstack/api/command/admin/iso/CopyIsoCmdByAdmin.java
   ./api/src/main/java/org/apache/cloudstack/api/command/admin/iso/RegisterIsoCmdByAdmin.java
   ./api/src/main/java/org/apache/cloudstack/api/command/admin/zone/ListZonesCmdByAdmin.java
   ./api/src/main/java/org/apache/cloudstack/api/command/admin/network/ListNetworksCmdByAdmin.java
   ./api/src/main/java/org/apache/cloudstack/api/command/admin/network/UpdateNetworkCmdByAdmin.java
   ./api/src/main/java/org/apache/cloudstack/api/command/admin/network/CreateNetworkCmdByAdmin.java
   ./api/src/main/java/org/apache/cloudstack/api/command/admin/template/ListTemplatesCmdByAdmin.java
   ./api/src/main/java/org/apache/cloudstack/api/command/admin/template/CreateTemplateCmdByAdmin.java
   ./api/src/main/java/org/apache/cloudstack/api/command/admin/template/RegisterTemplateCmdByAdmin.java
   ./api/src/main/java/org/apache/cloudstack/api/command/admin/template/UpdateTemplateCmdByAdmin.java
   ./api/src/main/java/org/apache/cloudstack/api/command/admin/template/CopyTemplateCmdByAdmin.java
   ./api/src/main/java/org/apache/cloudstack/api/command/admin/template/ListTemplatePermissionsCmdByAdmin.java
   ./api/src/main/java/org/apache/cloudstack/api/command/admin/loadbalancer/ListLoadBalancerRuleInstancesCmdByAdmin.java
   ./api/src/main/java/org/apache/cloudstack/api/command/admin/vm/StopVMCmdByAdmin.java
   ./api/src/main/java/org/apache/cloudstack/api/command/admin/vm/DestroyVMCmdByAdmin.java
   ./api/src/main/java/org/apache/cloudstack/api/command/admin/vm/ScaleVMCmdByAdmin.java
   ./api/src/main/java/org/apache/cloudstack/api/command/admin/vm/ListVMsCmdByAdmin.java
   ./api/src/main/java/org/apache/cloudstack/api/command/admin/vm/DeployVMCmdByAdmin.java
   ./api/src/main/java/org/apache/cloudstack/api/command/admin/vm/StartVMCmdByAdmin.java
   ./api/src/main/java/org/apache/cloudstack/api/command/admin/vm/RebootVMCmdByAdmin.java
   ./api/src/main/java/org/apache/cloudstack/api/command/admin/vm/RemoveNicFromVMCmdByAdmin.java
   ./api/src/main/java/org/apache/cloudstack/api/command/admin/vm/UpdateDefaultNicForVMCmdByAdmin.java
   ./api/src/main/java/org/apache/cloudstack/api/command/admin/vm/UpgradeVMCmdByAdmin.java
   ./api/src/main/java/org/apache/cloudstack/api/command/admin/vm/ResetVMSSHKeyCmdByAdmin.java
   ./api/src/main/java/org/apache/cloudstack/api/command/admin/vm/UpdateVMCmdByAdmin.java
   ./api/src/main/java/org/apache/cloudstack/api/command/admin/vm/AddNicToVMCmdByAdmin.java
   ./api/src/main/java/org/apache/cloudstack/api/command/admin/vm/RestoreVMCmdByAdmin.java
   ./api/src/main/java/org/apache/cloudstack/api/command/admin/vm/ResetVMPasswordCmdByAdmin.java
   ./api/src/main/java/org/apache/cloudstack/api/command/admin/address/ListPublicIpAddressesCmdByAdmin.java
   ./api/src/main/java/org/apache/cloudstack/api/command/admin/address/AcquirePodIpCmdByAdmin.java
   ./api/src/main/java/org/apache/cloudstack/api/command/admin/address/ReleasePodIpCmdByAdmin.java
   ./api/src/main/java/org/apache/cloudstack/api/command/admin/address/AssociateIPAddrCmdByAdmin.java
   ./api/src/main/java/org/apache/cloudstack/api/command/admin/vpc/ListVPCsCmdByAdmin.java
   ./api/src/main/java/org/apache/cloudstack/api/command/admin/vpc/CreateVPCCmdByAdmin.java
   ./api/src/main/java/org/apache/cloudstack/api/command/admin/vpc/UpdateVPCCmdByAdmin.java
   ./api/src/main/java/org/apache/cloudstack/api/command/admin/affinitygroup/UpdateVMAffinityGroupCmdByAdmin.java
   ./api/src/main/java/org/apache/cloudstack/api/command/admin/account/ListAccountsCmdByAdmin.java
   ./api/src/main/java/org/apache/cloudstack/api/command/admin/domain/ListDomainsCmdByAdmin.java
   ./api/src/main/java/org/apache/cloudstack/api/command/admin/volume/ResizeVolumeCmdByAdmin.java
   ./api/src/main/java/org/apache/cloudstack/api/command/admin/volume/ListVolumesCmdByAdmin.java
   ./api/src/main/java/org/apache/cloudstack/api/command/admin/volume/DestroyVolumeCmdByAdmin.java
   ./api/src/main/java/org/apache/cloudstack/api/command/admin/volume/UpdateVolumeCmdByAdmin.java
   ./api/src/main/java/org/apache/cloudstack/api/command/admin/volume/CreateVolumeCmdByAdmin.java
   ./api/src/main/java/org/apache/cloudstack/api/command/admin/volume/AttachVolumeCmdByAdmin.java
   ./api/src/main/java/org/apache/cloudstack/api/command/admin/volume/RecoverVolumeCmdByAdmin.java
   ./api/src/main/java/org/apache/cloudstack/api/command/admin/volume/MigrateVolumeCmdByAdmin.java
   ./api/src/main/java/org/apache/cloudstack/api/command/admin/volume/UploadVolumeCmdByAdmin.java
   ./api/src/main/java/org/apache/cloudstack/api/command/admin/volume/DetachVolumeCmdByAdmin.java
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] blueorangutan commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1342271993

   @rohityadavcloud a Jenkins job has been kicked to build packages. It will be bundled with  KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] blueorangutan commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1341358113

   Packaging result: :heavy_multiplication_x: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 4806


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] DaanHoogland commented on a diff in pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on code in PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#discussion_r1005800493


##########
server/src/main/java/com/cloud/api/query/QueryManagerImpl.java:
##########
@@ -1035,16 +1045,14 @@ private Pair<List<UserVmJoinVO>, Integer> searchForUserVMsInternal(ListVMsCmd cm
         Object backupOfferingId = cmd.getBackupOfferingId();
         Object isHaEnabled = cmd.getHaEnabled();
         Object pod = null;
-        Long clusterId = null;
+        Object clusterId = null;
         Object hostId = null;
         Object storageId = null;
         if (_accountMgr.isRootAdmin(caller.getId())) {
-            if (cmd instanceof ListVMsCmdByAdmin) {

Review Comment:
   if we remove this, do we still need the extra class, or can we just implement the extra parameters in `ListVMsMetricsCmd`?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] blueorangutan commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1291952384

   @shwstppr a Jenkins job has been kicked to build packages. It will be bundled with  KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] blueorangutan commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1297554430

   @nvazquez a Jenkins job has been kicked to build packages. It will be bundled with  KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] blueorangutan commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1345460405

   <b>Trillian test result (tid-5458)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 44590 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6851-t5458-kvm-centos7.zip
   Smoke tests completed. 98 look OK, 3 have errors, 0 did not run
   Only failed and skipped tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_08_upgrade_kubernetes_ha_cluster | `Failure` | 607.57 | test_kubernetes_clusters.py
   test_01_unmanage_vm_cycle | `Error` | 33.93 | test_vm_lifecycle_unmanage_import.py
   ContextSuite context=TestUnmanageVM>:teardown | `Error` | 33.96 | test_vm_lifecycle_unmanage_import.py
   test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | `Failure` | 470.04 | test_vpc_redundant.py
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] blueorangutan commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1345923349

   Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 4844


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] rohityadavcloud commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
rohityadavcloud commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1344990014

   @blueorangutan test


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] DaanHoogland commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1344417415

   @blueorangutan package


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] DaanHoogland commented on a diff in pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on code in PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#discussion_r1037272474


##########
plugins/metrics/src/main/java/org/apache/cloudstack/api/ListVMsMetricsCmd.java:
##########
@@ -42,9 +44,9 @@
  */
 @APICommand(name = ListVMsMetricsCmd.APINAME, description = "Lists VM metrics", responseObject = VmMetricsResponse.class,
         requestHasSensitiveInfo = false, responseHasSensitiveInfo = false,  responseView = ResponseObject.ResponseView.Full,
-        since = "4.9.3", authorized = {RoleType.Admin,  RoleType.ResourceAdmin, RoleType.DomainAdmin, RoleType.User})
+        since = "4.9.3", authorized = {RoleType.ResourceAdmin, RoleType.DomainAdmin, RoleType.User})
 @Deprecated(since = "4.17.0")

Review Comment:
   let's remove the deprecation annotation



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] blueorangutan commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1342140184

   @rohityadavcloud a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] blueorangutan commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1344464563

   Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 4834


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] shwstppr commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
shwstppr commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1344144770

   @DaanHoogland I've closed this PR as I don't think I have the complete understanding to produce a proper fix. Can you please look into the issue for the fix and comment if it is critical for 4.17.2 milestone


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] rohityadavcloud commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
rohityadavcloud commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1344263888

   @blueorangutan package


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] rohityadavcloud commented on a diff in pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
rohityadavcloud commented on code in PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#discussion_r1036953610


##########
plugins/metrics/src/main/java/org/apache/cloudstack/api/ListVMsMetricsCmdByAdmin.java:
##########
@@ -0,0 +1,74 @@
+// 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 org.apache.cloudstack.api;
+
+import org.apache.cloudstack.acl.RoleType;
+import org.apache.cloudstack.api.command.admin.AdminCmd;
+import org.apache.cloudstack.api.response.ClusterResponse;
+import org.apache.cloudstack.api.response.HostResponse;
+import org.apache.cloudstack.api.response.PodResponse;
+import org.apache.cloudstack.api.response.StoragePoolResponse;
+import org.apache.cloudstack.response.VmMetricsResponse;
+
+@APICommand(name = ListVMsMetricsCmd.APINAME, description = "Lists VM metrics", responseObject = VmMetricsResponse.class,
+        requestHasSensitiveInfo = false, responseHasSensitiveInfo = false,  responseView = ResponseObject.ResponseView.Full,
+        since = "4.9.3", authorized = {RoleType.Admin})

Review Comment:
   it might cause issue with apidocs generation - to be clear this isn't adding a new API; but an admin cmd (api with admin specific params)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] vladimirpetrov commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
vladimirpetrov commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1338191759

   @blueorangutan package


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] shwstppr closed pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
shwstppr closed pull request #6851: api,server: fix listing vm metrics for infra resources
URL: https://github.com/apache/cloudstack/pull/6851


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] DaanHoogland commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1344160805

   > @DaanHoogland I've closed this PR as I don't think I have the complete understanding to produce a proper fix. Can you please look into the issue for the fix and comment if it is critical for 4.17.2 milestone
   
   your fix is mostly good @shwstppr , I only ask to not introduce the reflection pattern for this change (and some minor things). I'll adress the changes.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] blueorangutan commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1344264356

   @rohityadavcloud a Jenkins job has been kicked to build packages. It will be bundled with  KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] rohityadavcloud commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
rohityadavcloud commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1326195017

   @blueorangutan package


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] blueorangutan commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1291554562

   UI build: :heavy_check_mark:
   Live QA URL: http://qa.cloudstack.cloud:8080/client/pr/6851 (SL-JID-2568)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] acs-robot commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
acs-robot commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1291550201

   Found UI changes, kicking a new UI QA build
   @blueorangutan ui


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] blueorangutan commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1340990708

   Packaging result: :heavy_multiplication_x: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 4798


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] blueorangutan commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1341224844

   @rohityadavcloud a Jenkins job has been kicked to build packages. It will be bundled with
   
    SystemVM template(s). I'll keep you posted as I make progress.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] blueorangutan commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1343019811

   @rohityadavcloud a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] rohityadavcloud commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
rohityadavcloud commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1343019069

   
   @blueorangutan test
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] blueorangutan commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1342005397

   Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 4812


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] rohityadavcloud commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
rohityadavcloud commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1345924676

   @blueorangutan test 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] DaanHoogland commented on a diff in pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on code in PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#discussion_r1010161974


##########
plugins/metrics/src/main/java/org/apache/cloudstack/api/ListVMsMetricsCmdByAdmin.java:
##########
@@ -0,0 +1,74 @@
+// 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 org.apache.cloudstack.api;
+
+import org.apache.cloudstack.acl.RoleType;
+import org.apache.cloudstack.api.command.admin.AdminCmd;
+import org.apache.cloudstack.api.response.ClusterResponse;
+import org.apache.cloudstack.api.response.HostResponse;
+import org.apache.cloudstack.api.response.PodResponse;
+import org.apache.cloudstack.api.response.StoragePoolResponse;
+import org.apache.cloudstack.response.VmMetricsResponse;
+
+@APICommand(name = ListVMsMetricsCmd.APINAME, description = "Lists VM metrics", responseObject = VmMetricsResponse.class,
+        requestHasSensitiveInfo = false, responseHasSensitiveInfo = false,  responseView = ResponseObject.ResponseView.Full,
+        since = "4.9.3", authorized = {RoleType.Admin})

Review Comment:
   I think we better remove the since attribute completely. I don't think it add value either way.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] blueorangutan commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1329339551

   @DaanHoogland 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.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] blueorangutan commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1326196115

   @rohityadavcloud a Jenkins job has been kicked to build packages. It will be bundled with  KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] rohityadavcloud closed pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
rohityadavcloud closed pull request #6851: api,server: fix listing vm metrics for infra resources
URL: https://github.com/apache/cloudstack/pull/6851


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] vladimirpetrov commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
vladimirpetrov commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1339425057

   Now we have exactly the opposite behavior - KVM VM deployment works but VMWare fails:
   
   ```
   2022-12-06 13:31:37,168 INFO  [c.c.a.ApiServer] (qtp1647766367-15:ctx-e93e5c38 ctx-ca4808ff) (logid:3c497499) Hypervisor passed to the deployVm call, is different from the hypervisor type of the template
   ```
   
   And it works when using cmk.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] sonarcloud[bot] commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1291589303

   SonarCloud Quality Gate failed.&nbsp; &nbsp; [![Quality Gate failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/failed-16px.png 'Quality Gate failed')](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=6851)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6851&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6851&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6851&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6851&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6851&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6851&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6851&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6851&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6851&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6851&resolved=false&types=CODE_SMELL) [![D](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/D-16px.png 'D')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6851&resolved=false&types=CODE_SMELL) [8 Code Smells](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6851&resolved=false&types=CODE_SMELL)
   
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/0-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6851&metric=new_coverage&view=list) [0.0% Coverage](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6851&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6851&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6851&metric=new_duplicated_lines_density&view=list)
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] blueorangutan commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1291958533

   @acs-robot a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] acs-robot commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
acs-robot commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1291957753

   Found UI changes, kicking a new UI QA build
   @blueorangutan ui


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] blueorangutan commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1343935113

   <b>Trillian test result (tid-5436)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 49521 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6851-t5436-kvm-centos7.zip
   Smoke tests completed. 99 look OK, 2 have errors, 0 did not run
   Only failed and skipped tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_02_upgrade_kubernetes_cluster | `Failure` | 507.61 | test_kubernetes_clusters.py
   test_08_upgrade_kubernetes_ha_cluster | `Failure` | 573.26 | test_kubernetes_clusters.py
   test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | `Failure` | 652.28 | test_vpc_redundant.py
   test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | `Failure` | 461.84 | test_vpc_redundant.py
   test_05_rvpc_multi_tiers | `Failure` | 600.70 | test_vpc_redundant.py
   test_05_rvpc_multi_tiers | `Error` | 600.72 | test_vpc_redundant.py
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] shwstppr commented on a diff in pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
shwstppr commented on code in PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#discussion_r1044503899


##########
server/src/main/java/com/cloud/api/query/QueryManagerImpl.java:
##########
@@ -1035,7 +1035,7 @@ private Pair<List<UserVmJoinVO>, Integer> searchForUserVMsInternal(ListVMsCmd cm
         Object backupOfferingId = cmd.getBackupOfferingId();
         Object isHaEnabled = cmd.getHaEnabled();
         Object pod = null;
-        Long clusterId = null;
+        Object clusterId = null;
         Object hostId = null;
         Object storageId = null;
         if (_accountMgr.isRootAdmin(caller.getId())) {

Review Comment:
   @DaanHoogland I am afraid the below code may not work as cmd will be an instance of `ListVMsMetricsCmdByAdmin` and not `ListVMsCmdByAdmin` 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] DaanHoogland commented on a diff in pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on code in PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#discussion_r1044518315


##########
server/src/main/java/com/cloud/api/query/QueryManagerImpl.java:
##########
@@ -1035,7 +1035,7 @@ private Pair<List<UserVmJoinVO>, Integer> searchForUserVMsInternal(ListVMsCmd cm
         Object backupOfferingId = cmd.getBackupOfferingId();
         Object isHaEnabled = cmd.getHaEnabled();
         Object pod = null;
-        Long clusterId = null;
+        Object clusterId = null;
         Object hostId = null;
         Object storageId = null;
         if (_accountMgr.isRootAdmin(caller.getId())) {

Review Comment:
   ok, you are right, 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.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] blueorangutan commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1344539152

   <b>Trillian Build Failed (tid-5447)<b/>


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] blueorangutan commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1345283531

   <b>Trillian test result (tid-5455)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 42629 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6851-t5455-kvm-centos7.zip
   Smoke tests completed. 99 look OK, 2 have errors, 0 did not run
   Only failed and skipped tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_08_upgrade_kubernetes_ha_cluster | `Failure` | 3634.02 | test_kubernetes_clusters.py
   test_09_delete_kubernetes_ha_cluster | `Failure` | 0.05 | test_kubernetes_clusters.py
   ContextSuite context=TestKubernetesCluster>:teardown | `Error` | 67.40 | test_kubernetes_clusters.py
   test_01_unmanage_vm_cycle | `Error` | 33.14 | test_vm_lifecycle_unmanage_import.py
   ContextSuite context=TestUnmanageVM>:teardown | `Error` | 33.17 | test_vm_lifecycle_unmanage_import.py
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] rohityadavcloud commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
rohityadavcloud commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1345299647

   @blueorangutan test


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] rohityadavcloud merged pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
rohityadavcloud merged PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] blueorangutan commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1346832813

   <b>Trillian test result (tid-5459)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 36996 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6851-t5459-kvm-centos7.zip
   Smoke tests completed. 101 look OK, 0 have errors, 0 did not run
   Only failed and skipped tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] DaanHoogland commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1344417043

   I re-applied your reflection bit. I created an issue to remove the `instanceof` operator in the similar cases.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] shwstppr commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
shwstppr commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1340534008

   > Now we have exactly the opposite behavior - KVM VM deployment works but VMWare fails:
   > 
   > ```
   > 2022-12-06 13:31:37,168 INFO  [c.c.a.ApiServer] (qtp1647766367-15:ctx-e93e5c38 ctx-ca4808ff) (logid:3c497499) Hypervisor passed to the deployVm call, is different from the hypervisor type of the template
   > ```
   > 
   > And it works when using cmk.
   
   It seems I've tagged the wrong issue to this PR. I've fixed that in desc now. Will check #6756 for a fix


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] rohityadavcloud commented on a diff in pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
rohityadavcloud commented on code in PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#discussion_r1042976077


##########
plugins/metrics/src/main/java/org/apache/cloudstack/api/ListVMsMetricsCmd.java:
##########
@@ -42,9 +44,9 @@
  */
 @APICommand(name = ListVMsMetricsCmd.APINAME, description = "Lists VM metrics", responseObject = VmMetricsResponse.class,
         requestHasSensitiveInfo = false, responseHasSensitiveInfo = false,  responseView = ResponseObject.ResponseView.Full,
-        since = "4.9.3", authorized = {RoleType.Admin,  RoleType.ResourceAdmin, RoleType.DomainAdmin, RoleType.User})
+        since = "4.9.3", authorized = {RoleType.ResourceAdmin, RoleType.DomainAdmin, RoleType.User})
 @Deprecated(since = "4.17.0")

Review Comment:
   ```suggestion
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] rohityadavcloud commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
rohityadavcloud commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1342139209

   @blueorangutan test


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] blueorangutan commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1345882953

   @rohityadavcloud a Jenkins job has been kicked to build packages. It will be bundled with  SystemVM template(s). I'll keep you posted as I make progress.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] blueorangutan commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1342325099

   Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 4813


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] DaanHoogland commented on a diff in pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on code in PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#discussion_r1044518315


##########
server/src/main/java/com/cloud/api/query/QueryManagerImpl.java:
##########
@@ -1035,7 +1035,7 @@ private Pair<List<UserVmJoinVO>, Integer> searchForUserVMsInternal(ListVMsCmd cm
         Object backupOfferingId = cmd.getBackupOfferingId();
         Object isHaEnabled = cmd.getHaEnabled();
         Object pod = null;
-        Long clusterId = null;
+        Object clusterId = null;
         Object hostId = null;
         Object storageId = null;
         if (_accountMgr.isRootAdmin(caller.getId())) {

Review Comment:
   ok, you are right, thanks. so it needs to be checking for both possibilities.
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] shwstppr closed pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
shwstppr closed pull request #6851: api,server: fix listing vm metrics for infra resources
URL: https://github.com/apache/cloudstack/pull/6851


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] DaanHoogland commented on a diff in pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on code in PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#discussion_r1005839394


##########
server/src/main/java/com/cloud/api/query/QueryManagerImpl.java:
##########
@@ -1035,16 +1045,14 @@ private Pair<List<UserVmJoinVO>, Integer> searchForUserVMsInternal(ListVMsCmd cm
         Object backupOfferingId = cmd.getBackupOfferingId();
         Object isHaEnabled = cmd.getHaEnabled();
         Object pod = null;
-        Long clusterId = null;
+        Object clusterId = null;
         Object hostId = null;
         Object storageId = null;
         if (_accountMgr.isRootAdmin(caller.getId())) {
-            if (cmd instanceof ListVMsCmdByAdmin) {

Review Comment:
   does that matter? they will just not be able to use the extra parameters.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] blueorangutan commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1291982395

   UI build: :heavy_check_mark:
   Live QA URL: http://qa.cloudstack.cloud:8080/client/pr/6851 (SL-JID-2573)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] DaanHoogland commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1329338903

   @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.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] blueorangutan commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1330031817

   <b>Trillian test result (tid-5268)</b>
   Environment: xenserver-71 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 40864 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6851-t5268-xenserver-71.zip
   Smoke tests completed. 100 look OK, 1 have errors, 0 did not run
   Only failed and skipped tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_08_upgrade_kubernetes_ha_cluster | `Failure` | 633.16 | test_kubernetes_clusters.py
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] DaanHoogland commented on a diff in pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on code in PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#discussion_r1037271018


##########
plugins/metrics/src/main/java/org/apache/cloudstack/api/ListVMsMetricsCmdByAdmin.java:
##########
@@ -0,0 +1,74 @@
+// 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 org.apache.cloudstack.api;
+
+import org.apache.cloudstack.acl.RoleType;
+import org.apache.cloudstack.api.command.admin.AdminCmd;
+import org.apache.cloudstack.api.response.ClusterResponse;
+import org.apache.cloudstack.api.response.HostResponse;
+import org.apache.cloudstack.api.response.PodResponse;
+import org.apache.cloudstack.api.response.StoragePoolResponse;
+import org.apache.cloudstack.response.VmMetricsResponse;
+
+@APICommand(name = ListVMsMetricsCmd.APINAME, description = "Lists VM metrics", responseObject = VmMetricsResponse.class,
+        requestHasSensitiveInfo = false, responseHasSensitiveInfo = false,  responseView = ResponseObject.ResponseView.Full,
+        since = "4.9.3", authorized = {RoleType.Admin})

Review Comment:
   it shouldn´t, there are others without this annotation parameter are there?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] blueorangutan commented on pull request #6851: api,server: fix listing vm metrics for infra resources

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6851:
URL: https://github.com/apache/cloudstack/pull/6851#issuecomment-1326249824

   Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 4638


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org