Skip to content

agent: Detect existing hosts with UEFI support #6139

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
nvazquez merged 4 commits into from
Apr 5, 2022
Merged

agent: Detect existing hosts with UEFI support #6139

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
26b738a
agent: Pass uefi enabled status as part of ready command
davidjumani Mar 18, 2022
11fdae3
Cleanup
davidjumani Mar 18, 2022
c7da0fc
Fix checkstyle
davidjumani Mar 18, 2022
ccbd188
Save uefi status if different
davidjumani Mar 18, 2022
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
15 changes: 15 additions & 0 deletions core/src/main/java/com/cloud/agent/api/ReadyAnswer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,12 @@

package com.cloud.agent.api;

import java.util.Map;

public class ReadyAnswer extends Answer {

private Map<String, String> detailsMap;

protected ReadyAnswer() {
}

Expand All @@ -31,4 +36,14 @@ public ReadyAnswer(ReadyCommand cmd, String details) {
super(cmd, false, details);
}

public ReadyAnswer(ReadyCommand cmd, Map<String, String> detailsMap) {
super(cmd, true, null);
this.detailsMap = detailsMap;
}

public Map<String, String> getDetailsMap() {
return detailsMap;
}


}
15 changes: 15 additions & 0 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 @@ -600,6 +600,21 @@ protected AgentAttache notifyMonitorsOfConnection(final AgentAttache attache, fi
// return the attache instead of null, even it is disconnectede
handleDisconnectWithoutInvestigation(attache, Event.AgentDisconnected, true, true);
}
if (answer instanceof ReadyAnswer) {
ReadyAnswer readyAnswer = (ReadyAnswer)answer;
Map<String, String> detailsMap = readyAnswer.getDetailsMap();
if (detailsMap != null) {
String uefiEnabled = detailsMap.get(Host.HOST_UEFI_ENABLE);
s_logger.debug(String.format("Got HOST_UEFI_ENABLE [%s] for hostId [%s]:", uefiEnabled, host.getUuid()));
if (uefiEnabled != null) {
_hostDao.loadDetails(host);
if (!uefiEnabled.equals(host.getDetails().get(Host.HOST_UEFI_ENABLE))) {
host.getDetails().put(Host.HOST_UEFI_ENABLE, uefiEnabled);
_hostDao.saveDetails(host);
}
}
}
}

agentStatusTransitTo(host, Event.Ready, _nodeId);
attache.ready();
Expand Down
25 changes: 24 additions & 1 deletion ...m/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtReadyCommandWrapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,41 @@

package com.cloud.hypervisor.kvm.resource.wrapper;

import java.util.HashMap;
import java.util.Map;

import com.cloud.agent.api.Answer;
import com.cloud.agent.api.ReadyAnswer;
import com.cloud.agent.api.ReadyCommand;
import com.cloud.host.Host;
import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
import com.cloud.resource.CommandWrapper;
import com.cloud.resource.ResourceWrapper;
import com.cloud.utils.script.Script;

import org.apache.log4j.Logger;

@ResourceWrapper(handles = ReadyCommand.class)
public final class LibvirtReadyCommandWrapper extends CommandWrapper<ReadyCommand, Answer, LibvirtComputingResource> {

private static final Logger s_logger = Logger.getLogger(LibvirtReadyCommandWrapper.class);

@Override
public Answer execute(final ReadyCommand command, final LibvirtComputingResource libvirtComputingResource) {
return new ReadyAnswer(command);
Map<String, String> hostDetails = new HashMap<String, String>();

if (hostSupportsUefi()) {
hostDetails.put(Host.HOST_UEFI_ENABLE, Boolean.TRUE.toString());
}

return new ReadyAnswer(command, hostDetails);
}

private boolean hostSupportsUefi() {
String cmd = "rpm -qa | grep -i ovmf";
Copy link
Copy Markdown
Member

@yadvr yadvr Jan 12, 2023

Choose a reason for hiding this comment

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

cc @davidjumani @weizhouapache @DaanHoogland we may need to fix this as we support both EL and Ubuntu, rpm isn't installed by default on Ubuntu. Probably just check if /sys/firmware/efi using File::exists?

s_logger.debug("Running command : " + cmd);
int result = Script.runSimpleBashScriptForExitValue(cmd);
s_logger.debug("Got result : " + result);
return result == 0;
}
}
5 changes: 0 additions & 5 deletions .../hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/VmwareServerDiscoverer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@
import com.cloud.exception.DiscoveryException;
import com.cloud.exception.InvalidParameterValueException;
import com.cloud.exception.ResourceInUseException;
import com.cloud.host.Host;
import com.cloud.host.HostVO;
import com.cloud.hypervisor.Hypervisor;
import com.cloud.hypervisor.Hypervisor.HypervisorType;
Expand Down Expand Up @@ -367,10 +366,6 @@ public VmwareServerDiscoverer() {
details.put("url", hostMo.getHostName());
details.put("username", username);
details.put("password", password);
boolean uefiLegacySupported = hostMo.isUefiLegacySupported();
Copy link
Copy Markdown
Contributor

@nvazquez nvazquez Mar 18, 2022

Choose a reason for hiding this comment

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

I think these could be kept, same for other discoverers

Copy link
Copy Markdown
Contributor Author

@davidjumani davidjumani Mar 18, 2022

Choose a reason for hiding this comment

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

It would be redundant as the code to check the UEFI support is subsequently called while adding a host

if (uefiLegacySupported) {
details.put(Host.HOST_UEFI_ENABLE, "true");
}
String guid = morHost.getType() + ":" + morHost.getValue() + "@" + url.getHost();
details.put("guid", guid);

Expand Down
12 changes: 11 additions & 1 deletion ...hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@
import com.cloud.dc.Vlan;
import com.cloud.exception.CloudException;
import com.cloud.exception.InternalErrorException;
import com.cloud.host.Host;
import com.cloud.host.Host.Type;
import com.cloud.hypervisor.Hypervisor.HypervisorType;
import com.cloud.hypervisor.guru.VMwareGuru;
Expand Down Expand Up @@ -3930,8 +3931,17 @@ protected Answer execute(ReadyCommand cmd) {
try {
VmwareContext context = getServiceContext();
VmwareHypervisorHost hyperHost = getHyperHost(context);

Map<String, String> hostDetails = new HashMap<String, String>();
ManagedObjectReference morHost = hyperHost.getMor();
HostMO hostMo = new HostMO(context, morHost);
boolean uefiLegacySupported = hostMo.isUefiLegacySupported();
if (uefiLegacySupported) {
hostDetails.put(Host.HOST_UEFI_ENABLE, Boolean.TRUE.toString());
}

if (hyperHost.isHyperHostConnected()) {
return new ReadyAnswer(cmd);
return new ReadyAnswer(cmd, hostDetails);
} else {
return new ReadyAnswer(cmd, "Host is not in connect state");
}
Expand Down
3 changes: 0 additions & 3 deletions ...enserver/src/main/java/com/cloud/hypervisor/xenserver/discoverer/XcpServerDiscoverer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -321,9 +321,6 @@ protected boolean poolHasHotFix(Connection conn, String hostIp, String hotFixUui
details.put("username", username);
params.put("username", username);
details.put("password", password);
if (isUefiSupported(prodVersion)) {
details.put(com.cloud.host.Host.HOST_UEFI_ENABLE, Boolean.TRUE.toString());
}
params.put("password", password);
params.put("zone", Long.toString(dcId));
params.put("guid", record.uuid);
Expand Down
4 changes: 0 additions & 4 deletions ...s/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
// under the License.
package com.cloud.hypervisor.xenserver.resource;

import static com.cloud.hypervisor.xenserver.discoverer.XcpServerDiscoverer.isUefiSupported;
import static com.cloud.utils.NumbersUtil.toHumanReadableSize;

import java.io.BufferedReader;
Expand Down Expand Up @@ -1779,9 +1778,6 @@ protected void fillHostInfo(final Connection conn, final StartupRoutingCommand c
}
details.put("product_brand", productBrand);
details.put("product_version", _host.getProductVersion());
if (isUefiSupported(_host.getProductVersion())) {
details.put(com.cloud.host.Host.HOST_UEFI_ENABLE, Boolean.TRUE.toString());
}
if (hr.softwareVersion.get("product_version_text_short") != null) {
details.put("product_version_text_short", hr.softwareVersion.get("product_version_text_short"));
cmd.setHypervisorVersion(hr.softwareVersion.get("product_version_text_short"));
Expand Down
13 changes: 12 additions & 1 deletion ...va/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixReadyCommandWrapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,19 @@

package com.cloud.hypervisor.xenserver.resource.wrapper.xenbase;

import java.util.Map;
import java.util.HashMap;
import java.util.Set;

import org.apache.log4j.Logger;
import org.apache.xmlrpc.XmlRpcException;

import static com.cloud.hypervisor.xenserver.discoverer.XcpServerDiscoverer.isUefiSupported;

import com.cloud.agent.api.Answer;
import com.cloud.agent.api.ReadyAnswer;
import com.cloud.agent.api.ReadyCommand;
import com.cloud.hypervisor.xenserver.resource.CitrixHelper;
import com.cloud.hypervisor.xenserver.resource.CitrixResourceBase;
import com.cloud.resource.CommandWrapper;
import com.cloud.resource.ResourceWrapper;
Expand All @@ -44,6 +49,7 @@ public final class CitrixReadyCommandWrapper extends CommandWrapper<ReadyCommand
public Answer execute(final ReadyCommand command, final CitrixResourceBase citrixResourceBase) {
final Connection conn = citrixResourceBase.getConnection();
final Long dcId = command.getDataCenterId();
Map<String, String> hostDetails = new HashMap<String, String>();
// Ignore the result of the callHostPlugin. Even if unmounting the
// snapshots dir fails, let Ready command
// succeed.
Expand All @@ -55,6 +61,11 @@ public Answer execute(final ReadyCommand command, final CitrixResourceBase citri
final Host host = Host.getByUuid(conn, citrixResourceBase.getHost().getUuid());
final Set<VM> vms = host.getResidentVMs(conn);
citrixResourceBase.destroyPatchVbd(conn, vms);

final Host.Record hr = host.getRecord(conn);
if (isUefiSupported(CitrixHelper.getProductVersion(hr))) {
hostDetails.put(com.cloud.host.Host.HOST_UEFI_ENABLE, Boolean.TRUE.toString());
}
} catch (final Exception e) {
}
try {
Expand All @@ -70,6 +81,6 @@ public Answer execute(final ReadyCommand command, final CitrixResourceBase citri
return new ReadyAnswer(command, "Unable to cleanup halted vms");
}

return new ReadyAnswer(command);
return new ReadyAnswer(command, hostDetails);
}
}
8 changes: 0 additions & 8 deletions server/src/main/java/com/cloud/hypervisor/kvm/discoverer/LibvirtServerDiscoverer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,8 +214,6 @@ private void setupAgentSecurity(final Connection sshConnection, final String age
@Override
public Map<? extends ServerResource, Map<String, String>>
find(long dcId, Long podId, Long clusterId, URI uri, String username, String password, List<String> hostTags) throws DiscoveryException {
boolean isUefiSupported = false;

ClusterVO cluster = _clusterDao.findById(clusterId);
if (cluster == null || cluster.getHypervisorType() != getHypervisorType()) {
if (s_logger.isInfoEnabled())
Expand Down Expand Up @@ -280,11 +278,6 @@ private void setupAgentSecurity(final Connection sshConnection, final String age
throw new DiscoveredWithErrorException(errorMsg);
}

if (SSHCmdHelper.sshExecuteCmd(sshConnection, "rpm -qa | grep -i ovmf", 3)) {
s_logger.debug("It's UEFI enabled KVM machine");
isUefiSupported = true;
}

List<PhysicalNetworkSetupInfo> netInfos = _networkMgr.getPhysicalNetworkInfo(dcId, getHypervisorType());
String kvmPrivateNic = null;
String kvmPublicNic = null;
Expand Down Expand Up @@ -368,7 +361,6 @@ private void setupAgentSecurity(final Connection sshConnection, final String age
Map<String, String> hostDetails = connectedHost.getDetails();
hostDetails.put("password", password);
hostDetails.put("username", username);
hostDetails.put(Host.HOST_UEFI_ENABLE, isUefiSupported == true ? Boolean.toString(true) : Boolean.toString(false));
_hostDao.saveDetails(connectedHost);
return resources;
} catch (DiscoveredWithErrorException e) {
Expand Down