Don't attempt to delete public IP address resource when using private IP (#381)

Author: Minreaux

.gitignore

@@ -2,6 +2,7 @@
 /target
 /work
 /.checkstyle
+/.vscode
 
 *.idea
 *.settings

src/main/java/com/microsoft/azure/vmagent/AzureVMCloud.java

@@ -913,7 +913,8 @@ private void handleFailure(
                                 try {
                                     getServiceDelegate().terminateVirtualMachine(
                                             vmName,
-                                            template.getResourceGroupName());
+                                            template.getResourceGroupName(),
+                                            template.getUsePrivateIP());
                                 } catch (AzureCloudException terminateEx) {
                                     LOGGER.log(
                                             Level.SEVERE,

src/main/java/com/microsoft/azure/vmagent/AzureVMManagementServiceDelegate.java

@@ -1767,7 +1767,7 @@ public void shutdownVirtualMachine(AzureVMAgent agent) {
     public static void terminateVirtualMachine(AzureVMAgent agent) throws AzureCloudException {
         AzureVMManagementServiceDelegate delegate = agent.getServiceDelegate();
         if (delegate != null) {
-            delegate.terminateVirtualMachine(agent.getNodeName(), agent.getResourceGroupName(), new ExecutionEngine());
+            delegate.terminateVirtualMachine(agent.getNodeName(), agent.getResourceGroupName(), agent.getTemplate().getUsePrivateIP(), new ExecutionEngine());
         }
     }
 
@@ -1779,8 +1779,9 @@ public static void terminateVirtualMachine(AzureVMAgent agent) throws AzureCloud
      */
     public void terminateVirtualMachine(
             String vmName,
-            String resourceGroupName) throws AzureCloudException {
-        terminateVirtualMachine(vmName, resourceGroupName, new ExecutionEngine());
+            String resourceGroupName,
+            boolean usePrivateIP) throws AzureCloudException {
+        terminateVirtualMachine(vmName, resourceGroupName, usePrivateIP, new ExecutionEngine());
     }
 
     /**
@@ -1792,6 +1793,7 @@ public void terminateVirtualMachine(
     public void terminateVirtualMachine(
             final String vmName,
             final String resourceGroupName,
+            final boolean usePrivateIP,
             ExecutionEngine executionEngine) throws AzureCloudException {
         try {
             if (virtualMachineExists(vmName, resourceGroupName)) {
@@ -1835,7 +1837,7 @@ public void terminateVirtualMachine(
         } finally {
             LOGGER.log(Level.INFO, "Clean operation starting for {0} NIC and IP", vmName);
             executionEngine.executeAsync((Callable<Void>) () -> {
-                removeIPName(resourceGroupName, vmName);
+                removeIPName(resourceGroupName, vmName, usePrivateIP);
                 return null;
             }, new NoRetryStrategy());
         }
@@ -1883,7 +1885,8 @@ public void removeStorageBlob(URI blobURI, String resourceGroupName) throws Exce
      *
      */
     public void removeIPName(String resourceGroupName,
-                             String vmName) {
+                             String vmName,
+                             boolean usePrivateIP) {
         final String nic = vmName + "NIC";
         try {
             LOGGER.log(Level.INFO, "Remove NIC {0}", nic);
@@ -1894,8 +1897,11 @@ public void removeIPName(String resourceGroupName,
 
         final String ip = vmName + "IPName";
         try {
-            LOGGER.log(Level.INFO, "Remove IP {0}", ip);
-            azureClient.publicIpAddresses().deleteByResourceGroup(resourceGroupName, ip);
+            // Does not attempt to remove public IP if private IP is used because public IP does not exist
+            if (!usePrivateIP) {
+                LOGGER.log(Level.INFO, "Remove IP {0}", ip);
+                azureClient.publicIpAddresses().deleteByResourceGroup(resourceGroupName, ip);
+            }
         } catch (Exception e) {
             LOGGER.log(Level.WARNING, "Exception while deleting IPName", e);
         }

src/test/java/com/microsoft/azure/vmagent/ITAzureVMManagementServiceDelegate.java

@@ -719,7 +719,7 @@ public void terminateVirtualMachineTest() {
             VirtualMachine vm = createAzureVM(vmName);
             ExecutionEngine executionEngineMock = mock(ExecutionEngine.class);
 
-            delegate.terminateVirtualMachine(vmName, testEnv.azureResourceGroup, executionEngineMock);
+            delegate.terminateVirtualMachine(vmName, testEnv.azureResourceGroup, false, executionEngineMock);
 
             verify(executionEngineMock).executeAsync(any(Callable.class), any(RetryStrategy.class));
 
@@ -738,7 +738,7 @@ public void terminateVirtualMachineTest_MissingVM() {
             ExecutionEngine executionEngineMock = mock(ExecutionEngine.class);
 
             //VM is missing so terminateVirtualMachine should be a no-op and no exception should be thrown
-            delegate.terminateVirtualMachine(vmName, testEnv.azureResourceGroup, executionEngineMock);
+            delegate.terminateVirtualMachine(vmName, testEnv.azureResourceGroup, false, executionEngineMock);
             verify(executionEngineMock).executeAsync(any(Callable.class), any(RetryStrategy.class));
 
         } catch (Exception e) {
@@ -753,7 +753,7 @@ public void removeIPNameTest() {
             final AzureVMDeploymentInfo deploymentInfo = createDefaultDeployment(1, null);
             final String nodeName = deploymentInfo.getVmBaseName() + "0";
 
-            delegate.removeIPName(testEnv.azureResourceGroup, nodeName);
+            delegate.removeIPName(testEnv.azureResourceGroup, nodeName, false);
             //should fail because the VM is still using them
             Assert.assertNotNull(
                     azureClient
@@ -768,7 +768,7 @@ public void removeIPNameTest() {
 
             // destroy the vm first
             azureClient.virtualMachines().deleteByResourceGroup(testEnv.azureResourceGroup, nodeName);
-            delegate.removeIPName(testEnv.azureResourceGroup, nodeName);
+            delegate.removeIPName(testEnv.azureResourceGroup, nodeName, false);
             ManagementException managementException = assertThrows(ManagementException.class, () ->
                     azureClient
                             .publicIpAddresses()