Skip to content

Better tracking host maintanence and handling of migration jobs #3425

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.

Sign up for GitHub

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

Merged
andrijapanicsb merged 19 commits into from
Dec 19, 2019
Merged

Better tracking host maintanence and handling of migration jobs #3425

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
2a2374c
Service layer changes for new way of tracking maintanence progress
Jun 26, 2019
766373e
Fixes after offline code review
Jun 27, 2019
b7b0f77
Fix marvin tests
Jul 4, 2019
8a05052
Change state name and add documentation
Jul 12, 2019
aa43675
Fix test
Jul 17, 2019
8a84a4b
Fix and add more unit tests for different caseS
Jul 21, 2019
47bb147
Fix and enhance Marvin Tests
Jul 21, 2019
2e09eed
Fixes for corner cases
Nov 13, 2019
7142394
More fixes and logging
Nov 14, 2019
f6704fc
UI fixes
Nov 14, 2019
438c800
Some minor changes and reducing VMs on host for more contained tests
Nov 18, 2019
7b04362
Fixed ssh client auth problem causing test failure
Nov 19, 2019
43f592a
Code review changes + fixes + some more logging
Nov 25, 2019
4b380c7
Fix flaky tests by adding delays between host states
Nov 26, 2019
a789933
Added fetching only enabled hosts for tests
Nov 27, 2019
7e654b9
Make port blocking KVM specific and refactor to handle failure
Nov 27, 2019
06c4521
Make failing migrations due to tagged host instead of port blocking
Nov 27, 2019
ca792ef
Added additional check for migrating VMs
Nov 28, 2019
1c90d88
Refactor to use single place for methods checking maintenance states
Nov 28, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 35 additions & 6 deletions api/src/main/java/com/cloud/resource/ResourceState.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,23 +16,33 @@
// under the License.
package com.cloud.resource;

import java.util.Arrays;
import java.util.List;
import java.util.Set;

import com.cloud.utils.fsm.StateMachine;

