Skip to content

CLOUDSTACK-9593: userdata: enforce data is a multiple of 4 characters - #1760

Merged
yadvr merged 1 commit into
apache:masterfrom
exoscale:fix/CLOUDSTACK-9593
Dec 28, 2017
Merged

CLOUDSTACK-9593: userdata: enforce data is a multiple of 4 characters#1760
yadvr merged 1 commit into
apache:masterfrom
exoscale:fix/CLOUDSTACK-9593

Conversation

@marcaurele

Copy link
Copy Markdown
Member

Python base64 requires that the string is a multiple of 4 characters but
the Apache codec does not. RFC says is not mandatory but the data should
not fail the VR script (vmdata.py).

Signed-off-by: Marc-Aurèle Brothier m@brothier.org

@wido

wido commented Nov 11, 2016

Copy link
Copy Markdown
Contributor

LGTM

@jburwell

Copy link
Copy Markdown
Contributor

@marcaurele this change looks a good check to add to LTS to as well. Could you please change the base branch to 4.9? Once you do, I will kick regression tests across all hypervisors in order to merge the fix.

@marcaurele
marcaurele changed the base branch from master to 4.9 November 15, 2016 09:01
@marcaurele
marcaurele changed the base branch from 4.9 to master November 15, 2016 09:01
@marcaurele
marcaurele changed the base branch from master to 4.9 November 15, 2016 09:05
@marcaurele

Copy link
Copy Markdown
Member Author

@jburwell done

@marcaurele

marcaurele commented Nov 17, 2016

Copy link
Copy Markdown
Member Author

I changed the code to make to use the validation of the base64 value which is decoded to return the encoded one. This will happen any necessary padding to the value.
Fixing the incorrect user data in the database is not as straightforward as I thought.

@yadvr

yadvr commented Nov 20, 2016

Copy link
Copy Markdown
Member

@marcaurele can you check why the Travis run failed

@marcaurele

Copy link
Copy Markdown
Member Author

@rhtyd the SQL function I'm using to fix current user data in the database is not present in MySQL 5.5, but only in 5.6 (TO_BASE64, FROM_BASE64). I have to find a workaround, either in SQL or in Java to fix previous data.

@marcaurele
marcaurele force-pushed the fix/CLOUDSTACK-9593 branch 3 times, most recently from b57339d to c494ed3 Compare November 24, 2016 07:09
@marcaurele

Copy link
Copy Markdown
Member Author

@rhtyd The DB update class for the DatabaseUpgradeChecker doesn't work on Travis. Is there something I'm doing wrong, or missed something?

@yadvr

yadvr commented Nov 25, 2016

Copy link
Copy Markdown
Member

@marcaurele an upgrade path such as Upgrade4910to4920 would only work if your destination/final version is 4.9.2.0, have a look at the db upgrade checker workflow and you'll see how to trigger your changes.

@marcaurele

Copy link
Copy Markdown
Member Author

right, I forgot about the match required with the version in the pom. I thought I should prepare the stuff for the next 4.9.x release but that cannot work unless the pom files are updated too. I will just move the code inside Upgrade490to4910.java to Upgrade4910to4920.java when it's available.

@yadvr

yadvr commented Jul 23, 2017

Copy link
Copy Markdown
Member

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

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

@blueorangutan

Copy link
Copy Markdown

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

@yadvr

yadvr commented Jul 23, 2017

Copy link
Copy Markdown
Member

@marcaurele due to db changes in older upgrade paths, this cannot be accepted in 4.9/4.10, please re-base and change base branch to master.

@marcaurele

Copy link
Copy Markdown
Member Author

@rhtyd I still don't get it why on new release there isn't the creation of the SQL migration files done by the RM. It's totally unclear where/how I should push my changes for the SQL upgrade.

@marcaurele
marcaurele force-pushed the fix/CLOUDSTACK-9593 branch from d340181 to 58c67d4 Compare July 27, 2017 09:32
@marcaurele
marcaurele changed the base branch from 4.9 to master July 27, 2017 09:32
@yadvr

yadvr commented Jul 27, 2017

Copy link
Copy Markdown
Member

@marcaurele we already have 4.10.0.0 release and branch cut, we won't be able to accept PRs with schema changes for 4.9 branch since 4.9.2->4.9.3 path ought not change as it's included in 4.10.0.0 release. As I suggested earlier, change the base branch of the PR to master and put schema changes in 4.10->4.11 upgrade path.

@yadvr

yadvr commented Jul 27, 2017

Copy link
Copy Markdown
Member

@marcaurele I checked there exists a 4.10.0.0 -> 4.11.0.0 upgrade path on master branch that you can use.

@marcaurele
marcaurele force-pushed the fix/CLOUDSTACK-9593 branch from 58c67d4 to 3394765 Compare July 28, 2017 06:55
@marcaurele

Copy link
Copy Markdown
Member Author

@rhtyd should be all set now

@yadvr

yadvr commented Jul 28, 2017

Copy link
Copy Markdown
Member

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

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

@marcaurele

Copy link
Copy Markdown
Member Author

@rhtyd conflict fixed

@yadvr

yadvr commented Dec 19, 2017

Copy link
Copy Markdown
Member

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

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

@blueorangutan

Copy link
Copy Markdown

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

@yadvr

yadvr commented Dec 19, 2017

Copy link
Copy Markdown
Member

@blueorangutan test

@blueorangutan

Copy link
Copy Markdown

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

@blueorangutan

Copy link
Copy Markdown

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

Test Result Time (s) Test File
test_01_vpc_privategw_acl Failure 46.70 test_privategw_acl.py
test_02_vpc_privategw_static_routes Failure 218.40 test_privategw_acl.py
test_03_vpc_privategw_restart_vpc_cleanup Failure 198.33 test_privategw_acl.py
test_04_rvpc_privategw_static_routes Failure 308.92 test_privategw_acl.py
test_02_create_template_with_checksum_sha1 Error 5.26 test_templates.py
test_03_create_template_with_checksum_sha256 Error 5.22 test_templates.py
test_04_create_template_with_checksum_md5 Error 5.24 test_templates.py
test_08_resize_volume Error 312.38 test_volumes.py
test_09_delete_detached_volume Error 186.87 test_volumes.py
test_10_list_volumes Error 172.32 test_volumes.py
test_01_vpc_remote_access_vpn Error 45.81 test_vpc_vpn.py
test_hostha_kvm_host_degraded Error 26.01 test_hostha_kvm.py

@yadvr

yadvr commented Dec 20, 2017

Copy link
Copy Markdown
Member

@marcaurele can you check volume related failures, maybe caused by your changes?

@marcaurele

Copy link
Copy Markdown
Member Author

@rhtyd failure due to VM not able to start/restart due to insufficient capacity. Are those tests run in parallel on the integration server?

@yadvr

yadvr commented Dec 20, 2017

Copy link
Copy Markdown
Member

@marcaurele no they are not run in parallel, but several times when there is an exception/issue on the mgmt server side this general error is thrown. I'm suspecting if this has to do with any exception (NPE etc?) when VM starts? I can check again later today, meanwhile see if you find any issue or think that tests failures were env caused.

@yadvr

yadvr commented Dec 24, 2017

Copy link
Copy Markdown
Member

Let's do a final test run and then this should be good to merge based on results
@blueorangutan package

Python base64 requires that the string is a multiple of 4 characters but
the Apache codec does not. RFC states is not mandatory so the data should
not fail the VR script (vmdata.py).

Signed-off-by: Marc-Aurèle Brothier <m@brothier.org>
Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
@yadvr
yadvr force-pushed the fix/CLOUDSTACK-9593 branch from 14c7820 to 7893605 Compare December 24, 2017 09:05
@yadvr

yadvr commented Dec 24, 2017

Copy link
Copy Markdown
Member

Fixed conflict. Re-testing...
@blueorangutan package

@apache apache deleted a comment from blueorangutan Dec 24, 2017
@apache apache deleted a comment from blueorangutan Dec 24, 2017
@yadvr

yadvr commented Dec 25, 2017

Copy link
Copy Markdown
Member

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

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

@blueorangutan

Copy link
Copy Markdown

Packaging result: ✔centos6 ✔centos7 ✖debian. JID-1486

@yadvr

yadvr commented Dec 25, 2017

Copy link
Copy Markdown
Member

@blueorangutan test

@blueorangutan

Copy link
Copy Markdown

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

@apache apache deleted a comment from blueorangutan Dec 26, 2017
@apache apache deleted a comment from blueorangutan Dec 26, 2017
@apache apache deleted a comment from blueorangutan Dec 26, 2017
@apache apache deleted a comment from blueorangutan Dec 26, 2017
@blueorangutan

Copy link
Copy Markdown

Trillian test result (tid-1917)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 34746 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr1760-t1917-kvm-centos7.zip
Intermitten failure detected: /marvin/tests/smoke/test_deploy_virtio_scsi_vm.py
Intermitten failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Intermitten failure detected: /marvin/tests/smoke/test_hostha_kvm.py
Smoke tests completed. 64 look OK, 2 have error(s)
Only failed tests results shown below:

Test Result Time (s) Test File
test_04_rvpc_network_garbage_collector_nics Failure 301.34 test_vpc_redundant.py
test_hostha_enable_ha_when_host_in_maintenance Error 0.83 test_hostha_kvm.py

@yadvr

yadvr commented Dec 28, 2017

Copy link
Copy Markdown
Member

Merging this based on two lgtms and test results (ignoring env related Trillian failures).

@yadvr
yadvr merged commit a26a502 into apache:master Dec 28, 2017
@marcaurele
marcaurele deleted the fix/CLOUDSTACK-9593 branch December 28, 2017 10:18
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.

6 participants