CLOUDSTACK-10178: Hotfixes to make 4.10 work by bwsw · Pull Request #2320 · apache/cloudstack

bwsw · 2017-11-11T15:57:23Z

This PR implements several fixes which are vital for CS 4.10 to work:

Fixes absent IPv6 network definition bugs for basic zone which lead to exceptions on KVM agent if it uses SGs (management and agent affected)
Fixes the case when template is created from a snapshot (CLOUDSTACK-10140, merged CLOUDSTACK-10140: Fix for when template is created from snapshot template.properties are corrupted #2322)
Fixes ubuntu/debian br_netfilter dependency (CLOUDSTACK-10138, merged CLOUDSTACK-10138: Load br_netfilter in security_group management script)
Fixes quota plugin bug ([CLOUDSTACK-10184] Re-work method QuotaResponseBuilderImpl.startOfNextDay and its test cases #2326)

Load br_netfilter in security_group management script.

exists in the system and should be loaded.

yadvr · 2017-11-20T07:35:46Z

 Documentation=http://www.cloudstack.org/
-Requires=libvirtd.service
-After=libvirtd.service
+Requires=libvirt-bin.service


@bwsw remove these, during packaging for debian/ubuntu the services file is fixed using sed. The libvirtd name for centos 6,7 and most other distros is libvirtd, while it's libvirt-bin for debian and derivatives.

I went ahead and fixed this to enable packaging+testing on this PR.

Great to hear, I'm not able to code until the next week.

yadvr · 2017-11-20T07:38:25Z

+                else:
+                    execute('iptables -I ' + vmchain + ' -p icmp --icmp-type ' + range + ' ' + direction + ' ' + ip + ' -j ' + action)
+            except:
+                pass


Should we log a warning or error be logged when an exception is hit?

@rhtyd I believe the code should be improved before inclusion to master, but I believe it should be done in management server side, because agent is too far from API to do it correctly. But warning is probably a good thing. I don't remember why excluded, but probably because 'execute' includes debug.

Basically, these try except for icmpv6, because (-1, -1) doesn't handled right for icmpv6.

yadvr · 2017-11-20T07:41:35Z

@blueorangutan package

…ases Also, removed @Local annotation that is not needed

Depending on the timezone you're running CS (before GMT timezones) you could experience that some jobs are marked as failed since the parent job got a null result despite its child job having successfully done the job. The child job got deleted by the CleanupTask ahead of time, due to a missing datetime conversion to GMT timezone. Jobs are failing with this message: Job failed with un-handled exception The fix intends to correct any datetime used in the code that should be using the GMT timezone instead of the local one since all DB datetime should be stored at GMT. Conflicts: server/src/com/cloud/test/DatabaseConfig.java

GabrielBrascher · 2017-12-06T18:23:26Z

Thanks for the PR @bwsw.

Can you please create a JIRA ticket for this PR and append the ticket id with PR name?

CLOUDSTACK-XXXXX: Hotfixes to make 4.10 work

Additionally, the description This PR implements several fixes which are vital for CS 4.10 to work doesn't tell much about this PR. Can you please provide more details? You can describe fixed bug(s), code improvements that have been done, etc.

Such documentation helps us keeping track of issues and PRs; thus, we can detect what is already being handled. For instance, PR #2355 (avoid sending a null IPv6 address to Security Group Python script) tackles a problem already handled by this PR.

bwsw · 2017-12-07T05:49:50Z

@GabrielBrascher updated. Please, take a look.

GabrielBrascher · 2017-12-07T18:06:45Z

Thanks, @bwsw.

yadvr · 2017-12-18T22:43:32Z

@blueorangutan package

blueorangutan · 2017-12-18T22:44:32Z

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

blueorangutan · 2017-12-18T23:25:22Z

Packaging result: ✔centos6 ✔centos7 ✔debian. JID-1409

yadvr · 2017-12-19T06:44:23Z

@blueorangutan test

blueorangutan · 2017-12-19T06:44:32Z

@rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

blueorangutan · 2017-12-19T13:57:09Z

Trillian test result (tid-1824)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 25952 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr2320-t1824-kvm-centos7.zip
Smoke tests completed. 54 look OK, 2 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File
test_02_edit_iso	`Failure`	0.10	test_iso.py
test_05_iso_permissions	`Failure`	0.07	test_iso.py
test_04_rvpc_privategw_static_routes	`Failure`	548.78	test_privategw_acl.py

yadvr · 2017-12-19T18:48:50Z

@bwsw I see most of these commits are already in master, do you want to squash your commits and if there are no new changes/commits for master we can merge using -X ours to effective ignore changes in master while forward merging. Thoughts?

yadvr · 2017-12-20T11:27:28Z

@wido @GabrielBrascher (and others) are you lgtm on this? My only issue with the change is that it's suppressing exceptions with an except: pass.

yadvr · 2017-12-28T09:59:05Z

Any update on this, ping?

bwsw · 2018-01-07T19:27:20Z

@rhtyd @wido @GabrielBrascher
Since it requires more and more PRs to include in 4.10 to fix bugs I would like to close this because It is no sense to keep it open in the perspective of 4.11.

yadvr · 2018-01-08T07:32:18Z

Sure thanks @bwsw

bwsw added 5 commits November 11, 2017 19:47

Implemented CLOUDSTACK-10138

11a1a33

Load br_netfilter in security_group management script.

Implemented fixes to make 4.10 work.

47c1e90

Improved debian installation script to find either rb_netfilter module

c844620

exists in the system and should be loaded.

fixed wrong id when building template from snapshot.

710c16d

CLOUDSTACK-10140 Fixed

4dc5dee

yadvr reviewed Nov 20, 2017

View reviewed changes

Fix packaging failures to enable Trillian testing

bf16818

rafaelweingartner and others added 3 commits November 20, 2017 18:18

Re-work method QuotaResponseBuilderImpl.startOfNextDay and its test c…

f2662d0

…ases Also, removed @Local annotation that is not needed

bwsw changed the title ~~Hotfixes to make 4.10 KVM agent work~~ Hotfixes to make 4.10 work Nov 23, 2017

bwsw mentioned this pull request Dec 6, 2017

CLOUDSTACK-10177: Only pass IPv6 address to Security Group Python scr… #2355

Merged

bwsw changed the title ~~Hotfixes to make 4.10 work~~ CLOUDSTACK-10178: Hotfixes to make 4.10 work Dec 7, 2017

wido added the component:kvm label Dec 7, 2017

wido self-requested a review December 7, 2017 10:47

wido added component:ipv6 component:security-group labels Dec 7, 2017

bwsw closed this Jan 8, 2018

Conversation

bwsw commented Nov 11, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yadvr Nov 20, 2017

Choose a reason for hiding this comment

Uh oh!

yadvr Nov 20, 2017

Choose a reason for hiding this comment

Uh oh!

bwsw Nov 20, 2017

Choose a reason for hiding this comment

Uh oh!

yadvr Nov 20, 2017

Choose a reason for hiding this comment

Uh oh!

bwsw Nov 20, 2017

Choose a reason for hiding this comment

Uh oh!

bwsw Nov 20, 2017

Choose a reason for hiding this comment

Uh oh!

yadvr commented Nov 20, 2017

Uh oh!

GabrielBrascher commented Dec 6, 2017

Uh oh!

bwsw commented Dec 7, 2017

Uh oh!

GabrielBrascher commented Dec 7, 2017

Uh oh!

yadvr commented Dec 18, 2017

Uh oh!

blueorangutan commented Dec 18, 2017

Uh oh!

blueorangutan commented Dec 18, 2017

Uh oh!

yadvr commented Dec 19, 2017

Uh oh!

blueorangutan commented Dec 19, 2017

Uh oh!

blueorangutan commented Dec 19, 2017

Uh oh!

yadvr commented Dec 19, 2017

Uh oh!

yadvr commented Dec 20, 2017

Uh oh!

yadvr commented Dec 28, 2017

Uh oh!

bwsw commented Jan 7, 2018

Uh oh!

yadvr commented Jan 8, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

bwsw commented Nov 11, 2017 •

edited

Loading