Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

CLOUDSTACK-6975: Prevent dnsmasq from starting on backup RvR #1509

Closed

Conversation

QuiteClose
Copy link
Contributor

CLOUDSTACK-6975 refers to service monitoring bringing up dnsmasq but this is no-longer accurate, as service monitoring is not active on the post-4.6 routers. These routers still suffer an essentially identical issue, however, because "dnsmasq needs to be restarted each time configure.py is called in order to avoid lease problems." As such, dnsmasq is still running on backup RvRs, causing the issues described in CLOUDSTACK-6975.

This PR is based on a patch submitted by @ustcweizhou. The code now checks the redundant state of the router before restarting dnsmasq.

RvR networks without this patch have dnsmasq running on both master and backup routers. RvR networks with this patch have dnsmasq running on only the master router.

Boris Schrijver and others added 30 commits January 21, 2016 11:51
More VR performance!Ping @remibergsma

Integration tests runnning now!

* pr/1356:
  More VR performance!

Signed-off-by: Remi Bergsma <github@remi.nl>
CLOUDSTACK-9254: Make longer names display pretty in UIThe arrow always fell off when I log in ;-)

Before change it looked like this:
<img width="1521" alt="screenshot_23_01_16_21_10" src="https://cloud.githubusercontent.com/assets/1630096/12533043/f4e4baa8-c223-11e5-83ed-7e77bbce3a6c.png">

Now also longer names display nicely:
<img width="1549" alt="screen shot 2016-01-23 at 22 19 41" src="https://cloud.githubusercontent.com/assets/1630096/12533051/19474d20-c224-11e5-85c4-8d5ba1b7e938.png">

Ping @borisroman

* pr/1362:
  CLOUDSTACK-9254: Make longer names display pretty

Signed-off-by: Remi Bergsma <github@remi.nl>
…STACK-9245

CLOUDSTACK-9245 - Cannot delete non-attached ACL that contains itemsThis PR fixes the issue when trying to delete ACL lists which contain item.

It seemed it was not thought of when the ACS project started, when most user were relying on the UI to execute those tasks. Nowadays, with automation all over the place and ACL lists containing hundreds of items, it's very hard to have to delete them 1 by 1 either via the UI. Writing scripts to do so might be a solution, but it would be much simpler to just delete non-attached ACLs with all its items in one go.

Also, destroying a VPC that contains ACL lists was "succeeding", but after that the ACL list/items were messing up:

```
list networkacls aclid=920d74b6-4d15-454f-b3a6-61e7a6ffd1a4
Error 431: Unable to find VPC associated with acl
{
  "cserrorcode": 4350,
  "errorcode": 431,
  "errortext": "Unable to find VPC associated with acl",
  "uuidList": []
}
```

So, it also cleans up ACLs when destroying VPCs

* pr/1358:
  CLOUDSTACK-9245 - Deletes ACL items when destroying the VPC or deleting the ACL itself
  CLOUDSTACK-9245 - Formatting NetworkACLServiceImpl class
  CLOUDSTACK-9245 - Formatting VpcManagerImpl class
  CLOUDSTACK-9245 - Formatting NetworkACLManagerImpl class

Signed-off-by: Remi Bergsma <github@remi.nl>
Signed-off-by: Remi Bergsma <github@remi.nl>
Set version to 4.7.2-SNAPSHOT in 4.7 branchAfter releasing 4.7.1, next version is 4.7.2-SNAPSHOT in 4.7 branch.

* pr/1367:
  Implement upgrade path 4.7.1 -> 4.7.2
  Updating pom.xml version numbers for release 4.7.2-SNAPSHOT
  update debian changelog

Signed-off-by: Remi Bergsma <github@remi.nl>
CLOUDSTACK-9256 add unique key for static routes in jsonStatic routes that are being set do not show up in the static_routes.json file. The reason for this is that the index that is used, is the gateway address, which is not unique. Hence stuff is overwritten and lost.

Ping @borisroman @wilderrodrigues @DaanHoogland

