quattor / aquilon

Configuration Management Database (3rd Generation)
www.quattor.org
Apache License 2.0
14 stars 19 forks source link

Use new include syntax #102

Closed jouvin closed 6 years ago

jouvin commented 6 years ago

Fixes #84

Change-Id: Iad0df0a4b27f5881fe3d7fe4d6ea6fbc7a22daa7

ned21 commented 6 years ago

How are we doing at getting the unit tests able to run on Travis? I would think this change would require updates to the unit tests?

XaF commented 6 years ago

Can this be rebased on the current master please? The tests used internally in Morgan Stanley changed a bit and requires commits that have been included in the last master. Thank you!

jouvin commented 6 years ago

No problem. Done.

XaF commented 6 years ago

Hi @jouvin Tests need to be updated in order to work with the new format for the include. It seems there is a number of tests expecting the old format that thus don't work anymore. Can you please update those ?

jouvin commented 6 years ago

@XaF hopefully I changed all the occurences in the tests but I don't know how to check that it works. Please check and let me know the places still failing, if any...

XaF commented 6 years ago

@jouvin can't you run the tests through the the runtests.py script ? (is there MS-specific stuff for those?)

./tests/runtests.py --config=./tests/unittest.conf --failfast --no-interactive

Also, please merge both commits into one as in the internal process, each commit has to pass the tests to be merged to master, and the first one requires the second one to pass the tests... Sorry for that!

jouvin commented 6 years ago

I had no clue on how to run the test and the potential impact on the production broker... I'll merge both commit no problem. I had them separate initially for easier tracking but it's easy to merge them!

jouvin commented 6 years ago

@XaF commits squashed. Let me know the result if you manage to run the tests, I will not have the time to do it in the coming days...

jrha commented 6 years ago

The tests are not currently easy to run outside of MS, the travis PR has found some/most of the issues, but I wouldn't be willing to estimate how close we are.

XaF commented 6 years ago

@jrha thanks for the feedback. Is this still active work? Copy-pasting manually results from internally-running tests does not seem like a reasonable long term workflow :(

@jouvin a number of things are failing. We will copy you the failing output in a few.

jouvin commented 6 years ago

@xaf you may want to attach to the issue the test output. This may be easier than a copy/paste... Or just email me the output.

Shaeli commented 6 years ago

@jouvin Here is the output of failing tests:

00:17:30 ====================================================================== 00:17:30 FAIL: test_12_verify_cat_utvcs1_client (broker.test_add_cluster.TestAddCluster) 00:17:30 ---------------------------------------------------------------------- 00:17:30 Traceback (most recent call last): 00:17:30 File "/var/tmp/pjenkslv/jenkins/workspace/aquilon-aqd-custom-master/src/tests/broker/test_add_cluster.py", line 73, in test_12_verify_cat_utvcs1_client 00:17:30 command) 00:17:30 File "/var/tmp/pjenkslv/jenkins/workspace/aquilon-aqd-custom-master/src/tests/broker/brokertest.py", line 491, in searchoutput 00:17:30 % (command, r, out)) 00:17:30 AssertionError: output for ['cat', '--cluster', 'utvcs1', '--client'] did not match 'include if_exists("features/" + value("/system/archetype/name") + "/hacluster/hapersonality/config");': 00:17:30 @@@ 00:17:30 'template cluster/utvcs1/client; 00:17:30 00:17:30 "/system/cluster/name" = "utvcs1"; 00:17:30 include { if_exists("features/" + value("/system/archetype/name") + "/hacluster/hapersonality/config") }; 00:17:30 ' 00:17:30 @@@ 00:17:30 00:17:30 00:17:30 =================================================================== 00:17:30 FAIL: test_155_cat_hacl1_client (broker.test_usecase_hacluster.TestUsecaseHACluster) 00:17:30 ---------------------------------------------------------------------- 00:17:30 Traceback (most recent call last): 00:17:30 File "/var/tmp/pjenkslv/jenkins/workspace/aquilon-aqd-custom-master/src/tests/broker/test_usecase_hacluster.py", line 222, in test_155_cat_hacl1_client 00:17:30 command) 00:17:30 File "/var/tmp/pjenkslv/jenkins/workspace/aquilon-aqd-custom-master/src/tests/broker/brokertest.py", line 491, in searchoutput 00:17:30 % (command, r, out)) 00:17:30 AssertionError: output for ['cat', '--cluster', 'hacl1', '--client'] did not match 'include if_exists("features/" + value("/system/archetype/name") + "/hacluster/hapersonality/config");': 00:17:30 @@@ 00:17:30 'template cluster/hacl1/client; 00:17:30 00:17:30 "/system/cluster/name" = "hacl1"; 00:17:30 "/system/cluster/resources/resourcegroup" = append(create("resource/cluster/hacl1/resourcegroup/hacl1g1/config")); 00:17:30 "/system/cluster/resources/resourcegroup" = append(create("resource/cluster/hacl1/resourcegroup/hacl1g2/config")); 00:17:30 "/system/cluster/resources/service_address" = append(create("resource/cluster/hacl1/service_address/hacl1/config")); 00:17:30 include { if_exists("features/" + value("/system/archetype/name") + "/hacluster/hapersonality/config") }; 00:17:30 ' 00:17:30 @@@

jrha commented 6 years ago

@XaF active yes, but I have many other things to do, so slow. There is nothing to stop anyone else forking my branch and having a go at figuring out what the remaining problems are though!

jouvin commented 6 years ago

@XaF @Shaeli should be better now... missed line detected by unit tests in fact!

Shaeli commented 6 years ago

It's working now, thanks!

XaF commented 6 years ago

@jouvin so actually, it was not working but because of a change in the version of the panc compiler (an error message that changed). With the new master it should work. Could you please rebase ? I will post that same message in other pull requests sharing the same issue.

jouvin commented 6 years ago

@XaF No problem, done!

XaF commented 6 years ago

Merged in 04116cb5fccd68d2d233858c9aeda05125201463