wido opened a new pull request, #14237:
URL: https://github.com/apache/cloudstack/pull/14237

   ### Description
   
   This PR removes agent commands that the management server stopped sending 
when the storage subsystem was introduced in 4.2, together with every handler 
for them in the hypervisor resources.
   
   **Depends on the KVM cleanup PR** 
https://github.com/apache/cloudstack/pull/14236, which removes the KVM wrappers 
for these commands and the `ManagedNfsStorageAdaptor`. The commits from that 
branch show up here until it is merged; only the last commit belongs to this PR.
   
   Nothing in the management server creates or subclasses any of these, so the 
handlers in every hypervisor resource were unreachable:
   
   AttachIsoCommand, BackupSnapshotCommand, CheckConsoleProxyLoadCommand, 
WatchConsoleProxyLoadCommand, CheckStorageAvailabilityCommand, CreateCommand, 
CreatePrivateTemplateFromSnapshotCommand, 
CreatePrivateTemplateFromVolumeCommand, CreateVolumeFromSnapshotCommand, 
ManageSnapshotCommand, PrimaryStorageDownloadCommand, RebootRouterCommand, 
UpgradeSnapshotCommand.
   
   Removed with them:
   
   - the `Answer` classes only they produced: `AttachIsoAnswer`, 
`BackupSnapshotAnswer`, `CreateAnswer`, `CreateVolumeFromSnapshotAnswer`, 
`ManageSnapshotAnswer`, `CreatePrivateTemplateAnswer`. 
`PrimaryStorageDownloadAnswer` stays, the KVM storage processor still uses it 
as a plain failure answer.
   - the XenServer wrappers and the VMware, simulator and console proxy 
handlers, the corresponding `VmwareStorageManager` and mock manager methods, 
and the dispatch branches that routed to them.
   - the `execute(PrimaryStorageDownloadCommand)` method of the 
`StoragePoolResource` interface and the `CreateCommand` entry in 
`AgentAttache`'s list of commands refused while connecting.
   - the unit tests in `core`, XenServer, Hyper-V and KVM that built these 
commands.
   - the `ManagedNFS` storage pool type. Its only adaptor is removed in the KVM 
cleanup PR and no code has ever created a pool of that type.
   
   Not touched, and worth a reviewer's eye:
   
   - the `ovm` and `ovm3` plugins are not part of the build and still reference 
these classes.
   - the Hyper-V .NET agent has its own C# handlers for `CreateCommand` and 
`PrimaryStorageDownloadCommand`.
   - log messages and configuration descriptions that merely mention the old 
command names.
   
   ### 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)
   - [x] Cleanup (Code refactoring and cleanup, that may add test cases)
   - [ ] Build/CI
   - [ ] Test (unit or integration test code)
   
   ### Feature/Enhancement Scale or Bug Severity
   
   #### Feature/Enhancement Scale
   
   - [ ] Major
   - [x] Minor
   
   #### Bug Severity
   
   - [ ] BLOCKER
   - [ ] Critical
   - [ ] Major
   - [ ] Minor
   - [ ] Trivial
   
   ### Screenshots (if appropriate):
   
   ### How Has This Been Tested?
   
   Every command was checked for `new X(` and `extends X` across all non-test 
Java in the tree; none is instantiated or subclassed outside the hypervisor 
handlers removed here. Non-Java files were searched as well (JSON fixtures, 
Python, SQL).
   
   `api`, `core`, `agent`, `engine/orchestration`, the XenServer, Hyper-V, KVM 
and simulator plugins compile and their unit tests pass with JDK 17. A 
full-tree `mvn -T 1C -Dsimulator test` passes: 147 modules, 13,188 tests, 0 
failures.
   
   **The `vmware` plugin was not compiled locally.** It needs the 
non-redistributable vSphere SDK, which is not available in the build 
environment used. Its changes (removal of dispatch branches and the matching 
handler methods in `VmwareResource`, `VmwareStorageManager(Impl)`, 
`VmwareSecondaryStorageResourceHandler` and `VMwareGuru`) were reviewed line by 
line but need the `noredist` CI build to confirm.
   
   #### How did you try to break this feature and the system with this change?
   
   Checked that no remaining dispatch chain starts with a removed branch (the 
console proxy resource and the VMware secondary storage handler both did, and 
were rewritten so the chain still begins with a valid `if`), and that no 
interface method was removed while an implementation still overrides 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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to