* pr/1364:
  CLOUDSTACK-9256 add unique key for static routes in json

Signed-off-by: Remi Bergsma <github@remi.nl>
…M templates

This commit includes three changes:
(1) Revert commit 13bf1ec to show the 'root disk size' field
(2) Set the default value from '1' to empty. This also changes the default value of customized service offerings from '1' to empty.
(3) show 'root disk size' field only when users choose a KVM template, as cloudstack supports deployvm with rootdisksize on KVM only.
CLOUDSTACK-6181: Add 'root disk size' field in instance wizard for KVM templatesThis commit includes three changes:
(1) Revert commit 13bf1ec to show the 'root disk size' field
(2) Set the default value from '1' to empty. This also changes the default value of customized service offerings from '1' to empty.
(3) show 'root disk size' field only when users choose a KVM template, as cloudstack supports deployvm with rootdisksize on KVM only.

* pr/1342:
  CLOUDSTACK-6181: Add 'root disk size' field in instance wizard for KVM templates

Signed-off-by: Remi Bergsma <github@remi.nl>
* 4.6:
  CLOUDSTACK-6181: Add 'root disk size' field in instance wizard for KVM templates
This is displayed on Infra tab and it's annoying that on the VPC
page only the link local is mentioned and not the hostname. That is
now corrected, so you have the correct details to login to the
router straight away.
…-host

Display hostname the VPC router runs onThe hostname a router is running on is only displayed on Infra tab and not on the VPC page (the link local is mentioned though). That is now corrected, so you have the correct details to login to the router straight away.

Before:
<img width="702" alt="screen shot 2016-01-29 at 20 58 58" src="https://cloud.githubusercontent.com/assets/1630096/12687083/f7de5760-c6cd-11e5-9f62-cdf2cce8aeed.png">

After:
<img width="704" alt="screen shot 2016-01-29 at 21 18 21" src="https://cloud.githubusercontent.com/assets/1630096/12687088/fc4d1c1e-c6cd-11e5-8573-cdde638264f7.png">

So the option 'Host' was added and displays the hypervisor the VPC runs on.

* pr/1384:
  Display hostname the VPC router runs on

Signed-off-by: Remi Bergsma <github@remi.nl>
Added jsonObj to newly created row in multiedit.js to stop listing the same VM again.
Fixed the hard coded string.
Added _l() to dictioanry keys.
…ce, Network, Egress rules

Now it breaks into two lines once the word goes out of the box.
Fixed the advanced search field issue.
It supports http also. Added http to the list.
CLOUDSTACK-9264: Make /32 static routes for private gw workStatic routes for private gateways that were /32 failed because the `route` command used had `-net` in it and a `/32` requires `-host` instead. I rewrote it to `ip` commands.

* pr/1383:
  CLOUDSTACK-9264: Make /32 static routes for private gw work

Signed-off-by: Remi Bergsma <github@remi.nl>
CLOUDSTACK-9266: Make deleting static routes in private gw workThe to-be-deleted static routes were removed from the json file, instead of putting them there with revoke=true. The script that parses the json now doesn't find it and thus does not delete it.

