Some bugfixes and style improvements

* Fix validation of 0, fix ulimits, don't compare mem_limit or mem_reservation until OS supports them

* Remove all instances of _.forEach

* ApplicationManager: have separate compareNetworksForUpdate and compareVolumesForUpdate

Signed-off-by: Pablo Carranza Velez <pablo@resin.io>

src/application-manager.coffee

@@ -433,7 +433,7 @@ module.exports = class ApplicationManager extends EventEmitter
 			})
 		return { removePairs, installPairs, updatePairs }
 
-	compareNetworksOrVolumesForUpdate: (model, { current, target }, appId) ->
+	_compareNetworksOrVolumesForUpdate: (model, { current, target }, appId) ->
 		outputPairs = []
 		currentNames = _.keys(current)
 		targetNames = _.keys(target)
@@ -474,6 +474,12 @@ module.exports = class ApplicationManager extends EventEmitter
 			})
 		return outputPairs
 
+	compareNetworksForUpdate: ({ current, target }, appId) =>
+		@_compareNetworksOrVolumesForUpdate(@networks, { current, target }, appId)
+
+	compareVolumesForUpdate: ({ current, target }, appId) =>
+		@_compareNetworksOrVolumesForUpdate(@volumes, { current, target }, appId)
+
 	# TODO: should we consider the case where several services use the same image?
 	# In such case we should do more complex matching to allow several image objects
 	# with the same name but different metadata. For now this shouldn't be necessary
@@ -647,8 +653,8 @@ module.exports = class ApplicationManager extends EventEmitter
 		if !currentApp?
 			currentApp = emptyApp
 		appId = targetApp.appId ? currentApp.appId
-		networkPairs = @compareNetworksOrVolumesForUpdate(@networks, { current: currentApp.networks, target: targetApp.networks }, appId)
-		volumePairs = @compareNetworksOrVolumesForUpdate(@volumes, { current: currentApp.volumes, target: targetApp.volumes }, appId)
+		networkPairs = @compareNetworksForUpdate({ current: currentApp.networks, target: targetApp.networks }, appId)
+		volumePairs = @compareVolumesForUpdate({ current: currentApp.volumes, target: targetApp.volumes }, appId)
 		{ removePairs, installPairs, updatePairs } = @compareServicesForUpdate(currentApp.services, targetApp.services)
 		imagePairs = @compareImagesForMetadataUpdate(availableImages, targetApp.services)
 		steps = []

src/compose/service.coffee

@@ -154,6 +154,7 @@ module.exports = class Service
 			@dnsOpt
 			@tmpfs
 			@extraHosts
+			@ulimits
 			@ulimitsArray
 			@stopSignal
 			@stopGracePeriod
@@ -241,9 +242,9 @@ module.exports = class Service
 
 			@ulimitsArray = _.map @ulimits, (value, name) ->
 				if _.isNumber(value) or _.isString(value)
-					return { Name: name, Soft: parseInt(value), Hard: parseInt(value) }
+					return { Name: name, Soft: checkInt(value), Hard: checkInt(value) }
 				else
-					return { Name: name, Soft: parseInt(value.soft), Hard: parseInt(value.hard) }
+					return { Name: name, Soft: checkInt(value.soft), Hard: checkInt(value.hard) }
 			if @init
 				@init = true
 
@@ -255,6 +256,11 @@ module.exports = class Service
 
 			if Array.isArray(@sysctls)
 				@sysctls = _.fromPairs(_.map(@sysctls, (v) -> _.split(v, '=')))
+			@sysctls = _.mapValues(@sysctls, String)
+
+			# Avoid problems with yaml parsing numbers as strings
+			for key in [ 'cpuShares', 'cpuQuota', 'oomScoreAdj' ]
+				this[key] = checkInt(this[key])
 
 	_addSupervisorApi: (opts) =>
 		@environment['RESIN_SUPERVISOR_PORT'] = opts.listenPort.toString()
@@ -315,7 +321,7 @@ module.exports = class Service
 		@expose = _.clone(@expose)
 		@expose = _.map(@expose, String)
 		if imageInfo?.Config?.ExposedPorts?
-			_.forEach imageInfo.Config.ExposedPorts, (v, k) =>
+			for own k, v of imageInfo.Config.ExposedPorts
 				port = k.match(/^([0-9]*)\/tcp$/)?[1]
 				if port? and !_.find(@expose, port)
 					@expose.push(port)
@@ -375,7 +381,7 @@ module.exports = class Service
 		boundContainerPorts = []
 		ports = []
 		expose = []
-		_.forEach container.HostConfig.PortBindings, (conf, port) ->
+		for own port, conf of container.HostConfig.PortBindings
 			containerPort = port.match(/^([0-9]*)\/tcp$/)?[1]
 			if containerPort?
 				boundContainerPorts.push(containerPort)
@@ -384,7 +390,7 @@ module.exports = class Service
 					ports.push("#{hostPort}:#{containerPort}")
 				else
 					ports.push(containerPort)
