You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by "harikrishna-patnala (via GitHub)" <gi...@apache.org> on 2023/10/17 05:21:37 UTC

[PR] Handle errors while scaling kubernetes cluster [cloudstack]

harikrishna-patnala opened a new pull request, #8107:
URL: https://github.com/apache/cloudstack/pull/8107

   ### Description
   
   This PR fixes the issue #7920 
   
   <!--- Describe your changes in DETAIL - And how has behaviour functionally changed. -->
   
   <!-- For new features, provide link to FS, dev ML discussion etc. -->
   <!-- In case of bug fix, the expected and actual behaviours, steps to reproduce. -->
   
   <!-- When "Fixes: #<id>" is specified, the issue/PR will automatically be closed when this PR gets merged -->
   <!-- For addressing multiple issues/PRs, use multiple "Fixes: #<id>" -->
   <!-- Fixes: # -->
   
   <!--- ********************************************************************************* -->
   <!--- NOTE: AUTOMATATION USES THE DESCRIPTIONS TO SET LABELS AND PRODUCE DOCUMENTATION. -->
   <!--- PLEASE PUT AN 'X' in only **ONE** box -->
   <!--- ********************************************************************************* -->
   
   ### Types of changes
   
   - [ ] Breaking change (fix or feature that would cause existing functionality to change)
   - [ ] New feature (non-breaking change which adds functionality)
   - [ ] 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)
   - [ ] build/CI
   
   
   #### Bug Severity
   
   - [ ] BLOCKER
   - [ ] Critical
   - [ ] Major
   - [ ] Minor
   - [ ] Trivial
   
   
   
   ### 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 -->
   
   
   


-- 
This is an automated message from the 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


Re: [PR] Handle errors while scaling kubernetes cluster [cloudstack]

Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on code in PR #8107:
URL: https://github.com/apache/cloudstack/pull/8107#discussion_r1372613484


