fix provisionCertificate api returns NPE when 'reconnect' parameter is true by dhlaluku · Pull Request #2756 · apache/cloudstack

dhlaluku · Jul 19, 2018

Description

This PR fixes NPE with the provisionCertificateCmd when reconnect is set to True.

Also fixes the following Marvin test failures:

test_certauthority_root.py

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)

GitHub Issue/PRs

Fixes: #2753

Screenshots (if appropriate):

How Has This Been Tested?

With cloudmonkey, see screenshot

Dev environment components:

Platform: ACS-4.12
Management OS: Ubuntu 18.04 LTS
Hypervisor: 1 Host running KVM with CentOS 7

Checklist:

I have read the CONTRIBUTING document.
My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
Testing
I have added tests to cover my changes.
All relevant new and existing integration tests have passed.
A full integration testsuite with all test that can run on my environment has passed.

…s used apache#2753

borisstoyanov · Jul 19, 2018

Great @dhlaluku
@blueorangutan package

blueorangutan · Jul 19, 2018

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

blueorangutan · Jul 19, 2018

Packaging result: ✔centos6 ✔centos7 ✔debian. JID-2189

dhlaluku · Jul 19, 2018

@blueorangutan test

blueorangutan · Jul 19, 2018

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

rafaelweingartner · Jul 19, 2018

engine/orchestration/src/main/java/com/cloud/agent/manager/ClusteredAgentManagerImpl.java