-		_.forEach container.Config.ExposedPorts, (conf, port) ->
+		for own port, conf of container.Config.ExposedPorts
 			containerPort = port.match(/^([0-9]*)\/tcp$/)?[1]
 			if containerPort? and !_.includes(boundContainerPorts, containerPort)
 				expose.push(containerPort)
@@ -556,8 +562,8 @@ module.exports = class Service
 			'environment'
 			'portBindings'
 			'exposedPorts'
-			'memLimit'
-			'memReservation'
+			#'memLimit'
+			#'memReservation'
 			'shmSize'
 			'cpuShares'
 			'cpuQuota'

src/device-config.coffee

@@ -107,7 +107,7 @@ module.exports = class DeviceConfig
 			conf.RESIN_HOST_LOG_TO_DISPLAY ?= ''
 			if initial or !conf.RESIN_SUPERVISOR_VPN_CONTROL?
 				conf.RESIN_SUPERVISOR_VPN_CONTROL = 'true'
-			_.forEach @configKeys, ({ envVarName, defaultValue }) ->
+			for own k, { envVarName, defaultValue } of @configKeys
 				conf[envVarName] ?= defaultValue ? ''
 			return conf
 
@@ -123,7 +123,7 @@ module.exports = class DeviceConfig
 						RESIN_HOST_LOG_TO_DISPLAY: (logToDisplayStatus ? '').toString()
 						RESIN_SUPERVISOR_VPN_CONTROL: (vpnStatus ? 'true').toString()
 					}
-					_.forEach @configKeys, ({ envVarName }, key) ->
+					for own key, { envVarName } of @configKeys
 						currentConf[envVarName] = (conf[key] ? '').toString()
 					return _.assign(currentConf, bootConfig)
 			)
@@ -154,7 +154,7 @@ module.exports = class DeviceConfig
 				'int': (a, b) ->
 					checkInt(a) == checkInt(b)
 			}
-			_.forEach @configKeys, ({ envVarName, varType }, key) ->
+			for own key, { envVarName, varType } of @configKeys
 				if !match[varType](current[envVarName], target[envVarName])
 					configChanges[key] = target[envVarName]
 					humanReadableConfigChanges[envVarName] = target[envVarName]
@@ -277,12 +277,11 @@ module.exports = class DeviceConfig
 				return false
 			@logger.logSystemMessage("Applying boot config: #{JSON.stringify(conf)}", {}, 'Apply boot config in progress')
 			configStatements = []
-			_.forEach conf, (val, key) ->
+			for own key, val of conf
 				if key is 'initramfs'
 					configStatements.push("#{key} #{val}")
 				else if _.isArray(val)
-					configStatements = configStatements.concat _.map val, (entry) ->
-						return "#{key}=#{entry}"
+					configStatements = configStatements.concat(_.map(val, (entry) -> "#{key}=#{entry}"))
 				else
 					configStatements.push("#{key}=#{val}")
 

src/lib/validation.coffee

@@ -2,7 +2,7 @@ _ = require 'lodash'
 
 exports.checkInt = checkInt = (s, options = {}) ->
 	# Make sure `s` exists and is not an empty string.
-	if !s
+	if !s?
 		return
 	i = parseInt(s, 10)
 	if isNaN(i)

src/proxyvisor.coffee

@@ -368,7 +368,7 @@ module.exports = class Proxyvisor
 	normaliseDependentDeviceTargetForDB: (device, appCommit) ->
 		Promise.try ->
 			apps = _.clone(device.apps ? {})
-			_.forEach apps, (app) ->
+			for app in apps
 				app.commit ?= appCommit
 				app.config ?= {}
 				app.environment ?= {}
@@ -447,10 +447,10 @@ module.exports = class Proxyvisor
 	imagesInUse: (current, target) ->
 		images = []
 		if current.dependent?.apps?
-			_.forEach current.dependent.apps, (app) ->
+			for app in current.dependent.apps
 				images.push app.image
 		if target.dependent?.apps?
-			_.forEach target.dependent.apps, (app) ->
+			for app in target.dependent.apps
 				images.push app.image
 		return images
 
@@ -463,7 +463,7 @@ module.exports = class Proxyvisor
 			devices: []
 			appId
 		}
-		_.forEach currentDevices, (device) =>
+		for device in currentDevices
 			if device.markedForDeletion
 				hookStep.devices.push({
 					uuid: device.uuid
@@ -527,7 +527,7 @@ module.exports = class Proxyvisor
 			appIdsToCheck = _.filter allAppIds, (appId) ->
 				# - if a step is in progress for this appId, ignore
 				!_.some(steps.concat(stepsInProgress), (step) -> step.appId == appId)
-			_.forEach appIdsToCheck, (appId) =>
+			for appId in appIdsToCheck
 				# - if there's current but not target, push a removeDependentApp step
 				if !targetApps[appId]?
 					steps.push({