• Icon: Patch Patch
    • Resolution: Fixed
    • Icon: Major Major
    • core
    • None
    • Platform: All, OS: All

      Dynamic labels on the master node are broken:

      • the first check is done when toComputer() returns null; the result is cached
        and no later checks are performed
      • lots of code is duplicated with hudson.model.Slave.

      The patch I'm attaching fixes this by:

      • consolidating the two connected variables dynamicLabels and
        dynamicInstanceHash into a single instance of a new class, fixing race
        conditions between the two and making code dealing with them cleaner
      • pulling the common code between Hudson and Slave for dealing with dynamic
        labels up into Node

      I hope the style etc is ok, please let me know if you need more changes done to
      be able to land the patch. I could see a bunch of improvements in the labelling
      area but I didn't want to make a bigger patch than needed to fix what we want to
      do in the squid project (where we want to tie jobs to the arch-os-version of
      nodes and we are entering it by hand at the moment).

      This patch would allow the OSLabeller to be enabled (I used it as a testbed),
      but it doesn't meet our project needs, and enabling it is a separate conceptual
      step - thus its been left disabled.

          [JENKINS-4235] Fix DynamicLabels for the master node

          lifeless added a comment -

          Created an attachment (id=846)
          Fix dynamic labels for the master node.

          lifeless added a comment - Created an attachment (id=846) Fix dynamic labels for the master node.

          lifeless added a comment -

          Oh, I also fixed a

          {tiny}

          shallow bug in Slave.getComputer where there was
          duplicate code between it and Node - the toComputer() interface is identical and
          appears to meet the expectation exactly. It was helpful as I tried to determine
          what was different and what was the same with the getDynamicLabels functions,
          but not strictly needed for this patch.

          Still, cleanup is cleanup .

          lifeless added a comment - Oh, I also fixed a {tiny} shallow bug in Slave.getComputer where there was duplicate code between it and Node - the toComputer() interface is identical and appears to meet the expectation exactly. It was helpful as I tried to determine what was different and what was the same with the getDynamicLabels functions, but not strictly needed for this patch. Still, cleanup is cleanup .

          Alan Harder added a comment -

          Is the patch missing the DynamicLabels class?

          Alan Harder added a comment - Is the patch missing the DynamicLabels class?

          lifeless added a comment -

          Urgh, yes it is. skew between svn and netbeans - sorry. Manually svn adding and
          regenerating the diff now.

          lifeless added a comment - Urgh, yes it is. skew between svn and netbeans - sorry. Manually svn adding and regenerating the diff now.

          lifeless added a comment -

          Created an attachment (id=847)
          Patch with DynamicLabels.java

          lifeless added a comment - Created an attachment (id=847) Patch with DynamicLabels.java

          Code changed in hudson
          User: : kohsuke
          Path:
          trunk/hudson/main/core/src/main/java/hudson/model/DynamicLabels.java
          trunk/hudson/main/core/src/main/java/hudson/model/Hudson.java
          trunk/hudson/main/core/src/main/java/hudson/model/Node.java
          trunk/hudson/main/core/src/main/java/hudson/model/Slave.java
          trunk/www/changelog.html
          http://fisheye4.cenqua.com/changelog/hudson/?cs=20863
          Log:
          [FIXED JENKINS-4235] Applied a patch, with a bit of changes. This will be in 1.321.

          SCM/JIRA link daemon added a comment - Code changed in hudson User: : kohsuke Path: trunk/hudson/main/core/src/main/java/hudson/model/DynamicLabels.java trunk/hudson/main/core/src/main/java/hudson/model/Hudson.java trunk/hudson/main/core/src/main/java/hudson/model/Node.java trunk/hudson/main/core/src/main/java/hudson/model/Slave.java trunk/www/changelog.html http://fisheye4.cenqua.com/changelog/hudson/?cs=20863 Log: [FIXED JENKINS-4235] Applied a patch, with a bit of changes. This will be in 1.321.

            Unassigned Unassigned
            lifeless lifeless
            Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

              Created:
              Updated:
              Resolved: