Skip to content
This repository has been archived by the owner on Jul 25, 2020. It is now read-only.

Checks existence of node before getting SecurityGroups and Images #360

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -115,14 +115,15 @@ public Image call() throws Exception {
URI uri = api.getVirtualMachineApi(resourceGroupName)
.capture(regionAndId.id(), cloneTemplate.getName(), CONTAINER_NAME);
checkState(uri != null && imageAvailablePredicate.apply(uri),
"Image %s was not created within the configured time limit", cloneTemplate.getName());
"Image for node %s was not created within the configured time limit", cloneTemplate.getName());

List<ResourceDefinition> definitions = api.getJobApi().captureStatus(uri);
checkState(definitions.size() == 1,
"Expected one resource definition after creating the image but %s were returned", definitions.size());

Image image = resourceDefinitionToImage.create(cloneTemplate.getSourceNodeId(), cloneTemplate.getName())
.apply(definitions.get(0));
checkState(image != null, "Image for node %s was not created", cloneTemplate.getSourceNodeId());
logger.debug(">> created %s", image);
return image;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,9 @@ public Set<SecurityGroup> listSecurityGroupsForNode(String nodeId) {
ResourceGroup resourceGroup = resourceGroupMap.getUnchecked(regionAndId.region());

VirtualMachine vm = api.getVirtualMachineApi(resourceGroup.name()).get(regionAndId.id());
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you use checkNotNull, maybe?

Copy link
Member

Choose a reason for hiding this comment

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

I'd say it is ok as-is? taht check would throw a NPE, but in this case an IAE makes more sense?

if (vm == null) {
throw new IllegalArgumentException("Node " + regionAndId.id() + " was not found");
}
List<IdReference> networkInterfacesIdReferences = vm.properties().networkProfile().networkInterfaces();
List<NetworkSecurityGroup> networkGroups = new ArrayList<NetworkSecurityGroup>();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,9 @@ public Image apply(ResourceDefinition input) {
ResourceGroup resourceGroup = resourceGroupMap.getUnchecked(regionAndId.region());

VirtualMachine vm = api.getVirtualMachineApi(resourceGroup.name()).get(regionAndId.id());
if (vm == null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above

return null;
}
String storageAccountName = storageProfileToStorageAccountName.apply(vm.properties().storageProfile());

VMImage.Builder builder = VMImage.customImage().group(resourceGroup.name()).storage(storageAccountName)
Expand Down