Fix config validation for value range and code coverage improvement

Author: sudo87

server/src/main/java/org/apache/cloudstack/vm/lease/VMLeaseManager.java

@@ -19,6 +19,7 @@
 package org.apache.cloudstack.vm.lease;
 
 import com.cloud.utils.component.Manager;
+import com.google.common.annotations.VisibleForTesting;
 import org.apache.cloudstack.framework.config.ConfigKey;
 
 import java.util.List;
@@ -29,7 +30,9 @@ public interface VMLeaseManager extends Manager {
 
     enum ExpiryAction {
         STOP,
-        DESTROY
+        DESTROY,
+        @VisibleForTesting
+        UNKNOWN
     }
 
     enum LeaseActionExecution {
@@ -45,15 +48,15 @@ enum LeaseActionExecution {
             "Re-enabling feature will not cause lease expiry actions on grandfathered instances",
             true, List.of(ConfigKey.Scope.Global));
 
-    ConfigKey<Long> InstanceLeaseSchedulerInterval = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED, Long.class,
+    ConfigKey<Integer> InstanceLeaseSchedulerInterval = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED, Integer.class,
             "instance.lease.scheduler.interval", "3600", "VM Lease Scheduler interval in seconds",
             false, List.of(ConfigKey.Scope.Global));
 
-    ConfigKey<Long> InstanceLeaseExpiryEventSchedulerInterval = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED, Long.class,
+    ConfigKey<Integer> InstanceLeaseExpiryEventSchedulerInterval = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED, Integer.class,
             "instance.lease.eventscheduler.interval", "86400", "Lease expiry event Scheduler interval in seconds",
             false, List.of(ConfigKey.Scope.Global));
 
-    ConfigKey<Long> InstanceLeaseExpiryEventDaysBefore = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED, Long.class,
+    ConfigKey<Integer> InstanceLeaseExpiryEventDaysBefore = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED, Integer.class,
             "instance.lease.expiryevent.daysbefore", "7", "Indicates how many days in advance, expiry events will be created before expiry.",
             true, List.of(ConfigKey.Scope.Global));
 

server/src/main/java/org/apache/cloudstack/vm/lease/VMLeaseManagerImpl.java