Example after adding/removing some:
```
root@r-3-VM:/var/cache/cloud# cat /etc/cloudstack/staticroutes.json
{
    "1.2.3.0/24": {
        "gateway": "172.16.0.1",
        "ip_address": "172.16.0.2",
        "network": "1.2.3.0/24",
        "revoke": true
    },
    "1.2.3.4/32": {
        "gateway": "172.16.0.1",
        "ip_address": "172.16.0.2",
        "network": "1.2.3.4/32",
        "revoke": true
    },
    "1.2.33.3/32": {
        "gateway": "172.16.0.1",
        "ip_address": "172.16.0.2",
        "network": "1.2.33.3/32",
        "revoke": true
    },
    "1.22.2.2/32": {
        "gateway": "172.16.0.1",
        "ip_address": "172.16.0.2",
        "network": "1.22.2.2/32",
        "revoke": true
    },
    "10.1.2.1/32": {
        "gateway": "172.16.0.1",
        "ip_address": "172.16.0.2",
        "network": "10.1.2.1/32",
        "revoke": true
    },
    "10.1.200.0/25": {
        "gateway": "172.16.0.1",
        "ip_address": "172.16.0.2",
        "network": "10.1.200.0/25",
        "revoke": true
    },
    "10.11.12.13/32": {
        "gateway": "172.16.0.1",
        "ip_address": "172.16.0.2",
        "network": "10.11.12.13/32",
        "revoke": true
    },
    "172.16.1.3/32": {
        "gateway": "172.16.0.1",
        "ip_address": "172.16.0.2",
        "network": "172.16.1.3/32",
        "revoke": true
    },
    "172.16.15.14/32": {
        "gateway": "172.16.0.1",
        "ip_address": "172.16.0.2",
        "network": "172.16.15.14/32",
        "revoke": false
    },
    "172.16.17.0/25": {
        "gateway": "172.16.0.1",
        "ip_address": "172.16.0.2",
        "network": "172.16.17.0/25",
        "revoke": false
    },
    "id": "staticroutes"
}
```

This results in:
```
root@r-3-VM:/var/cache/cloud# ip route show
default via 192.168.23.1 dev eth1
169.254.0.0/16 dev eth0  proto kernel  scope link  src 169.254.1.67
172.16.0.0/24 dev eth2  proto kernel  scope link  src 172.16.0.2
172.16.15.14 via 172.16.0.1 dev eth2
172.16.17.0/25 via 172.16.0.1 dev eth2
192.168.23.0/24 dev eth1  proto kernel  scope link  src 192.168.23.4
```

Two static routes left, the rest deleted:
```
172.16.15.14 via 172.16.0.1 dev eth2
172.16.17.0/25 via 172.16.0.1 dev eth2
```

That also matches the UI:

<img width="1327" alt="screen shot 2016-01-30 at 06 34 06" src="https://cloud.githubusercontent.com/assets/1630096/12693933/83e67d80-c71b-11e5-9241-9f478522b7a4.png">

* pr/1386:
  CLOUDSTACK-9266: Make deleting static routes in private gw work

Signed-off-by: Remi Bergsma <github@remi.nl>
The field we use most are now on the top:
- name
- state
- hypervisor
- link local ip
- redundant state

The other fields are nice, but not needed most of the time.
…_4.7

CLOUDSTACK-9267: String is not localized on create instance wizards.Repro Steps:
==========
1. Setup basic environments as normal.
2. Open a browser, go to Web Console.
3. Upload a iso template.
4. Go to "Instances" page and add new instance.
5. Check the strings on create instance wizards.

Expected Result:
=============
All the strings should be localized on create instance wizards.

Actual Result:
===========
Some strings are not localized on create instance wizards.

Language:
========
JA -> Fail
SC -> Fail

Fix:
===
Fixed the hard coded string.

* pr/1390:
  CLOUDSTACK-9267: String is not localized on create instance wizards.

Signed-off-by: Rafael Weingärtner <rafael@apache.org>
rohityadavcloud and others added 19 commits March 14, 2016 12:21
The default umask of 0022 is set in Ubuntu and other packages. Set the same
in case of CentOS startup scripts. Use umask 022 in the injectkeys.sh script

Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
CLOUDSTACK-9285 for 4.7.xPer Daan's request, here is a pull request for the 4.7.x release.

* pr/1430:
  Cloudstack 9285 for 4.7.x
  CLOUDSTACK-9285 - Address original on start exception(s) and newline cleanup
  Cloudstack 9285 for 4.7.x

Signed-off-by: Will Stevens <williamstevens@gmail.com>
… determine the storage pool of the snapshot
Add ability to download templates in SwiftThis PR adds the ability to download templates when using Swift as a secondary storage. Uses the "temp_url" feature of Swift so that tempates can be downloaded without authenticaiton.

