[WIP] Fix service updates

cli/lib/kontena/cli/stacks/service_generator.rb

@@ -31,45 +31,43 @@ def parse_data(options)
       data['links'] = parse_links(options['links'] || [])
       data['external_links'] = parse_links(options['external_links'] || [])
       data['ports'] = parse_stringified_ports(options['ports'] || [])
-      data['memory'] = parse_memory(options['mem_limit'].to_s) if options['mem_limit']
-      data['memory_swap'] = parse_memory(options['memswap_limit'].to_s) if options['memswap_limit']
-      data['shm_size'] = parse_memory(options['shm_size'].to_s) if options['shm_size']
-      data['cpus'] = options['cpus'] if options['cpus']
-      data['cpu_shares'] = options['cpu_shares'] if options['cpu_shares']
+      data['memory'] = options['mem_limit'] ? parse_memory(options['mem_limit'].to_s) : nil
+      data['memory_swap'] = options['memswap_limit'] ? parse_memory(options['memswap_limit'].to_s) : nil
+      data['shm_size'] = options['shm_size'] ? parse_memory(options['shm_size'].to_s) : nil
+      data['cpus'] = options['cpus'] ? options['cpus'] : nil
+      data['cpu_shares'] = options['cpu_shares'] ? options['cpu_shares'] : nil
       data['volumes'] = options['volumes'] || []
       data['volumes_from'] = options['volumes_from'] || []
-      data['cmd'] = Shellwords.split(options['command']) if options['command']
+      data['cmd'] = options['command'] ? Shellwords.split(options['command']) : []
       data['affinity'] = options['affinity'] || []
-      data['user'] = options['user'] if options['user']
+      data['user'] = options['user'] ? options['user'] : nil
       data['stateful'] = options['stateful'] == true
-      data['privileged'] = options['privileged'] unless options['privileged'].nil?
-      data['cap_add'] = options['cap_add'] if options['cap_add']
-      data['cap_drop'] = options['cap_drop'] if options['cap_drop']
-      data['net'] = options['net'] if options['net']
-      data['pid'] = options['pid'] if options['pid']
-      data['log_driver'] = options['log_driver'] if options['log_driver']
-      data['log_opts'] = options['log_opt'] if options['log_opt'] && !options['log_opt'].empty?
+      data['privileged'] = options['privileged'] || false
+      data['cap_add'] = options['cap_add'] ? options['cap_add'] : []
+      data['cap_drop'] = options['cap_drop'] ? options['cap_drop'] : []
+      data['net'] = options['net'] ? options['net'] : nil
+      data['pid'] = options['pid'] ? options['pid'] : nil
+      data['log_driver'] = options['log_driver'] ? options['log_driver'] : nil
+      data['log_opts'] = options['log_opt'] ? options['log_opt'] : {}
+      data['hooks'] = options['hooks'] || {}
+      data['secrets'] = options['secrets'] ? options['secrets'] : []
+      data['certificates'] = options['certificates'] ? options['certificates'] : []
+      data['build'] = options['build'] ? parse_build_options(options) : nil
+      data['health_check'] = parse_health_check(options)
+      data['stop_signal'] = options['stop_signal'] ? options['stop_signal'] : nil
+      data['stop_grace_period'] = options['stop_grace_period'] ? options['stop_grace_period'] : nil
+      data['read_only'] = options['read_only'] || false
+      data['entrypoint'] = options['entrypoint'] ? options['entrypoint'] : nil
+
       deploy_opts = options['deploy'] || {}
       data['strategy'] = deploy_opts['strategy'] if deploy_opts['strategy']
       deploy = {
         'wait_for_port' => deploy_opts['wait_for_port'],
         'min_health' => deploy_opts['min_health']
       }
-      if deploy_opts.has_key?('interval')
-        deploy['interval'] = parse_relative_time(deploy_opts['interval'])
-      else
-        deploy['interval'] = nil
-      end
+      deploy['interval'] = deploy_opts['interval'] ? parse_relative_time(deploy_opts['interval']) : nil
       data['deploy_opts'] = deploy
-      data['hooks'] = options['hooks'] || {}
-      data['secrets'] = options['secrets'] if options['secrets']
-      data['certificates'] = options['certificates'] if options['certificates']
-      data['build'] = parse_build_options(options) if options['build']
-      data['health_check'] = parse_health_check(options)
-      data['stop_signal'] = options['stop_signal'] if options['stop_signal']
-      data['stop_grace_period'] = options['stop_grace_period'] if options['stop_grace_period']
-      data['read_only'] = options['read_only'] || false
-      data['entrypoint'] = options['entrypoint'] if options['entrypoint']
+
       data
     end
 

