You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by "DaanHoogland (via GitHub)" <gi...@apache.org> on 2023/07/18 06:12:29 UTC
[GitHub] [cloudstack] DaanHoogland opened a new pull request, #7746: make control.cidr dynamic
DaanHoogland opened a new pull request, #7746:
URL: https://github.com/apache/cloudstack/pull/7746
### Description
This PR attempts to make choosing local ip choices adhere to a more dynamics control.cidr so people can actually choose their control network....
<!--- Describe your changes in DETAIL - And how has behaviour functionally changed. -->
<!-- For new features, provide link to FS, dev ML discussion etc. -->
<!-- In case of bug fix, the expected and actual behaviours, steps to reproduce. -->
<!-- When "Fixes: #<id>" is specified, the issue/PR will automatically be closed when this PR gets merged -->
<!-- For addressing multiple issues/PRs, use multiple "Fixes: #<id>" -->
Fixes: #6715
<!--- ********************************************************************************* -->
<!--- NOTE: AUTOMATATION USES THE DESCRIPTIONS TO SET LABELS AND PRODUCE DOCUMENTATION. -->
<!--- PLEASE PUT AN 'X' in only **ONE** box -->
<!--- ********************************************************************************* -->
### Types of changes
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] New feature (non-breaking change which adds functionality)
- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] Enhancement (improves an existing feature and functionality)
- [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
### Feature/Enhancement Scale or Bug Severity
#### Feature/Enhancement Scale
- [x] Major
- [ ] Minor
#### Bug Severity
- [ ] BLOCKER
- [ ] Critical
- [ ] Major
- [x] Minor
- [ ] Trivial
### Screenshots (if appropriate):
### How Has This Been Tested?
<!-- Please describe in detail how you tested your changes. -->
<!-- Include details of your testing environment, and the tests you ran to -->
<!-- see how your change affects other areas of the code, etc. -->
<!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/main/CONTRIBUTING.md) document -->
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on a diff in pull request #7746: make control.cidr dynamic
Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on code in PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#discussion_r1293424888
##########
engine/components-api/src/main/java/com/cloud/configuration/ConfigurationManager.java:
##########
@@ -20,6 +20,7 @@
import java.util.Map;
import java.util.Set;
+import net.sf.ehcache.management.ManagementServer;
Review Comment:
this is not required
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7746: make control.cidr dynamic
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#issuecomment-1667511275
Packaging result [SF]: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: el9 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 6688
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GutoVeronezi commented on pull request #7746: make control.cidr dynamic
Posted by "GutoVeronezi (via GitHub)" <gi...@apache.org>.
GutoVeronezi commented on PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#issuecomment-1643835052
@DaanHoogland, there are some refactorings being done that are not related to the PR proposal; would not be better to create a new PR for the refactoring and separate the contexts?
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7746: make control.cidr dynamic
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#issuecomment-1677411673
@DaanHoogland a [SF] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on a diff in pull request #7746: make control.cidr and control.gateway dynamic
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on code in PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#discussion_r1309883867
##########
engine/components-api/src/main/java/com/cloud/configuration/ConfigurationManager.java:
##########
@@ -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<>(
+ String.class,
+ "control.cidr",
+ "Advanced",
+ "169.254.0.0/16",
Review Comment:
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 .
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] wido commented on a diff in pull request #7746: make control.cidr and control.gateway dynamic
Posted by "wido (via GitHub)" <gi...@apache.org>.
wido commented on code in PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#discussion_r1310112588
##########
engine/components-api/src/main/java/com/cloud/configuration/ConfigurationManager.java:
##########
@@ -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<>(
+ String.class,
+ "control.cidr",
+ "Advanced",
+ "169.254.0.0/16",
Review Comment:
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.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] make control.cidr and control.gateway dynamic [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland closed pull request #7746: make control.cidr and control.gateway dynamic
URL: https://github.com/apache/cloudstack/pull/7746
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] make control.cidr and control.gateway dynamic [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#issuecomment-1743056612
i disagree with this PR myself
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7746: make control.cidr dynamic
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#issuecomment-1639555755
@DaanHoogland a [SF] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7746: make control.cidr dynamic
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#issuecomment-1641771329
Packaging result [SF]: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: el9 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 6509
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7746: make control.cidr dynamic
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#issuecomment-1641201571
<b>[SF] Trillian test result (tid-7106)</b>
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server r8
Total time taken: 49009 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7746-t7106-vmware-67u3.zip
Smoke tests completed. 107 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_02_upgrade_kubernetes_cluster | `Failure` | 379.10 | test_kubernetes_clusters.py
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7746: make control.cidr dynamic
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#issuecomment-1668757877
<b>[SF] Trillian test result (tid-7315)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 53573 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7746-t7315-kvm-centos7.zip
Smoke tests completed. 106 look OK, 2 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_list_vms_metrics_admin | `Error` | 3610.63 | test_metrics_api.py
test_list_vms_metrics_history | `Error` | 7.46 | test_metrics_api.py
test_list_volumes_metrics_history | `Error` | 4.37 | test_metrics_api.py
test_01_migrate_VM_and_root_volume | `Error` | 88.02 | test_vm_life_cycle.py
test_02_migrate_VM_with_two_data_disks | `Error` | 62.70 | test_vm_life_cycle.py
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7746: make control.cidr dynamic
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#issuecomment-1667351023
<b>[SF] Trillian Build Failed (tid-7302)<b/>
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Rubueno commented on pull request #7746: make control.cidr dynamic
Posted by "Rubueno (via GitHub)" <gi...@apache.org>.
Rubueno commented on PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#issuecomment-1653436441
> > Thanks for spending time to work on this Daan! LGTM
>
> Did you test it @Rubueno ?
No, not yet. I've not had the time to do so. Pretty swamped!
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] wido commented on pull request #7746: make control.cidr and control.gateway dynamic
Posted by "wido (via GitHub)" <gi...@apache.org>.
wido commented on PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#issuecomment-1685377973
> > > @Rubueno @wido I gave this another look and it is not going to work. The link local addresses are created on createPod and changing the configurations will break any SVM created after changing the settings. We'll need a more extensive disign. Can you give some input on what a valid scenario for you would be?
> > > thanks
> >
> >
> > We should move to IPv6 Link-Local at all for the SSVM :-) These addresses with IPv4 aren't needed at all ;-) Makes life much easier, I promise!
> > Anyway, this isn't the biggest problem now.
> > In RFC5549 the IPv4 address 169.254.0.1 is used for routing IPv4 traffic via IPv6, something which you will use when using EVPN+BGP+VXLAN.
> > Therefor you want to change the control CIDR to something else. It's 'stupid' that we allocate a /16 to this as that's not needed at all, a /20 by default should be more then enough.
> > Any existing installations work just fine. Maybe we should just change the default from this /16 to a /20 (169.254.240.0/20) and be done with it for now.
>
> thanks @wido
>
> @DaanHoogland I think the main problem for this PR is, all link local IPs are saved in database table `op_dc_link_local_ip_address_alloc` which is not needed at all. We need to save only the allocated IPs in the table and remove all unused IPs. Some classes need to be updated to support the IP allocation from cidr/gateway, instead of picking up IPs from `op_dc_link_local_ip_address_alloc` table.
And in addition I think we should opt for a /20 by default for all new installations. If we do that, this whole problem is kind of solved. For new deployments the /20 would be used and never get into the way of BGP with v4 via v6 routing.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #7746: make control.cidr dynamic
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#issuecomment-1678487078
@blueorangutan test
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7746: make control.cidr dynamic
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#issuecomment-1641902969
<b>[SF] Trillian Build Failed (tid-7121)<b/>
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #7746: make control.cidr dynamic
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#issuecomment-1643752965
@blueorangutan package
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on a diff in pull request #7746: make control.cidr dynamic
Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on code in PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#discussion_r1293425930
##########
server/src/main/java/com/cloud/network/guru/ControlNetworkGuru.java:
##########
@@ -215,18 +215,6 @@ public boolean configure(String name, Map<String, Object> params) throws Configu
Map<String, String> dbParams = _configDao.getConfiguration(params);
- _cidr = dbParams.get(Config.ControlCidr.toString());
- if (_cidr == null) {
- _cidr = NetUtils.getLinkLocalCIDR();
- }
-
- _gateway = dbParams.get(Config.ControlGateway.toString());
- if (_gateway == null) {
- _gateway = NetUtils.getLinkLocalGateway();
- }
Review Comment:
ok, looks good
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7746: make control.cidr and control.gateway dynamic
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#issuecomment-1680602531
Packaging result [SF]: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: el9 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 6801
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #7746: make control.cidr dynamic
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#issuecomment-1667404009
@blueorangutan package
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #7746: make control.cidr dynamic
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#issuecomment-1667527899
@blueorangutan test keepEnv
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on a diff in pull request #7746: make control.cidr dynamic
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on code in PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#discussion_r1293421469
##########
server/src/main/java/com/cloud/network/guru/ControlNetworkGuru.java:
##########
@@ -215,18 +215,6 @@ public boolean configure(String name, Map<String, Object> params) throws Configu
Map<String, String> dbParams = _configDao.getConfiguration(params);
- _cidr = dbParams.get(Config.ControlCidr.toString());
- if (_cidr == null) {
- _cidr = NetUtils.getLinkLocalCIDR();
- }
-
- _gateway = dbParams.get(Config.ControlGateway.toString());
- if (_gateway == null) {
- _gateway = NetUtils.getLinkLocalGateway();
- }
Review Comment:
set but not in this location
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on a diff in pull request #7746: make control.cidr and control.gateway dynamic
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on code in PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#discussion_r1308401998
##########
engine/components-api/src/main/java/com/cloud/configuration/ConfigurationManager.java:
##########
@@ -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<>(
+ String.class,
+ "control.cidr",
+ "Advanced",
+ "169.254.0.0/16",
Review Comment:
```suggestion
"169.254.240.0/20",
```
like this and abandon all other changes in this PR, right @wido ?
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7746: make control.cidr dynamic
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#issuecomment-1641105414
<b>[SF] Trillian test result (tid-7105)</b>
Environment: xenserver-71 (x2), Advanced Networking with Mgmt server 7
Total time taken: 44065 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7746-t7105-xenserver-71.zip
Smoke tests completed. 107 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_create_pvlan_network | `Error` | 0.05 | test_pvlan.py
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on a diff in pull request #7746: make control.cidr dynamic
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on code in PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#discussion_r1291225660
##########
server/src/main/java/com/cloud/configuration/Config.java:
##########
@@ -885,14 +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),
Review Comment:
I'll add it (and move it as well)
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7746: make control.cidr dynamic
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#issuecomment-1641779909
@DaanHoogland a [SF] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #7746: make control.cidr dynamic
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#issuecomment-1643884249
> @DaanHoogland, there are some refactorings being done that are not related to the PR proposal; would not be better to create a new PR for the refactoring and separate the contexts?
I could do @GutoVeronezi , but I find that I do these refactoring for undestanding the software and the problem. Thus the changes are usually intertwined a bit. I gather you ask this because it makes reviewing harder?
(btw, i still have to think of a way to verify this change)
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7746: make control.cidr dynamic
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#issuecomment-1641682819
Packaging result [SF]: :heavy_multiplication_x: el7 :heavy_multiplication_x: el8 :heavy_multiplication_x: el9 :heavy_multiplication_x: debian :heavy_multiplication_x: suse15. SL-JID 6508
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #7746: make control.cidr dynamic
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#issuecomment-1665620656
@blueorangutan package
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7746: make control.cidr dynamic
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#issuecomment-1639969252
@DaanHoogland a [SF] Trillian-Jenkins matrix job (centos7 mgmt + xenserver71, rocky8 mgmt + vmware67u3, centos7 mgmt + kvmcentos7) has been kicked to run smoke tests
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #7746: make control.cidr and control.gateway dynamic
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#issuecomment-1681960222
@Rubueno @wido I gave this another look and it is not going to work. The link local addresses are created on createPod and changing the configurations will break any SVM created after changing the settings. We'll need a more extensive disign. Can you give some input on what a valid scenario for you would be?
thanks
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] harikrishna-patnala commented on a diff in pull request #7746: make control.cidr dynamic
Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on code in PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#discussion_r1269107003
##########
engine/components-api/src/main/java/com/cloud/configuration/ConfigurationManager.java:
##########
@@ -54,10 +55,30 @@
*/
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<>(
Review Comment:
we need to add this config to getConfigKeys() method @DaanHoogland
https://github.com/shapeblue/cloudstack/blob/1275db4081aa8b09148909d6e6b65bc377d4992f/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java#L7630
##########
server/src/main/java/com/cloud/network/guru/ControlNetworkGuru.java:
##########
@@ -21,10 +21,12 @@
import javax.inject.Inject;
import javax.naming.ConfigurationException;
-import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
+import com.cloud.network.NetworkModel;
Review Comment:
reorder this please
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Rubueno commented on pull request #7746: make control.cidr dynamic
Posted by "Rubueno (via GitHub)" <gi...@apache.org>.
Rubueno commented on PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#issuecomment-1644644253
Thanks for spending time to work on this Daan! LGTM
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7746: make control.cidr dynamic
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#issuecomment-1641128654
<b>[SF] Trillian test result (tid-7107)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 45451 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7746-t7107-kvm-centos7.zip
Smoke tests completed. 107 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_01_migrate_VM_and_root_volume | `Error` | 78.67 | test_vm_life_cycle.py
test_02_migrate_VM_with_two_data_disks | `Error` | 54.42 | test_vm_life_cycle.py
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7746: make control.cidr dynamic
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#issuecomment-1641668455
@DaanHoogland a [SF] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GutoVeronezi commented on pull request #7746: make control.cidr dynamic
Posted by "GutoVeronezi (via GitHub)" <gi...@apache.org>.
GutoVeronezi commented on PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#issuecomment-1643923645
> > @DaanHoogland, there are some refactorings being done that are not related to the PR proposal; would not be better to create a new PR for the refactoring and separate the contexts?
>
> I could do @GutoVeronezi , but I find that I do these refactoring for undestanding the software and the problem. Thus the changes are usually intertwined a bit. I gather you ask this because it makes reviewing harder?
>
> (btw, i still have to think of a way to verify this change)
In this scale of changes (~250 lines), the reviewing still is quite simple. Personally, I like to separate the contexts when possible, to better organize and track the changes. It is just a suggestion, though. :grin:
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] codecov[bot] commented on pull request #7746: make control.cidr dynamic
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#issuecomment-1639629427
## [Codecov](https://app.codecov.io/gh/apache/cloudstack/pull/7746?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
> Merging [#7746](https://app.codecov.io/gh/apache/cloudstack/pull/7746?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (d7c1f28) into [4.18](https://app.codecov.io/gh/apache/cloudstack/commit/939ee9e1534a1e6013a74f92dbb151666b3887e3?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (939ee9e) will **increase** coverage by `0.00%`.
> The diff coverage is `66.66%`.
```diff
@@ Coverage Diff @@
## 4.18 #7746 +/- ##
=========================================
Coverage 13.02% 13.02%
- Complexity 9026 9028 +2
=========================================
Files 2719 2720 +1
Lines 256867 256830 -37
Branches 40051 40042 -9
=========================================
+ Hits 33445 33447 +2
+ Misses 219233 219196 -37
+ Partials 4189 4187 -2
```
| [Impacted Files](https://app.codecov.io/gh/apache/cloudstack/pull/7746?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [...ck/engine/subsystem/api/storage/VolumeService.java](https://app.codecov.io/gh/apache/cloudstack/pull/7746?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZW5naW5lL2FwaS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvY2xvdWRzdGFjay9lbmdpbmUvc3Vic3lzdGVtL2FwaS9zdG9yYWdlL1ZvbHVtZVNlcnZpY2UuamF2YQ==) | `0.00% <ø> (ø)` | |
| [...ine/subsystem/api/storage/type/VolumeTypeBase.java](https://app.codecov.io/gh/apache/cloudstack/pull/7746?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZW5naW5lL2FwaS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvY2xvdWRzdGFjay9lbmdpbmUvc3Vic3lzdGVtL2FwaS9zdG9yYWdlL3R5cGUvVm9sdW1lVHlwZUJhc2UuamF2YQ==) | `10.00% <ø> (ø)` | |
| [...e/subsystem/api/storage/type/VolumeTypeHelper.java](https://app.codecov.io/gh/apache/cloudstack/pull/7746?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZW5naW5lL2FwaS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvY2xvdWRzdGFjay9lbmdpbmUvc3Vic3lzdGVtL2FwaS9zdG9yYWdlL3R5cGUvVm9sdW1lVHlwZUhlbHBlci5qYXZh) | `100.00% <ø> (ø)` | |
| [.../main/java/com/cloud/network/IpAddressManager.java](https://app.codecov.io/gh/apache/cloudstack/pull/7746?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZW5naW5lL2NvbXBvbmVudHMtYXBpL3NyYy9tYWluL2phdmEvY29tL2Nsb3VkL25ldHdvcmsvSXBBZGRyZXNzTWFuYWdlci5qYXZh) | `100.00% <ø> (ø)` | |
| [...n/java/com/cloud/vm/VmWorkJobWakeupDispatcher.java](https://app.codecov.io/gh/apache/cloudstack/pull/7746?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZW5naW5lL29yY2hlc3RyYXRpb24vc3JjL21haW4vamF2YS9jb20vY2xvdWQvdm0vVm1Xb3JrSm9iV2FrZXVwRGlzcGF0Y2hlci5qYXZh) | `0.00% <ø> (ø)` | |
| [...tack/engine/orchestration/NetworkOrchestrator.java](https://app.codecov.io/gh/apache/cloudstack/pull/7746?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZW5naW5lL29yY2hlc3RyYXRpb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2Nsb3Vkc3RhY2svZW5naW5lL29yY2hlc3RyYXRpb24vTmV0d29ya09yY2hlc3RyYXRvci5qYXZh) | `6.06% <0.00%> (ø)` | |
| [.../cloud/configuration/dao/ResourceCountDaoImpl.java](https://app.codecov.io/gh/apache/cloudstack/pull/7746?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZW5naW5lL3NjaGVtYS9zcmMvbWFpbi9qYXZhL2NvbS9jbG91ZC9jb25maWd1cmF0aW9uL2Rhby9SZXNvdXJjZUNvdW50RGFvSW1wbC5qYXZh) | `8.84% <ø> (ø)` | |
| [...in/java/com/cloud/dc/dao/ClusterVSMMapDaoImpl.java](https://app.codecov.io/gh/apache/cloudstack/pull/7746?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZW5naW5lL3NjaGVtYS9zcmMvbWFpbi9qYXZhL2NvbS9jbG91ZC9kYy9kYW8vQ2x1c3RlclZTTU1hcERhb0ltcGwuamF2YQ==) | `0.00% <ø> (ø)` | |
| [...in/java/com/cloud/dc/dao/DomainVlanMapDaoImpl.java](https://app.codecov.io/gh/apache/cloudstack/pull/7746?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZW5naW5lL3NjaGVtYS9zcmMvbWFpbi9qYXZhL2NvbS9jbG91ZC9kYy9kYW8vRG9tYWluVmxhbk1hcERhb0ltcGwuamF2YQ==) | `54.54% <ø> (ø)` | |
| [...main/java/com/cloud/dc/dao/VlanDetailsDaoImpl.java](https://app.codecov.io/gh/apache/cloudstack/pull/7746?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZW5naW5lL3NjaGVtYS9zcmMvbWFpbi9qYXZhL2NvbS9jbG91ZC9kYy9kYW8vVmxhbkRldGFpbHNEYW9JbXBsLmphdmE=) | `0.00% <ø> (ø)` | |
| ... and [96 more](https://app.codecov.io/gh/apache/cloudstack/pull/7746?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | |
... and [10 files with indirect coverage changes](https://app.codecov.io/gh/apache/cloudstack/pull/7746/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #7746: make control.cidr dynamic
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#issuecomment-1649540632
> Thanks for spending time to work on this Daan! LGTM
Did you test it @Rubueno ?
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #7746: make control.cidr dynamic
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#issuecomment-1649542539
> > > @DaanHoogland, there are some refactorings being done that are not related to the PR proposal; would not be better to create a new PR for the refactoring and separate the contexts?
> >
> >
> > I could do @GutoVeronezi , but I find that I do these refactoring for undestanding the software and the problem. Thus the changes are usually intertwined a bit. I gather you ask this because it makes reviewing harder?
> > (btw, i still have to think of a way to verify this change)
>
> In this scale of changes (~250 lines), the reviewing still is quite simple. Personally, I like to separate the contexts when possible, to better organize and track the changes. It is just a suggestion, though. grin
noted
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #7746: make control.cidr dynamic
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#issuecomment-1639554455
@blueorangutan package
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7746: make control.cidr dynamic
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#issuecomment-1641808637
<b>[SF] Trillian Build Failed (tid-7119)<b/>
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #7746: make control.cidr dynamic
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#issuecomment-1667289362
@blueorangutan test
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #7746: make control.cidr dynamic
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#issuecomment-1677409185
@blueorangutan package
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] wido commented on a diff in pull request #7746: make control.cidr and control.gateway dynamic
Posted by "wido (via GitHub)" <gi...@apache.org>.
wido commented on code in PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#discussion_r1308479382
##########
engine/components-api/src/main/java/com/cloud/configuration/ConfigurationManager.java:
##########
@@ -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<>(
+ String.class,
+ "control.cidr",
+ "Advanced",
+ "169.254.0.0/16",
Review Comment:
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.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7746: make control.cidr and control.gateway dynamic
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#issuecomment-1679766204
<b>[SF] Trillian test result (tid-7409)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 60109 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7746-t7409-kvm-centos7.zip
Smoke tests completed. 106 look OK, 2 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
ContextSuite context=TestISOUsage>:setup | `Error` | 0.00 | test_usage.py
test_01_migrate_VM_and_root_volume | `Error` | 81.80 | test_vm_life_cycle.py
test_02_migrate_VM_with_two_data_disks | `Error` | 57.55 | test_vm_life_cycle.py
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on pull request #7746: make control.cidr and control.gateway dynamic
Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#issuecomment-1683565479
> > @Rubueno @wido I gave this another look and it is not going to work. The link local addresses are created on createPod and changing the configurations will break any SVM created after changing the settings. We'll need a more extensive disign. Can you give some input on what a valid scenario for you would be?
> > thanks
>
> We should move to IPv6 Link-Local at all for the SSVM :-) These addresses with IPv4 aren't needed at all ;-) Makes life much easier, I promise!
>
> Anyway, this isn't the biggest problem now.
>
> In RFC5549 the IPv4 address 169.254.0.1 is used for routing IPv4 traffic via IPv6, something which you will use when using EVPN+BGP+VXLAN.
>
> Therefor you want to change the control CIDR to something else. It's 'stupid' that we allocate a /16 to this as that's not needed at all, a /20 by default should be more then enough.
>
> Any existing installations work just fine. Maybe we should just change the default from this /16 to a /20 (169.254.240.0/20) and be done with it for now.
thanks @wido
@DaanHoogland I think the main problem for this PR is, all link local IPs are saved in database table `op_dc_link_local_ip_address_alloc` which is not needed at all.
We need to save only the allocated IPs in the table and remove all unused IPs.
Some classes need to be updated to support the IP allocation from cidr/gateway, instead of picking up IPs from `op_dc_link_local_ip_address_alloc` table.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #7746: make control.cidr dynamic
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#issuecomment-1641661458
@blueorangutan package
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7746: make control.cidr dynamic
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#issuecomment-1667529800
@DaanHoogland a [SF] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7746: make control.cidr dynamic
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#issuecomment-1667291733
@DaanHoogland a [SF] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #7746: make control.cidr dynamic
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#issuecomment-1639968865
@blueorangutan test matrix
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on a diff in pull request #7746: make control.cidr and control.gateway dynamic
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on code in PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#discussion_r1311183007
##########
engine/components-api/src/main/java/com/cloud/configuration/ConfigurationManager.java:
##########
@@ -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<>(
+ String.class,
+ "control.cidr",
+ "Advanced",
+ "169.254.0.0/16",
Review Comment:
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.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7746: make control.cidr dynamic
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#issuecomment-1665731452
Packaging result [SF]: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: el9 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 6667
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] wido commented on pull request #7746: make control.cidr and control.gateway dynamic
Posted by "wido (via GitHub)" <gi...@apache.org>.
wido commented on PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#issuecomment-1683546525
> @Rubueno @wido I gave this another look and it is not going to work. The link local addresses are created on createPod and changing the configurations will break any SVM created after changing the settings. We'll need a more extensive disign. Can you give some input on what a valid scenario for you would be?
>
> thanks
We should move to IPv6 Link-Local at all for the SSVM :-) These addresses with IPv4 aren't needed at all ;-) Makes life much easier, I promise!
Anyway, this isn't the biggest problem now.
In RFC5549 the IPv4 address 169.254.0.1 is used for routing IPv4 traffic via IPv6, something which you will use when using EVPN+BGP+VXLAN.
Therefor you want to change the control CIDR to something else. It's 'stupid' that we allocate a /16 to this as that's not needed at all, a /20 by default should be more then enough.
Any existing installations work just fine. Maybe we should just change the default from this /16 to a /20 (169.254.240.0/20) and be done with it for now.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on a diff in pull request #7746: make control.cidr dynamic
Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on code in PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#discussion_r1291031697
##########
server/src/main/java/com/cloud/configuration/Config.java:
##########
@@ -885,14 +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),
Review Comment:
@DaanHoogland
should ControlGateway be dynamic as well ?
##########
server/src/main/java/com/cloud/network/guru/ControlNetworkGuru.java:
##########
@@ -215,18 +215,6 @@ public boolean configure(String name, Map<String, Object> params) throws Configu
Map<String, String> dbParams = _configDao.getConfiguration(params);
- _cidr = dbParams.get(Config.ControlCidr.toString());
- if (_cidr == null) {
- _cidr = NetUtils.getLinkLocalCIDR();
- }
-
- _gateway = dbParams.get(Config.ControlGateway.toString());
- if (_gateway == null) {
- _gateway = NetUtils.getLinkLocalGateway();
- }
Review Comment:
@DaanHoogland
should _gateway be set ?
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7746: make control.cidr dynamic
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#issuecomment-1641865788
<b>[SF] Trillian Build Failed (tid-7120)<b/>
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7746: make control.cidr dynamic
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#issuecomment-1643753576
@DaanHoogland a [SF] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7746: make control.cidr dynamic
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#issuecomment-1643825188
Packaging result [SF]: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: el9 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 6522
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #7746: make control.cidr dynamic
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#issuecomment-1641777359
@blueorangutan test
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7746: make control.cidr dynamic
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#issuecomment-1639636737
Packaging result [SF]: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: el9 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 6498
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7746: make control.cidr dynamic
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#issuecomment-1665621942
@DaanHoogland a [SF] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7746: make control.cidr dynamic
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#issuecomment-1667406319
@DaanHoogland a [SF] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on a diff in pull request #7746: make control.cidr and control.gateway dynamic
Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on code in PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#discussion_r1309923306
##########
engine/components-api/src/main/java/com/cloud/configuration/ConfigurationManager.java:
##########
@@ -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<>(
+ String.class,
+ "control.cidr",
+ "Advanced",
+ "169.254.0.0/16",
Review Comment:
@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.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7746: make control.cidr dynamic
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#issuecomment-1677560889
Packaging result [SF]: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: el9 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 6773
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7746: make control.cidr dynamic
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#issuecomment-1678487498
@DaanHoogland a [SF] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on a diff in pull request #7746: make control.cidr dynamic
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on code in PR #7746:
URL: https://github.com/apache/cloudstack/pull/7746#discussion_r1293520428
##########
engine/components-api/src/main/java/com/cloud/configuration/ConfigurationManager.java:
##########
@@ -20,6 +20,7 @@
import java.util.Map;
import java.util.Set;
+import net.sf.ehcache.management.ManagementServer;
Review Comment:
hm, how did that get in?
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org