public enum ResourceState {
Creating, Enabled, Disabled, PrepareForMaintenance, ErrorInMaintenance, Maintenance, Error;
Creating,
Enabled,
Disabled,
ErrorInPrepareForMaintenance,
PrepareForMaintenance,
ErrorInMaintenance,
Maintenance,
Error;

public enum Event {
InternalCreated("Resource is created"),
Enable("Admin enables"),
Disable("Admin disables"),
AdminAskMaintenace("Admin asks to enter maintenance"),
AdminAskMaintenance("Admin asks to enter maintenance"),
AdminCancelMaintenance("Admin asks to cancel maintenance"),
InternalEnterMaintenance("Resource enters maintenance"),
UpdatePassword("Admin updates password of host"),
UnableToMigrate("Management server migrates VM failed"),
UnableToMigrate("Migration of VM failed, such as from scheduled HAWork"),
UnableToMaintain("Management server has exhausted all legal operations and attempts to put into maintenance has failed"),
ErrorsCorrected("Errors were corrected on a resource attempting to enter maintenance but encountered errors"),
Error("An internal error happened"),
DeleteHost("Admin delete a host"),

Expand Down Expand Up @@ -84,6 +94,16 @@ public static String[] toString(ResourceState... states) {
return strs;
}

public static boolean isMaintenanceState(ResourceState state) {
return Arrays.asList(ResourceState.Maintenance, ResourceState.ErrorInMaintenance,
ResourceState.PrepareForMaintenance, ResourceState.ErrorInPrepareForMaintenance).contains(state);
}

public static boolean canAttemptMaintenance(ResourceState state) {
return !Arrays.asList(ResourceState.Maintenance, ResourceState.PrepareForMaintenance,
ResourceState.ErrorInPrepareForMaintenance).contains(state);
}

protected static final StateMachine<ResourceState, Event> s_fsm = new StateMachine<ResourceState, Event>();
static {
s_fsm.addTransition(null, Event.InternalCreated, ResourceState.Enabled);
Expand All @@ -92,22 +112,31 @@ public static String[] toString(ResourceState... states) {
s_fsm.addTransition(ResourceState.Enabled, Event.Enable, ResourceState.Enabled);
s_fsm.addTransition(ResourceState.Enabled, Event.InternalCreated, ResourceState.Enabled);
s_fsm.addTransition(ResourceState.Enabled, Event.Disable, ResourceState.Disabled);
s_fsm.addTransition(ResourceState.Enabled, Event.AdminAskMaintenace, ResourceState.PrepareForMaintenance);
s_fsm.addTransition(ResourceState.Enabled, Event.AdminAskMaintenance, ResourceState.PrepareForMaintenance);
s_fsm.addTransition(ResourceState.Enabled, Event.InternalEnterMaintenance, ResourceState.Maintenance);
s_fsm.addTransition(ResourceState.Disabled, Event.Enable, ResourceState.Enabled);
s_fsm.addTransition(ResourceState.Disabled, Event.Disable, ResourceState.Disabled);
s_fsm.addTransition(ResourceState.Disabled, Event.InternalCreated, ResourceState.Disabled);
s_fsm.addTransition(ResourceState.PrepareForMaintenance, Event.InternalEnterMaintenance, ResourceState.Maintenance);
s_fsm.addTransition(ResourceState.PrepareForMaintenance, Event.AdminCancelMaintenance, ResourceState.Enabled);
s_fsm.addTransition(ResourceState.PrepareForMaintenance, Event.UnableToMigrate, ResourceState.ErrorInMaintenance);
s_fsm.addTransition(ResourceState.PrepareForMaintenance, Event.UnableToMigrate, ResourceState.ErrorInPrepareForMaintenance);
s_fsm.addTransition(ResourceState.PrepareForMaintenance, Event.UnableToMaintain, ResourceState.ErrorInMaintenance);
s_fsm.addTransition(ResourceState.PrepareForMaintenance, Event.InternalCreated, ResourceState.PrepareForMaintenance);
s_fsm.addTransition(ResourceState.Maintenance, Event.AdminCancelMaintenance, ResourceState.Enabled);
s_fsm.addTransition(ResourceState.Maintenance, Event.InternalCreated, ResourceState.Maintenance);
s_fsm.addTransition(ResourceState.Maintenance, Event.DeleteHost, ResourceState.Disabled);
s_fsm.addTransition(ResourceState.ErrorInPrepareForMaintenance, Event.InternalCreated, ResourceState.ErrorInPrepareForMaintenance);
s_fsm.addTransition(ResourceState.ErrorInPrepareForMaintenance, Event.Disable, ResourceState.Disabled);
s_fsm.addTransition(ResourceState.ErrorInPrepareForMaintenance, Event.DeleteHost, ResourceState.Disabled);
s_fsm.addTransition(ResourceState.ErrorInPrepareForMaintenance, Event.InternalEnterMaintenance, ResourceState.Maintenance);
s_fsm.addTransition(ResourceState.ErrorInPrepareForMaintenance, Event.AdminCancelMaintenance, ResourceState.Enabled);
s_fsm.addTransition(ResourceState.ErrorInPrepareForMaintenance, Event.UnableToMigrate, ResourceState.ErrorInPrepareForMaintenance);
s_fsm.addTransition(ResourceState.ErrorInPrepareForMaintenance, Event.UnableToMaintain, ResourceState.ErrorInMaintenance);
s_fsm.addTransition(ResourceState.ErrorInPrepareForMaintenance, Event.ErrorsCorrected, ResourceState.PrepareForMaintenance);
s_fsm.addTransition(ResourceState.ErrorInMaintenance, Event.InternalCreated, ResourceState.ErrorInMaintenance);
s_fsm.addTransition(ResourceState.ErrorInMaintenance, Event.AdminAskMaintenance, ResourceState.PrepareForMaintenance);
s_fsm.addTransition(ResourceState.ErrorInMaintenance, Event.Disable, ResourceState.Disabled);
s_fsm.addTransition(ResourceState.ErrorInMaintenance, Event.DeleteHost, ResourceState.Disabled);
s_fsm.addTransition(ResourceState.ErrorInMaintenance, Event.InternalEnterMaintenance, ResourceState.Maintenance);
s_fsm.addTransition(ResourceState.ErrorInMaintenance, Event.AdminCancelMaintenance, ResourceState.Enabled);
s_fsm.addTransition(ResourceState.Error, Event.InternalCreated, ResourceState.Error);
s_fsm.addTransition(ResourceState.Disabled, Event.DeleteHost, ResourceState.Disabled);
Expand Down
22 changes: 13 additions & 9 deletions api/src/main/java/org/apache/cloudstack/api/command/admin/host/PrepareForMaintenanceCmd.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@
// under the License.
package org.apache.cloudstack.api.command.admin.host;

import org.apache.log4j.Logger;

import org.apache.cloudstack.api.APICommand;
import org.apache.cloudstack.api.ApiCommandJobType;
import org.apache.cloudstack.api.ApiConstants;
Expand All @@ -27,10 +25,12 @@
import org.apache.cloudstack.api.ServerApiException;
import org.apache.cloudstack.api.response.HostResponse;
import org.apache.cloudstack.context.CallContext;
import org.apache.log4j.Logger;

import com.cloud.event.EventTypes;
import com.cloud.host.Host;
import com.cloud.user.Account;
import com.cloud.utils.exception.CloudRuntimeException;

@APICommand(name = "prepareHostForMaintenance", description = "Prepares a host for maintenance.", responseObject = HostResponse.class,
requestHasSensitiveInfo = false, responseHasSensitiveInfo = false)
Expand Down Expand Up @@ -99,13 +99,17 @@ public Long getInstanceId() {

@Override
public void execute() {
Host result = _resourceService.maintain(this);
if (result != null) {
HostResponse response = _responseGenerator.createHostResponse(result);
response.setResponseName("host");
this.setResponseObject(response);
} else {
throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to prepare host for maintenance");
try {
Host result = _resourceService.maintain(this);
if (result != null) {
HostResponse response = _responseGenerator.createHostResponse(result);
response.setResponseName("host");
this.setResponseObject(response);
} else {
throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to prepare host for maintenance");
}
} catch (CloudRuntimeException exception) {
throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to prepare host for maintenance due to: " + exception.getMessage());
}
}
}
1 change: 1 addition & 0 deletions engine/components-api/src/main/java/com/cloud/ha/HighAvailabilityManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ enum Step {

boolean hasPendingHaWork(long vmId);

boolean hasPendingMigrationsWork(long vmId);
/**
* @return
*/
Expand Down
7 changes: 1 addition & 6 deletions engine/components-api/src/main/java/com/cloud/resource/ResourceManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,6 @@
*/
public interface ResourceManager extends ResourceService, Configurable {

ConfigKey<Integer> HostMaintenanceRetries = new ConfigKey<>("Advanced", Integer.class,

@yadvr yadvr Nov 21, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@anuragaw @nvazquez I thought we wanted to have a global setting to define max retries? Can you check the requirement and the implementation regarding this, or if any other setting is used instead?

@yadvr yadvr Nov 21, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Saw MaxRetries usage later in the code, in this case if we're deprecating/removing this ConfigKey, please add an upgrade path to delete the configuration from cloud.configuration as well.

@anuragaw anuragaw Nov 22, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. Will do.

@anuragaw anuragaw Nov 25, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

"host.maintenance.retries","20",
"Number of retries when preparing a host into Maintenance Mode is faulty before failing",
true, ConfigKey.Scope.Cluster);

ConfigKey<Boolean> KvmSshToAgentEnabled = new ConfigKey<>("Advanced", Boolean.class,
"kvm.ssh.to.agent","true",
"Number of retries when preparing a host into Maintenance Mode is faulty before failing",
Expand Down Expand Up @@ -97,7 +92,7 @@ public interface ResourceManager extends ResourceService, Configurable {

boolean umanageHost(long hostId);

boolean maintenanceFailed(long hostId);
boolean migrateAwayFailed(long hostId, long vmId);

public boolean maintain(final long hostId) throws AgentUnavailableException;

Expand Down
6 changes: 4 additions & 2 deletions engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1583,7 +1583,7 @@ protected void runInContext() {
final HostVO h = sc.find();
if (h != null) {
final ResourceState resourceState = h.getResourceState();
if (resourceState == ResourceState.Disabled || resourceState == ResourceState.Maintenance || resourceState == ResourceState.ErrorInMaintenance) {
if (resourceState == ResourceState.Disabled || resourceState == ResourceState.Maintenance) {
/*
* Host is in non-operation state, so no investigation and direct put agent to Disconnected
*/
Expand All @@ -1605,7 +1605,9 @@ protected void runInContext() {
}

final QueryBuilder<HostVO> sc = QueryBuilder.create(HostVO.class);
sc.and(sc.entity().getResourceState(), Op.IN, ResourceState.PrepareForMaintenance, ResourceState.ErrorInMaintenance);
sc.and(sc.entity().getResourceState(), Op.IN,
ResourceState.PrepareForMaintenance,
ResourceState.ErrorInPrepareForMaintenance);
final List<HostVO> hosts = sc.list();

for (final HostVO host : hosts) {
Expand Down
32 changes: 17 additions & 15 deletions ...ion/src/main/java/org/apache/cloudstack/engine/datacenter/entity/api/db/EngineHostVO.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,10 @@
// under the License.
package org.apache.cloudstack.engine.datacenter.entity.api.db;

import com.cloud.host.Status;
import com.cloud.hypervisor.Hypervisor.HypervisorType;
import com.cloud.resource.ResourceState;
import com.cloud.storage.Storage.StoragePoolType;
import com.cloud.utils.NumbersUtil;
import com.cloud.utils.db.GenericDao;
import com.cloud.utils.db.StateMachine;
import org.apache.cloudstack.api.Identity;
import org.apache.cloudstack.engine.datacenter.entity.api.DataCenterResourceEntity.State;
import org.apache.cloudstack.engine.datacenter.entity.api.DataCenterResourceEntity.State.Event;
import java.util.Date;
import java.util.List;
import java.util.Map;
import java.util.UUID;

import javax.persistence.Column;
import javax.persistence.DiscriminatorColumn;
Expand All @@ -42,10 +36,18 @@
import javax.persistence.Temporal;
import javax.persistence.TemporalType;
import javax.persistence.Transient;
import java.util.Date;
import java.util.List;
import java.util.Map;
import java.util.UUID;

import org.apache.cloudstack.api.Identity;
import org.apache.cloudstack.engine.datacenter.entity.api.DataCenterResourceEntity.State;
import org.apache.cloudstack.engine.datacenter.entity.api.DataCenterResourceEntity.State.Event;

import com.cloud.host.Status;
import com.cloud.hypervisor.Hypervisor.HypervisorType;
import com.cloud.resource.ResourceState;
import com.cloud.storage.Storage.StoragePoolType;
import com.cloud.utils.NumbersUtil;
import com.cloud.utils.db.GenericDao;
import com.cloud.utils.db.StateMachine;

@Entity
@Table(name = "host")
Expand Down Expand Up @@ -730,7 +732,7 @@ public boolean isDisabled() {

@Override
public boolean isInMaintenanceStates() {
return (getResourceState() == ResourceState.Maintenance || getResourceState() == ResourceState.ErrorInMaintenance || getResourceState() == ResourceState.PrepareForMaintenance);
return ResourceState.isMaintenanceState(getResourceState());
}

public long getUpdated() {
Expand Down
26 changes: 13 additions & 13 deletions engine/schema/src/main/java/com/cloud/host/HostVO.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,11 @@
// under the License.
package com.cloud.host;

import com.cloud.agent.api.VgpuTypesInfo;
import com.cloud.hypervisor.Hypervisor.HypervisorType;
import com.cloud.resource.ResourceState;
import com.cloud.storage.Storage.StoragePoolType;
import com.cloud.utils.NumbersUtil;
import com.cloud.utils.db.GenericDao;
import java.util.Date;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.UUID;

import javax.persistence.Column;
import javax.persistence.DiscriminatorColumn;
Expand All @@ -38,11 +37,13 @@
import javax.persistence.Temporal;
import javax.persistence.TemporalType;
import javax.persistence.Transient;
import java.util.Date;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.UUID;

import com.cloud.agent.api.VgpuTypesInfo;
import com.cloud.hypervisor.Hypervisor.HypervisorType;
import com.cloud.resource.ResourceState;
import com.cloud.storage.Storage.StoragePoolType;
import com.cloud.utils.NumbersUtil;
import com.cloud.utils.db.GenericDao;

@Entity
@Table(name = "host")
Expand Down Expand Up @@ -714,9 +715,8 @@ public void setResourceState(ResourceState state) {

@Override
public boolean isInMaintenanceStates() {
return (getResourceState() == ResourceState.Maintenance || getResourceState() == ResourceState.ErrorInMaintenance || getResourceState() == ResourceState.PrepareForMaintenance);
return ResourceState.isMaintenanceState(getResourceState());
}

@Override
public boolean isDisabled() {
return (getResourceState() == ResourceState.Disabled);
Expand Down
1 change: 1 addition & 0 deletions engine/schema/src/main/resources/META-INF/db/schema-41300to41400-cleanup.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,4 @@
-- Schema upgrade cleanup from 4.13.0.0 to 4.14.0.0
--;

DELETE FROM `cloud`.`configuration` WHERE name = 'host.maintenance.retries';
Loading