cli/lib/kontena/cli/stacks/service_generator_v2.rb

@@ -6,7 +6,7 @@ class ServiceGeneratorV2 < ServiceGenerator
 
     def parse_data(options)
       data = super(options)
-      data['net'] = options['network_mode'] if options['network_mode']
+      data['net'] = options['network_mode'] ? options['network_mode'] : nil
       data['log_driver'] = options.dig('logging', 'driver')
       data['log_opts'] = options.dig('logging', 'options')
       if options['depends_on']

cli/spec/kontena/cli/stacks/service_generator_spec.rb

@@ -58,12 +58,12 @@
         expect(result['cmd']).to eq(data['command'].split(' '))
       end
 
-      it 'does not return cmd if not set' do
+      it 'returns array if not set' do
         data = {
           'image' => 'foo/bar:latest'
         }
         result = subject.send(:parse_data, data)
-        expect(result.has_key?('cmd')).to be_falsey
+        expect(result['cmd']).to eq([])
       end
     end
 
@@ -99,12 +99,12 @@
         expect(result['user']).to eq('user')
       end
 
-      it 'does not return user if not set' do
+      it 'returns nil if user not set' do
         data = {
           'image' => 'foo/bar:latest'
         }
         result = subject.send(:parse_data, data)
-        expect(result.has_key?('user')).to be_falsey
+        expect(result['user']).to be_nil
       end
     end
 
@@ -158,12 +158,12 @@
         expect(result['privileged']).to eq(false)
       end
 
-      it 'does not return privileged if not set' do
+      it 'returns false if not set' do
         data = {
           'image' => 'foo/bar:latest'
         }
         result = subject.send(:parse_data, data)
-        expect(result['privileged']).to be_nil
+        expect(result['privileged']).to be_falsey
       end
     end
 
@@ -179,12 +179,12 @@
         expect(result['cap_add']).to eq(data['cap_add'])
       end
 
-      it 'does not return cap_add if not set' do
+      it 'returns empty array if not set' do
         data = {
           'image' => 'foo/bar:latest'
         }
         result = subject.send(:parse_data, data)
-        expect(result['cap_add']).to be_nil
+        expect(result['cap_add']).to eq([])
       end
     end
 
@@ -200,12 +200,12 @@
         expect(result['cap_drop']).to eq(data['cap_drop'])
       end
 
-      it 'does not return cap_drop if not set' do
+      it 'returns empty array if not set' do
         data = {
           'image' => 'foo/bar:latest'
         }
         result = subject.send(:parse_data, data)
-        expect(result['cap_drop']).to be_nil
+        expect(result['cap_drop']).to eq([])
       end
     end
 
@@ -219,7 +219,7 @@
         expect(result['net']).to eq('host')
       end
 
-      it 'does not return pid if not set' do
+      it 'returns nil if not set' do
         data = {
           'image' => 'foo/bar:latest'
         }
@@ -238,7 +238,7 @@
         expect(result['pid']).to eq('host')
       end
 
-      it 'does not return pid if not set' do
+      it 'returns nil if not set' do
         data = {
           'image' => 'foo/bar:latest'
         }
@@ -257,7 +257,7 @@
         expect(result['log_driver']).to eq('syslog')
       end
 
-      it 'does not return log_driver if not set' do
+      it 'returns nil if not set' do
         data = {
           'image' => 'foo/bar:latest'
         }
@@ -280,12 +280,12 @@
         expect(result['log_opts']).to eq(data['log_opt'])
       end
 
-      it 'does not return log_opts if log_opt is not set' do
+      it 'returns empty hash if log_opt is not set' do
         data = {
           'image' => 'foo/bar:latest'
         }
         result = subject.send(:parse_data, data)
-        expect(result['log_opts']).to be_nil
+        expect(result['log_opts']).to eq({})
       end
     end
 
@@ -359,7 +359,7 @@
         expect(result['hooks']).to eq(data['hooks'])
       end
 
-      it 'does returns empty hook hash if not defined' do
+      it 'does return empty hook hash if not defined' do
         data = {'image' => 'foo/bar:latest'}
         result = subject.send(:parse_data, data)
         expect(result['hooks']).to eq({})