* pr/1332:
  Add ability to download templates in Swift

Signed-off-by: Will Stevens <williamstevens@gmail.com>
CLOUDSTACK-9297 - Reworked logic in StorageSystemSnapshotStrategy and XenserverSnapshotStrategyThe ticket this PR fixes was opened because KVM-specific code had been added to the StorageSystemSnapshotStrategy class and that class' canHandle method was only prepared to handle managed storage being used with XenServer (and a case was hit for KVM that triggered a CloudRuntimeException to be thrown).

To solve the problem, I moved the KVM logic to the default snapshot strategy class, which is (unfortunately) named XenserverSnapshotStrategy.

I plan to rename XenserverSnapshotStrategy to something like DefaultSnapshotStrategy in 4.9.

My guess is that when XenserverSnapshotStrategy was originally written, it was written only for XenServer, but has since that time had its usage increased to support other hypervisors (with non-managed storage).

* pr/1441:
  CLOUDSTACK-9297: delete snapshot without id is failing with Unable to determine the storage pool of the snapshot

Signed-off-by: Will Stevens <williamstevens@gmail.com>
…ement_47

Improve ordering of fields of VPC router detail tabThe field we use most are now on the top:
- name
- state
- hypervisor
- link local ip
- redundant state

The other fields are nice, but not needed most of the time.

