Conversation
…nger sends
The KVM agent still registered wrappers for a set of commands that date
back to the pre-4.2 storage model. None of these commands is created or
subclassed anywhere in the management server any more, so the wrappers
could never be reached:
AttachIsoCommand, BackupSnapshotCommand, CheckConsoleProxyLoadCommand,
WatchConsoleProxyLoadCommand, CheckStorageAvailabilityCommand,
CreateCommand, CreatePrivateTemplateFromSnapshotCommand,
CreatePrivateTemplateFromVolumeCommand, CreateVolumeFromSnapshotCommand,
ManageSnapshotCommand, PrimaryStorageDownloadCommand,
RebootRouterCommand, UpgradeSnapshotCommand
Along with the wrappers and their shared console proxy base class, this
removes what only they used:
- the createtmplt.sh and createvm.sh lookups in
LibvirtComputingResource. The storage processor has its own
createtmplt.sh lookup; createvm.sh was looked up at startup but never
executed.
- the buildTemplateLocation, buildQCOW2Processor and
retrieveBashScriptPath helpers in LibvirtUtilitiesHelper and the
BASH_SCRIPT_PATH constant.
- fields that were declared but never read: heartBeatPath,
hostOsVersion, javaTempDir, and an unused local in
getMacAddressToNicNumPair.
- the 27 unit tests that exercised the removed wrappers.
The Command classes themselves stay in core because the XenServer,
VMware and simulator resources still reference them.
Nothing in the tree references these scripts any more. Nine of them have no callers at all: create_private_template.sh, createvm.sh, delvm.sh, get_domr_kernel.sh, get_iqn.sh, importmpl.sh, listvmdisk.sh, listvmdisksize.sh, managevolume.sh createvm.sh was still located at agent startup until the previous commit, but was never executed. The other three are stale copies of scripts that live in scripts/storage/secondary. The secondary storage code resolves them from that directory only, so the qcow2 copies were never run: createvolume.sh, listvmtmplt.sh, listvolume.sh createtmplt.sh, managesnapshot.sh and resizevolume.sh stay, the KVM agent still uses them.
- IvsVifDriver: VIF driver for Big Switch IVS. It could only be
activated by naming it in libvirt.vif.driver in agent.properties,
and the Big Switch plugin itself never refers to it.
- ManagedNfsStorageAdaptor: picked up by the storage adaptor
reflection scan, but the management server never creates a pool of
type ManagedNFS, so the adaptor was never selected. The enum value
is left in place.
- LibvirtNetworkDef: builder for libvirt <network> XML, never used.
- KVMGuestOsMapper, KVMStorageResource, StorageAdaptorInfo,
KVMVirtualDisk: no references anywhere in the tree.
These commands belong to the storage model that predates the storage
subsystem introduced in 4.2. Nothing in the management server creates
or subclasses any of them any more, 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 was removed
earlier and no code has ever created a pool of that type.
Not touched: the ovm and ovm3 plugins, which are not part of the build
and still reference these classes; the Hyper-V .NET agent, which has its
own C# handlers for CreateCommand and PrimaryStorageDownloadCommand;
and log messages and configuration descriptions that merely mention the
old command names.
The vmware plugin needs the non redistributable vSphere SDK to compile,
which is not available in this build environment; its changes were
reviewed but not compiled.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #14237 +/- ##
============================================
- Coverage 19.91% 19.90% -0.02%
+ Complexity 20200 20154 -46
============================================
Files 6373 6326 -47
Lines 577230 574724 -2506
Branches 70696 70489 -207
============================================
- Hits 114974 114393 -581
+ Misses 449690 447797 -1893
+ Partials 12566 12534 -32
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 #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:
Answerclasses only they produced:AttachIsoAnswer,BackupSnapshotAnswer,CreateAnswer,CreateVolumeFromSnapshotAnswer,ManageSnapshotAnswer,CreatePrivateTemplateAnswer.PrimaryStorageDownloadAnswerstays, the KVM storage processor still uses it as a plain failure answer.VmwareStorageManagerand mock manager methods, and the dispatch branches that routed to them.execute(PrimaryStorageDownloadCommand)method of theStoragePoolResourceinterface and theCreateCommandentry inAgentAttache's list of commands refused while connecting.core, XenServer, Hyper-V and KVM that built these commands.ManagedNFSstorage 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:
ovmandovm3plugins are not part of the build and still reference these classes.CreateCommandandPrimaryStorageDownloadCommand.Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
Every command was checked for
new X(andextends Xacross 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-treemvn -T 1C -Dsimulator testpasses: 147 modules, 13,188 tests, 0 failures.The
vmwareplugin 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 inVmwareResource,VmwareStorageManager(Impl),VmwareSecondaryStorageResourceHandlerandVMwareGuru) were reviewed line by line but need thenoredistCI 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.