@@ -378,10 +378,10 @@
         expect(result['secrets']).to eq(data['secrets'])
       end
 
-      it 'does not return secrets if not defined' do
+      it 'returns empty array if not defined' do
         data = {'image' => 'foo/bar:latest'}
         result = subject.send(:parse_data, data)
-        expect(result['secrets']).to be_nil
+        expect(result['secrets']).to eq([])
       end
     end
 

server/app/models/grid_service.rb

@@ -13,7 +13,7 @@ class GridService
   field :stateful, type: Boolean, default: false
   field :user, type: String
   field :container_count, type: Integer, default: 1
-  field :cmd, type: Array
+  field :cmd, type: Array, default: []
   field :entrypoint, type: String
   field :ports, type: Array, default: []
   field :env, type: Array, default: []

server/app/mutations/grid_services/common.rb

@@ -223,14 +223,14 @@ module ClassMethods
 
       def common_validations
         optional do
-          string :strategy
+          string :strategy, nils: true
           integer :instances
           integer :container_count # @todo: deprecated by instances
-          string :user
+          string :user, nils: true
           array :cmd do
             string
           end
-          string :entrypoint
+          string :entrypoint, nils: true
           array :env do
             string matches: /\A[^=]+=/, max_length: 128 * 1024
           end
@@ -284,27 +284,27 @@ def common_validations
           array :volumes_from do
             string
           end
-          float :cpus
-          integer :cpu_shares, min: 0, max: 1024
-          integer :memory
-          integer :memory_swap
-          integer :shm_size
-          boolean :privileged
+          float :cpus, nils: true
+          integer :cpu_shares, min: 0, max: 1024, nils: true
+          integer :memory, nils: true
+          integer :memory_swap, nils: true
+          integer :shm_size, nils: true
+          boolean :privileged, nils: true
           array :cap_add do
             string
           end
           array :cap_drop do
             string
           end
-          string :net, matches: /\A(bridge|host|container:.+)\z/
+          string :net, default: 'bridge', matches: /\A(bridge|host|container:.+)\z/
           hash :log_opts do
             string :*
           end
-          string :log_driver
+          string :log_driver, nils: true
           array :devices do
             string
           end
-          string :pid, in: ['host']
+          string :pid, in: ['host'], nils: true
           boolean :read_only
           hash :hooks do
             optional do
@@ -352,8 +352,8 @@ def common_validations
               integer :initial_delay, default: 10
             end
           end
-          string :stop_signal, matches: /\A((SIG)([A-Z0-9]+|RTMIN\+\d+|RTMAX-\d+)|\d+)\z/i
-          string :stop_grace_period, matches: Duration::VALIDATION_PATTERN
+          string :stop_signal, nils: true, matches: /\A((SIG)([A-Z0-9]+|RTMIN\+\d+|RTMAX-\d+)|\d+)\z/i
+          string :stop_grace_period, nils: true, matches: Duration::VALIDATION_PATTERN
         end
       end
     end

server/app/mutations/grid_services/update.rb

@@ -52,60 +52,65 @@ def validate
 
     def execute
       attributes = {}
