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

dhlaluku · 2018-07-19T10:23:04Z

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 · 2018-07-19T10:23:51Z

Great @dhlaluku
@blueorangutan package

blueorangutan · 2018-07-19T10:24:51Z

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

blueorangutan · 2018-07-19T11:09:00Z

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

dhlaluku · 2018-07-19T11:10:59Z

@blueorangutan test

blueorangutan · 2018-07-19T11:11:50Z

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

rafaelweingartner · 2018-07-19T13:58:36Z

@@ -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 · 2018-07-19T21:36:39Z

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 · 2018-07-20T03:48:04Z

@blueorangutan package

blueorangutan · 2018-07-20T03:48:50Z

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

blueorangutan · 2018-07-20T04:39:34Z

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

dhlaluku · 2018-07-20T04:40:57Z

@blueorangutan package

blueorangutan · 2018-07-20T04:41:51Z

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

blueorangutan · 2018-07-20T05:15:33Z

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

dhlaluku · 2018-07-20T05:16:31Z

@blueorangutan test

blueorangutan · 2018-07-20T05:16:51Z

@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 · 2018-07-20T13:17:19Z

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:masterJul 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

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

Uh oh!

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!

rafaelweingartnerJul 19, 2018

Choose a reason for hiding this comment

Uh oh!

dhlalukuJul 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