@@ -203,7 +203,7 @@ protected void runInContext() {
             try {
                 if (scanLock.lock(ACQUIRE_GLOBAL_LOCK_TIMEOUT_FOR_COOPERATION)) {
                     try {
-                        List<UserVmJoinVO> leaseExpiringForInstances = userVmJoinDao.listLeaseInstancesExpiringInDays(InstanceLeaseExpiryEventDaysBefore.value().intValue());
+                        List<UserVmJoinVO> leaseExpiringForInstances = userVmJoinDao.listLeaseInstancesExpiringInDays(InstanceLeaseExpiryEventDaysBefore.value());
                         for (UserVmJoinVO instance : leaseExpiringForInstances) {
                             String leaseExpiryEventMsg =  String.format("Lease expiring for for instance: %s (id: %s) with action: %s",
                                     instance.getName(), instance.getUuid(), instance.getLeaseExpiryAction());
@@ -220,11 +220,6 @@ protected void runInContext() {
         }
     }
 
-    @Override
-    public Map<String, Object> getConfigParams() {
-        return super.getConfigParams();
-    }
-
     protected void reallyRun() {
         // fetch user_instances having leaseDuration configured and has expired
         List<UserVmJoinVO> leaseExpiredInstances = userVmJoinDao.listEligibleInstancesWithExpiredLease();

server/src/test/java/org/apache/cloudstack/vm/lease/VMLeaseManagerImplTest.java

@@ -27,9 +27,11 @@
 import com.cloud.utils.db.GlobalLock;
 import com.cloud.utils.exception.CloudRuntimeException;
 import com.cloud.vm.VirtualMachine;
+import com.cloud.vm.VmDetailConstants;
 import com.cloud.vm.dao.UserVmDetailsDao;
 import org.apache.cloudstack.api.command.user.vm.DestroyVMCmd;
 import org.apache.cloudstack.api.command.user.vm.StopVMCmd;
+import org.apache.cloudstack.framework.config.ConfigKey;
 import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
 import org.apache.cloudstack.framework.jobs.AsyncJobDispatcher;
 import org.apache.cloudstack.framework.jobs.AsyncJobManager;
@@ -47,6 +49,7 @@
 import org.springframework.context.ApplicationContext;
 
 import javax.naming.ConfigurationException;
+import java.lang.reflect.Field;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashMap;
@@ -56,6 +59,7 @@
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyBoolean;
 import static org.mockito.ArgumentMatchers.anyLong;
@@ -246,6 +250,57 @@ public void testGetLeaseExpiryInvalidAction() {
         assertNull(vmLeaseManager.getLeaseExpiryAction(vm));
     }
 
+    @Test
+    public void testGetComponentName() {
+        assertEquals(vmLeaseManager.getConfigComponentName(), "VMLeaseManager");
+    }
+
+    @Test
+    public void testConfigKeys() {
+        assertEquals(vmLeaseManager.getConfigKeys().length, 4);
+    }
+
+    @Test
+    public void testConfigure() throws Exception {
+        overrideDefaultConfigValue(VMLeaseManager.InstanceLeaseEnabled, "true");
+        vmLeaseManager.configure("VMLeaseManagerImpl", new HashMap<>());
+    }
+
+    @Test
+    public void testDefaultExpiryAction() {
+        UserVmJoinVO vm = createMockVm(1L, VM_UUID, VM_NAME, VirtualMachine.State.Running, false);
+        assertNull(vmLeaseManager.executeExpiryAction(vm, VMLeaseManager.ExpiryAction.UNKNOWN, 123L));
+    }
+
+    @Test
+    public void testStopShouldShutdownExecutors() {
+        assertTrue(vmLeaseManager.stop());
+    }
+
+    @Test
+    public void testCancelLeaseOnExistingInstances() {
+        UserVmJoinVO vm = createMockVm(1L, VM_UUID, VM_NAME, VirtualMachine.State.Running, true);
+        when(userVmJoinDao.listLeaseInstancesExpiringInDays(-1)).thenReturn(List.of(vm));
+        try (MockedStatic<ActionEventUtils> utilities = Mockito.mockStatic(ActionEventUtils.class)) {
+            utilities.when(() -> ActionEventUtils.onStartedActionEvent(Mockito.anyLong(), Mockito.anyLong(), Mockito.anyString(),
+                    Mockito.anyString(), Mockito.anyLong(), Mockito.anyString(), Mockito.anyBoolean(), Mockito.anyLong())).thenReturn(1L);
+            vmLeaseManager.cancelLeaseOnExistingInstances();
+            verify(userVmDetailsDao).addDetail(1L, VmDetailConstants.INSTANCE_LEASE_EXECUTION, VMLeaseManager.LeaseActionExecution.CANCELLED.name(), false);
+        }
+    }
+
+    @Test
+    public void testOnLeaseFeatureToggleEnabled() throws Exception {
+        overrideDefaultConfigValue(VMLeaseManager.InstanceLeaseEnabled, "true");
+        vmLeaseManager.onLeaseFeatureToggle();
+    }
+
+    @Test
+    public void testOnLeaseFeatureToggleDisabled() throws Exception {
+        overrideDefaultConfigValue(VMLeaseManager.InstanceLeaseEnabled, "false");
+        vmLeaseManager.onLeaseFeatureToggle();
+    }
+
     private UserVmJoinVO createMockVm(Long id, String uuid, String name, VirtualMachine.State state, boolean deleteProtection) {
         return createMockVm(id, uuid, name, state, deleteProtection, "STOP");
     }
@@ -260,4 +315,10 @@ private UserVmJoinVO createMockVm(Long id, String uuid, String name, VirtualMach
         when(vm.getLeaseExpiryAction()).thenReturn(expiryAction);
         return vm;
     }
+
+    private void overrideDefaultConfigValue(final ConfigKey configKey, final String value) throws IllegalAccessException, NoSuchFieldException {
+        final Field f = ConfigKey.class.getDeclaredField("_defaultValue");
+        f.setAccessible(true);
+        f.set(configKey, value);
+    }
 }