DaanHoogland commented on code in PR #7170:
URL: https://github.com/apache/cloudstack/pull/7170#discussion_r1152849220
##########
server/src/main/java/com/cloud/resource/ResourceManagerImpl.java:
##########
@@ -1774,73 +1780,149 @@ public boolean checkAndMaintain(final long hostId) {
return hostInMaintenance;
}
+ private ResourceState.Event
getResourceEventFromAllocationStateString(String allocationState) {
+ final ResourceState.Event resourceEvent =
ResourceState.Event.toEvent(allocationState);
+ if (resourceEvent != ResourceState.Event.Enable && resourceEvent !=
ResourceState.Event.Disable) {
+ throw new InvalidParameterValueException(String.format("Invalid
allocation state: %s, " +
+ "only Enable/Disable are allowed", allocationState));
+ }
+ return resourceEvent;
+ }
+
+ private void handleAutoEnableDisableKVMHost(boolean
autoEnableDisableKVMSetting,
+ boolean
isUpdateFromHostHealthCheck,
+ HostVO host, DetailVO
hostDetail,
+ ResourceState.Event
resourceEvent) {
+ if (autoEnableDisableKVMSetting) {
+ if (!isUpdateFromHostHealthCheck && hostDetail != null &&
+ !Boolean.parseBoolean(hostDetail.getValue()) &&
resourceEvent == ResourceState.Event.Enable) {
+ hostDetail.setValue(Boolean.TRUE.toString());
+ _hostDetailsDao.update(hostDetail.getId(), hostDetail);
+ } else if (!isUpdateFromHostHealthCheck && hostDetail != null &&
+ Boolean.parseBoolean(hostDetail.getValue()) &&
resourceEvent == ResourceState.Event.Disable) {
+ s_logger.info(String.format("The setting %s is enabled but the
host %s is manually set into %s state," +
+ "ignoring future auto enabling of the host
based on health check results",
+ AgentManager.EnableKVMAutoEnableDisable.key(),
host.getName(), resourceEvent));
+ hostDetail.setValue(Boolean.FALSE.toString());
+ _hostDetailsDao.update(hostDetail.getId(), hostDetail);
+ } else if (hostDetail == null) {
+ String autoEnableValue = !isUpdateFromHostHealthCheck ?
Boolean.FALSE.toString() : Boolean.TRUE.toString();
+ hostDetail = new DetailVO(host.getId(),
ApiConstants.AUTO_ENABLE_KVM_HOST, autoEnableValue);
+ _hostDetailsDao.persist(hostDetail);
+ }
+ }
+ }
+ private boolean updateHostAllocationState(HostVO host, String
allocationState,
+ boolean
isUpdateFromHostHealthCheck) throws NoTransitionException {
+ boolean autoEnableDisableKVMSetting =
AgentManager.EnableKVMAutoEnableDisable.valueIn(host.getClusterId()) &&
+ host.getHypervisorType() == HypervisorType.KVM;
+ ResourceState.Event resourceEvent =
getResourceEventFromAllocationStateString(allocationState);
+ DetailVO hostDetail = _hostDetailsDao.findDetail(host.getId(),
ApiConstants.AUTO_ENABLE_KVM_HOST);
+
+ if ((host.getResourceState() == ResourceState.Enabled && resourceEvent
== ResourceState.Event.Enable) ||
+ (host.getResourceState() == ResourceState.Disabled &&
resourceEvent == ResourceState.Event.Disable)) {
+ s_logger.info(String.format("The host %s is already on the
allocated state", host.getName()));
+ return false;
+ }
+
+ if (isAutoEnableAttemptForADisabledHost(autoEnableDisableKVMSetting,
isUpdateFromHostHealthCheck, hostDetail, resourceEvent)) {
+ s_logger.debug(String.format("The setting '%s' is enabled and the
health check succeeds on the host, " +
+ "but the host has been manually disabled
previously, ignoring auto enabling",
+ AgentManager.EnableKVMAutoEnableDisable.key()));
+ return false;
+ }
+
+ handleAutoEnableDisableKVMHost(autoEnableDisableKVMSetting,
isUpdateFromHostHealthCheck, host,
+ hostDetail, resourceEvent);
+
+ resourceStateTransitTo(host, resourceEvent, _nodeId);
+ return true;
+ }
+
+ private boolean isAutoEnableAttemptForADisabledHost(boolean
autoEnableDisableKVMSetting,
+ boolean
isUpdateFromHostHealthCheck,
+ DetailVO hostDetail,
ResourceState.Event resourceEvent) {
+ return autoEnableDisableKVMSetting && isUpdateFromHostHealthCheck &&
hostDetail != null &&
+ !Boolean.parseBoolean(hostDetail.getValue()) && resourceEvent
== ResourceState.Event.Enable;
+ }
+
+ private void updateHostName(HostVO host, String name) {
+ s_logger.debug("Updating Host name to: " + name);
+ host.setName(name);
+ _hostDao.update(host.getId(), host);
+ }
+
+ private void updateHostGuestOSCategory(Long hostId, Long
guestOSCategoryId) {
+ // Verify that the guest OS Category exists
+ if (!(guestOSCategoryId > 0) ||
_guestOSCategoryDao.findById(guestOSCategoryId) == null) {
+ throw new InvalidParameterValueException("Please specify a valid
guest OS category.");
+ }
+
+ final GuestOSCategoryVO guestOSCategory =
_guestOSCategoryDao.findById(guestOSCategoryId);
+ final DetailVO guestOSDetail = _hostDetailsDao.findDetail(hostId,
"guest.os.category.id");
+
+ if (guestOSCategory != null &&
!GuestOSCategoryVO.CATEGORY_NONE.equalsIgnoreCase(guestOSCategory.getName())) {
+ // Create/Update an entry for guest.os.category.id
+ if (guestOSDetail != null) {
+
guestOSDetail.setValue(String.valueOf(guestOSCategory.getId()));
+ _hostDetailsDao.update(guestOSDetail.getId(), guestOSDetail);
+ } else {
+ final Map<String, String> detail = new HashMap<String,
String>();
+ detail.put("guest.os.category.id",
String.valueOf(guestOSCategory.getId()));
+ _hostDetailsDao.persist(hostId, detail);
+ }
+ } else {
+ // Delete any existing entry for guest.os.category.id
+ if (guestOSDetail != null) {
+ _hostDetailsDao.remove(guestOSDetail.getId());
+ }
+ }
+ }
+
+ private void updateHostTags(HostVO host, Long hostId, List<String>
hostTags) {
+ List<VMInstanceVO> activeVMs = _vmDao.listByHostId(hostId);
+ s_logger.warn(String.format("The following active VMs [%s] are using
the host [%s]. " +
+ "Updating the host tags will not affect them.", activeVMs,
host));
+
+ if (s_logger.isDebugEnabled()) {
+ s_logger.debug("Updating Host Tags to :" + hostTags);
+ }
+ _hostTagsDao.persist(hostId, new ArrayList<>(new HashSet<>(hostTags)));
+ }
+
@Override
public Host updateHost(final UpdateHostCmd cmd) throws
NoTransitionException {
- Long hostId = cmd.getId();
- String name = cmd.getName();
- Long guestOSCategoryId = cmd.getOsCategoryId();
+ return updateHost(cmd.getId(), cmd.getName(), cmd.getOsCategoryId(),
+ cmd.getAllocationState(), cmd.getUrl(), cmd.getHostTags(),
cmd.getAnnotation(), false);
+ }
+ private Host updateHost(Long hostId, String name, Long guestOSCategoryId,
String allocationState,
+ String url, List<String> hostTags, String
annotation, boolean isUpdateFromHostHealthCheck) throws NoTransitionException {
Review Comment:
No big problem with the code but ;
why maintain the second method? Can we just pass the cmd as parameter object
and do all teh gets in the method?
--
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]
