Skip to content

Commit 9f72e75

Browse files
kubernetes: clean up temporary provisioning scripts on the management server
The CKS action workers copy provisioning scripts (deploy-cloudstack-secret, deploy-provider, deploy-csi-driver, delete-pv-reclaimpolicy-delete, autoscale-kube-cluster and upgrade-kubernetes.sh) to the cluster nodes via retrieveScriptFiles() + copyScripts()/copyScriptFile(). Each script is written to a local temp file via File.createTempFile(), but that local copy is never deleted after it has been SCP'd to the node. As a result every cluster deploy, provider/CSI deploy, autoscale enable, PV cleanup and Kubernetes upgrade leaks one *.sh file per script into the management server's temp directory, growing without bound over the cluster lifetime. On a long-running management server this accumulates to hundreds of stale scripts in /tmp. This adds cleanupScriptFiles()/deleteScriptFileQuietly() to the base action worker and invokes it in a finally block around every copy site (deployProvider, deployCsiDriver, autoscaleCluster, deletePVsWithReclaimPolicyDelete and the upgrade worker), so the local temp copies are removed once they have been transferred to the node. The upgrade worker additionally removes its upgrade-kubernetes.sh temp file after the node loop completes. No functional change to the provisioning itself: the scripts are still written, copied and executed exactly as before; only the leftover local copies are deleted. Covered by new unit tests for cleanupScriptFiles()/deleteScriptFileQuietly() (temp files deleted, null/missing files handled without throwing).
1 parent a503a52 commit 9f72e75

4 files changed

Lines changed: 106 additions & 16 deletions

File tree

plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterActionWorker.java

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -761,6 +761,25 @@ protected void copyScriptFile(String nodeAddress, final int sshPort, File file,
761761
}
762762
}
763763

