RE-53 Add lint check to validate job name length #525
Conversation
mattt416
force-pushed
the
mattt/RE-53
branch
2 times, most recently
from
45b709f to
c86b1fa
Compare
|
Modified this script to error on job names > 50 chars for testing, and you can see the results here: |
We've run into situations where long job names cause python scripts in virtualenvs to fail. This is due to a kernel limitation that imposes the max number of characters that can exist in an interpreter line in a script. This commit adds a new check that scans all job name lengths and then errors if any exceeds 77 chars.
|
As it stands, the only issues I've seen with this relate to the use of Python virtual environments. While very long job names will be difficult to read, I wouldn't want us to be forcing the awkward naming of jobs just to meet this limit. Instead of creating the venv in the workspace, could we use /tmp instead? |
|
@git-harry That would only be helpful for the rpc-gating venv(s) - what about venvs created in the test itself? FYI this issue is informed by pypa/pip#1773 and pypa/virtualenv#596. |
There was a problem hiding this comment.
I like this as it informs us at the point where a job is being defined, so it makes sense to add it. If we exclusively run jobs on single-use-slaves then we could perhaps figure out how to change the workspace to be shorter and higher up in the file system hierarchy, but for the moment this at least keeps us informed and acts to prevent running into this known issue.
|
@odyssey4me you're assuming any venv created by the test is in the workspace, what about venvs created in subdirectories of the workspace as happens here https://github.com/rcbops/rpc-gating/pull/523/files#diff-6ab1426463e7a9fa99ef939e5f30246c This change isn't guaranteed to protect against the issue in all cases and the subset of cases for which it does guarantee to solve the problem can be achieved in a different manner without placing a restriction on the job name length. |
|
@git-harry It's not foolproof, no, but this still looks like something to at least try to prevent new jobs going in until we can figure out a better way to solve this. Ideally, in my mind, we should be able to have Jenkins use some sort of short-form directory name for the workspace. This would be ideal as it will clean up after itself so that the jobs can still be universally used on single-use-slaves and long-lived-slaves. If we do workarounds like using temp directories outside of the workspace, then we have to make sure that the jobs clean up after themselves otherwise long-lived-slaves get cluttered or fail jobs due to clashes when two jobs use the same spaces (we've seen this in the apt artifact builder). |
|
All nodes where a venv is used are allocated via common.use_node, so we can modify that function to allocate a workspace with a shorter path (using the ws step) and ensure it is cleaned up. However we'd have to work out where to put it, so the jenkins user has all the required perms. Tmp may not be the best especially if it is tmpfs? |
|
@hughsaunders Would it perhaps be possible for the workspace directory name to be a fixed size hash of the job name instead of the full job name string? That way we could have the workspace folder just be |
|
I'm not married to this idea -- I went with the job name length checking idea as that was what was suggested by @git-harry in RE-53. :) As we're moving away from CIT Jenkins (where /var/lib/jenkins is necessary because that's the partition with the most available space), we can easily move away from /var/lib/jenkins/workspace to another directory higher up the tree. To be honest though, /var/lib/jenkins/workspace only accounts for 17% of the overall max job name length. |
|
In answer to @odyssey4me's question, we could use a hash, and a 8 char hash would prob do. |
We've run into situations where long job names cause python scripts in
virtualenvs to fail. This is due to a kernel limitation that imposes
the max number of characters that can exist in an interpreter line in a
script.
This commit adds a new check that scans all job name lengths and then
errors if any exceeds 77 chars.
Issue: RE-53