From e9f5d4e2a96f21411268f1ae41c181bafc375255 Mon Sep 17 00:00:00 2001 From: Brad House Date: Mon, 9 Nov 2026 19:05:57 +0000 Subject: [PATCH] CKS: don't fail when a node still has an ISO attached attachIsoKubernetesVMs attaches the binaries ISO to every node before an upgrade, a scale-up or a start, and relied on the attach replacing whatever ISO the node had. Since an Instance can have several CD-ROMs, attachIso refuses an ISO that is already attached, and any ISO beyond the number of CD-ROMs, one by default. A node left holding the binaries ISO by an operation that did not finish, for example because the management server stopped, then made every later operation fail before it started. Leave the binaries ISO in place if the node has it already, and detach any other ISO from the node first, as the attach used to replace it. Signed-off-by: Brad House --- .../KubernetesClusterActionWorker.java | 15 ++ .../KubernetesClusterActionWorkerIsoTest.java | 135 ++++++++++++++++++ 2 files changed, 150 insertions(+) create mode 100644 plugins/integrations/kubernetes-service/src/test/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterActionWorkerIsoTest.java 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 07c9f12a50e2..efae1c465a23 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 @@ -139,6 +139,7 @@ import com.cloud.vm.dao.UserVmDao; import com.cloud.vm.dao.VMInstanceDao; import com.cloud.vm.dao.VMInstanceDetailsDao; +import com.cloud.vm.dao.VmIsoMapDao; import static com.cloud.kubernetes.cluster.KubernetesServiceHelper.KubernetesClusterNodeType.CONTROL; import static com.cloud.kubernetes.cluster.KubernetesServiceHelper.KubernetesClusterNodeType.ETCD; @@ -196,6 +197,8 @@ public class KubernetesClusterActionWorker { @Inject protected UserVmDao userVmDao; @Inject + protected VmIsoMapDao vmIsoMapDao; + @Inject protected VMInstanceDetailsDao vmInstanceDetailsDao; @Inject protected UserVmService userVmService; @@ -699,6 +702,18 @@ protected void attachIsoKubernetesVMs(List clusterVMs, final KubernetesS CallContext vmContext = CallContext.register(CallContext.current(), ApiCommandResourceType.VirtualMachine); vmContext.putContextParameter(VirtualMachine.class, vm.getUuid()); try { + // A node can still hold an ISO from an operation that did not finish. An Instance can not have the + // same ISO attached twice, nor more ISOs than it has CD-ROMs. + UserVmVO userVm = userVmDao.findById(vm.getId()); + Long attachedIsoId = userVm != null ? userVm.getIsoId() : null; + if (Long.valueOf(iso.getId()).equals(attachedIsoId) || vmIsoMapDao.findByVmIdIsoId(vm.getId(), iso.getId()) != null) { + logger.info("Binaries ISO is already attached to VM: {} in cluster: {}", vm, kubernetesCluster); + continue; + } + if (attachedIsoId != null) { + logger.info("Detaching ISO {} from VM: {} in cluster: {} to attach the binaries ISO", attachedIsoId, vm, kubernetesCluster); + templateService.detachIso(vm.getId(), attachedIsoId, true); + } templateService.attachIso(iso.getId(), vm.getId(), true); if (logger.isInfoEnabled()) { logger.info("Attached binaries ISO for VM: {} in cluster: {}", vm, kubernetesCluster); diff --git a/plugins/integrations/kubernetes-service/src/test/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterActionWorkerIsoTest.java b/plugins/integrations/kubernetes-service/src/test/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterActionWorkerIsoTest.java new file mode 100644 index 000000000000..759cd7968db6 --- /dev/null +++ b/plugins/integrations/kubernetes-service/src/test/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterActionWorkerIsoTest.java @@ -0,0 +1,135 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package com.cloud.kubernetes.cluster.actionworkers; + +import java.util.List; + +import org.apache.cloudstack.api.ApiCommandResourceType; +import org.apache.cloudstack.context.CallContext; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.MockedStatic; +import org.mockito.Mockito; +import org.mockito.junit.MockitoJUnitRunner; + +import com.cloud.kubernetes.cluster.KubernetesCluster; +import com.cloud.kubernetes.cluster.KubernetesClusterManagerImpl; +import com.cloud.kubernetes.cluster.dao.KubernetesClusterDao; +import com.cloud.kubernetes.version.KubernetesSupportedVersion; +import com.cloud.storage.Storage; +import com.cloud.storage.VMTemplateVO; +import com.cloud.storage.dao.VMTemplateDao; +import com.cloud.template.TemplateApiService; +import com.cloud.template.VirtualMachineTemplate; +import com.cloud.uservm.UserVm; +import com.cloud.vm.UserVmVO; +import com.cloud.vm.VmIsoMapVO; +import com.cloud.vm.dao.UserVmDao; +import com.cloud.vm.dao.VmIsoMapDao; + +/** + * Attaching the binaries ISO to the nodes of a cluster, some of which may still have an ISO attached. + */ +@RunWith(MockitoJUnitRunner.class) +public class KubernetesClusterActionWorkerIsoTest { + + private static final long BINARIES_ISO_ID = 10L; + private static final long OTHER_ISO_ID = 11L; + private static final long VM_ID = 2L; + + @Mock + KubernetesClusterManagerImpl kubernetesClusterManager; + + @Mock + KubernetesCluster kubernetesCluster; + + @Mock + TemplateApiService templateService; + + @Mock + VMTemplateDao templateDao; + + @Mock + UserVmDao userVmDao; + + @Mock + VmIsoMapDao vmIsoMapDao; + + @Mock + KubernetesSupportedVersion version; + + @Mock + UserVm vm; + + KubernetesClusterActionWorker actionWorker; + + @Before + public void setUp() { + kubernetesClusterManager.kubernetesClusterDao = Mockito.mock(KubernetesClusterDao.class); + actionWorker = new KubernetesClusterActionWorker(kubernetesCluster, kubernetesClusterManager); + actionWorker.templateService = templateService; + actionWorker.templateDao = templateDao; + actionWorker.userVmDao = userVmDao; + actionWorker.vmIsoMapDao = vmIsoMapDao; + Mockito.when(version.getIsoId()).thenReturn(BINARIES_ISO_ID); + VMTemplateVO iso = Mockito.mock(VMTemplateVO.class); + Mockito.when(iso.getId()).thenReturn(BINARIES_ISO_ID); + Mockito.when(iso.getFormat()).thenReturn(Storage.ImageFormat.ISO); + Mockito.when(iso.getState()).thenReturn(VirtualMachineTemplate.State.Active); + Mockito.when(templateDao.findById(BINARIES_ISO_ID)).thenReturn(iso); + Mockito.when(vm.getId()).thenReturn(VM_ID); + } + + private void attachBinariesIso(Long attachedIsoId, boolean binariesIsoInOtherCdrom) { + UserVmVO userVm = Mockito.mock(UserVmVO.class); + Mockito.when(userVm.getIsoId()).thenReturn(attachedIsoId); + Mockito.when(userVmDao.findById(VM_ID)).thenReturn(userVm); + Mockito.lenient().when(vmIsoMapDao.findByVmIdIsoId(VM_ID, BINARIES_ISO_ID)).thenReturn(binariesIsoInOtherCdrom ? Mockito.mock(VmIsoMapVO.class) : null); + try (MockedStatic callContext = Mockito.mockStatic(CallContext.class)) { + callContext.when(() -> CallContext.register(Mockito.any(), Mockito.any(ApiCommandResourceType.class))).thenReturn(Mockito.mock(CallContext.class)); + actionWorker.attachIsoKubernetesVMs(List.of(vm), version); + } + } + + @Test + public void testAttachIsoKubernetesVMsNoIsoAttached() { + attachBinariesIso(null, false); + Mockito.verify(templateService, Mockito.never()).detachIso(Mockito.anyLong(), Mockito.any(), Mockito.any()); + Mockito.verify(templateService).attachIso(BINARIES_ISO_ID, VM_ID, true); + } + + @Test + public void testAttachIsoKubernetesVMsBinariesIsoAlreadyAttached() { + attachBinariesIso(BINARIES_ISO_ID, false); + Mockito.verifyNoInteractions(templateService); + } + + @Test + public void testAttachIsoKubernetesVMsBinariesIsoAttachedToOtherCdrom() { + attachBinariesIso(OTHER_ISO_ID, true); + Mockito.verifyNoInteractions(templateService); + } + + @Test + public void testAttachIsoKubernetesVMsOtherIsoAttached() { + attachBinariesIso(OTHER_ISO_ID, false); + Mockito.verify(templateService).detachIso(VM_ID, OTHER_ISO_ID, true); + Mockito.verify(templateService).attachIso(BINARIES_ISO_ID, VM_ID, true); + } +}