Greg Padgett has posted comments on this change. Change subject: webadmin, backend: control of hosted engine maintenance mode ......................................................................
Patch Set 3: (7 comments) See master patch at http://gerrit.ovirt.org/#/c/24605/. http://gerrit.ovirt.org/#/c/23531/3/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MaintenanceNumberOfVdssCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MaintenanceNumberOfVdssCommand.java: Line 202: clustersAsSet.add(vds.getVdsGroupId()); Line 203: Line 204: List<String> nonMigratableVmDescriptionsToFrontEnd = new ArrayList<String>(); Line 205: for (VM vm : vms) { Line 206: if (vm.isHostedEngine()) { > not so familiar with the arch, but what if there is only one ha agent confi Good point, same goes for any case where the ha agent might fail to migrate the engine vm. Seems to me the maintenance operation should fail in this case, but it's something I'll need to test. At any rate, it require manual intervention. Not sure if it's something we should investigate for this patch set or submit a fix for later. Line 207: // The Hosted Engine vm is migrated by the HA agent Line 208: continue; Line 209: } Line 210: if (vm.getMigrationSupport() != MigrationSupport.MIGRATABLE) { http://gerrit.ovirt.org/#/c/23531/3/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/SetHaMaintenanceCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/SetHaMaintenanceCommand.java: Line 15: import java.util.Collections; Line 16: import java.util.List; Line 17: Line 18: @NonTransactiveCommandAttribute Line 19: public class SetHaMaintenanceCommand extends VdsCommand<VdsActionParameters> { > declare params generic type with SetHaMaintenanceParameters, will save the Done Line 20: Line 21: public SetHaMaintenanceCommand(VdsActionParameters vdsActionParameters) { Line 22: super(vdsActionParameters); Line 23: } http://gerrit.ovirt.org/#/c/23531/3/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/SetHaMaintenanceParameters.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/SetHaMaintenanceParameters.java: Line 17: public SetHaMaintenanceParameters() { Line 18: } Line 19: Line 20: public HaMaintenanceMode getMode() { Line 21: return this.mode; > minor: I personally think that 'this' is redundant here. Done Line 22: } Line 23: Line 24: public boolean getIsEnabled() { Line 25: return this.enabled; http://gerrit.ovirt.org/#/c/23531/3/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/auditloghandling/AuditLogDirector.java File backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/auditloghandling/AuditLogDirector.java: Line 361: severities.put(AuditLogType.VDS_STORAGE_VDS_STATS_FAILED, AuditLogSeverity.ERROR); Line 362: severities.put(AuditLogType.VDS_LOW_DISK_SPACE, AuditLogSeverity.WARNING); Line 363: severities.put(AuditLogType.VDS_LOW_DISK_SPACE_ERROR, AuditLogSeverity.ERROR); Line 364: severities.put(AuditLogType.VDS_ACTIVATE_ASYNC, AuditLogSeverity.NORMAL); Line 365: severities.put(AuditLogType.VDS_ACTIVATE_MANUAL_HA, AuditLogSeverity.WARNING); > duplicate line Done (should have been VDS_ACTIVATE_MANUAL_HA_ASYNC) Line 366: severities.put(AuditLogType.VDS_ACTIVATE_FAILED_ASYNC, AuditLogSeverity.NORMAL); Line 367: severities.put(AuditLogType.VDS_SET_NON_OPERATIONAL_VM_NETWORK_IS_BRIDGELESS, AuditLogSeverity.WARNING); Line 368: severities.put(AuditLogType.EMULATED_MACHINES_INCOMPATIBLE_WITH_CLUSTER, AuditLogSeverity.WARNING); Line 369: severities.put(AuditLogType.VDS_TIME_DRIFT_ALERT, AuditLogSeverity.WARNING); http://gerrit.ovirt.org/#/c/23531/3/backend/manager/modules/dal/src/main/resources/bundles/AuditLogMessages.properties File backend/manager/modules/dal/src/main/resources/bundles/AuditLogMessages.properties: Line 207: VDC_START=Starting oVirt Engine. Line 208: VDC_STOP=Stopping oVirt Engine. Line 209: VDS_ACTIVATE=Host ${VdsName} was activated by ${UserName}. Line 210: VDS_ACTIVATE_ASYNC=Host ${VdsName} was autorecovered. Line 211: VDS_ACTIVATE_MANUAL_HA=Host ${VdsName} was activated by ${UserName}, but Hosted Engine HA may still be in maintenance mode. If necessary, please correct this manually. > Looks a bit confusing to me. Done, used your suggested wording. Also fixed it in LocalizedEnums.properties. Line 212: VDS_ACTIVATE_MANUAL_HA_ASYNC=Host ${VdsName} was autorecovered, but Hosted Engine HA may still be in maintenance mode. If necessary, please correct this manually. Line 213: VDS_ACTIVATE_FAILED=Failed to activate Host ${VdsName}.(User: ${UserName}). Line 214: VDS_ACTIVATE_FAILED_ASYNC=Failed to autorecover Host ${VdsName}. Line 215: HOST_REFRESHED_CAPABILITIES=Successfully refreshed the capabilities of host ${VdsName}. http://gerrit.ovirt.org/#/c/23531/3/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmListModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmListModel.java: Line 1866: return; Line 1867: } Line 1868: Line 1869: SetHaMaintenanceParameters params = new SetHaMaintenanceParameters(vm.getRunOnVds(), HaMaintenanceMode.GLOBAL, enabled); Line 1870: Frontend.getInstance().runAction(VdcActionType.SetHaMaintenance, params, null, this); > minor: you can use runAction overload with type and params. Done Line 1871: } Line 1872: Line 1873: private void preSave() Line 1874: { Line 2388: VDS vds = response.getReturnValue(); Line 2389: setHaMaintenanceAvailability(vds.getHighlyAvailableIsConfigured()); Line 2390: } Line 2391: } Line 2392: }, true)); > I'm afraid we cannot query the server for each click, especially in the vm Removed the query... maybe it being a hosted engine vm on cluster version 3.4 is enough to qualify it for the maintenance action anyway. Line 2393: } Line 2394: Line 2395: private void setHaMaintenanceAvailability(boolean isAvailable) { Line 2396: getEnableGlobalHaMaintenanceCommand().setIsExecutionAllowed(isAvailable); -- To view, visit http://gerrit.ovirt.org/23531 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0f76f7ad63bcf6d7871c362b46cfa6e928eb9c74 Gerrit-PatchSet: 3 Gerrit-Project: ovirt-engine Gerrit-Branch: ovirt-engine-3.4 Gerrit-Owner: Greg Padgett <gpadg...@redhat.com> Gerrit-Reviewer: Doron Fediuck <dfedi...@redhat.com> Gerrit-Reviewer: Gilad Chaplik <gchap...@redhat.com> Gerrit-Reviewer: Greg Padgett <gpadg...@redhat.com> Gerrit-Reviewer: Itamar Heim <ih...@redhat.com> Gerrit-Reviewer: Jiří Moskovčák <jmosk...@redhat.com> Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
