Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-29547

"master" value is silently ignored and empty value is set for "Restrict where..." in Tied Label Slicer section

      Configuration Slicer page -> "Tied Label Slicer".

      I move some of my jenkins jobs to a new row and set "master" value on the left.
      I expect these jobs to get this "restricted to run on master" setting.

      actual:
      when I open individual configuration screen for those jobs, they still have "restrict where it can be run" checkbox selected,
      but the label value is empty. there is NO "master" text in it.
      if I go back to Configuration Slicer page -> Tied Label Slicer, I can see those jobs in a separate row with "master" value on the left, which is good.
      but it is simply not shown on individual Job Config pages (when you click "Configure" for a particular job in Jenkins UI).

      note: setting labels other than "master" seems to work fine in my environment.

      also, I have 0 executors available on the "master", which may or may not be relevant to this. I think it's not, but decided to share just in case.

      maybe "master" is considered to be a "default" label and not always shown in some UI pages... but that's weird because I can open "Configure Job" page and type "master" value as the node restriction and it stays there...

          [JENKINS-29547] "master" value is silently ignored and empty value is set for "Restrict where..." in Tied Label Slicer section

          mdonohue added a comment - - edited

          I've been looking at the code, comparing the configuration of a freestyle project manually, and via the label slicer. I think this is a bug in Jenkins, since using the configuration screen sets the 'assignedNode' private field directly based on what was typed in the HTML form. In this case the 'master' string goes directly into the field.

          But any other plugin that wants to modify the 'assignedNode' field needs to call 'setAssignedNode', and that function does extra checking. If you set the assigned node to "master" it switches that to 'null'. There are other comments indicating that 'null' is the preferred representation for the master node, but the AbstractProject configuration code doesn't maintain that invariant.

          mdonohue added a comment - - edited I've been looking at the code, comparing the configuration of a freestyle project manually, and via the label slicer. I think this is a bug in Jenkins, since using the configuration screen sets the 'assignedNode' private field directly based on what was typed in the HTML form. In this case the 'master' string goes directly into the field. But any other plugin that wants to modify the 'assignedNode' field needs to call 'setAssignedNode', and that function does extra checking. If you set the assigned node to "master" it switches that to 'null'. There are other comments indicating that 'null' is the preferred representation for the master node, but the AbstractProject configuration code doesn't maintain that invariant.

          Daniel Beck added a comment -

          mdonohue getAssignedLabel() returns the master node's self-label ("master") when canRoam is false but the label is null, so that seems consistent with the setter.

          Daniel Beck added a comment - mdonohue getAssignedLabel() returns the master node's self-label ("master") when canRoam is false but the label is null , so that seems consistent with the setter.

          mdonohue added a comment -

          Yes, getAssignedNode and setAssignedNode are consistent about converting 'master' to/from a null value in the field. The problem is that the configuration section for AbstractProject does not follow that convention.

          see getAssignedLabelString and AbstractProject.submit. Here are the relevant lines of code, where you can see no attempt is made to convert 'master' into a null value for assignedNode. You can also verify this by looking at config.xml for a project after setting the label to 'master', where you will see <assignedNode>master</assignedNode> in the file.

                  if(req.getParameter("hasSlaveAffinity")!=null) {
                      assignedNode = Util.fixEmptyAndTrim(req.getParameter("_.assignedLabelString"));
                  } else {
                      assignedNode = null;
                  }
                  canRoam = assignedNode==null;
          
          

          mdonohue added a comment - Yes, getAssignedNode and setAssignedNode are consistent about converting 'master' to/from a null value in the field. The problem is that the configuration section for AbstractProject does not follow that convention. see getAssignedLabelString and AbstractProject.submit. Here are the relevant lines of code, where you can see no attempt is made to convert 'master' into a null value for assignedNode. You can also verify this by looking at config.xml for a project after setting the label to 'master', where you will see <assignedNode>master</assignedNode> in the file. if (req.getParameter( "hasSlaveAffinity" )!= null ) { assignedNode = Util.fixEmptyAndTrim(req.getParameter( "_.assignedLabelString" )); } else { assignedNode = null ; } canRoam = assignedNode== null ;

          mdonohue added a comment -

          My proposed fix to Jenkins core is here: https://github.com/mdonohue/jenkins/commit/2cb89d3e269318644fabd1acb4861690e662aef9

          I am having difficulty testing this code right now - JDK8 on Windows 10 results in several unit test failures doing a 'mvn install' on jenkins. I think JDK7 will do better, but I couldn't find any documentation of that on the jenkins wiki. JDK7 testing will happen another day.

          mdonohue added a comment - My proposed fix to Jenkins core is here: https://github.com/mdonohue/jenkins/commit/2cb89d3e269318644fabd1acb4861690e662aef9 I am having difficulty testing this code right now - JDK8 on Windows 10 results in several unit test failures doing a 'mvn install' on jenkins. I think JDK7 will do better, but I couldn't find any documentation of that on the jenkins wiki. JDK7 testing will happen another day.

          mdonohue added a comment -

          JDK7 didn't fare any better. Does Jenkins-core build on windows cleanly?

          mdonohue added a comment - JDK7 didn't fare any better. Does Jenkins-core build on windows cleanly?

          mdonohue added a comment -

          And here is the pull request: https://github.com/jenkinsci/jenkins/pull/1881

          I'm still working on getting it accepted.

          mdonohue added a comment - And here is the pull request: https://github.com/jenkinsci/jenkins/pull/1881 I'm still working on getting it accepted.

          mdonohue added a comment -

          I don't know how to get any more attention for this one. I just added a comment to the pull request.

          mdonohue added a comment - I don't know how to get any more attention for this one. I just added a comment to the pull request.

          Daniel Beck added a comment -

          mdonohue With ~550 PRs this year so far, the time of the handful of people able to review PRs is stretched thin.

          Pinging people often helps (place an at character before their GitHub user name, e.g. @jtnord, not jtnord), as does having tests that show your change is safe (if it's not obvious).

          Daniel Beck added a comment - mdonohue With ~550 PRs this year so far, the time of the handful of people able to review PRs is stretched thin. Pinging people often helps (place an at character before their GitHub user name, e.g. @jtnord , not jtnord ), as does having tests that show your change is safe (if it's not obvious).

          JC G added a comment -

          Hi,

          Can someone tell me whether a fix is available to avoid this behaviour, as I use a Jenkins 1.580 ?

          The Label Expression parameter is just set to null for jobs I move to the master label, but when launched, they run on the master, that's exactly what I want.

          How can I be sure it will work like that any time ?

          Can I consider this is just a matter of display ?

           

          Thanks in advance

          JC G added a comment - Hi, Can someone tell me whether a fix is available to avoid this behaviour, as I use a Jenkins 1.580 ? The Label Expression parameter is just set to null for jobs I move to the master label, but when launched, they run on the master, that's exactly what I want. How can I be sure it will work like that any time ? Can I consider this is just a matter of display ?   Thanks in advance

          Dirk Heinrichs added a comment - - edited

          Can this be fixed, please? I don't understand why the label field being "null" means "master". This is highly confusing. Just write "master" into it. This is especially true since the help of the text input field states (under Notes): "An empty expression will always evaluate to true, matching all agents." and then explicitely lists master under Examples.

          Dirk Heinrichs added a comment - - edited Can this be fixed, please? I don't understand why the label field being "null" means "master". This is highly confusing. Just write "master" into it. This is especially true since the help of the text input field states (under Notes ): "An empty expression will always evaluate to true , matching all agents." and then explicitely lists master under Examples .

            mdonohue mdonohue
            alskor Alex Java
            Votes:
            3 Vote for this issue
            Watchers:
            5 Start watching this issue

              Created:
              Updated: