kvm: send unsupported answer only when applicable (#2714)

Throw specific NPE child when command is known not to be known. Add unit tests.
This commit is contained in:
dahn 2018-06-21 07:33:43 +02:00 committed by Rohit Yadav
parent d4e302dcc6
commit f02e402ebb
4 changed files with 55 additions and 9 deletions

View File

@ -29,6 +29,15 @@ import com.cloud.agent.api.Answer;
import com.cloud.agent.api.Command;
public abstract class RequestWrapper {
static public class CommandNotSupported extends NullPointerException {
public CommandNotSupported(String msg) {
super(msg);
}
public CommandNotSupported(String msg, Throwable cause) {
super(msg);
initCause(cause);
}
}
private static final Logger s_logger = Logger.getLogger(RequestWrapper.class);
@ -52,7 +61,7 @@ public abstract class RequestWrapper {
keepResourceClass = keepResourceClass2;
} catch (final ClassCastException e) {
throw new NullPointerException("No key found for '" + command.getClass() + "' in the Map!");
throw new CommandNotSupported("No key found for '" + command.getClass() + "' in the Map!");
}
}
return resource;
@ -69,14 +78,14 @@ public abstract class RequestWrapper {
final Class<? extends Command> commandClass2 = (Class<? extends Command>) keepCommandClass.getSuperclass();
if (commandClass2 == null) {
throw new NullPointerException("All the COMMAND hierarchy tree has been visited but no compliant key has been found for '" + commandClass + "'.");
throw new CommandNotSupported("All the COMMAND hierarchy tree has been visited but no compliant key has been found for '" + commandClass + "'.");
}
commandWrapper = resourceCommands.get(commandClass2);
keepCommandClass = commandClass2;
} catch (final ClassCastException e) {
throw new NullPointerException("No key found for '" + keepCommandClass.getClass() + "' in the Map!");
throw new CommandNotSupported("No key found for '" + keepCommandClass.getClass() + "' in the Map!");
} catch (final NullPointerException e) {
// Will now traverse all the resource hierarchy. Returning null
// is not a problem.
@ -102,7 +111,7 @@ public abstract class RequestWrapper {
final Class<? extends ServerResource> resourceClass2 = (Class<? extends ServerResource>) keepResourceClass.getSuperclass();
if (resourceClass2 == null) {
throw new NullPointerException("All the SERVER-RESOURCE hierarchy tree has been visited but no compliant key has been found for '" + command.getClass() + "'.");
throw new CommandNotSupported("All the SERVER-RESOURCE hierarchy tree has been visited but no compliant key has been found for '" + command.getClass() + "'.");
}
final Hashtable<Class<? extends Command>, CommandWrapper> resourceCommands2 = retrieveResource(command,
@ -110,10 +119,8 @@ public abstract class RequestWrapper {
keepResourceClass = resourceClass2;
commandWrapper = retrieveCommands(command.getClass(), resourceCommands2);
} catch (final ClassCastException e) {
throw new NullPointerException("No key found for '" + command.getClass() + "' in the Map!");
} catch (final NullPointerException e) {
throw e;
} catch (final ClassCastException | NullPointerException e) {
throw new CommandNotSupported("No key found for '" + command.getClass() + "' in the Map!", e);
}
}
return commandWrapper;

View File

@ -47,6 +47,7 @@ import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.parsers.ParserConfigurationException;
import com.cloud.resource.RequestWrapper;
import org.apache.cloudstack.storage.to.PrimaryDataStoreTO;
import org.apache.cloudstack.storage.to.VolumeObjectTO;
import org.apache.cloudstack.utils.hypervisor.HypervisorUtils;
@ -1432,13 +1433,21 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv
return true;
}
/**
* This finds a command wrapper to handle the command and executes it.
* If no wrapper is found an {@see UnsupportedAnswer} is sent back.
* Any other exceptions are to be caught and wrapped in an generic {@see Answer}, marked as failed.
*
* @param cmd the instance of a {@see Command} to execute.
* @return the for the {@see Command} appropriate {@see Answer} or {@see UnsupportedAnswer}
*/
@Override
public Answer executeRequest(final Command cmd) {
final LibvirtRequestWrapper wrapper = LibvirtRequestWrapper.getInstance();
try {
return wrapper.execute(cmd, this);
} catch (final Exception e) {
} catch (final RequestWrapper.CommandNotSupported cmde) {
return Answer.createUnsupportedCommandAnswer(cmd);
}
}

View File

@ -72,6 +72,9 @@ public class LibvirtRequestWrapper extends RequestWrapper {
commandWrapper = retryWhenAllFails(command, resourceClass, resourceCommands);
}
if (commandWrapper == null) {
throw new CommandNotSupported("No way to handle " + command.getClass());
}
return commandWrapper.execute(command, serverResource);
}
}

View File

@ -38,6 +38,8 @@ import javax.xml.xpath.XPathConstants;
import javax.xml.xpath.XPathExpressionException;
import javax.xml.xpath.XPathFactory;
import com.cloud.agent.api.Command;
import com.cloud.agent.api.UnsupportedAnswer;
import com.cloud.hypervisor.kvm.resource.LibvirtVMDef.CpuTuneDef;
import org.apache.commons.lang.SystemUtils;
import org.joda.time.Duration;
@ -5191,4 +5193,29 @@ public class LibvirtComputingResourceTest {
Assert.assertEquals(CpuTuneDef.MIN_QUOTA, cpuTuneDef.getQuota());
Assert.assertEquals((int) (CpuTuneDef.MIN_QUOTA / pct), cpuTuneDef.getPeriod());
}
@Test
public void testUnknownCommand() {
libvirtComputingResource = new LibvirtComputingResource();
Command cmd = new Command() {
@Override public boolean executeInSequence() {
return false;
}
};
Answer ans = libvirtComputingResource.executeRequest(cmd);
assertTrue(ans instanceof UnsupportedAnswer);
}
@Test
public void testKnownCommand() {
libvirtComputingResource = new LibvirtComputingResource();
Command cmd = new PingTestCommand() {
@Override public boolean executeInSequence() {
throw new NullPointerException("test succeeded");
}
};
Answer ans = libvirtComputingResource.executeRequest(cmd);
assertFalse(ans instanceof UnsupportedAnswer);
assertTrue(ans instanceof Answer);
}
}