@@ -368,9 +368,10 @@ public void reconnect(final long hostId) throws CloudRuntimeException, AgentUnav
        if (result == null) {
            super.reconnect(hostId);



Instead of adding an else block and then an if nested, what about simply using a return; statement here?

Thanks, that is actually quite good since simplifies things and removes redundant code.

rafaelweingartner

Thanks!

blueorangutan · Jul 19, 2018

Trillian test result (tid-2866)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 35720 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr2756-t2866-kvm-centos7.zip
Intermitten failure detected: /marvin/tests/smoke/test_deploy_virtio_scsi_vm.py
Intermitten failure detected: /marvin/tests/smoke/test_privategw_acl.py
Intermitten failure detected: /marvin/tests/smoke/test_public_ip_range.py
Intermitten failure detected: /marvin/tests/smoke/test_templates.py
Intermitten failure detected: /marvin/tests/smoke/test_usage.py
Intermitten failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
Intermitten failure detected: /marvin/tests/smoke/test_volumes.py
Intermitten failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Intermitten failure detected: /marvin/tests/smoke/test_hostha_kvm.py
Smoke tests completed. 61 look OK, 7 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File
ContextSuite context=TestDeployVirtioSCSIVM>:setup	`Error`	0.00	test_deploy_virtio_scsi_vm.py
test_03_vpc_privategw_restart_vpc_cleanup	`Failure`	1146.06	test_privategw_acl.py
test_04_extract_template	`Failure`	128.31	test_templates.py
ContextSuite context=TestISOUsage>:setup	`Error`	0.00	test_usage.py
test_01_secure_vm_migration	`Error`	133.94	test_vm_life_cycle.py
test_02_unsecure_vm_migration	`Error`	133.94	test_vm_life_cycle.py
test_03_secured_to_nonsecured_vm_migration	`Error`	133.96	test_vm_life_cycle.py
test_04_nonsecured_to_secured_vm_migration	`Error`	132.91	test_vm_life_cycle.py
test_06_download_detached_volume	`Failure`	137.58	test_volumes.py
test_11_migrate_volume_and_change_offering	`Error`	129.13	test_volumes.py
test_hostha_enable_ha_when_host_in_maintenance	`Error`	3.58	test_hostha_kvm.py

dhlaluku · Jul 20, 2018

@blueorangutan package

blueorangutan · Jul 20, 2018

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

blueorangutan · Jul 20, 2018

Packaging result: ✔centos6 ✖centos7 ✔debian. JID-2191

dhlaluku · Jul 20, 2018

@blueorangutan package

blueorangutan · Jul 20, 2018

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

blueorangutan · Jul 20, 2018

Packaging result: ✔centos6 ✔centos7 ✔debian. JID-2193

dhlaluku · Jul 20, 2018

@blueorangutan test

blueorangutan · Jul 20, 2018

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

borisstoyanov

The Certificate error is gone now, LGTM!
Once we get this merged we can test #2753

blueorangutan · Jul 20, 2018

Trillian test result (tid-2869)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 27201 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr2756-t2869-kvm-centos7.zip
Intermitten failure detected: /marvin/tests/smoke/test_deploy_virtio_scsi_vm.py
Intermitten failure detected: /marvin/tests/smoke/test_primary_storage.py
Intermitten failure detected: /marvin/tests/smoke/test_privategw_acl.py
Intermitten failure detected: /marvin/tests/smoke/test_snapshots.py
Intermitten failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
Intermitten failure detected: /marvin/tests/smoke/test_host_maintenance.py
Intermitten failure detected: /marvin/tests/smoke/test_hostha_kvm.py
Smoke tests completed. 61 look OK, 7 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File
ContextSuite context=TestDeployVirtioSCSIVM>:setup	`Error`	0.00	test_deploy_virtio_scsi_vm.py
test_01_add_primary_storage_disabled_host	`Error`	0.60	test_primary_storage.py
test_01_primary_storage_nfs	`Error`	0.12	test_primary_storage.py
ContextSuite context=TestStorageTags>:setup	`Error`	0.22	test_primary_storage.py
test_03_vpc_privategw_restart_vpc_cleanup	`Failure`	1126.37	test_privategw_acl.py
test_02_list_snapshots_with_removed_data_store	`Error`	1.15	test_snapshots.py
test_01_secure_vm_migration	`Error`	132.90	test_vm_life_cycle.py
test_02_unsecure_vm_migration	`Error`	132.90	test_vm_life_cycle.py
test_03_secured_to_nonsecured_vm_migration	`Error`	132.93	test_vm_life_cycle.py
test_04_nonsecured_to_secured_vm_migration	`Error`	132.91	test_vm_life_cycle.py
test_08_migrate_vm	`Error`	14.79	test_vm_life_cycle.py
test_01_cancel_host_maintenace_with_no_migration_jobs	`Failure`	0.11	test_host_maintenance.py
test_02_cancel_host_maintenace_with_migration_jobs	`Error`	2.37	test_host_maintenance.py
test_hostha_enable_ha_when_host_in_maintenance	`Error`	4.58	test_hostha_kvm.py
test_hostha_kvm_host_fencing	`Failure`	645.66	test_hostha_kvm.py
test_hostha_kvm_host_recovering	`Failure`	619.79	test_hostha_kvm.py

…s true (apache#2756) This PR fixes NPE with the provisionCertificateCmd when reconnect is set to True. Also fixes the following Marvin test failures: - test_certauthority_root.py

fix provisionCertificate api returns NPE when 'reconnect' parameter i…

9293595

…s used apache#2753

dhlaluku mentioned this pull request Jul 19, 2018

provisionCertificate api returns NPE when 'reconnect' parameter is used. #2753

Closed

rafaelweingartner reviewed Jul 19, 2018

View reviewed changes

Removing nested if statements

3c72c54

rafaelweingartner approved these changes Jul 19, 2018

View reviewed changes

borisstoyanov approved these changes Jul 20, 2018

View reviewed changes

DaanHoogland merged commit a6e0204 into apache:master Jul 20, 2018

dhlaluku deleted the bugfix-provision-certificate-api-npe branch July 20, 2018 13:19

borisstoyanov mentioned this pull request Jul 24, 2018

Host is still unsecured when provisionCertificate api is called #2763

Closed

dhlaluku mentioned this pull request Aug 16, 2018

kvm: Properly report available memory to Management Server #2795

Merged

12 tasks

Search code, repositories, users, issues, pull requests...

Conversation

dhlaluku commented Jul 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Types of changes

GitHub Issue/PRs

Screenshots (if appropriate):

How Has This Been Tested?

Checklist:

Uh oh!

borisstoyanov commented Jul 19, 2018

Uh oh!

blueorangutan commented Jul 19, 2018

Uh oh!

blueorangutan commented Jul 19, 2018

Uh oh!

dhlaluku commented Jul 19, 2018

Uh oh!

blueorangutan commented Jul 19, 2018

Uh oh!

rafaelweingartner Jul 19, 2018

Choose a reason for hiding this comment

Uh oh!

dhlaluku Jul 19, 2018

Choose a reason for hiding this comment

Uh oh!

rafaelweingartner left a comment

Choose a reason for hiding this comment

Uh oh!

blueorangutan commented Jul 19, 2018

Uh oh!

dhlaluku commented Jul 20, 2018

Uh oh!

blueorangutan commented Jul 20, 2018

Uh oh!

blueorangutan commented Jul 20, 2018

Uh oh!

dhlaluku commented Jul 20, 2018

Uh oh!

blueorangutan commented Jul 20, 2018

Uh oh!

blueorangutan commented Jul 20, 2018

Uh oh!

dhlaluku commented Jul 20, 2018

Uh oh!

blueorangutan commented Jul 20, 2018

Uh oh!

borisstoyanov left a comment

Choose a reason for hiding this comment

Uh oh!

blueorangutan commented Jul 20, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

dhlaluku commented Jul 19, 2018 •

edited

Loading