Result:
![screen shot 2016-02-21 at 20 11 49](https://cloud.githubusercontent.com/assets/1630096/13204809/37ce63c0-d8d9-11e5-8861-35c1c1a406cf.png)

Before:
![vpc_router_details_before](https://cloud.githubusercontent.com/assets/1630096/13204811/41510f10-d8d9-11e5-850f-549a3333b063.png)

* pr/1422:
  Improve ordering of fields of VPC router detail tab

Signed-off-by: Will Stevens <williamstevens@gmail.com>
Cloudstack-9285 exception log additionAfter discussion with @miguelaferreira on the previous PR related to Cloudstack-9285, we decided on adding additional exception logging for this issue.

After adding it, the logs look like this in our lab:

2016-04-07 15:44:03,298 WARN  [cloud.agent.Agent] (Agent-Handler-1:null) (logid:7225632a) NIO Connection Exception com.cloud.utils.exception.NioConnectionException: Connection closed with -1 on reading size.  <<-- new exception logging
2016-04-07 15:44:03,298 INFO  [cloud.agent.Agent] (Agent-Handler-1:null) (logid:7225632a) Attempted to connect to the server, but received an unexpected exception, trying again... << --original logging from previous PR.

* pr/1479:
  Additional exception logging for Cloudstack-9285

Signed-off-by: Will Stevens <williamstevens@gmail.com>
…_variables

speedup iptables by prefetching the variables-- This PR is replacing speedup iptables setup apache#1449
-- Squashing commits and cleanup

PR against 4.7 as discussed with Remi Bergsma. This will speed up the iptables creation on the virtual router.

Testing showed the following:
with current code:
root@kvm704:~# time /usr/share/cloudstack-common/scripts/network/domr/router_proxy.sh vr_cfg.sh 169.254.1.176 -c /var/cache/cloud/VR-12f28879-de7e-44d2-8dbe-b93a04bd3ba4.cfg
real 2m56.401s
user 0m0.012s
sys 0m0.012s

modified version:
root@kvm704:~# time /usr/share/cloudstack-common/scripts/network/domr/router_proxy.sh vr_cfg.sh 169.254.1.176 -c /var/cache/cloud/VR-12f28879-de7e-44d2-8dbe-b93a04bd3ba4.cfg
real 1m35.762s
user 0m0.020s
sys 0m0.004s

* pr/1487:
  speedup iptables by prefetching the variables

Signed-off-by: Will Stevens <williamstevens@gmail.com>
…_4.7

CLOUDSTACK-9268: Display VM in Load balancing rule in UISteps of Repro:
=============
1:Create VMs
2:Make LoadBalancing rule in GUI
Name:WWW
PrivatePort:80
PublicPort:80
Add VMs:some VMs

Expected Result:
==============
The VMs which has been already assigned is should not be listed when you add the VM to an existing rule.

Actual Result:
===========
The VMs which has been already assigned is still being listed when you add the VM to an existing rule.

Fix:
===
Added jsonObj to newly created row in multiedit.js to stop listing the same VM again.

* pr/1394:
  CLOUDSTACK-9268: Display VM in Load balancing rule in UI

Signed-off-by: Koushik Das <koushik@apache.org>
…_4.7

CLOUDSTACK-9272: No option in UI to add GSLB with service type "HTTP"Steps to Repro:
============
Go to Regions -> Local -> View GSLB -> Add GSLB
Click on the service type dropdown
Observe http is missing. Please see the attached snapshot.

Expected Behaviour:
================
As it supports http also, So http should be in the list.

Actual Behaviour:
==============
http is missing from the list.

Fix:
===
It supports http also. Added http to the list.

Snapshot:
========
<img width="531" alt="gslb-http-missing-nitin" src="https://cloud.githubusercontent.com/assets/12583725/12772818/21513dc0-ca5b-11e5-822e-e2dd2426da65.png">

* pr/1399:
  CLOUDSTACK-9272: No option in UI to add GSLB with service type "HTTP"

Signed-off-by: Koushik Das <koushik@apache.org>
…_4.7

CLOUDSTACK-9270: UI alignment gone bad in multiple places - VM Instance, Network, Egress rulesSteps to Repro:
============
Please see the snapshots attached.

Fix:
===
Now it breaks into two lines once the word goes out of the box.
Fixed the advanced search field issue.

Network Section:
=============
![network-section-nitin](https://cloud.githubusercontent.com/assets/12583725/12762032/66f27d32-ca13-11e5-8d0a-9c8f99a12a99.png)

Adding VM to LB Rule:
==================
![adding-vm-to-lb-rule-nitin](https://cloud.githubusercontent.com/assets/12583725/12762050/76083cd0-ca13-11e5-873d-0037bc13a444.png)

Affinity Group Section:
==================
![affinity-group-section-nitin](https://cloud.githubusercontent.com/assets/12583725/12762066/881c2a1c-ca13-11e5-8905-f72676151130.png)

Fixed Affinity Group Section:
======================
![fix-affinity-group-section-nitin](https://cloud.githubusercontent.com/assets/12583725/12762083/990a673a-ca13-11e5-9d63-f895f9a1875c.png)

* pr/1398:
  CLOUDSTACK-9270: UI alignment gone bad in multiple places - VM Instance, Network, Egress rules

Signed-off-by: Koushik Das <koushik@apache.org>
systemvm: preserve file permissions, set default umask- In injectkeys.sh which is used to inject new public keys everytime cloudstack
  starts; while copying files preserve the mode/ownership. This ensures the
  scripts have same mode bits as originally configured in the iso file
- The default umask of 0022 is set in Ubuntu and other packages. Set the same
  in case of CentOS startup scripts

cc @abhinandanprateek @wido @remibergsma @DaanHoogland @jburwell

* pr/1420:
  systemvm: set default umask 022 in injectkeys.sh

Signed-off-by: Will Stevens <williamstevens@gmail.com>
Check the existence of 'forceencap' parameter before useCheck the existence of 'forceencap' parameter before use.

Error seen:

```
Traceback (most recent call last):
  File "/opt/cloud/bin/update_config.py", line 140, in <module>
    process_file()
  File "/opt/cloud/bin/update_config.py", line 54, in process_file
    finish_config()
  File "/opt/cloud/bin/update_config.py", line 44, in finish_config
    returncode = configure.main(sys.argv)
  File "/opt/cloud/bin/configure.py", line 1003, in main
    vpns.process()
  File "/opt/cloud/bin/configure.py", line 488, in process
    self.configure_ipsec(self.dbag[vpn])
  File "/opt/cloud/bin/configure.py", line 544, in configure_ipsec
    file.addeq(" forceencaps=%s" % CsHelper.bool_to_yn(obj['encap']))
KeyError: 'encap'
```

* pr/1402:
  Check the existence of 'forceencap' parameter before use

Signed-off-by: Will Stevens <williamstevens@gmail.com>
…s-zones-template

CLOUDSTACK-9172 Added cross zones check to delete template and isoAdded a check to ignore the zoneid, in the delete template UI, if the template is cross zones.

reference : CLOUDSTACK-9172

* pr/1505:
  CLOUDSTACK-9172 Added cross zones check to delete template and iso

Signed-off-by: Will Stevens <williamstevens@gmail.com>
@ustcweizhou
Copy link
Contributor

LGTM, tested

@swill
Copy link
Contributor

swill commented Apr 22, 2016

@dsclose this PR is currently opened against apache:4.7.1-RC20160120T2318. Should this be merged into 4.7 and then we will forward merge it to 4.8 and master?

Need one more LGTM code review and I would like to verify this PR via CI.

@QuiteClose
Copy link
Contributor Author

@swill I raised the PR against apache:4.7.1-RC20160120T2318 because it seemed sensible to include it in the latest 4.7 branch. I'd assumed it could be forward merged but as this is only my second PR for Cloudstack I don't think I should be making any recommendations.

It's not clear to me how to determine which branch I should raise PRs against - but that's probably a question for the mailing list rather than here. Let me know if you want it based off something else and I'll sort that out.

@swill
Copy link
Contributor

swill commented Apr 22, 2016

Lets close this PR and open it based off the 4.7 branch. @ustcweizhou's LGTM vote will still count on the new PR. Lets just reference this PR in the new PR so we remember to count his vote. :)

Basically how this works is as follows:

  • The current release is 4.8.
  • We support fixes in one release prior the the current release, and those fixes are put against the main branch, so in this case 4.7. I am not sure of the release cycle on this branch, but at some point a newer minor release may be cut from this branch (something like 4.7.2 for example).
  • Whenever a fix is made in a previous release, the fix is then forward merged into the later releases. This is so the commit hash is consistent between all branches making it easier to understand if a specific fix is in a specific branch. So if a commit is added to the 4.7 branch, it will be forward merged to the 4.8 branch, and then forward merged again to master.
  • We treat master as the next release and add new features there and make sure everything is stable for the next release (in this case 4.9).

Does that all make sense? Don't be shy if you have questions. I will do my best to support you. :)

@QuiteClose
Copy link
Contributor Author

Rebased off of 4.7 branch and opened PR #1514 - closing this PR.

@QuiteClose QuiteClose closed this Apr 22, 2016
@QuiteClose
Copy link
Contributor Author

@swill and thank you for you're support - much appreciated :-)

asfgit pushed a commit that referenced this pull request May 26, 2016
CLOUDSTACK-6975: Prevent dnsmasq from starting on backup redundant RvRRebase of PR #1509 against the 4.7 branch as requested by @swill

One LGTM from @ustcweizhou carried from previous PR. Previous PR will be closed.

Description from PR #1509:

CLOUDSTACK-6975 refers to service monitoring bringing up dnsmasq but this is no-longer accurate, as service monitoring is not active on the post-4.6 routers. These routers still suffer an essentially identical issue, however, because "dnsmasq needs to be restarted each time configure.py is called in order to avoid lease problems." As such, dnsmasq is still running on backup RvRs, causing the issues described in CLOUDSTACK-6975.

This PR is based on a patch submitted by @ustcweizhou. The code now checks the redundant state of the router before restarting dnsmasq.

RvR networks without this patch have dnsmasq running on both master and backup routers. RvR networks with this patch have dnsmasq running on only the master router.

* pr/1514:
  CLOUDSTACK-6975: Prevent dnsmasq from starting on backup redundant RvR.

Signed-off-by: Will Stevens <williamstevens@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet