diff --git a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterActionWorker.java b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterActionWorker.java index 4fcf6fa7686d..baa1c93ca551 100644 --- a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterActionWorker.java +++ b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterActionWorker.java @@ -761,6 +761,25 @@ protected void copyScriptFile(String nodeAddress, final int sshPort, File file, } } + /** + * Deletes the local temporary copies of the scripts created by {@link #retrieveScriptFiles()} + * once they have been copied to the cluster node(s). Without this, every deploy/autoscale/PV-cleanup + * action leaks a *.sh file in the management server's tmp directory. + */ + protected void cleanupScriptFiles() { + deleteScriptFileQuietly(deploySecretsScriptFile); + deleteScriptFileQuietly(deployProviderScriptFile); + deleteScriptFileQuietly(deployCsiDriverScriptFile); + deleteScriptFileQuietly(deletePvScriptFile); + deleteScriptFileQuietly(autoscaleScriptFile); + } + + protected void deleteScriptFileQuietly(File file) { + if (file != null && file.exists() && !file.delete()) { + logger.debug("Failed to delete temporary Kubernetes cluster script file: {}", file.getAbsolutePath()); + } + } + protected boolean taintControlNodes() { StringBuilder commands = new StringBuilder(); List vmMapVOList = getKubernetesClusterVMMaps(); @@ -820,7 +839,11 @@ protected boolean deployProvider() { if (!result.first()) { logMessage(Level.INFO, "Provider files missing. Adding them now", null); retrieveScriptFiles(); - copyScripts(publicIpAddress, sshPort); + try { + copyScripts(publicIpAddress, sshPort); + } finally { + cleanupScriptFiles(); + } if (!createCloudStackSecret(keys)) { logTransitStateAndThrow(Level.ERROR, String.format("Failed to setup keys for Kubernetes cluster %s", @@ -857,7 +880,11 @@ protected boolean deployCsiDriver() { if (!result.first()) { logMessage(Level.INFO, "CSI files missing. Adding them now", null); retrieveScriptFiles(); - copyScripts(publicIpAddress, sshPort); + try { + copyScripts(publicIpAddress, sshPort); + } finally { + cleanupScriptFiles(); + } if (!createCloudStackSecret(keys)) { logTransitStateAndThrow(Level.ERROR, String.format("Failed to setup keys for Kubernetes cluster %s", diff --git a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterResourceModifierActionWorker.java b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterResourceModifierActionWorker.java index a1b2294bcd4d..167f9a80d228 100644 --- a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterResourceModifierActionWorker.java +++ b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterResourceModifierActionWorker.java @@ -951,7 +951,11 @@ protected boolean autoscaleCluster(boolean enable, Long minSize, Long maxSize) { if (!result.first()) { logMessage(Level.INFO, "Autoscaling files missing. Adding them now", null); retrieveScriptFiles(); - copyScripts(publicIpAddress, sshPort); + try { + copyScripts(publicIpAddress, sshPort); + } finally { + cleanupScriptFiles(); + } if (!createCloudStackSecret(keys)) { logTransitStateAndThrow(Level.ERROR, String.format("Failed to setup keys for Kubernetes cluster %s", @@ -1003,19 +1007,23 @@ public boolean deletePVsWithReclaimPolicyDelete() { if (Boolean.FALSE.equals(result.first())) { logMessage(Level.INFO, "PV delete script missing. Adding it now", null); retrieveScriptFiles(); - if (deletePvScriptFile != null) { - copyScriptFile(publicIpAddress, sshPort, deletePvScriptFile, deletePvScriptFilename); - logMessage(Level.INFO, "Executing PV deletion script (this may take several minutes)...", null); - result = SshHelper.sshExecute(publicIpAddress, sshPort, getControlNodeLoginUser(), - pkFile, null, command, 10000, 10000, 600000); // 10 minute timeout - if (Boolean.FALSE.equals(result.first())) { - logMessage(Level.ERROR, "PV deletion script failed: " + result.second(), null); - throw new CloudRuntimeException(result.second()); + try { + if (deletePvScriptFile != null) { + copyScriptFile(publicIpAddress, sshPort, deletePvScriptFile, deletePvScriptFilename); + logMessage(Level.INFO, "Executing PV deletion script (this may take several minutes)...", null); + result = SshHelper.sshExecute(publicIpAddress, sshPort, getControlNodeLoginUser(), + pkFile, null, command, 10000, 10000, 600000); // 10 minute timeout + if (Boolean.FALSE.equals(result.first())) { + logMessage(Level.ERROR, "PV deletion script failed: " + result.second(), null); + throw new CloudRuntimeException(result.second()); + } + logMessage(Level.INFO, "PV deletion script completed successfully", null); + } else { + logMessage(Level.WARN, "PV delete script file not found in resources, skipping PV deletion", null); + return false; } - logMessage(Level.INFO, "PV deletion script completed successfully", null); - } else { - logMessage(Level.WARN, "PV delete script file not found in resources, skipping PV deletion", null); - return false; + } finally { + cleanupScriptFiles(); } } else { logMessage(Level.INFO, "PV deletion script completed successfully", null); diff --git a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterUpgradeWorker.java b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterUpgradeWorker.java index 4c2725fc2a2f..2498e5493c5c 100644 --- a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterUpgradeWorker.java +++ b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterUpgradeWorker.java @@ -63,6 +63,12 @@ protected void retrieveScriptFiles() { upgradeScriptFile = retrieveScriptFile(upgradeScriptFilename); } + @Override + protected void cleanupScriptFiles() { + super.cleanupScriptFiles(); + deleteScriptFileQuietly(upgradeScriptFile); + } + private Pair runInstallScriptOnVM(final UserVm vm, final int index) throws Exception { int nodeSshPort = sshPort == 22 ? sshPort : sshPort + index; String nodeAddress = (index > 0 && sshPort == 22) ? vm.getPrivateIpAddress() : publicIpAddress; @@ -176,7 +182,11 @@ public boolean upgradeCluster() throws CloudRuntimeException { retrieveScriptFiles(); stateTransitTo(kubernetesCluster.getId(), KubernetesCluster.Event.UpgradeRequested); attachIsoKubernetesVMs(clusterVMs, upgradeVersion); - upgradeKubernetesClusterNodes(); + try { + upgradeKubernetesClusterNodes(); + } finally { + cleanupScriptFiles(); + } detachIsoKubernetesVMs(clusterVMs); KubernetesClusterVO kubernetesClusterVO = kubernetesClusterDao.findById(kubernetesCluster.getId()); kubernetesClusterVO.setKubernetesVersionId(upgradeVersion.getId()); diff --git a/plugins/integrations/kubernetes-service/src/test/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterActionWorkerTest.java b/plugins/integrations/kubernetes-service/src/test/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterActionWorkerTest.java index a25ec55cc04a..051d05bf1889 100644 --- a/plugins/integrations/kubernetes-service/src/test/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterActionWorkerTest.java +++ b/plugins/integrations/kubernetes-service/src/test/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterActionWorkerTest.java @@ -16,6 +16,7 @@ // under the License. package com.cloud.kubernetes.cluster.actionworkers; +import java.io.File; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -229,4 +230,48 @@ public void testGetMergedAffinityGroupIdsExplicitDedicationAlreadyInList() { Assert.assertTrue(result.contains(99L)); Assert.assertTrue(result.contains(2L)); } + + @Test + public void testCleanupScriptFilesDeletesAllTempFiles() throws Exception { + actionWorker.deploySecretsScriptFile = File.createTempFile("deploy-cloudstack-secret", ".sh"); + actionWorker.deployProviderScriptFile = File.createTempFile("deploy-provider", ".sh"); + actionWorker.deployCsiDriverScriptFile = File.createTempFile("deploy-csi-driver", ".sh"); + actionWorker.deletePvScriptFile = File.createTempFile("delete-pv-reclaimpolicy-delete", ".sh"); + actionWorker.autoscaleScriptFile = File.createTempFile("autoscale-kube-cluster", ".sh"); + + List files = Arrays.asList(actionWorker.deploySecretsScriptFile, actionWorker.deployProviderScriptFile, + actionWorker.deployCsiDriverScriptFile, actionWorker.deletePvScriptFile, actionWorker.autoscaleScriptFile); + for (File f : files) { + Assert.assertTrue(f.exists()); + } + + actionWorker.cleanupScriptFiles(); + + for (File f : files) { + Assert.assertFalse("Temporary script file should have been deleted: " + f.getAbsolutePath(), f.exists()); + } + } + + @Test + public void testDeleteScriptFileQuietlyHandlesNullAndMissingFiles() throws Exception { + // null must not throw + actionWorker.deleteScriptFileQuietly(null); + + // a File that does not exist must not throw + File missing = new File(System.getProperty("java.io.tmpdir"), "cks-nonexistent-" + UUID.randomUUID() + ".sh"); + Assert.assertFalse(missing.exists()); + actionWorker.deleteScriptFileQuietly(missing); + + // an existing file is deleted + File present = File.createTempFile("cks-present", ".sh"); + Assert.assertTrue(present.exists()); + actionWorker.deleteScriptFileQuietly(present); + Assert.assertFalse(present.exists()); + } + + @Test + public void testCleanupScriptFilesWithNullFieldsDoesNotThrow() { + // No retrieveScriptFiles() was called, so all file fields are null. + actionWorker.cleanupScriptFiles(); + } }