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

vSphere plugin fails to start a VM for pipeline jobs that request a node with no specific label

    Details

    • Similar Issues:

      Description

      When a pipeline job requests a node with no label specified a null appears to be passed to the vSphere plugin. One can argue if a null is the appropriate value. However for sure the plugin should be starting any VM template that is configured, because an empty label should match any VM.
      See the following error message I see in the logs instead:

      WARNING: provision(null,1): Failed.
      java.lang.NullPointerException
              at org.jenkinsci.plugins.vSphereCloud.provision(vSphereCloud.java:334)
              at hudson.slaves.NodeProvisioner$StandardStrategyImpl.apply(NodeProvisioner.java:715)
              at hudson.slaves.NodeProvisioner.update(NodeProvisioner.java:320)
              at hudson.slaves.NodeProvisioner.access$000(NodeProvisioner.java:61)
              at hudson.slaves.NodeProvisioner$NodeProvisionerInvoker.doRun(NodeProvisioner.java:807)
              at hudson.triggers.SafeTimerTask.run(SafeTimerTask.java:72)
              at jenkins.security.ImpersonatingScheduledExecutorService$1.run(ImpersonatingScheduledExecutorService.java:58)
              at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
              at java.util.concurrent.FutureTask.runAndReset(FutureTask.java:308)
              at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$301(ScheduledThreadPoolExecutor.java:180)
              at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:294)
              at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
              at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
              at java.lang.Thread.run(Thread.java:748)
      

        Attachments

          Activity

          Hide
          pjdarton pjdarton added a comment -

          Yes, I have to agree with you there - that's a bug.
          It's not a bug I have time to fix myself, but I can at least point the way forwards so, if anyone feels like having a go and submitting a PR that fixes the issue, please do.

          So the issue is that the provision(label,number) method isn't using the same (null-proof) logic as the getTemplates(label) method to determine if a node matches the given label or not. What's needed is to extract that null-proof logic into a method that we can call from both places, and then use it from both methods.

          So, I'd guess that what we need is a common method

          private static boolean meetsLabelRequirements(final Label requirements, final Mode actualMode,
                  final Set<LabelAtom> actualLabel) {
              if (actualMode == Node.Mode.NORMAL) {
                  if (requirements == null || requirements.matches(actualLabel)) {
                      return true;
                  }
              } else if (actualMode == Node.Mode.EXCLUSIVE) {
                  if (requirements != null && requirements.matches(actualLabel)) {
                      return true;
                  }
              }
              return false;
          }
          

          ...and then to refactor the getTemplates(final Label label) method so that it says

          private List<vSphereCloudSlaveTemplate> getTemplates(final Label requirements) {
              if (this.templates == null)
                  return Collections.emptyList();
              List<vSphereCloudSlaveTemplate> matchingTemplates = new ArrayList<vSphereCloudSlaveTemplate>();
              for (vSphereCloudSlaveTemplate t : this.templates) {
                  final Set<LabelAtom> actualLabel = t.getLabelSet();
                  final Mode actualMode = t.getMode();
                  if (meetsLabelRequirements(requirements, actualMode, actualLabel)) {
                      matchingTemplates.add(t);
                  }
              }
              return matchingTemplates;
          }
          

          to change the buggy line in the provision method to be

          if (n.getComputer().isOffline() && meetsLabelRequirements(label, n.getMode(), n.getAssignedLabels())) {
          

          ...and (and this is the bit I don't have time for) test it to make damned sure that it's not broken anything and that it fixes the problem.

          If it's all ok, wrap it up in a pull-request referencing this issue (see https://github.com/jenkinsci/vsphere-cloud-plugin/blob/master/CONTRIBUTING.md for more details).

          If you, or someone else, does that, I'd be happy to review and merge it so it'll be in the next release.

          Show
          pjdarton pjdarton added a comment - Yes, I have to agree with you there - that's a bug. It's not a bug I have time to fix myself, but I can at least point the way forwards so, if anyone feels like having a go and submitting a PR that fixes the issue, please do. So the issue is that the provision(label,number) method isn't using the same (null-proof) logic as the getTemplates(label) method to determine if a node matches the given label or not. What's needed is to extract that null-proof logic into a method that we can call from both places, and then use it from both methods. So, I'd guess that what we need is a common method private static boolean meetsLabelRequirements( final Label requirements, final Mode actualMode, final Set<LabelAtom> actualLabel) { if (actualMode == Node.Mode.NORMAL) { if (requirements == null || requirements.matches(actualLabel)) { return true ; } } else if (actualMode == Node.Mode.EXCLUSIVE) { if (requirements != null && requirements.matches(actualLabel)) { return true ; } } return false ; } ...and then to refactor the getTemplates(final Label label) method so that it says private List<vSphereCloudSlaveTemplate> getTemplates( final Label requirements) { if ( this .templates == null ) return Collections.emptyList(); List<vSphereCloudSlaveTemplate> matchingTemplates = new ArrayList<vSphereCloudSlaveTemplate>(); for (vSphereCloudSlaveTemplate t : this .templates) { final Set<LabelAtom> actualLabel = t.getLabelSet(); final Mode actualMode = t.getMode(); if (meetsLabelRequirements(requirements, actualMode, actualLabel)) { matchingTemplates.add(t); } } return matchingTemplates; } to change the buggy line in the provision method to be if (n.getComputer().isOffline() && meetsLabelRequirements(label, n.getMode(), n.getAssignedLabels())) { ...and (and this is the bit I don't have time for) test it to make damned sure that it's not broken anything and that it fixes the problem. If it's all ok, wrap it up in a pull-request referencing this issue (see https://github.com/jenkinsci/vsphere-cloud-plugin/blob/master/CONTRIBUTING.md for more details). If you, or someone else, does that, I'd be happy to review and merge it so it'll be in the next release.

            People

            • Assignee:
              Unassigned
              Reporter:
              aszostak Artur Szostak
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated: