search

quattor / ncm-cdispd

Node Configuration Manager Configuration Dispatch Daemon
www.quattor.org
Other
2 stars 5 forks source link

Proper initialisation on ICLIST list of components #34

Closed stdweird closed 7 years ago

stdweird commented 7 years ago

Fixes #32

stdweird commented 7 years ago

Not tested, in particular the tainted mode

ned21 commented 7 years ago

LGTM. How do you want to do the testing? Can we get jenkins to generate a rpm so sites can just install it on a few test machines?

stdweird commented 7 years ago

retest this please

stdweird commented 7 years ago

test this please

stdweird commented 7 years ago

test this please

stdweird commented 7 years ago

@ned21 i was going to deploy this on our systems :smile: (and jenkins now also makes rpms)

ned21 commented 7 years ago

I installed the rpm on a 16.6 host and saw it run without issue

stdweird commented 7 years ago

same here tested on a few systems, rolling it out to more systems soon

jouvin commented 7 years ago

I had no time to test it on real machines yet... but the code changes LGTM. Thanks @stdweird for the cleanup (and apologies for the nasty typo I am probably responsible for...).

jouvin commented 7 years ago

I managed to (easily) reproduce the problem with 16.8.0. I'll try to validate the the proposed fix asap...

stdweird commented 7 years ago

@jouvin thx, jenkins has rpms https://jenkins1.ugent.be/job/ncm-cdispd-pr-builder/27/org.quattor.daemon$ncm-cdispd/ you can use

jouvin commented 7 years ago

I tested this PR and was able to assess that it does fix the issue with undefined ICLIST. For the record, the issue was originally trigged and then reproduced by adding:

"type": "deprecated"

to one of the interface properties.

As for me, the PR is ready to be merged.