Including instance details in KubernetesClusterResponse by davidjumani · Pull Request #4420 · apache/cloudstack

davidjumani · 2020-10-22T08:52:13Z

Description

Returning list of virtual machine nodes of a cluster rather than just their IDs. Simplifies the UI as well (Reduces API calls and checks post 4.15 release in primate for backward compatibility)

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)

How Has This Been Tested?

(kubeadmin) 🐱 > list kubernetesclusters id=3d2d10cd-db04-4c4f-87f0-7405d45333e8 
{
  "count": 1,
  "kubernetescluster": [
    {
      .....
      "name": "cluster-1",
      "virtualmachines": [
        {
          .....
          "displayname": "cluster-1-master-1754f2cc2da",
          .....
        },
        {
         .....
          "displayname": "cluster-1-node-1754f2cd5f1",
          .....
        }
      ],
    }
  ]
}

davidjumani · 2020-10-22T08:53:54Z

@blueorangutan package

blueorangutan · 2020-10-22T08:55:09Z

@davidjumani a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

davidjumani · 2020-10-22T09:24:27Z

@blueorangutan package

blueorangutan · 2020-10-22T09:25:03Z

@davidjumani a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

blueorangutan · 2020-10-22T09:57:53Z

Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2262

davidjumani · 2020-10-22T09:59:33Z

@blueorangutan test

blueorangutan · 2020-10-22T10:00:08Z

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

DaanHoogland · 2020-10-22T11:47:30Z

+                UserVmJoinVO userVM = userVmJoinDao.findById(vmMapVO.getVmId());
                if (userVM != null) {
-                    vmIds.add(userVM.getUuid());
+                    UserVmResponse vmResponse = ApiDBUtils.newUserVmResponse(respView, "virtualmachine", userVM,


would like this string constant defined

blueorangutan · 2020-10-22T21:29:44Z

Trillian test result (tid-3045)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 39498 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4420-t3045-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Smoke tests completed. 83 look OK, 2 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File
test_03_deploy_and_upgrade_kubernetes_cluster	`Failure`	259.96	test_kubernetes_clusters.py
test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers	`Failure`	301.90	test_vpc_redundant.py

GabrielBrascher · 2020-10-23T01:31:56Z

        response.setNetworkId(ntwk.getUuid());
        response.setAssociatedNetworkName(ntwk.getName());
-        List<String> vmIds = new ArrayList<String>();
+        List<UserVmResponse> vmIds = new ArrayList<UserVmResponse>();


@davidjumani considering that this variable has changed from a list of VM IDs to a list of VM Responses I would suggest renaming it.
What do you think of vmResponses or vmResponseList?

davidjumani · 2020-10-23T02:54:06Z

@blueorangutan package

blueorangutan · 2020-10-23T02:55:12Z

@davidjumani a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

blueorangutan · 2020-10-23T03:27:28Z

Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2267

davidjumani · 2020-10-23T03:30:44Z

@blueorangutan test

blueorangutan · 2020-10-23T03:32:07Z

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

blueorangutan · 2020-10-23T15:29:31Z

Trillian test result (tid-3061)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 41056 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4420-t3061-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_diagnostics.py
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Smoke tests completed. 85 look OK, 0 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File

DaanHoogland · 2020-10-27T06:39:05Z

@GabrielBrascher is this lgty?

shwstppr

LGTM on code changes and functionality works. Only concern, will this make listKubernetesCluster API response too big!

davidjumani · 2020-10-27T06:44:08Z

@shwstppr This is needed for the cluster auotscaler (rather return the entire result than give listVMs access to an external service)
Also makes primate simpler (reduces backwards compatibility work)

DaanHoogland · 2020-10-27T10:13:30Z

satisfied @shwstppr ? can we merge?

shwstppr

LGTM. @davidjumani seems okay based on auto-scaler, Primate requirements cc @DaanHoogland

Including instance details in KubernetesClusterResponse

6ccb9a3

davidjumani requested review from DaanHoogland and shwstppr October 22, 2020 08:52

davidjumani added component:api type:enhancement labels Oct 22, 2020

davidjumani added this to the 4.15.0.0 milestone Oct 22, 2020

Fixing tests

42f92f6

davidjumani force-pushed the cks-vms branch from ba7c048 to 42f92f6 Compare October 22, 2020 09:02

DaanHoogland approved these changes Oct 22, 2020

View reviewed changes

GabrielBrascher reviewed Oct 23, 2020

View reviewed changes

Refactoring

e844a1c

davidjumani mentioned this pull request Oct 27, 2020

Fixing cks instance view apache/cloudstack-primate#832

Merged

davidjumani requested review from DaanHoogland and GabrielBrascher October 27, 2020 06:36

shwstppr reviewed Oct 27, 2020

View reviewed changes

DaanHoogland approved these changes Oct 27, 2020

View reviewed changes

shwstppr approved these changes Oct 27, 2020

View reviewed changes

DaanHoogland merged commit 8a8947a into apache:master Oct 27, 2020

DaanHoogland deleted the cks-vms branch October 27, 2020 13:44

Conversation

davidjumani commented Oct 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Types of changes

How Has This Been Tested?

Uh oh!

davidjumani commented Oct 22, 2020

Uh oh!

blueorangutan commented Oct 22, 2020

Uh oh!

davidjumani commented Oct 22, 2020

Uh oh!

blueorangutan commented Oct 22, 2020

Uh oh!

blueorangutan commented Oct 22, 2020

Uh oh!

davidjumani commented Oct 22, 2020

Uh oh!

blueorangutan commented Oct 22, 2020

Uh oh!

DaanHoogland Oct 22, 2020

Choose a reason for hiding this comment

Uh oh!

blueorangutan commented Oct 22, 2020

Uh oh!

GabrielBrascher Oct 23, 2020

Choose a reason for hiding this comment

Uh oh!

davidjumani commented Oct 23, 2020

Uh oh!

blueorangutan commented Oct 23, 2020

Uh oh!

blueorangutan commented Oct 23, 2020

Uh oh!

davidjumani commented Oct 23, 2020

Uh oh!

blueorangutan commented Oct 23, 2020

Uh oh!

blueorangutan commented Oct 23, 2020

Uh oh!

DaanHoogland commented Oct 27, 2020

Uh oh!

shwstppr left a comment

Choose a reason for hiding this comment

Uh oh!

davidjumani commented Oct 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DaanHoogland commented Oct 27, 2020

Uh oh!

shwstppr left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

davidjumani commented Oct 22, 2020 •

edited

Loading

davidjumani commented Oct 27, 2020 •

edited

Loading