##########
plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterScaleWorker.java:
##########
@@ -299,8 +298,8 @@ private void scaleKubernetesClusterOffering() throws CloudRuntimeException {
             boolean result = false;
             try {
                 result = userVmManager.upgradeVirtualMachine(userVM.getId(), serviceOffering.getId(), new HashMap<String, String>());
-            } catch (ResourceUnavailableException | ManagementServerException | ConcurrentOperationException | VirtualMachineMigrationException e) {
-                logTransitStateAndThrow(Level.ERROR, String.format("Scaling Kubernetes cluster : %s failed, unable to scale cluster VM : %s", kubernetesCluster.getName(), userVM.getDisplayName()), kubernetesCluster.getId(), KubernetesCluster.Event.OperationFailed, e);
+            } catch (CloudRuntimeException | ResourceUnavailableException | ManagementServerException | VirtualMachineMigrationException e) {

Review Comment:
   ConcurrentOperationException is extending CloudRuntimeException, so its not required or it is not allowed to have both.
   
   `public class ConcurrentOperationException extends CloudRuntimeException {`



-- 
This is an automated message from the 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


Re: [PR] Handle errors while scaling kubernetes cluster [cloudstack]

Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on code in PR #8107:
URL: https://github.com/apache/cloudstack/pull/8107#discussion_r1365376966


##########
plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterScaleWorker.java:
##########
@@ -299,8 +298,8 @@ private void scaleKubernetesClusterOffering() throws CloudRuntimeException {
             boolean result = false;
             try {
                 result = userVmManager.upgradeVirtualMachine(userVM.getId(), serviceOffering.getId(), new HashMap<String, String>());
-            } catch (ResourceUnavailableException | ManagementServerException | ConcurrentOperationException | VirtualMachineMigrationException e) {
-                logTransitStateAndThrow(Level.ERROR, String.format("Scaling Kubernetes cluster : %s failed, unable to scale cluster VM : %s", kubernetesCluster.getName(), userVM.getDisplayName()), kubernetesCluster.getId(), KubernetesCluster.Event.OperationFailed, e);
+            } catch (CloudRuntimeException | ResourceUnavailableException | ManagementServerException | VirtualMachineMigrationException e) {

Review Comment:
   is `ConcurrentOperationException` no longer an issue?



-- 
This is an automated message from the 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


Re: [PR] Handle errors while scaling kubernetes cluster [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8107:
URL: https://github.com/apache/cloudstack/pull/8107#issuecomment-1846108568

   <b>[SF] Trillian test result (tid-8499)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 40740 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8107-t8499-kvm-centos7.zip
   Smoke tests completed. 109 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


Re: [PR] Handle errors while scaling kubernetes cluster [cloudstack]

Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on PR #8107:
URL: https://github.com/apache/cloudstack/pull/8107#issuecomment-1844844728

   This is ready for review cc @kiranchavala @shwstppr @DaanHoogland 


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

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

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


Re: [PR] Handle errors while scaling kubernetes cluster [cloudstack]

Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on PR #8107:
URL: https://github.com/apache/cloudstack/pull/8107#issuecomment-1844667583

   I'm seeing some other cases where the cluster is stuck in "Scaling" state. Trying to fix them as well.


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

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

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


Re: [PR] Handle errors while scaling kubernetes cluster [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8107:
URL: https://github.com/apache/cloudstack/pull/8107#issuecomment-1844973509

   @DaanHoogland a [SL] 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


Re: [PR] Handle errors while scaling kubernetes cluster [cloudstack]

Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on code in PR #8107:
URL: https://github.com/apache/cloudstack/pull/8107#discussion_r1420209019


##########
plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterResourceModifierActionWorker.java:
##########
@@ -788,6 +788,9 @@ public KubernetesClusterVO doInTransaction(TransactionStatus status) {
                 if (autoscaleEnabled != null) {
                     updatedCluster.setAutoscalingEnabled(autoscaleEnabled.booleanValue());
                 }
+                if (state != null) {

Review Comment:
   yes, this is not required, kind of duplicate update code since we have fixing that in update query. Reverted this change.



-- 
This is an automated message from the 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


Re: [PR] Handle errors while scaling kubernetes cluster [cloudstack]

Posted by "shwstppr (via GitHub)" <gi...@apache.org>.
shwstppr commented on code in PR #8107:
URL: https://github.com/apache/cloudstack/pull/8107#discussion_r1418642201


##########
plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterResourceModifierActionWorker.java:
##########
@@ -788,6 +788,9 @@ public KubernetesClusterVO doInTransaction(TransactionStatus status) {
                 if (autoscaleEnabled != null) {
                     updatedCluster.setAutoscalingEnabled(autoscaleEnabled.booleanValue());
                 }
+                if (state != null) {

Review Comment:
   @harikrishna-patnala curious, why do we need to update the state manually? I feel that should be updated by FSM.
   Also, I don't see a real use where code is explicitly updating the state with this.



##########
plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterScaleWorker.java:
##########
@@ -412,7 +411,7 @@ private void scaleKubernetesClusterSize() throws CloudRuntimeException {
         } else { // upscale, same node count handled above
             scaleUpKubernetesClusterSize(newVmRequiredCount);
         }
-        kubernetesCluster = updateKubernetesClusterEntry(clusterSize, null);
+        kubernetesCluster = updateKubernetesClusterEntry(clusterSize, serviceOffering);

Review Comment:
   Will this have any effect in some cases?
   What happens if scale is called for both size and offering, the new size is lesser and for some reason offering scaling fails? Will it already update the cluster's CPU and memory based on the new offering?



-- 
This is an automated message from the 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


Re: [PR] Handle errors while scaling kubernetes cluster [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8107:
URL: https://github.com/apache/cloudstack/pull/8107#issuecomment-1846906419

   @harikrishna-patnala a [SL] 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


Re: [PR] Handle errors while scaling kubernetes cluster [cloudstack]

Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on code in PR #8107:
URL: https://github.com/apache/cloudstack/pull/8107#discussion_r1418519143


##########
plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterResourceModifierActionWorker.java:
##########
@@ -768,11 +768,11 @@ protected String getKubernetesClusterNodeNamePrefix() {
     }
 
     protected KubernetesClusterVO updateKubernetesClusterEntry(final Long cores, final Long memory,
-        final Long size, final Long serviceOfferingId, final Boolean autoscaleEnabled, final Long minSize, final Long maxSize) {
+                                                               final Long size, final Long serviceOfferingId, final Boolean autoscaleEnabled, final Long minSize, final Long maxSize, KubernetesCluster.State state) {
         return Transaction.execute(new TransactionCallback<KubernetesClusterVO>() {
                 @Override
                 public KubernetesClusterVO doInTransaction(TransactionStatus status) {
-                KubernetesClusterVO updatedCluster = kubernetesClusterDao.createForUpdate(kubernetesCluster.getId());
+                KubernetesClusterVO updatedCluster = kubernetesClusterDao.findById(kubernetesCluster.getId());

Review Comment:
   This is a bigger change because, I observed createForUpdate() is returning an object with all null or default values and assumption is that we have to update that entry for all the columns.
   
   This is causing few issues with scaling operations
   1. When I tried to change the node count and change the compute offering at the same time. Compute offering change on few nodes is missing. This happened because of above code
   2. The state issue, causing NPE (the actual bug raised here)



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

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

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


Re: [PR] Handle errors while scaling kubernetes cluster [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8107:
URL: https://github.com/apache/cloudstack/pull/8107#issuecomment-1767560211

   <b>[SF] Trillian test result (tid-7993)</b>
   Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server r8
   Total time taken: 50614 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8107-t7993-vmware-67u3.zip
   Smoke tests completed. 107 look OK, 1 have errors, 0 did not run
   Only failed and skipped tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_01_deploy_vm_on_specific_host | `Error` | 3603.69 | test_vm_deployment_planner.py
   test_02_deploy_vm_on_specific_cluster | `Error` | 4.38 | test_vm_deployment_planner.py
   test_03_deploy_vm_on_specific_pod | `Error` | 4.40 | test_vm_deployment_planner.py
   test_04_deploy_vm_on_host_override_pod_and_cluster | `Error` | 4.46 | test_vm_deployment_planner.py
   test_05_deploy_vm_on_cluster_override_pod | `Error` | 12.56 | test_vm_deployment_planner.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


Re: [PR] Handle errors while scaling kubernetes cluster [cloudstack]

Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on PR #8107:
URL: https://github.com/apache/cloudstack/pull/8107#issuecomment-1766352163

   @blueorangutan test rocky8 vmware-67u3


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

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

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


Re: [PR] Handle errors while scaling kubernetes cluster [cloudstack]

Posted by "shwstppr (via GitHub)" <gi...@apache.org>.
shwstppr commented on PR #8107:
URL: https://github.com/apache/cloudstack/pull/8107#issuecomment-1801374917

   @harikrishna-patnala will you be able to make changes for cases reported by @kiranchavala ?


-- 
This is an automated message from the 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


Re: [PR] Handle errors while scaling kubernetes cluster [cloudstack]

Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on PR #8107:
URL: https://github.com/apache/cloudstack/pull/8107#issuecomment-1847933582

   @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


Re: [PR] Handle errors while scaling kubernetes cluster [cloudstack]

Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on PR #8107:
URL: https://github.com/apache/cloudstack/pull/8107#issuecomment-1849300406

   @kiranchavala would you like to continue testing this and also please verify the issue that you've raised


-- 
This is an automated message from the 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


Re: [PR] Handle errors while scaling kubernetes cluster [cloudstack]

Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on PR #8107:
URL: https://github.com/apache/cloudstack/pull/8107#issuecomment-1846904735

   @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


Re: [PR] Handle errors while scaling kubernetes cluster [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8107:
URL: https://github.com/apache/cloudstack/pull/8107#issuecomment-1765782838

   Packaging result [SF]: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: el9 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 7393


-- 
This is an automated message from the 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


Re: [PR] Handle errors while scaling kubernetes cluster [cloudstack]

Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #8107:
URL: https://github.com/apache/cloudstack/pull/8107#issuecomment-1812627316

   @harikrishna-patnala any progress/prognosis?


-- 
This is an automated message from the 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


Re: [PR] Handle errors while scaling kubernetes cluster [cloudstack]

Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on code in PR #8107:
URL: https://github.com/apache/cloudstack/pull/8107#discussion_r1418519143


##########
plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterResourceModifierActionWorker.java:
##########
@@ -768,11 +768,11 @@ protected String getKubernetesClusterNodeNamePrefix() {
     }
 
     protected KubernetesClusterVO updateKubernetesClusterEntry(final Long cores, final Long memory,
-        final Long size, final Long serviceOfferingId, final Boolean autoscaleEnabled, final Long minSize, final Long maxSize) {
+                                                               final Long size, final Long serviceOfferingId, final Boolean autoscaleEnabled, final Long minSize, final Long maxSize, KubernetesCluster.State state) {
         return Transaction.execute(new TransactionCallback<KubernetesClusterVO>() {
                 @Override
                 public KubernetesClusterVO doInTransaction(TransactionStatus status) {
-                KubernetesClusterVO updatedCluster = kubernetesClusterDao.createForUpdate(kubernetesCluster.getId());
+                KubernetesClusterVO updatedCluster = kubernetesClusterDao.findById(kubernetesCluster.getId());

Review Comment:
   This is a bigger change because, I observed createForUpdate() is returning an object with all null or default values and assumption is that we have to update that entry for all the columns.
   
   This is causing few issues with scaling operations
   1. When I tried to change the node count and change the compute offering at the same time. Compute offering change on few nodes is missing. This happened because of above code
   2. Above issue is causing after effects where I could not change the compute offering of the cluster anymore as there are differences in the compute offerings of the nodes
   3. The state issue, causing NPE (the actual bug raised here)



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

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

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


Re: [PR] Handle errors while scaling kubernetes cluster [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8107:
URL: https://github.com/apache/cloudstack/pull/8107#issuecomment-1848365427

   <b>[SF] Trillian test result (tid-8534)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 40913 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8107-t8534-kvm-centos7.zip
   Smoke tests completed. 108 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_migrate_vm | `Error` | 45.83 | test_vm_life_cycle.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


Re: [PR] Handle errors while scaling kubernetes cluster [cloudstack]

Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on PR #8107:
URL: https://github.com/apache/cloudstack/pull/8107#issuecomment-1805209441

   checking it now, I'll see if that case can be covered here.


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

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

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


Re: [PR] Handle errors while scaling kubernetes cluster [cloudstack]

Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #8107:
URL: https://github.com/apache/cloudstack/pull/8107#issuecomment-1801923774

   > @harikrishna-patnala will you be able to make changes for cases reported by @kiranchavala ?
   
   and if you can't, is it worth merging without?/will you create a new issue for 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


Re: [PR] Handle errors while scaling kubernetes cluster [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8107:
URL: https://github.com/apache/cloudstack/pull/8107#issuecomment-1844849096

   @harikrishna-patnala a [SL] 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


Re: [PR] Handle errors while scaling kubernetes cluster [cloudstack]

Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #8107:
URL: https://github.com/apache/cloudstack/pull/8107#issuecomment-1844971826

   @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


Re: [PR] Handle errors while scaling kubernetes cluster [cloudstack]

Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on PR #8107:
URL: https://github.com/apache/cloudstack/pull/8107#issuecomment-1765713706

   @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


Re: [PR] Handle errors while scaling kubernetes cluster [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8107:
URL: https://github.com/apache/cloudstack/pull/8107#issuecomment-1766354750

   @harikrishna-patnala a [SF] Trillian-Jenkins test job (rocky8 mgmt + vmware-67u3) has been kicked to run smoke tests


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

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

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


Re: [PR] Handle errors while scaling kubernetes cluster [cloudstack]

Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on PR #8107:
URL: https://github.com/apache/cloudstack/pull/8107#issuecomment-1842460261

   Sorry @DaanHoogland and @shwstppr could not finish this completely. Resuming it now.


-- 
This is an automated message from the 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


Re: [PR] Handle errors while scaling kubernetes cluster [cloudstack]

Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #8107:
URL: https://github.com/apache/cloudstack/pull/8107#issuecomment-1844936753

   ## [Codecov](https://app.codecov.io/gh/apache/cloudstack/pull/8107?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   Attention: `905 lines` in your changes are missing coverage. Please review.
   > Comparison is base [(`29c7b31`)](https://app.codecov.io/gh/apache/cloudstack/commit/29c7b3167eb62d7290a9e7ac51d1dada878620a4?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 13.02% compared to head [(`6a419bc`)](https://app.codecov.io/gh/apache/cloudstack/pull/8107?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 13.11%.
   > Report is 114 commits behind head on 4.18.
   
   | [Files](https://app.codecov.io/gh/apache/cloudstack/pull/8107?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
   |---|---|---|
   | [...n/java/com/cloud/network/IpAddressManagerImpl.java](https://app.codecov.io/gh/apache/cloudstack/pull/8107?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c2VydmVyL3NyYy9tYWluL2phdmEvY29tL2Nsb3VkL25ldHdvcmsvSXBBZGRyZXNzTWFuYWdlckltcGwuamF2YQ==) | 11.01% | [105 Missing :warning: ](https://app.codecov.io/gh/apache/cloudstack/pull/8107?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [...ava/com/cloud/upgrade/dao/Upgrade41800to41810.java](https://app.codecov.io/gh/apache/cloudstack/pull/8107?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZW5naW5lL3NjaGVtYS9zcmMvbWFpbi9qYXZhL2NvbS9jbG91ZC91cGdyYWRlL2Rhby9VcGdyYWRlNDE4MDB0bzQxODEwLmphdmE=) | 1.05% | [94 Missing :warning: ](https://app.codecov.io/gh/apache/cloudstack/pull/8107?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [...java/com/cloud/agent/manager/AgentManagerImpl.java](https://app.codecov.io/gh/apache/cloudstack/pull/8107?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZW5naW5lL29yY2hlc3RyYXRpb24vc3JjL21haW4vamF2YS9jb20vY2xvdWQvYWdlbnQvbWFuYWdlci9BZ2VudE1hbmFnZXJJbXBsLmphdmE=) | 0.00% | [66 Missing :warning: ](https://app.codecov.io/gh/apache/cloudstack/pull/8107?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [...src/main/java/com/cloud/upgrade/GuestOsMapper.java](https://app.codecov.io/gh/apache/cloudstack/pull/8107?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZW5naW5lL3NjaGVtYS9zcmMvbWFpbi9qYXZhL2NvbS9jbG91ZC91cGdyYWRlL0d1ZXN0T3NNYXBwZXIuamF2YQ==) | 20.73% | [65 Missing :warning: ](https://app.codecov.io/gh/apache/cloudstack/pull/8107?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [...ud/hypervisor/kvm/storage/KVMStorageProcessor.java](https://app.codecov.io/gh/apache/cloudstack/pull/8107?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGx1Z2lucy9oeXBlcnZpc29ycy9rdm0vc3JjL21haW4vamF2YS9jb20vY2xvdWQvaHlwZXJ2aXNvci9rdm0vc3RvcmFnZS9LVk1TdG9yYWdlUHJvY2Vzc29yLmphdmE=) | 0.00% | [47 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/cloudstack/pull/8107?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [...ain/java/com/cloud/api/query/QueryManagerImpl.java](https://app.codecov.io/gh/apache/cloudstack/pull/8107?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c2VydmVyL3NyYy9tYWluL2phdmEvY29tL2Nsb3VkL2FwaS9xdWVyeS9RdWVyeU1hbmFnZXJJbXBsLmphdmE=) | 0.00% | [35 Missing :warning: ](https://app.codecov.io/gh/apache/cloudstack/pull/8107?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [...ain/java/com/cloud/storage/dao/GuestOSDaoImpl.java](https://app.codecov.io/gh/apache/cloudstack/pull/8107?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZW5naW5lL3NjaGVtYS9zcmMvbWFpbi9qYXZhL2NvbS9jbG91ZC9zdG9yYWdlL2Rhby9HdWVzdE9TRGFvSW1wbC5qYXZh) | 0.00% | [28 Missing :warning: ](https://app.codecov.io/gh/apache/cloudstack/pull/8107?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [...in/java/com/cloud/server/ManagementServerImpl.java](https://app.codecov.io/gh/apache/cloudstack/pull/8107?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c2VydmVyL3NyYy9tYWluL2phdmEvY29tL2Nsb3VkL3NlcnZlci9NYW5hZ2VtZW50U2VydmVySW1wbC5qYXZh) | 0.00% | [24 Missing :warning: ](https://app.codecov.io/gh/apache/cloudstack/pull/8107?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [.../apache/cloudstack/vm/UnmanagedVMsManagerImpl.java](https://app.codecov.io/gh/apache/cloudstack/pull/8107?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9jbG91ZHN0YWNrL3ZtL1VubWFuYWdlZFZNc01hbmFnZXJJbXBsLmphdmE=) | 48.93% | [19 Missing and 5 partials :warning: ](https://app.codecov.io/gh/apache/cloudstack/pull/8107?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [...va/org/apache/cloudstack/ldap/LdapManagerImpl.java](https://app.codecov.io/gh/apache/cloudstack/pull/8107?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGx1Z2lucy91c2VyLWF1dGhlbnRpY2F0b3JzL2xkYXAvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2Nsb3Vkc3RhY2svbGRhcC9MZGFwTWFuYWdlckltcGwuamF2YQ==) | 0.00% | [20 Missing :warning: ](https://app.codecov.io/gh/apache/cloudstack/pull/8107?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | ... and [59 more](https://app.codecov.io/gh/apache/cloudstack/pull/8107?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | |
   
   <details><summary>Additional details and impacted files</summary>
   
   
   ```diff
   @@             Coverage Diff              @@
   ##               4.18    #8107      +/-   ##
   ============================================
   + Coverage     13.02%   13.11%   +0.09%     
   - Complexity     9032     9133     +101     
   ============================================
     Files          2720     2720              
     Lines        257080   257661     +581     
     Branches      40088    40172      +84     
   ============================================
   + Hits          33476    33802     +326     
   - Misses       219400   219568     +168     
   - Partials       4204     4291      +87     
   ```
   
   
   
   </details>
   
   [:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/cloudstack/pull/8107?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   


-- 
This is an automated message from the 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


Re: [PR] Handle errors while scaling kubernetes cluster [cloudstack]

Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on PR #8107:
URL: https://github.com/apache/cloudstack/pull/8107#issuecomment-1780453331

   > Code LGTM but needs testing. In my original issue, dynamic scaling config was set to true. It was the hypervisor plugin that was returning error during scaling. Using same vCPU and 2GB to 4GB scaling should reproduce the original issue on VMware. cc @harikrishna-patnala @kiranchavala
   
   In both cases, CloudRuntimeException is thrown @shwstppr , so this covers that error as well.


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

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

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


Re: [PR] Handle errors while scaling kubernetes cluster [cloudstack]

Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on PR #8107:
URL: https://github.com/apache/cloudstack/pull/8107#issuecomment-1780458917

   > KubernetesClusterScaleWorker.java:460
   
   Thanks for testing @kiranchavala .
   For sure this error or this case is not part of this PR but worth checking the case and fix it. I'll test it again with your reproduction steps.


-- 
This is an automated message from the 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


Re: [PR] Handle errors while scaling kubernetes cluster [cloudstack]

Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on code in PR #8107:
URL: https://github.com/apache/cloudstack/pull/8107#discussion_r1420207760


##########
plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterScaleWorker.java:
##########
@@ -412,7 +411,7 @@ private void scaleKubernetesClusterSize() throws CloudRuntimeException {
         } else { // upscale, same node count handled above
             scaleUpKubernetesClusterSize(newVmRequiredCount);
         }
-        kubernetesCluster = updateKubernetesClusterEntry(clusterSize, null);
+        kubernetesCluster = updateKubernetesClusterEntry(clusterSize, serviceOffering);

Review Comment:
   reverted this change @shwstppr This is not required as we are now correctly updating the existing cluster. Previously the core issue was because of updating the cluster wrongly.



-- 
This is an automated message from the 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


Re: [PR] Handle errors while scaling kubernetes cluster [cloudstack]

Posted by "shwstppr (via GitHub)" <gi...@apache.org>.
shwstppr commented on PR #8107:
URL: https://github.com/apache/cloudstack/pull/8107#issuecomment-1846888241

   @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


Re: [PR] Handle errors while scaling kubernetes cluster [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8107:
URL: https://github.com/apache/cloudstack/pull/8107#issuecomment-1765714027

   @harikrishna-patnala a [SF] 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


Re: [PR] Handle errors while scaling kubernetes cluster [cloudstack]

Posted by "shwstppr (via GitHub)" <gi...@apache.org>.
shwstppr merged PR #8107:
URL: https://github.com/apache/cloudstack/pull/8107


-- 
This is an automated message from the 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


Re: [PR] Handle errors while scaling kubernetes cluster [cloudstack]

Posted by "shwstppr (via GitHub)" <gi...@apache.org>.
shwstppr commented on PR #8107:
URL: https://github.com/apache/cloudstack/pull/8107#issuecomment-1840510615

   @harikrishna-patnala any update on 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


Re: [PR] Handle errors while scaling kubernetes cluster [cloudstack]

Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on PR #8107:
URL: https://github.com/apache/cloudstack/pull/8107#issuecomment-1844845503

   @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


Re: [PR] Handle errors while scaling kubernetes cluster [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8107:
URL: https://github.com/apache/cloudstack/pull/8107#issuecomment-1844953832

   Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 7962


-- 
This is an automated message from the 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


Re: [PR] Handle errors while scaling kubernetes cluster [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8107:
URL: https://github.com/apache/cloudstack/pull/8107#issuecomment-1847935323

   @harikrishna-patnala a [SL] 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


Re: [PR] Handle errors while scaling kubernetes cluster [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8107:
URL: https://github.com/apache/cloudstack/pull/8107#issuecomment-1846980563

   Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 7986


-- 
This is an automated message from the 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