Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removing an old, unused NetApp plug-in #2670

Merged

Conversation

Projects
None yet
6 participants
@mike-tutkowski
Copy link
Member

commented May 23, 2018

Description

NetApp doesn't support this old plug-in anymore nor do we believe anyone is actually still using it. Removing this plug-in will also remove a CloudStack non-OSS dependency on the manageontap.jar file.

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)

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

This comment has been minimized.

Copy link
Member Author

commented May 23, 2018

A couple questions for reviewers:

  • Is it OK that I removed the cloudvoladm file? I see this file was removed in the past and then put back. It seems specific to this plug-in, so I removed it.

  • Is it OK that I updated the .sql files and the Upgrade2213to2214.java file?

Thanks!

@rhtyd rhtyd added this to the 4.12.0.0 milestone May 24, 2018

@rhtyd

rhtyd approved these changes May 24, 2018

Copy link
Member

left a comment

LGTM.
Mike, I'm okay to remove any tables from other sql files including removal of tables in recent upgrade paths if those are not used at all like the following:

DROP TABLE IF EXISTS `cloud`.`netapp_volume`;
DROP TABLE IF EXISTS `cloud`.`netapp_pool`;
DROP TABLE IF EXISTS `cloud`.`netapp_lun`;

Can you start a conversation on both dev and user MLs, to see if any users are using this that can potentially block removal of this feature/plugin.

@rhtyd rhtyd added the type:Cleanup label May 24, 2018

@mike-tutkowski

This comment has been minimized.

Copy link
Member Author

commented May 24, 2018

@mike-tutkowski mike-tutkowski force-pushed the mike-tutkowski:remove-old-netapp-plug-in branch from e319715 to ba860aa May 26, 2018

@GabrielBrascher
Copy link
Member

left a comment

I am +1 on removing this plug-in. Code LGTM!

@mike-tutkowski mike-tutkowski force-pushed the mike-tutkowski:remove-old-netapp-plug-in branch from ba860aa to c7d6376 Jun 8, 2018

@mike-tutkowski

This comment has been minimized.

Copy link
Member Author

commented Jun 8, 2018

@rhtyd Looks like we have enough LGTMs and the basic tests are passing. Should we run more extensive tests?

@mike-tutkowski

This comment has been minimized.

Copy link
Member Author

commented Jun 18, 2018

@blueorangutan package

@blueorangutan

This comment has been minimized.

Copy link

commented Jun 18, 2018

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

@blueorangutan

This comment has been minimized.

Copy link

commented Jun 18, 2018

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

@DaanHoogland

This comment has been minimized.

Copy link
Contributor

commented Jun 18, 2018

@blueorangutan

This comment has been minimized.

Copy link

commented Jun 18, 2018

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

@blueorangutan

This comment has been minimized.

Copy link

commented Jun 19, 2018

Trillian test result (tid-2764)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 20767 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr2670-t2764-kvm-centos7.zip
Intermitten failure detected: /marvin/tests/smoke/test_certauthority_root.py
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. 59 look OK, 8 have error(s)
Only failed tests results shown below:

Test Result Time (s) Test File
test_provision_certificate Error 5.14 test_certauthority_root.py
ContextSuite context=TestDeployVirtioSCSIVM>:setup Error 0.00 test_deploy_virtio_scsi_vm.py
test_01_add_primary_storage_disabled_host Error 0.49 test_primary_storage.py
test_01_primary_storage_nfs Error 0.09 test_primary_storage.py
ContextSuite context=TestStorageTags>:setup Error 0.17 test_primary_storage.py
test_03_vpc_privategw_restart_vpc_cleanup Error 141.28 test_privategw_acl.py
test_02_list_snapshots_with_removed_data_store Error 1.11 test_snapshots.py
test_01_secured_vm_migration Error 36.34 test_vm_life_cycle.py
test_02_not_secured_vm_migration Error 34.30 test_vm_life_cycle.py
test_03_secured_to_nonsecured_vm_migration Error 35.32 test_vm_life_cycle.py
test_04_nonsecured_to_secured_vm_migration Error 35.32 test_vm_life_cycle.py
test_08_migrate_vm Error 14.60 test_vm_life_cycle.py
test_01_cancel_host_maintenace_with_no_migration_jobs Failure 5.26 test_host_maintenance.py
test_02_cancel_host_maintenace_with_migration_jobs Error 1.21 test_host_maintenance.py
test_hostha_enable_ha_when_host_in_maintenance Error 3.42 test_hostha_kvm.py
@DaanHoogland

This comment has been minimized.

Copy link
Contributor

commented Jun 19, 2018

@mike-tutkowski I hope these failures do not signify the state of master at this moment. You can ignore hostha, privategateway and CA, but the rest I'm not sure

@mike-tutkowski

This comment has been minimized.

Copy link
Member Author

commented Jun 19, 2018

I reviewed the test failures and none appears to be applicable to this PR.

Since this PR seems pretty low risk with respect to its likelihood to generate test failures, I'm going to go ahead and merge it.

@mike-tutkowski mike-tutkowski merged commit 9776157 into apache:master Jun 19, 2018

2 checks passed

Jenkins This pull request looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@mike-tutkowski mike-tutkowski deleted the mike-tutkowski:remove-old-netapp-plug-in branch Jun 19, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.