-      attributes[:strategy] = self.strategy if self.strategy
-      attributes[:image_name] = self.image if self.image
-      attributes[:container_count] = self.container_count if self.container_count
-      attributes[:container_count] = self.instances if self.instances
-      attributes[:user] = self.user if self.user
-      attributes[:cpus] = self.cpus if self.cpus
-      attributes[:cpu_shares] = self.cpu_shares if self.cpu_shares
-      attributes[:memory] = self.memory if self.memory
-      attributes[:memory_swap] = self.memory_swap if self.memory_swap
-      attributes[:shm_size] = self.shm_size if self.shm_size
-      attributes[:privileged] = self.privileged unless self.privileged.nil?
-      attributes[:cap_add] = self.cap_add if self.cap_add
-      attributes[:cap_drop] = self.cap_drop if self.cap_drop
-      attributes[:cmd] = self.cmd if self.cmd
-      attributes[:env] = self.build_grid_service_envs(self.env) if self.env
-      attributes[:net] = self.net if self.net
-      attributes[:ports] = self.ports if self.ports
-      attributes[:affinity] = self.affinity if self.affinity
-      attributes[:log_driver] = self.log_driver if self.log_driver
-      attributes[:log_opts] = self.log_opts if self.log_opts
-      attributes[:devices] = self.devices if self.devices
-      attributes[:deploy_opts] = self.deploy_opts if self.deploy_opts
-      attributes[:health_check] = self.health_check if self.health_check
-      attributes[:volumes_from] = self.volumes_from if self.volumes_from
-      attributes[:stop_signal] = self.stop_signal if self.stop_signal
-      attributes[:stop_grace_period] = parse_duration(self.stop_grace_period) if self.stop_grace_period
-      attributes[:read_only] = self.read_only unless self.read_only.nil?
+      attributes[:strategy] = self.strategy if inputs.has_key?('strategy')
+      attributes[:image_name] = self.image if inputs.has_key?('image')
+      attributes[:container_count] = self.container_count if inputs.has_key?('container_count')
+      attributes[:container_count] = self.instances if inputs.has_key?('instances')
+      attributes[:user] = self.user if inputs.has_key?('user')
+      attributes[:cpus] = self.cpus if inputs.has_key?('cpus')
+      attributes[:cpu_shares] = self.cpu_shares if inputs.has_key?('cpu_shares')
+      attributes[:memory] = self.memory if inputs.has_key?('memory')
+      attributes[:memory_swap] = self.memory_swap if inputs.has_key?('memory_swap')
+      attributes[:shm_size] = self.shm_size if inputs.has_key?('shm_size')
+      attributes[:privileged] = self.privileged unless inputs.has_key?('privileged')
+      attributes[:cap_add] = self.cap_add if inputs.has_key?('cap_add')
+      attributes[:cap_drop] = self.cap_drop if inputs.has_key?('cap_drop')
+      attributes[:cmd] = self.cmd if inputs.has_key?('cmd')
+      attributes[:env] = self.build_grid_service_envs(self.env) if inputs.has_key?('env')
+      attributes[:net] = self.net if inputs.has_key?('net')
+      attributes[:ports] = self.ports if inputs.has_key?('ports')
+      attributes[:affinity] = self.affinity if inputs.has_key?('affinity')
+      attributes[:log_driver] = self.log_driver if inputs.has_key?('log_driver')
+      attributes[:log_opts] = self.log_opts if inputs.has_key?('log_opts')
+      attributes[:devices] = self.devices if inputs.has_key?('devices')
+      attributes[:deploy_opts] = self.deploy_opts if inputs.has_key?('deploy_opts')
+      attributes[:health_check] = self.health_check if inputs.has_key?('health_check')
+      attributes[:volumes_from] = self.volumes_from if inputs.has_key?('volumes_from')
+      attributes[:stop_signal] = self.stop_signal if inputs.has_key?('stop_signal')
+      attributes[:read_only] = self.read_only if inputs.has_key?('read_only')
+
+      if inputs.has_key?('stop_grace_period')
+        attributes[:stop_grace_period] = self.stop_grace_period ? parse_duration(self.stop_grace_period) : GridService.new.stop_grace_period
+      end
 
       embeds_changed = false
 
-      if self.links
+      if inputs.has_key?('links')
         attributes[:grid_service_links] = build_grid_service_links(
           self.grid_service.grid_service_links.to_a,
           self.grid_service.grid, grid_service.stack, self.links
         )
         embeds_changed ||= attributes[:grid_service_links] != self.grid_service.grid_service_links.to_a
       end
 
-      if self.hooks
+      if inputs.has_key?('hooks')
         attributes[:hooks] = self.build_grid_service_hooks(self.grid_service.hooks.to_a)
         embeds_changed ||= attributes[:hooks] != self.grid_service.hooks.to_a
       end
 
-      if self.secrets
+      if inputs.has_key?('secrets')
         attributes[:secrets] = self.build_grid_service_secrets(self.grid_service.secrets.to_a)
         embeds_changed ||= attributes[:secrets] != self.grid_service.secrets.to_a
       end
-      if self.volumes
+
+      if inputs.has_key?('volumes')
         attributes[:service_volumes] = self.build_service_volumes(self.grid_service.service_volumes.to_a,
           self.grid_service.grid, self.grid_service.stack
         )
         embeds_changed ||= attributes[:service_volumes] != self.grid_service.service_volumes.to_a
       end
-      if self.certificates
+
+      if inputs.has_key?('certificates')
         attributes[:certificates] = self.build_grid_service_certificates(self.grid_service.certificates.to_a)
         embeds_changed ||= attributes[:certificates] != self.grid_service.certificates.to_a
       end