saltstack / salt-winrepo-ng

Jinja templated winrepo
Other
107 stars 182 forks source link

travis-ci new sls file setting "skip_urltest" is leading to yaml compilation error #487

Open TheBigBear opened 8 years ago

TheBigBear commented 8 years ago

the new (and unofficial) setting 'skip_urltest' in sls file (e.g. duplicati.sls) leads to yaml compilation problem report. :-(

when running salt-run winrepo.genrepo on a 2016.3.0 master you get

event:
    ----------
    error:
        Failed to compile /srv/salt/win/repo/salt-winrepo/duplicati.sls.
suffix:
    progress
event:
    ----------
    error:
        Failed to compile /srv/salt/win/repo/salt-winrepo/duplicati_x86.sls.
suffix:
    progress

@moebiuseye was kind enough to help me, a non-programmer, to get the travis-ci tests running again and in the process he 'invented' a new switch in the sls file that allows travis-ci to know when the 404 urltest should not be run against a particular sls file.

This new setting 'skip_urltest' works just fine, but unfortunately it results in the above yaml compilation error.

Question mainly to @moebiuseye could you please think about re-working the 'skip_urltest' workaround and maybe add a 'skip_urltest' section to the .travis/travis.yml or maybe a new .travis/skip_urltest or .travis/urltest_skip file that would contain a simple one per line list of sls file names where the 404 url test should not be run against?

Also at same time it might be good to get the travis tests to start to be modular, so there can be other tests (and some of them might need 'skip' files or sections as well?) There are of course many other tests that could be added over time, and some of them I could possibly handle myself, even as a non-programmer. One of the first would be a simple flagging of any trailing whitespace at end of lines. Another a test for any use of TAB instead of characters to indent sls files. Also generally those tests should be run only against the changed sls file that is being committed and not against every single file in the repo. That applies also to the URL test, it should be run only against the currently changed sls file in the submission. BUT there should be a cron or otherwise scheduled job that checks for URLs that have gone bad. A committer merging a PR should not be 'forced' to deal with a bad URL or 404 error in another unrelated sls file.

Basically the travis-ci files and setup should be 100% external to the actual winrepo sls files. They should not require new settings or even commented out lines to tell travis-ci how to work on them.

BTW saltstack might also look at moving the travis-ci files into their automated jenkins test-suite, so this travis-ci test suite might be superceeded at some point. Any ideas, or feedback or thoughts on this?

moebiuseye commented 8 years ago

I'm taking the time to find the right solution to only check changing files.

Although I still think that all files should be checked for 404: We're looking for download links that no longer work. Testing for a 404 only when a change has been made makes no sense.

Another thing I'm working on is validating the advertised content type. Some of these links lead to text/plain or text/html documents. These links should be considered stale.

So, First thing first: I'm looking into the PyGit module and other alternatives. This may take a while.

TheBigBear commented 8 years ago

@moebiuseye thanks for looking into this. yes i fully agree that all URLs should be checked for any 404s on a cron or schedule fashion once or twice a day,or even hourly. but when a committer wants to merge in a PR they only care about the URLs and any other modified content in the sls files being changed or committed, only the changes they have just added. And they want to commit those there and then and be told if they made copy and paste error or the link isn't working as they think it should. but they don't really want to have to touch any other file before the travis-ci test come back green again. so the test ofg checking all URLs is valuable to maintain a good winrepo, but it shouldn't run on ALL URLs on every single change.

No problem if it takes time, I am sure the result will be well worth it, thanks for looking at it. Please feel free at any time to bounce of any ideas off of me or others here in this issue. And keep in mind the saltstack QA engineers might get involved and some of these tests might get integrated with their jenkins ci platform and integration.

moebiuseye commented 8 years ago

I've been working on this a bit.

Test it and tell me what you think of it:

https://gist.github.com/moebiuseye/44455610563c648ac7efa48dcd85161e

moebiuseye commented 8 years ago

This script is pretty much not working right now. Reposting soon.

moebiuseye commented 8 years ago

I pushed the new working version (same link above)

TheBigBear commented 8 years ago

@moebiuseye thanks for this. just a quick update to say that for the last 3 days I have been struggling to get any python code to run on any of my linux platforms. i have tried on centos 7 centos 6 and tonight on ubuntu 16.04, thinking I would get it finally started. But I am still having issues with importing (different) python modules, ready for the script.

but I am running into a series of beginner newbie python issues of running and importing python modules like pycurl or even yaml.

So i have not yet, been able to test your code, but nothing necessarily wrong or directly to do with your code, or not that I could demonstrate.

What release of a linux distro are you using where your script runs on? do you use python virtualenv at all, or not?

Maybe I can try this on travis-ci direct? How do i get it to run with -c or -t , is that even possible? Or will I have to have it run in -t mode by default and do a manual or cron job of running it as -c?

moebiuseye commented 8 years ago

Make sure you're using python 2 and not python 3.

You can place that flag on the .travis.yml file just like in this commit: https://github.com/moebiuseye/salt-winrepo-ng/commit/8c90e20001ea64d19462cc9a39d0fec71974cca3

TheBigBear commented 8 years ago

@moebiuseye thanks that helped. Do you have any ideas why tihs fails after 30 seconds?

my travis-ci fails within 30 secs

something to do with git, obviously. ( But i did add pygit to the "requirements.txt" file and as far as I can tell that does get installed and has no issue? with the pygit missing it complains about not having any git module to import?)

$ python --version
Python 2.7.9
$ pip --version
pip 6.0.7 from /home/travis/virtualenv/python2.7.9/lib/python2.7/site-packages (python 2.7)
install
pip install -r ./.travis/requirements.txt
./.travis/tests.py --travis

<cut some>

Traceback (most recent call last):
  File "./.travis/tests.py", line 19, in <module>
    head=git.Repo(".").commit("HEAD")
AttributeError: 'module' object has no attribute 'Repo'
moebiuseye commented 8 years ago

Okay. Run it with the command python2 ./.travis/tests.py --travis. I just realized my shebang (the first line) of my script is completely wrong.

Patching this week-end.

moebiuseye commented 8 years ago

It's gitpython not pygit.

TheBigBear commented 8 years ago

@moebiuseye ok, i'll change to gitpython and see what happens. It is definitely 100% running python 2 2.7.9 to be precise.

TheBigBear commented 8 years ago

@moebiuseye thanks, gitpython and NOT pygit was the solution, in this case. ;-) Travis-CI has gone green again! Thanks!

TheBigBear commented 8 years ago

@moebiuseye how are things? did you ever continue working on a more modular structure and ways to exclude specific tests purely inside of the travis files and not having to use or create non standard values in the 'sls' files themselves, which lead to compilation errors and warnings?

moebiuseye commented 8 years ago

Hey! Sorry my holidays made me forget about all this :) . Just a couple of weeks and I'll get back into it.