Copilot commented on code in PR #13884:
URL: https://github.com/apache/cloudstack/pull/13884#discussion_r3926885033
##########
plugins/network-elements/elastic-loadbalancer/src/main/java/com/cloud/network/lb/ElasticLoadBalancerManager.java:
##########
@@ -31,6 +32,18 @@ public interface ElasticLoadBalancerManager {
public static final int DEFAULT_ELB_VM_RAMSIZE = 128; // 512 MB
public static final int DEFAULT_ELB_VM_CPU_MHZ = 256; // 500
MHz
+ ConfigKey<Integer> ElasticLoadBalancerVmMemory = new
ConfigKey<>("Advanced", Integer.class,
"network.loadbalancer.basiczone.elb.vm.ram.size", "512",
+ "Memory in MB for the elastic load balancer vm", true);
Review Comment:
ElasticLoadBalancerVmMemory is introduced with a default of 512MB, but the
existing DB schema migration that adds this config key inserts a default of 128
(see schema-228to229.sql). This mismatch changes the effective default
depending on whether the configuration row exists and makes upgrades/new
installs inconsistent.
##########
plugins/storage/image/default/src/main/java/org/apache/cloudstack/storage/datastore/driver/CloudStackImageStoreDriverImpl.java:
##########
@@ -130,12 +130,8 @@ private String generateCopyUrl(String ipAddress, String
fileName, String filePat
String hostname = ipAddress;
String scheme = "http";
- boolean _sslCopy = false;
- String sslCfg =
_configDao.getValue(Config.SecStorageEncryptCopy.toString());
+ boolean _sslCopy =
SecondaryStorageVmManager.SecStorageEncryptCopy.value();
String _ssvmUrlDomain =
_configDao.getValue("secstorage.ssl.cert.domain");
Review Comment:
generateCopyUrl() now uses
SecondaryStorageVmManager.SecStorageEncryptCopy.value(), but still fetches the
cert domain via the raw string key "secstorage.ssl.cert.domain". Since
SecStorageSecureCopyCert was added as a ConfigKey, using it here keeps
configuration access consistent and avoids typos / divergent defaults.
##########
engine/storage/cache/src/main/java/org/apache/cloudstack/storage/cache/manager/StorageCacheManagerImpl.java:
##########
@@ -159,13 +162,23 @@ public void setRunLevel(int level) {
@Override
public boolean configure(String name, Map<String, Object> params) throws
ConfigurationException {
- cacheReplacementEnabled =
Boolean.parseBoolean(configDao.getValue(Config.StorageCacheReplacementEnabled.key()));
- cacheReplaceMentInterval =
NumbersUtil.parseInt(configDao.getValue(Config.StorageCacheReplacementInterval.key()),
86400);
- workers =
NumbersUtil.parseInt(configDao.getValue(Config.ExpungeWorkers.key()), 10);
+ cacheReplacementEnabled = StorageCacheReplacementEnabled.value();
+ cacheReplaceMentInterval = StorageCacheReplacementInterval.value();
+ workers =
NumbersUtil.parseInt(configDao.getValue(StorageManager.ExpungeWorkers.key()),
10);
Review Comment:
workers is still being derived via configDao.getValue(..., 10), which
hardcodes a fallback of 10 even though ExpungeWorkers now exists as a ConfigKey
with its own default. This can unintentionally spawn 10 cache-replacement
threads when the config is absent/uninitialized and also bypasses the new
ConfigKey access pattern used above.
##########
server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManager.java:
##########
@@ -64,6 +64,9 @@ public interface VirtualNetworkApplianceManager extends
Manager, VirtualNetworkA
ConfigKey<String> RouterTemplateOvm3 = new ConfigKey<>(String.class,
RouterTemplateOvm3CK, "Advanced", "SystemVM Template (Ovm3)",
"Name of the default router template on Ovm3.", true,
ConfigKey.Scope.Zone, null);
+ ConfigKey<Integer> RouterRamSize = new ConfigKey<>("Hidden",
Integer.class, "router.ram.size", "512",
+ "Default RAM for router VM (in MB).", true);
Review Comment:
RouterRamSize ConfigKey default is set to 512MB, but existing schema/data
migrations set router.ram.size to 256 (e.g. Upgrade442to450 updates it to 256).
Keeping 512 here changes the effective default when the configuration row is
missing/reset and is inconsistent with current behavior.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
