Skip to content

make control.cidr and control.gateway dynamic #7746

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
DaanHoogland wants to merge 7 commits into from
Closed

make control.cidr and control.gateway dynamic #7746

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e83c568
make control.cidr dynamic
DaanHoogland Jul 18, 2023
3d390b1
merge mistake fixed
DaanHoogland Jul 19, 2023
8cd48ac
some open ends
DaanHoogland Jul 20, 2023
34dabe4
double var
DaanHoogland Aug 4, 2023
a7b9fbe
dynamic gateway value (per zone)
DaanHoogland Aug 14, 2023
efd2cd9
import
DaanHoogland Aug 14, 2023
3dd5858
control cidr/gateway not dynamoic
DaanHoogland Aug 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 35 additions & 56 deletions engine/components-api/src/main/java/com/cloud/configuration/ConfigurationManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import java.util.Map;
import java.util.Set;

import org.apache.cloudstack.framework.config.ConfigKey;
import org.apache.cloudstack.framework.config.impl.ConfigurationSubGroupVO;

import com.cloud.dc.ClusterVO;
Expand Down Expand Up @@ -54,10 +55,40 @@
*/
public interface ConfigurationManager {

public static final String MESSAGE_CREATE_POD_IP_RANGE_EVENT = "Message.CreatePodIpRange.Event";
public static final String MESSAGE_DELETE_POD_IP_RANGE_EVENT = "Message.DeletePodIpRange.Event";
public static final String MESSAGE_CREATE_VLAN_IP_RANGE_EVENT = "Message.CreateVlanIpRange.Event";
public static final String MESSAGE_DELETE_VLAN_IP_RANGE_EVENT = "Message.DeleteVlanIpRange.Event";
String MESSAGE_CREATE_POD_IP_RANGE_EVENT = "Message.CreatePodIpRange.Event";
String MESSAGE_DELETE_POD_IP_RANGE_EVENT = "Message.DeletePodIpRange.Event";
String MESSAGE_CREATE_VLAN_IP_RANGE_EVENT = "Message.CreateVlanIpRange.Event";
String MESSAGE_DELETE_VLAN_IP_RANGE_EVENT = "Message.DeleteVlanIpRange.Event";

ConfigKey<Boolean> SystemVMUseLocalStorage = new ConfigKey<>(
Boolean.class,
"system.vm.use.local.storage",
"Advanced",
"false",
"Indicates whether to use local storage pools or shared storage pools for system VMs.",
false,
ConfigKey.Scope.Zone,
null);

ConfigKey<String> ControlCidr = new ConfigKey<>(
Comment thread
DaanHoogland marked this conversation as resolved.
String.class,
"control.cidr",
"Advanced",
"169.254.0.0/16",

@DaanHoogland DaanHoogland Aug 29, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"169.254.0.0/16",
"169.254.240.0/20",

like this and abandon all other changes in this PR, right @wido ?

@wido wido Aug 29, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost @DaanHoogland , but also in:

  • NetUtils.java
  • AgentProperties.java
  • configuration/Config.java

If we change it to a /20 in those files we should be good.

@DaanHoogland DaanHoogland Aug 30, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will only work for new environments. I don't think this will work when upgrading environment. These hardcoded values of an already otherwise deplayed config item will break systems and needs a bit more TLC .

@weizhouapache weizhouapache Aug 30, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DaanHoogland
as I understand, Wido wants /20 to be the default in new environments.
It would be good to update the config description to emphasize that the new value only applies on new pods.

@wido wido Aug 30, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. For upgrades we shouldn't touch anything. Unless somebody is going to use BGP Unnumbered in their existing environment. Otherwise just using the /20 for new deployments is fine.

@DaanHoogland DaanHoogland Aug 31, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, this means that the hardcoded values in NetUtils.java and AgentProperties.java have to be intelligent... we can not assume returning the one or the other in these methods. That complicates things a bit.

"Changes the cidr for the control network traffic. Defaults to using link local. Must be unique within pods",
false,
ConfigKey.Scope.Zone,
null);

ConfigKey<String> ControlGateway = new ConfigKey<>(
String.class,
"control.gateway",
"Advanced",
"169.254.0.1",
"gateway for the control network traffic",
false,
ConfigKey.Scope.Zone,
null);

/**
* @param offering
Expand All @@ -78,58 +109,6 @@ public interface ConfigurationManager {
*/
String updateConfiguration(long userId, String name, String category, String value, String scope, Long id);

// /**
// * Creates a new service offering
// *
// * @param name
// * @param cpu
// * @param ramSize
// * @param speed
// * @param displayText
// * @param localStorageRequired
// * @param offerHA
// * @param domainId
// * @param volatileVm
// * @param hostTag
// * @param networkRate
// * TODO
// * @param id
// * @param useVirtualNetwork
// * @param deploymentPlanner
// * @param details
// * @param bytesReadRate
// * @param bytesWriteRate
// * @param iopsReadRate
// * @param iopsWriteRate
// * @return ID
// */
// ServiceOfferingVO createServiceOffering(long userId, boolean isSystem, VirtualMachine.Type vm_typeType, String name, int cpu, int ramSize, int speed, String displayText, boolean localStorageRequired,
// boolean offerHA, boolean limitResourceUse, boolean volatileVm, String tags, Long domainId, String hostTag, Integer networkRate, String deploymentPlanner, Map<String, String> details,
// Long bytesReadRate, Long bytesWriteRate, Long iopsReadRate, Long iopsWriteRate);

// /**
// * Creates a new disk offering
// *
// * @param domainId
// * @param name
// * @param description
// * @param numGibibytes
// * @param tags
// * @param isCustomized
// * @param localStorageRequired
// * @param isDisplayOfferingEnabled
// * @param isCustomizedIops (is admin allowing users to set custom iops?)
// * @param minIops
// * @param maxIops
// * @param bytesReadRate
// * @param bytesWriteRate
// * @param iopsReadRate
// * @param iopsWriteRate
// * @return newly created disk offering
// */
// DiskOfferingVO createDiskOffering(Long domainId, String name, String description, Long numGibibytes, String tags, boolean isCustomized,
// boolean localStorageRequired, boolean isDisplayOfferingEnabled, Boolean isCustomizedIops, Long minIops, Long maxIops,
// Long bytesReadRate, Long bytesWriteRate, Long iopsReadRate, Long iopsWriteRate);

/**
* Creates a new pod
Expand Down
4 changes: 2 additions & 2 deletions ...ments/elastic-loadbalancer/src/main/java/com/cloud/network/lb/LoadBalanceRuleHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService;
import org.apache.log4j.Logger;

import com.cloud.configuration.ConfigurationManagerImpl;
import com.cloud.configuration.ConfigurationManager;
import com.cloud.dc.DataCenter;
import com.cloud.dc.Pod;
import com.cloud.dc.PodVlanMapVO;
Expand Down Expand Up @@ -286,7 +286,7 @@ private DomainRouterVO deployELBVm(Network guestNetwork, final DeployDestination
}
}

ServiceOfferingVO elasticLbVmOffering = _serviceOfferingDao.findDefaultSystemOffering(ServiceOffering.elbVmDefaultOffUniqueName, ConfigurationManagerImpl.SystemVMUseLocalStorage.valueIn(dest.getDataCenter().getId()));
ServiceOfferingVO elasticLbVmOffering = _serviceOfferingDao.findDefaultSystemOffering(ServiceOffering.elbVmDefaultOffUniqueName, ConfigurationManager.SystemVMUseLocalStorage.valueIn(dest.getDataCenter().getId()));
elbVm = new DomainRouterVO(id, elasticLbVmOffering.getId(), vrProvider.getId(), VirtualMachineName.getSystemVmName(id, _instance, ELB_VM_NAME_PREFIX),
template.getId(), template.getHypervisorType(), template.getGuestOSId(), owner.getDomainId(), owner.getId(), userId, false, RedundantState.UNKNOWN,
elasticLbVmOffering.isOfferHA(), false, null);
Expand Down
7 changes: 4 additions & 3 deletions ...cer/src/main/java/org/apache/cloudstack/network/lb/InternalLoadBalancerVMManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,13 @@
import javax.inject.Inject;
import javax.naming.ConfigurationException;

import org.apache.log4j.Logger;

import org.apache.cloudstack.context.CallContext;
import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService;
import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
import org.apache.cloudstack.lb.ApplicationLoadBalancerRuleVO;
import org.apache.cloudstack.lb.dao.ApplicationLoadBalancerRuleDao;
import org.apache.log4j.Logger;

import com.cloud.agent.AgentManager;
import com.cloud.agent.api.Answer;
Expand All @@ -45,7 +46,7 @@
import com.cloud.agent.api.to.LoadBalancerTO;
import com.cloud.agent.manager.Commands;
import com.cloud.configuration.Config;
import com.cloud.configuration.ConfigurationManagerImpl;
import com.cloud.configuration.ConfigurationManager;
import com.cloud.dc.DataCenter;
import com.cloud.dc.DataCenterVO;
import com.cloud.dc.dao.DataCenterDao;
Expand Down Expand Up @@ -629,7 +630,7 @@ protected List<DomainRouterVO> findOrDeployInternalLbVm(final Network guestNetwo
final LinkedHashMap<Network, List<? extends NicProfile>> networks = createInternalLbVmNetworks(guestNetwork, plan, requestedGuestIp);
long internalLbVmOfferingId = _internalLbVmOfferingId;
if (internalLbVmOfferingId == 0L) {
ServiceOfferingVO serviceOffering = _serviceOfferingDao.findDefaultSystemOffering(ServiceOffering.internalLbVmDefaultOffUniqueName, ConfigurationManagerImpl.SystemVMUseLocalStorage.valueIn(dest.getDataCenter().getId()));
ServiceOfferingVO serviceOffering = _serviceOfferingDao.findDefaultSystemOffering(ServiceOffering.internalLbVmDefaultOffUniqueName, ConfigurationManager.SystemVMUseLocalStorage.valueIn(dest.getDataCenter().getId()));
internalLbVmOfferingId = serviceOffering.getId();
}
//Pass startVm=false as we are holding the network lock that needs to be released at the end of vm allocation
Expand Down
9 changes: 0 additions & 9 deletions server/src/main/java/com/cloud/configuration/Config.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -885,15 +885,6 @@ public enum Config {
"0",
"Default disk I/O writerate in bytes per second allowed in User vm's disk.",
null),
ControlCidr(
"Advanced",
ManagementServer.class,
String.class,
"control.cidr",
"169.254.0.0/16",
"Changes the cidr for the control network traffic. Defaults to using link local. Must be unique within pods",
null),
ControlGateway("Advanced", ManagementServer.class, String.class, "control.gateway", "169.254.0.1", "gateway for the control network traffic", null),
HostCapacityTypeToOrderClusters(
"Advanced",
ManagementServer.class,
Expand Down
6 changes: 2 additions & 4 deletions server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -460,8 +460,6 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
private Set<String> overprovisioningFactorsForValidation;
public static final String VM_USERDATA_MAX_LENGTH_STRING = "vm.userdata.max.length";

public static final ConfigKey<Boolean> SystemVMUseLocalStorage = new ConfigKey<Boolean>(Boolean.class, "system.vm.use.local.storage", "Advanced", "false",
"Indicates whether to use local storage pools or shared storage pools for system VMs.", false, ConfigKey.Scope.Zone, null);

public final static ConfigKey<Long> BYTES_MAX_READ_LENGTH= new ConfigKey<Long>(Long.class, "vm.disk.bytes.maximum.read.length", "Advanced", "0",
"Maximum Bytes read burst duration (seconds). If '0' (zero) then does not check for maximum burst length.", true, ConfigKey.Scope.Global, null);
Expand Down Expand Up @@ -2307,7 +2305,7 @@ public HostPodVO createPod(final long userId, final String podName, final DataCe
_zoneDao.addPrivateIpAddress(zone.getId(), pod.getId(), startIpFinal, endIpFinal, false, null);
}

final String[] linkLocalIpRanges = NetUtils.getLinkLocalIPRange(_configDao.getValue(Config.ControlCidr.key()));
final String[] linkLocalIpRanges = NetUtils.getLinkLocalIPRange(ControlCidr.valueIn(zone.getId()));
if (linkLocalIpRanges.length > 1) {
_zoneDao.addLinkLocalIpAddress(zone.getId(), pod.getId(), linkLocalIpRanges[0], linkLocalIpRanges[1]);
}
Expand Down Expand Up @@ -7628,7 +7626,7 @@ public String getConfigComponentName() {

@Override
public ConfigKey<?>[] getConfigKeys() {
return new ConfigKey<?>[] {SystemVMUseLocalStorage, IOPS_MAX_READ_LENGTH, IOPS_MAX_WRITE_LENGTH,
return new ConfigKey<?>[] {SystemVMUseLocalStorage, ControlCidr, ControlGateway, IOPS_MAX_READ_LENGTH, IOPS_MAX_WRITE_LENGTH,
BYTES_MAX_READ_LENGTH, BYTES_MAX_WRITE_LENGTH, ADD_HOST_ON_SERVICE_RESTART_KVM, SET_HOST_DOWN_TO_MAINTENANCE, VM_SERVICE_OFFERING_MAX_CPU_CORES,
VM_SERVICE_OFFERING_MAX_RAM_SIZE, VM_USERDATA_MAX_LENGTH, MIGRATE_VM_ACROSS_CLUSTERS,
ENABLE_ACCOUNT_SETTINGS_FOR_DOMAIN, ENABLE_DOMAIN_SETTINGS_FOR_CHILD_DOMAIN, ALLOW_DOMAIN_ADMINS_TO_CREATE_TAGGED_OFFERINGS
Expand Down
6 changes: 3 additions & 3 deletions server/src/main/java/com/cloud/consoleproxy/ConsoleProxyManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@
import com.cloud.agent.manager.Commands;
import com.cloud.cluster.ClusterManager;
import com.cloud.configuration.Config;
import com.cloud.configuration.ConfigurationManagerImpl;
import com.cloud.configuration.ConfigurationManager;
import com.cloud.configuration.ZoneConfig;
import com.cloud.dc.DataCenter;
import com.cloud.dc.DataCenter.NetworkType;
Expand Down Expand Up @@ -696,7 +696,7 @@ protected Map<String, Object> createProxyInstance(long dataCenterId, VMTemplateV

ServiceOfferingVO serviceOffering = serviceOfferingVO;
if (serviceOffering == null) {
serviceOffering = serviceOfferingDao.findDefaultSystemOffering(ServiceOffering.consoleProxyDefaultOffUniqueName, ConfigurationManagerImpl.SystemVMUseLocalStorage.valueIn(dataCenterId));
serviceOffering = serviceOfferingDao.findDefaultSystemOffering(ServiceOffering.consoleProxyDefaultOffUniqueName, ConfigurationManager.SystemVMUseLocalStorage.valueIn(dataCenterId));
}
ConsoleProxyVO proxy =
new ConsoleProxyVO(id, serviceOffering.getId(), name, template.getId(), template.getHypervisorType(), template.getGuestOSId(), dataCenterId,
Expand Down Expand Up @@ -888,7 +888,7 @@ public boolean isZoneReady(Map<Long, ZoneHostInfo> zoneHostInfoMap, long dataCen
}

if (templateHostRef != null) {
Boolean useLocalStorage = BooleanUtils.toBoolean(ConfigurationManagerImpl.SystemVMUseLocalStorage.valueIn(dataCenterId));
Boolean useLocalStorage = BooleanUtils.toBoolean(ConfigurationManager.SystemVMUseLocalStorage.valueIn(dataCenterId));
List<Pair<Long, Integer>> l = consoleProxyDao.getDatacenterStoragePoolHostInfo(dataCenterId, useLocalStorage);
if (CollectionUtils.isNotEmpty(l) && l.get(0).second() > 0) {
return true;
Expand Down
26 changes: 13 additions & 13 deletions server/src/main/java/com/cloud/deploy/DeploymentPlanningManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,20 +31,11 @@
import javax.inject.Inject;
import javax.naming.ConfigurationException;

import org.apache.cloudstack.affinity.AffinityGroupDomainMapVO;
import com.cloud.storage.VMTemplateVO;
import com.cloud.storage.dao.VMTemplateDao;
import com.cloud.user.AccountVO;
import com.cloud.user.dao.AccountDao;
import com.cloud.exception.StorageUnavailableException;
import com.cloud.utils.db.Filter;
import com.cloud.utils.fsm.StateMachine2;

import org.apache.cloudstack.framework.config.ConfigKey;
import org.apache.cloudstack.framework.config.Configurable;
import org.apache.commons.collections.CollectionUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.log4j.Logger;

import org.apache.cloudstack.affinity.AffinityGroupDomainMapVO;
import org.apache.cloudstack.affinity.AffinityGroupProcessor;
import org.apache.cloudstack.affinity.AffinityGroupService;
import org.apache.cloudstack.affinity.AffinityGroupVMMapVO;
Expand All @@ -57,6 +48,8 @@
import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
import org.apache.cloudstack.engine.subsystem.api.storage.StoragePoolAllocator;
import org.apache.cloudstack.framework.config.ConfigKey;
import org.apache.cloudstack.framework.config.Configurable;
import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
import org.apache.cloudstack.framework.messagebus.MessageBus;
import org.apache.cloudstack.framework.messagebus.MessageSubscriber;
Expand All @@ -77,7 +70,7 @@
import com.cloud.capacity.CapacityManager;
import com.cloud.capacity.dao.CapacityDao;
import com.cloud.configuration.Config;
import com.cloud.configuration.ConfigurationManagerImpl;
import com.cloud.configuration.ConfigurationManager;
import com.cloud.dc.ClusterDetailsDao;
import com.cloud.dc.ClusterDetailsVO;
import com.cloud.dc.ClusterVO;
Expand All @@ -95,6 +88,7 @@
import com.cloud.exception.AffinityConflictException;
import com.cloud.exception.ConnectionException;
import com.cloud.exception.InsufficientServerCapacityException;
import com.cloud.exception.StorageUnavailableException;
import com.cloud.gpu.GPU;
import com.cloud.host.DetailVO;
import com.cloud.host.Host;
Expand All @@ -115,26 +109,32 @@
import com.cloud.storage.StorageManager;
import com.cloud.storage.StoragePool;
import com.cloud.storage.StoragePoolHostVO;
import com.cloud.storage.VMTemplateVO;
import com.cloud.storage.Volume;
import com.cloud.storage.VolumeVO;
import com.cloud.storage.dao.DiskOfferingDao;
import com.cloud.storage.dao.GuestOSCategoryDao;
import com.cloud.storage.dao.GuestOSDao;
import com.cloud.storage.dao.StoragePoolHostDao;
import com.cloud.storage.dao.VMTemplateDao;
import com.cloud.storage.dao.VolumeDao;
import com.cloud.user.AccountManager;
import com.cloud.user.AccountVO;
import com.cloud.user.dao.AccountDao;
import com.cloud.utils.DateUtil;
import com.cloud.utils.NumbersUtil;
import com.cloud.utils.Pair;
import com.cloud.utils.component.Manager;
import com.cloud.utils.component.ManagerBase;
import com.cloud.utils.db.DB;
import com.cloud.utils.db.Filter;
import com.cloud.utils.db.SearchCriteria;
import com.cloud.utils.db.Transaction;
import com.cloud.utils.db.TransactionCallback;
import com.cloud.utils.db.TransactionStatus;
import com.cloud.utils.exception.CloudRuntimeException;
import com.cloud.utils.fsm.StateListener;
import com.cloud.utils.fsm.StateMachine2;
import com.cloud.vm.DiskProfile;
import com.cloud.vm.VMInstanceVO;
import com.cloud.vm.VirtualMachine;
Expand Down Expand Up @@ -1728,7 +1728,7 @@ protected Pair<Map<Volume, List<StoragePool>>, List<Volume>> findSuitablePoolsFo
if (vmProfile.getType() != VirtualMachine.Type.User) {
DataCenterVO zone = _dcDao.findById(plan.getDataCenterId());
assert (zone != null) : "Invalid zone in deployment plan";
Boolean useLocalStorageForSystemVM = ConfigurationManagerImpl.SystemVMUseLocalStorage.valueIn(zone.getId());
Boolean useLocalStorageForSystemVM = ConfigurationManager.SystemVMUseLocalStorage.valueIn(zone.getId());
if (useLocalStorageForSystemVM != null) {
useLocalStorage = useLocalStorageForSystemVM.booleanValue();
s_logger.debug("System VMs will use " + (useLocalStorage ? "local" : "shared") + " storage for zone id=" + plan.getDataCenterId());
Expand Down
Loading