Update how we merge custom labels with the defaults

[#163783467]

jobs/kubelet/templates/bin/kubelet_ctl.erb

@@ -33,7 +33,14 @@ DATA_DIR=/var/vcap/store/kubernetes
   if iaas=="vsphere"
     labels << "failure-domain.beta.kubernetes.io/zone=#{spec.az}"
   end
-  labels = labels.join(',')
+
+  if_p('k8s-args') do |args|
+    custom_labels = args.fetch('node-labels', "").split(",")
+    labels = custom_labels + labels
+    args.delete('node-labels')
+  end
+
+  labels = labels.join(",")
 %>
 
 # shellcheck disable=SC1091
@@ -101,19 +108,11 @@ start_kubelet() {
   kubelet \
   <%-
     if_p('k8s-args') do |args|
-      node_labels = args.select { |f,_| f == 'node-labels' }
-      if node_labels.empty?
-        args['node-labels'] = labels
-      else
-        node_labels = "#{node_labels['node-labels']},#{labels}"
-      end
       args.each do |flag, value|
         valueString = ""
 
         if value.nil?
           # Do nothing to supports args-less flags (--example)
-        elsif flag == 'node-labels'
-          valueString = "=#{node_labels}" # << labels
         elsif value.is_a? Array
           valueString = "=#{value.join(",")}"
         elsif value.is_a? Hash
@@ -125,15 +124,12 @@ start_kubelet() {
     <%= "--#{flag}#{valueString}" %> \
   <%-
       end
-  end.else do
-  -%>
-    <%= "--node-labels=#{labels}" %> \
-  <%-
-  end
+    end
   -%>
     <% if include_config -%>--cloud-config=${cloud_config}<% end %> \
     <% if !iaas.nil? -%>--cloud-provider=${cloud_provider}<% end %> \
     --hostname-override=$(get_hostname_override) \
+    --node-labels=<%= labels %> \
     --config="/var/vcap/jobs/kubelet/config/kubeletconfig.yml" \
   1>> $LOG_DIR/kubelet.stdout.log \
   2>> $LOG_DIR/kubelet.stderr.log

spec/kubelet_ctl_spec.rb

@@ -5,6 +5,13 @@
 require 'fileutils'
 require 'open3'
 
+def get_node_labels(rendered_kubelet_ctl)
+  node_labels = rendered_kubelet_ctl.split("\n").select { |line| line[/--node-labels=/i] }
+  expect(node_labels.length).to be(1)
+  labels = node_labels[0].match(/--node-labels=(.*) \\/).captures[0]
+  labels.split(",")
+end
+
 describe 'kubelet_ctl' do
   let(:rendered_template) do
     compiled_template('kubelet', 'bin/kubelet_ctl', {}, {}, {}, 'z1', 'fake-bosh-ip', 'fake-bosh-id')
@@ -29,9 +36,26 @@
       }
     }
     rendered_kubelet_ctl = compiled_template('kubelet', 'bin/kubelet_ctl', manifest_properties, {}, {}, 'z1', 'fake-bosh-ip', 'fake-bosh-id')
-    expect(rendered_kubelet_ctl).to include(',bosh.id=fake-bosh-id')
-    expect(rendered_kubelet_ctl).to include(',k8s.node=custom')
-    expect(rendered_kubelet_ctl).to include('foo=bar')
+    labels = get_node_labels(rendered_kubelet_ctl)
+
+    expect(labels).to include('bosh.zone=z1')
+    expect(labels).to include('spec.ip=fake-bosh-ip')
+    expect(labels).to include('bosh.id=fake-bosh-id')
+    expect(labels).to include('k8s.node=custom')
+    expect(labels).to include('foo=bar')
+  end
+
+  it 'labels the kubelet with default labels' do
+    manifest_properties = {
+      'k8s-args' => {
+      }
+    }
+    rendered_kubelet_ctl = compiled_template('kubelet', 'bin/kubelet_ctl', manifest_properties, {}, {}, 'z1', 'fake-bosh-ip', 'fake-bosh-id')
+    labels = get_node_labels(rendered_kubelet_ctl)
+
+    expect(labels).to include('bosh.zone=z1')
+    expect(labels).to include('spec.ip=fake-bosh-ip')
+    expect(labels).to include('bosh.id=fake-bosh-id')
   end
 
   it 'has no http proxy when no proxy is defined' do