764+
/**
765+
* Deletes the local temporary copies of the scripts created by {@link #retrieveScriptFiles()}
766+
* once they have been copied to the cluster node(s). Without this, every deploy/autoscale/PV-cleanup
767+
* action leaks a *.sh file in the management server's tmp directory.
768+
*/
769+
protected void cleanupScriptFiles() {
770+
deleteScriptFileQuietly(deploySecretsScriptFile);
771+
deleteScriptFileQuietly(deployProviderScriptFile);
772+
deleteScriptFileQuietly(deployCsiDriverScriptFile);
773+
deleteScriptFileQuietly(deletePvScriptFile);
774+
deleteScriptFileQuietly(autoscaleScriptFile);
775+
}
776+
777+
protected void deleteScriptFileQuietly(File file) {
778+
if (file != null && file.exists() && !file.delete()) {
779+
logger.debug("Failed to delete temporary Kubernetes cluster script file: {}", file.getAbsolutePath());
780+
}
781+
}
782+
764783
protected boolean taintControlNodes() {
765784
StringBuilder commands = new StringBuilder();
766785
List<KubernetesClusterVmMapVO> vmMapVOList = getKubernetesClusterVMMaps();
@@ -820,7 +839,11 @@ protected boolean deployProvider() {
820839
if (!result.first()) {
821840
logMessage(Level.INFO, "Provider files missing. Adding them now", null);
822841
retrieveScriptFiles();
823-
copyScripts(publicIpAddress, sshPort);
842+
try {
843+
copyScripts(publicIpAddress, sshPort);
844+
} finally {
845+
cleanupScriptFiles();
846+
}
824847

825848
if (!createCloudStackSecret(keys)) {
826849
logTransitStateAndThrow(Level.ERROR, String.format("Failed to setup keys for Kubernetes cluster %s",
@@ -857,7 +880,11 @@ protected boolean deployCsiDriver() {
857880
if (!result.first()) {
858881
logMessage(Level.INFO, "CSI files missing. Adding them now", null);
859882
retrieveScriptFiles();
860-
copyScripts(publicIpAddress, sshPort);
883+
try {
884+
copyScripts(publicIpAddress, sshPort);
885+
} finally {
886+
cleanupScriptFiles();
887+
}
861888

862889
if (!createCloudStackSecret(keys)) {
863890
logTransitStateAndThrow(Level.ERROR, String.format("Failed to setup keys for Kubernetes cluster %s",

plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterResourceModifierActionWorker.java

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -951,7 +951,11 @@ protected boolean autoscaleCluster(boolean enable, Long minSize, Long maxSize) {
951951
if (!result.first()) {
952952
logMessage(Level.INFO, "Autoscaling files missing. Adding them now", null);
953953
retrieveScriptFiles();
954-
copyScripts(publicIpAddress, sshPort);
954+
try {
955+
copyScripts(publicIpAddress, sshPort);
956+
} finally {
957+
cleanupScriptFiles();
958+
}
955959

956960
if (!createCloudStackSecret(keys)) {
957961
logTransitStateAndThrow(Level.ERROR, String.format("Failed to setup keys for Kubernetes cluster %s",
@@ -1003,19 +1007,23 @@ public boolean deletePVsWithReclaimPolicyDelete() {
10031007
if (Boolean.FALSE.equals(result.first())) {
10041008
logMessage(Level.INFO, "PV delete script missing. Adding it now", null);
10051009
retrieveScriptFiles();
1006-
if (deletePvScriptFile != null) {
1007-
copyScriptFile(publicIpAddress, sshPort, deletePvScriptFile, deletePvScriptFilename);
1008-
logMessage(Level.INFO, "Executing PV deletion script (this may take several minutes)...", null);
1009-
result = SshHelper.sshExecute(publicIpAddress, sshPort, getControlNodeLoginUser(),
1010-
pkFile, null, command, 10000, 10000, 600000); // 10 minute timeout
1011-
if (Boolean.FALSE.equals(result.first())) {
1012-
logMessage(Level.ERROR, "PV deletion script failed: " + result.second(), null);
1013-
throw new CloudRuntimeException(result.second());
1010+
try {
1011+
if (deletePvScriptFile != null) {
1012+
copyScriptFile(publicIpAddress, sshPort, deletePvScriptFile, deletePvScriptFilename);
1013+
logMessage(Level.INFO, "Executing PV deletion script (this may take several minutes)...", null);
1014+
result = SshHelper.sshExecute(publicIpAddress, sshPort, getControlNodeLoginUser(),
1015+
pkFile, null, command, 10000, 10000, 600000); // 10 minute timeout
1016+
if (Boolean.FALSE.equals(result.first())) {
1017+
logMessage(Level.ERROR, "PV deletion script failed: " + result.second(), null);
1018+
throw new CloudRuntimeException(result.second());
1019+
}
1020+
logMessage(Level.INFO, "PV deletion script completed successfully", null);
1021+
} else {
1022+
logMessage(Level.WARN, "PV delete script file not found in resources, skipping PV deletion", null);
1023+
return false;
10141024
}
1015-
logMessage(Level.INFO, "PV deletion script completed successfully", null);
1016-
} else {
1017-
logMessage(Level.WARN, "PV delete script file not found in resources, skipping PV deletion", null);
1018-
return false;
1025+
} finally {
1026+
cleanupScriptFiles();
10191027
}
10201028
} else {
10211029
logMessage(Level.INFO, "PV deletion script completed successfully", null);

plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterUpgradeWorker.java

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,12 @@ protected void retrieveScriptFiles() {
6363
upgradeScriptFile = retrieveScriptFile(upgradeScriptFilename);
6464
}
6565

66+
@Override
67+
protected void cleanupScriptFiles() {
68+
super.cleanupScriptFiles();
69+
deleteScriptFileQuietly(upgradeScriptFile);
70+
}
71+
6672
private Pair<Boolean, String> runInstallScriptOnVM(final UserVm vm, final int index) throws Exception {
6773
int nodeSshPort = sshPort == 22 ? sshPort : sshPort + index;
6874
String nodeAddress = (index > 0 && sshPort == 22) ? vm.getPrivateIpAddress() : publicIpAddress;
@@ -176,7 +182,11 @@ public boolean upgradeCluster() throws CloudRuntimeException {
176182
retrieveScriptFiles();
177183
stateTransitTo(kubernetesCluster.getId(), KubernetesCluster.Event.UpgradeRequested);
178184
attachIsoKubernetesVMs(clusterVMs, upgradeVersion);
179-
upgradeKubernetesClusterNodes();
185+
try {
186+
upgradeKubernetesClusterNodes();
187+
} finally {
188+
cleanupScriptFiles();
189+
}
180190
detachIsoKubernetesVMs(clusterVMs);
181191
KubernetesClusterVO kubernetesClusterVO = kubernetesClusterDao.findById(kubernetesCluster.getId());
182192
kubernetesClusterVO.setKubernetesVersionId(upgradeVersion.getId());

plugins/integrations/kubernetes-service/src/test/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterActionWorkerTest.java

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
// under the License.
1717
package com.cloud.kubernetes.cluster.actionworkers;
1818

19+
import java.io.File;
1920
import java.util.ArrayList;
2021
import java.util.Arrays;
2122
import java.util.Collections;
@@ -229,4 +230,48 @@ public void testGetMergedAffinityGroupIdsExplicitDedicationAlreadyInList() {
229230
Assert.assertTrue(result.contains(99L));
230231
Assert.assertTrue(result.contains(2L));
231232
}
233+
234+
@Test
235+
public void testCleanupScriptFilesDeletesAllTempFiles() throws Exception {
236+
actionWorker.deploySecretsScriptFile = File.createTempFile("deploy-cloudstack-secret", ".sh");
237+
actionWorker.deployProviderScriptFile = File.createTempFile("deploy-provider", ".sh");
238+
actionWorker.deployCsiDriverScriptFile = File.createTempFile("deploy-csi-driver", ".sh");
239+
actionWorker.deletePvScriptFile = File.createTempFile("delete-pv-reclaimpolicy-delete", ".sh");
240+
actionWorker.autoscaleScriptFile = File.createTempFile("autoscale-kube-cluster", ".sh");
241+
242+
List<File> files = Arrays.asList(actionWorker.deploySecretsScriptFile, actionWorker.deployProviderScriptFile,
243+
actionWorker.deployCsiDriverScriptFile, actionWorker.deletePvScriptFile, actionWorker.autoscaleScriptFile);
244+
for (File f : files) {
245+
Assert.assertTrue(f.exists());
246+
}
247+
248+
actionWorker.cleanupScriptFiles();
249+
250+
for (File f : files) {
251+
Assert.assertFalse("Temporary script file should have been deleted: " + f.getAbsolutePath(), f.exists());
252+
}
253+
}
254+
255+
@Test
256+
public void testDeleteScriptFileQuietlyHandlesNullAndMissingFiles() throws Exception {
257+
// null must not throw
258+
actionWorker.deleteScriptFileQuietly(null);
259+
260+
// a File that does not exist must not throw
261+
File missing = new File(System.getProperty("java.io.tmpdir"), "cks-nonexistent-" + UUID.randomUUID() + ".sh");
262+
Assert.assertFalse(missing.exists());
263+
actionWorker.deleteScriptFileQuietly(missing);
264+
265+
// an existing file is deleted
266+
File present = File.createTempFile("cks-present", ".sh");
267+
Assert.assertTrue(present.exists());
268+
actionWorker.deleteScriptFileQuietly(present);
269+
Assert.assertFalse(present.exists());
270+
}
271+
272+
@Test
273+
public void testCleanupScriptFilesWithNullFieldsDoesNotThrow() {
274+
// No retrieveScriptFiles() was called, so all file fields are null.
275+
actionWorker.cleanupScriptFiles();
276+
}
232277
}

0 commit comments

Comments
 (0)