[GitHub] cloudstack issue #1743: CLOUDSTACK-8326: Always fill UDP checksums in DHCP r...
Github user The-Loeki commented on the issue: https://github.com/apache/cloudstack/pull/1743 See https://bugzilla.redhat.com/show_bug.cgi?id=910619 for what's actually going on (and what actually should be going on) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack issue #1603: Vrouter fixes pylint
Github user The-Loeki commented on the issue: https://github.com/apache/cloudstack/pull/1603 Hi :) sorry I was unable to process your request to merge your PR with mine, as you can probably tell by now this would've oversized & muddied both PR's ;) Depending on the uptake of the CS team here I'm quite willing to help you with it, because as I mentioned IMHO this should've been done from the start on. So the `.format()` syntax is less compatible as you mention. It is however only incompatible with long-deprecated Python versions and CS SysVM's are compatible so that's not really an issue. While of course it is a matter of personal taste, the official message from Python pplz however is that `.format()` is intended to replace the `%s` formatting: https://docs.python.org/2/library/stdtypes.html#str.format , which is why I and the other OSS projects I work on prefer it. The logging one is an excellent remark though, wading through the docs I notice that they've added the `.format()` syntax as per 3.2: https://docs.python.org/3/howto/logging-cookbook.html#use-of-alternative-formatting-styles I'm not sure from the top of my head which version the SysVM's are using, but if it's not Py3 it oughta be (here comes the next big-ass PR ;) ) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack issue #1603: Vrouter fixes pylint
Github user The-Loeki commented on the issue:
https://github.com/apache/cloudstack/pull/1603
kudos for all the hard work! Most of this should've been done a long time
ago as `pylint` is one of the first firewalls of code quality ensurance.
Side notes:
* no fan of `%s` syntax; recommend further refactoring to employ
`.format()` which should make the code a lot more elegant.
* Strings oughta be single-quoted unless double quotes are necessary
e.g.:
```
-self.fw.append(
-["nat", "",
- "-I PREROUTING -i ppp+ -m tcp --dport 53 -j DNAT --to-destination
%s" %
- local_ip
- ]
-)
+self.fw.append([
+'nat', '',
+'-I PREROUTING -i ppp+ -m tcp --dport 53 -j DNAT --to-destination
{}'.format(
+local_ip
+)])
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Fixes for VirtualRouters in Basic Network...
Github user The-Loeki commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1547#discussion_r65132038
--- Diff: systemvm/patches/debian/config/etc/init.d/cloud-early-config ---
@@ -1207,33 +1145,27 @@ setup_secstorage() {
fi
setup_apache2 $ETH2_IP
+ # Deprecated, should move to Cs Python all of it
+ sed -e "s///" \
+-e "s///" \
+-e "s/Listen .*:80/Listen $ETH2_IP:80/g" \
+-e "s/Listen .*:443/Listen $ETH2_IP:443/g" \
+-e "s/NameVirtualHost .*:80/NameVirtualHost $ETH2_IP:80/g"
/etc/apache2/vhost.template > /etc/apache2/sites-enabled/vhost-${ETH2_IP}.conf
+
log_it "setting up apache2 for post upload of volume/template"
a2enmod proxy
a2enmod proxy_http
a2enmod headers
- SSL_FILE="/etc/apache2/sites-available/default-ssl"
- PATTERN="RewriteRule ^\/upload\/(.*)"
- CORS_PATTERN="Header set Access-Control-Allow-Origin"
- if [ -f $SSL_FILE ]; then
-if grep -q "$PATTERN" $SSL_FILE ; then
- log_it "rewrite rules already exist in file $SSL_FILE"
-else
-log_it "adding rewrite rules to file: $SSL_FILE"
-sed -i -e "s/<\/VirtualHost>/RewriteEngine On \n&/" $SSL_FILE
-sed -i -e "s/<\/VirtualHost>/RewriteCond %{HTTPS} =on \n&/"
$SSL_FILE
-sed -i -e "s/<\/VirtualHost>/RewriteCond %{REQUEST_METHOD} =POST
\n&/" $SSL_FILE
-sed -i -e "s/<\/VirtualHost>/RewriteRule ^\/upload\/(.*)
http:\/\/127.0.0.1:8210\/upload?uuid=\$1 [P,L] \n&/" $SSL_FILE
-fi
-if grep -q "$CORS_PATTERN" $SSL_FILE ; then
- log_it "cors rules already exist in file $SSL_FILE"
-else
-log_it "adding cors rules to file: $SSL_FILE"
-sed -i -e "s/<\/VirtualHost>/Header always set
Access-Control-Allow-Origin \"*\" \n&/" $SSL_FILE
-sed -i -e "s/<\/VirtualHost>/Header always set
Access-Control-Allow-Methods \"POST, OPTIONS\" \n&/" $SSL_FILE
-sed -i -e "s/<\/VirtualHost>/Header always set
Access-Control-Allow-Headers \"x-requested-with, Content-Type, origin,
authorization, accept, client-security-token, x-signature, x-metadata,
x-expires\" \n&/" $SSL_FILE
-fi
- fi
+ cat >/etc/apache2/cors.conf <http://127.0.0.1:8210/upload?uuid=\$1 [P,L]
+Header always set Access-Control-Allow-Origin "*"
+Header always set Access-Control-Allow-Methods "POST, OPTIONS"
+Header always set Access-Control-Allow-Headers "x-requested-with,
Content-Type, origin, authorization, accept, client-security-token,
x-signature, x-metadata, x-expires"
+CORS
--- End diff --
It should only generate on a SecStore which is why the `Include` of must be
conditional
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Fixes for VirtualRouters in Basic Network...
Github user The-Loeki commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1547#discussion_r65131979
--- Diff: systemvm/patches/debian/config/etc/init.d/cloud-early-config ---
@@ -1207,33 +1145,27 @@ setup_secstorage() {
fi
setup_apache2 $ETH2_IP
+ # Deprecated, should move to Cs Python all of it
+ sed -e "s///" \
+-e "s///" \
+-e "s/Listen .*:80/Listen $ETH2_IP:80/g" \
+-e "s/Listen .*:443/Listen $ETH2_IP:443/g" \
+-e "s/NameVirtualHost .*:80/NameVirtualHost $ETH2_IP:80/g"
/etc/apache2/vhost.template > /etc/apache2/sites-enabled/vhost-${ETH2_IP}.conf
--- End diff --
It didn't fail, it just didn't happen because that piece of shellscript is
exclusively for the SecStore, while VirtualRouter already utilizes the Python
bits.
Fixing that migration is unfortunately out of scope for this PR.
The `NameVirtualHost` directive is now added to `CsApp.py`, tnx 4 testing
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Fixes for VirtualRouters in Basic Network...
Github user The-Loeki commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1547#discussion_r65131836
--- Diff: systemvm/patches/debian/config/etc/init.d/cloud-early-config ---
@@ -801,38 +802,29 @@ setup_sshd(){
setup_vpc_apache2() {
log_it "Setting up apache web server for VPC"
chkconfig apache2 off
- rm -f /etc/apache2/conf.d/vhost*.conf
- [ -f /etc/apache2/sites-available/default ] && echo ""
>/etc/apache2/sites-available/default
- [ -f /etc/apache2/sites-available/default-ssl ] && echo
"">/etc/apache2/sites-available/default-ssl
- [ -f /etc/apache2/ports.conf ] && echo "">/etc/apache2/ports.conf
- [ -f /etc/apache2/ports.conf ] && echo "">/etc/apache2/ports.conf
- [ -f /etc/apache2/ports.conf ] && echo "">/etc/apache2/ports.conf
- [ -f /etc/apache2/conf.d/security ] && sed -i -e "s/^ServerTokens
.*/ServerTokens Prod/g" /etc/apache2/conf.d/security
- [ -f /etc/apache2/conf.d/security ] && sed -i -e "s/^ServerSignature
.*/ServerSignature Off/g" /etc/apache2/conf.d/security
-
- # Disable listing of http://SSVM-IP/icons folder for security issue. see
article
http://www.i-lateral.com/tutorials/disabling-the-icons-folder-on-an-ubuntu-web-server/
- [ -f /etc/apache2/mods-available/alias.conf ] && sed -i s/"Options
Indexes MultiViews"/"Options -Indexes MultiViews"/
/etc/apache2/mods-available/alias.conf
-
- echo "Options -Indexes" > /var/www/html/.htaccess
+ clean_ipalias_config
+ setup_apache2_common
}
clean_ipalias_config() {
-rm -f /etc/apache2/conf.d/ports.*.meta-data.conf
-rm -f /etc/apache2/sites-available/ipAlias*
-rm -f /etc/apache2/sites-enabled/ipAlias*
-rm -rf /etc/failure_config
+ # Old
+ rm -f /etc/apache2/conf.d/ports.*.meta-data.conf
+ rm -f /etc/apache2/sites-available/ipAlias*
+ rm -f /etc/apache2/sites-enabled/ipAlias*
+ rm -f /etc/apache2/conf.d/vhost*.conf
+ rm -f /etc/apache2/ports.conf
+ rm -f /etc/apache2/sites-available/default
+ rm -f /etc/apache2/sites-available/default-ssl
--- End diff --
I do & I just did ;)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Fixes for VirtualRouters in Basic Network...
Github user The-Loeki commented on the pull request: https://github.com/apache/cloudstack/pull/1547#issuecomment-220043104 'aight, pulling that string reveals a few more problems to fix unfortunately. * `cloud-init-early` shell script creates/modifies `ports.conf`, `default`, `default-ssl`. Considering the work towards the Python based code and the deprecation and bitrot already in that script, it seems most logical to stop `cloud-init-early` from touching Apaches' ports & default vhost files. * the SSL config in `default-ssl` differs from the one in `vhostexample.conf`, and I'm assuming the former is the correct one. If someone can confirm this I'll hack up the necessary patch --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: Fixes for VirtualRouters in Basic Network...
Github user The-Loeki commented on the pull request: https://github.com/apache/cloudstack/pull/1547#issuecomment-220022831 @linas-h1p tnx for the intel. Strange that I didn't encounter this, I'll go check --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: Fixes for VirtualRouters in Basic Network...
Github user The-Loeki commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1547#discussion_r63691374
--- Diff: systemvm/patches/debian/config/opt/cloud/bin/cs/CsConfig.py ---
@@ -58,25 +58,35 @@ def get_level(self):
return self.__LOG_LEVEL
def is_vpc(self):
-return self.cl.get_type() == "vpcrouter"
+return self.cl.get_type() == 'vpcrouter'
def is_router(self):
-return self.cl.get_type() == "router"
+return self.cl.get_type() == 'router'
+
+def is_dhcp(self):
+return self.cl.get_type() == 'dhcpsrvr'
+
+def has_dns(self):
+return not self.use_extdns()
+
+def has_metadata(self):
+return any((self.is_vpc(), self.is_router(), self.is_dhcp()))
def get_domain(self):
return self.cl.get_domain()
+def use_extdns(self):
+return self.cmdline().idata().get('useextdns', 'false') == 'true'
+
def get_dns(self):
+conf = self.cmdline().idata()
dns = []
-if not self.cl.get_use_ext_dns():
-if not self.is_vpc() and self.cl.is_redundant():
--- End diff --
@ustcweizhou / @DaanHoogland: This should revert the behaviour
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Fixes for VirtualRouters in Basic Network...
Github user The-Loeki commented on the pull request: https://github.com/apache/cloudstack/pull/1547#issuecomment-220004141 @linas-h1p thanks for noticing, that fix got lost in commit msg translation. I've amended the `CsAddress.py` commit to (un)fix the whitespacing & readded the `arpPing()` fix which caused your `KeyError`. The `ipaliases` stuff is for Pods which have multiple IP ranges in the same Guest VLAN, which is at least a Basic Networking feature. They indeed don't bring a `gateway` key with them, but CS assumes there is one to ping to verify the connection is up. By adding this tweak and reformatting the `ipalias.json` the ip aliases are now processed by the same code which adds & removes addresses, which prevents code duplication and additional files & bugs. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: Fixes for VirtualRouters in Basic Network...
Github user The-Loeki commented on the pull request: https://github.com/apache/cloudstack/pull/1547#issuecomment-219754165 Maybe, but for now it's still just one conditional which needs just one decision/explanation. On the plus side, if the decision can be taken in CsConfig correctly and CsGuestNetwork can employ the same logic this reduces code duplication and improves the overall quality. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: Fixes for VirtualRouters in Basic Network...
Github user The-Loeki commented on the pull request: https://github.com/apache/cloudstack/pull/1547#issuecomment-219709644 @DaanHoogland is correct; we're running this code in test-production. Keep in mind however that we only use Basic Networking Zones, so I did my best to keep the behaviour in other circumstances (barring obvious bugs) the same, but I can't test those either. Regarding @ustcweizhou 's comment; the only change I perceive there is that, analogous to what's in the CsGuestNetwork, the code seems to imply that the guest_gateway_ip will be added as DNS server in the case the SysVM is either a VPC or a redundant router. As I encountered a merge conflict while building this PR, I think this is a relatively new conditional which seems to mirror the one (incorrectly) in CsGuestNetwork (one depends on the is_vpc() or is_redundant(), the other depends on the availability of the `router_guest_gateway` key). If this both is true, I propose to change the `has_dns()` method to incorporate this logic and merge the same decision in CsGuestNetwork with that. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: Fixes for VirtualRouters in Basic Network...
Github user The-Loeki commented on the pull request: https://github.com/apache/cloudstack/pull/1547#issuecomment-219094194 Like this then :dart: --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: Fixes for VirtualRouters in Basic Network...
Github user The-Loeki commented on the pull request: https://github.com/apache/cloudstack/pull/1547#issuecomment-219062321 @DaanHoogland like it better now? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: Fixes for VirtualRouters in Basic Network...
Github user The-Loeki commented on the pull request: https://github.com/apache/cloudstack/pull/1547#issuecomment-219054107 @DaanHoogland click on the (...) :dancers: --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: Fixes for VirtualRouters in Basic Network...
Github user The-Loeki commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1547#discussion_r63188699
--- Diff: systemvm/patches/debian/config/opt/cloud/bin/cs/CsConfig.py ---
@@ -66,8 +66,13 @@ def is_router(self):
def get_domain(self):
return self.cl.get_domain()
+def use_extdns(self):
+return self.cmdline().idata().get('useextdns', 'false') == 'true'
+
def get_dns(self):
+conf = self.cmdline().idata()
dns = []
+<<<<<<< e09480728f4ed77d0bd0bdc6a6c0966d90f7ee26
--- End diff --
Yeah spotted it just too late... Thought I fixxed it but my git-merge-fu
isn't fully up to spec yet ;)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Fixes for VirtualRouters in Basic Network...
GitHub user The-Loeki opened a pull request: https://github.com/apache/cloudstack/pull/1547 Fixes for VirtualRouters in Basic Networking, especially with mutliple ranges in VLANs During the last few modifications on the SystemVM scripts, it turns out quite a lot of stuff broke in our setups. This PR fixes a number of things: * Multiple IP's per VLAN interface are now supported & working again, including DNS, DHCP ranges, password and metadata services * `useextdns` fixed (I had a small merge conflict with an attempt to fix this at 4.7, but these fixes are more comprehensive) * CLOUDSTACK-8303 * Apache configs better in line with best-practices and distro-expected locations * Added a few more helper functions & getters & setters for utility You can merge this pull request into a Git repository by running: $ git pull https://github.com/PCextreme/cloudstack vrouter-fixes Alternatively you can review and apply these changes as the patch at: https://github.com/apache/cloudstack/pull/1547.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #1547 commit e09480728f4ed77d0bd0bdc6a6c0966d90f7ee26 Author: Ronald van Zantvoort <ron...@pcextreme.nl> Date: 2016-05-12T16:56:37Z Virtual Router fixes: * Add IP addresses to interfaces when ip_aliases.json gets sent * Generate multiple dnsmasq range sets when applicable * Don't try to arping if there's no gateway associated with an address commit 44c932f091f8403ecb69549904e62b5cf11a1efa Author: Ronald van Zantvoort <ron...@pcextreme.nl> Date: 2016-05-12T17:25:50Z Virtual Router fixes: * obey useextdns configuration setting commit 55d1c3821d118bb6869bdb0bdf3918a1ffcef49a Author: Ronald van Zantvoort <ron...@pcextreme.nl> Date: 2016-05-12T21:46:09Z Virtual Router fixes: * DHCP lease times from infinite to ~a month +- 60 hours (fixes CLOUDSTACK-8303) commit 9ac3f0914780c4be67ea4c3e7b4d3144b8908856 Author: Ronald van Zantvoort <ron...@pcextreme.nl> Date: 2016-05-13T10:31:46Z Virtual Router fixes: * Fix broken to_str() * minor cleanup of imports commit c3b46e95d141f9e570dc75fd2c7b208560682a18 Author: Ronald van Zantvoort <ron...@pcextreme.nl> Date: 2016-05-13T12:14:29Z Virtual Router fixes: * Add has_metadata(), has_dns() and is_dhcp() to Config * CsIP post_config logic fixes for metadata, routing & dnsmasq services commit 054000ac3366359b4f1a563f8ccdb13ebb2f86e6 Author: Ronald van Zantvoort <ron...@pcextreme.nl> Date: 2016-05-13T13:03:43Z Virtual Router fixes: * VHosts to sites-enabled * VHost confs by IP rather than by intf to accomodate multiple IP's per intf * expose config to CsApp * Change VHost server name to something sane-ish rather than a Korean squatted cloudinternal.com host * CsDnsmasq: Only open DNS port if the SysVM is supposed to offer it commit 2d6fde2e1365fc417ea89eb6c8b7c0846eccd684 Author: Ronald van Zantvoort <ron...@pcextreme.nl> Date: 2016-05-13T13:43:17Z Virtual Router fixes cleanup commit 073db0462111e20375da28cdf2eb0706d418cc0b Author: Ronald van Zantvoort <ron...@pcextreme.nl> Date: 2016-05-13T13:56:10Z Just our luck on the merge conflict there --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
