diff --git a/app/models/service.rb b/app/models/service.rb index f9bb2d8..9e9dc4e 100644 --- a/app/models/service.rb +++ b/app/models/service.rb @@ -6,8 +6,6 @@ class Service using FleetAdapter::StringExtensions - MAX_PORT = 65536 - attr_accessor :id, :name, :source, :links, :command, :ports, :expose, :environment, :volumes, :deployment, :prefix @@ -105,9 +103,6 @@ def unit_block def service_block docker_rm = "-/usr/bin/docker rm #{prefix}" service_registration = "/usr/bin/etcdctl set app/#{name.upcase}/#{name.upcase}_SERVICE_HOST ${COREOS_PRIVATE_IPV4}" - if min_port - service_registration += " ; /usr/bin/etcdctl set app/#{name.upcase}/#{name.upcase}_SERVICE_PORT #{min_port}" - end { # A hack to be able to have two ExecStartPre values @@ -143,17 +138,6 @@ def docker_run_string ].flatten.compact.join(' ').strip end - def min_port - if ports.empty? - min_container_port = MAX_PORT - else - min_container_port = ports.sort_by { |port_binding| port_binding[:containerPort]}.first[:containerPort] - end - min_exposed = expose.sort.first || MAX_PORT - min_port = [min_container_port.to_i, min_exposed.to_i].min - min_port unless min_port == MAX_PORT - end - def port_flags return unless ports @@ -183,36 +167,40 @@ def link_flags link_alias = link[:alias].upcase if link[:alias] link_name = link[:name].sanitize.upcase + min_port = link[:exposed_ports].sort_by { |exposed_port| exposed_port[:containerPort] }.first + link_vars.push( { variable: (link_alias ? "#{link_alias}_SERVICE_HOST" : "#{link_name}_SERVICE_HOST").upcase, value: "`/usr/bin/etcdctl get app/#{link_name}/#{link_name}_SERVICE_HOST`" }, - { - variable: (link_alias ? "#{link_alias}_SERVICE_PORT" : "#{link_name}_SERVICE_PORT").upcase, - value: "`/usr/bin/etcdctl get app/#{link_name}/#{link_name}_SERVICE_PORT`" - }, { variable: (link_alias ? "#{link_alias}_PORT" : "#{link_name}_PORT").upcase, - value: "#{link[:protocol]}://`/usr/bin/etcdctl get app/#{link_name}/#{link_name}_SERVICE_HOST`:`/usr/bin/etcdctl get app/#{link_name}/#{link_name}_SERVICE_PORT`" - }, - { - variable: (link_alias ? "#{link_alias}_PORT_#{link[:port]}_#{link[:protocol]}" : "#{link_name}_PORT_#{link[:port]}_#{link[:protocol]}").upcase, - value: "#{link[:protocol]}://`/usr/bin/etcdctl get app/#{link_name}/#{link_name}_SERVICE_HOST`:`/usr/bin/etcdctl get app/#{link_name}/#{link_name}_SERVICE_PORT`" - }, - { - variable: (link_alias ? "#{link_alias}_PORT_#{link[:port]}_#{link[:protocol]}_PROTO" : "#{link_name}_PORT_#{link[:port]}_#{link[:protocol]}_PROTO").upcase, - value: link[:protocol] - }, - { - variable: (link_alias ? "#{link_alias}_PORT_#{link[:port]}_#{link[:protocol]}_PORT" : "#{link_name}_PORT_#{link[:port]}_#{link[:protocol]}_PORT").upcase, - value: "`/usr/bin/etcdctl get app/#{link_name}/#{link_name}_SERVICE_PORT`" - }, - { - variable: (link_alias ? "#{link_alias}_PORT_#{link[:port]}_#{link[:protocol]}_ADDR" : "#{link_name}_PORT_#{link[:port]}_#{link[:protocol]}_ADDR").upcase, - value: "`/usr/bin/etcdctl get app/#{link_name}/#{link_name}_SERVICE_HOST`" + value: "#{min_port[:protocol]}://`/usr/bin/etcdctl get app/#{link_name}/#{link_name}_SERVICE_HOST`:#{min_port[:hostPort]}" } ) + + # Docker-esque container linking variables + link[:exposed_ports].each do |exposed_port| + link_vars.push( + { + variable: (link_alias ? "#{link_alias}_PORT_#{exposed_port[:containerPort]}_#{exposed_port[:protocol]}" : "#{link_name}_PORT_#{exposed_port[:containerPort]}_#{exposed_port[:protocol]}").upcase, + value: "#{exposed_port[:protocol]}://`/usr/bin/etcdctl get app/#{link_name}/#{link_name}_SERVICE_HOST`:#{exposed_port[:hostPort]}" + }, + { + variable: (link_alias ? "#{link_alias}_PORT_#{exposed_port[:containerPort]}_#{exposed_port[:protocol]}_PROTO" : "#{link_name}_PORT_#{exposed_port[:containerPort]}_#{exposed_port[:protocol]}_PROTO").upcase, + value: exposed_port[:protocol] + }, + { + variable: (link_alias ? "#{link_alias}_PORT_#{exposed_port[:containerPort]}_#{exposed_port[:protocol]}_PORT" : "#{link_name}_PORT_#{exposed_port[:containerPort]}_#{exposed_port[:protocol]}_PORT").upcase, + value: exposed_port[:hostPort] + }, + { + variable: (link_alias ? "#{link_alias}_PORT_#{exposed_port[:containerPort]}_#{exposed_port[:protocol]}_ADDR" : "#{link_name}_PORT_#{exposed_port[:containerPort]}_#{exposed_port[:protocol]}_ADDR").upcase, + value: "`/usr/bin/etcdctl get app/#{link_name}/#{link_name}_SERVICE_HOST`" + } + ) + end end link_vars.map { |link| "-e #{link[:variable]}=#{link[:value]}" } diff --git a/app/models/service_sorter.rb b/app/models/service_sorter.rb index 2967d8f..28b6488 100644 --- a/app/models/service_sorter.rb +++ b/app/models/service_sorter.rb @@ -60,36 +60,22 @@ def set_link_port_and_protocol(service, dependency) return if service[:links].to_a.empty? exposed_ports = ports_and_protocols_for(dependency) - unless min_port = exposed_ports.sort_by { |exposed_port| exposed_port[:port] }.first - raise ArgumentError, "#{dependency[:name]} does not expose a port" - end - service[:links].find { |link| link[:name] == dependency[:name] } - .merge!({ port: min_port[:port], protocol: min_port[:protocol] }) + service[:links].find { |link| link[:name] == dependency[:name] }.merge!({ exposed_ports: exposed_ports }) end # Finds the explicitly exposed ports (:ports and :expose) on the dependency and # creates a hash of the port and protocol for each def ports_and_protocols_for(service) - ports = [] - - if service[:ports] - mapped_ports = service[:ports].map do |exposed_port| - { port: exposed_port[:hostPort], protocol: (exposed_port[:protocol] || 'tcp') } - end - ports.push(mapped_ports).flatten! + if service[:ports].to_a.empty? + raise ArgumentError, "#{service[:name]} does not have an explicit port binding" end - if service[:expose] - exposed_ports = service[:expose].map do |exposed_port| - { port: exposed_port, protocol: 'tcp' } - end - ports.push(exposed_ports).flatten! + mapped_ports = service[:ports].map do |exposed_port| + exposed_port.merge!({ protocol: 'tcp' }) unless exposed_port.has_key?(:proto) end - - return ports + mapped_ports.flatten end - end end end diff --git a/spec/api/services_spec.rb b/spec/api/services_spec.rb index ea5c223..7401761 100644 --- a/spec/api/services_spec.rb +++ b/spec/api/services_spec.rb @@ -60,7 +60,7 @@ end it 'includes an error message indicating the service should expose a port' do - expected = { error: 'dependency does not expose a port' }.to_json + expected = { error: 'dependency does not have an explicit port binding' }.to_json post '/v1/services', request_body expect(last_response.body).to eq expected end diff --git a/spec/models/service_sorter_spec.rb b/spec/models/service_sorter_spec.rb index 23fcc0f..86b88b7 100644 --- a/spec/models/service_sorter_spec.rb +++ b/spec/models/service_sorter_spec.rb @@ -20,23 +20,9 @@ it 'adds the port and protocol of the dependency to the dependent link hash' do expect(subject.last[:links].first[:name]).to eq 'DB' expect(subject.last[:links].first[:alias]).to eq 'DB_1' - expect(subject.last[:links].first[:port]).to eq 3306 - expect(subject.last[:links].first[:protocol]).to eq 'tcp' - end - - context 'when a dependent container exposes ports' do - before do - @db_service = services.find { |service| service[:name] == 'DB' } - @db_service[:expose] = [80] - end - - it 'uses the lowest numbered exposed port as the link port' do - expect(subject.last[:links].first[:port]).to eq 80 - end - - it "uses 'tcp' as the link protocol" do - expect(subject.last[:links].first[:protocol]).to eq 'tcp' - end + expect(subject.last[:links].first[:exposed_ports].first[:hostPort]).to eq 1111 + expect(subject.last[:links].first[:exposed_ports].first[:containerPort]).to eq 3306 + expect(subject.last[:links].first[:exposed_ports].first[:protocol]).to eq 'tcp' end context 'when a container links to itself' do @@ -66,18 +52,6 @@ end end - - context 'when a dependency does not expose a port through expose or port bindings' do - before do - @db_service = services.find { |service| service[:name] == 'DB' } - @db_service[:expose] = [] - @db_service[:ports] = [] - end - - it 'raises an exception' do - expect{ described_class.sort(services) }.to raise_error(ArgumentError, /does not expose a port/) - end - end end describe '.get_service_names_for' do @@ -95,26 +69,19 @@ end describe '.ports_and_protocols_for' do - it 'responds with mapped ports when there are only mapped ports' do - service = { ports: [{ hostPort: 80, protocol: 'tcp' }] } - expect(described_class.send(:ports_and_protocols_for, service)).to match_array([{ port: 80, protocol: 'tcp' }]) - end - - - it 'responds with exposed ports when there are only exposed ports' do - service = { expose: [80] } - expect(described_class.send(:ports_and_protocols_for, service)).to match_array([{ port: 80, protocol: 'tcp' }]) + it 'raises an exception if there are no ports' do + service = { ports: [] } + expect{ described_class.send(:ports_and_protocols_for, service) }.to raise_error(ArgumentError, /does not have an explicit port binding/) end - it 'combines mapped and exposed ports if there are both' do - service = { expose: [80], ports: [{ hostPort: 8080, protocol: 'tcp' }] } - expect(described_class.send(:ports_and_protocols_for, service)).to match_array([{ port: 80, protocol: 'tcp' }, - { port: 8080, protocol: 'tcp' }]) + it 'responds with mapped ports' do + service = { ports: [{ hostPort: 80, containerPort: 1234, protocol: 'tcp' }] } + expect(described_class.send(:ports_and_protocols_for, service)).to match_array([{ hostPort: 80, containerPort: 1234, protocol: 'tcp' }]) end - it 'returns an empty array if there no mapped or exposed ports' do - service = { expose: [], ports: [] } - expect(described_class.send(:ports_and_protocols_for, service)).to match_array([]) + it 'adds the tcp protocol if no protocol type is defined' do + service = { ports: [{ hostPort: 80, containerPort: 1234 }] } + expect(described_class.send(:ports_and_protocols_for, service)).to match_array([{ hostPort: 80, containerPort: 1234, protocol: 'tcp' }]) end end end diff --git a/spec/models/service_spec.rb b/spec/models/service_spec.rb index 5c3273d..2252cb6 100644 --- a/spec/models/service_spec.rb +++ b/spec/models/service_spec.rb @@ -9,7 +9,7 @@ name: 'foo', source: 'bar', command: '/bin/bash', - ports: [{ containerPort: 3306 }], + ports: [{ hostPort: 3306, containerPort: 3306 }], expose: [3306], environment: [{ variable: 'DB_PASSWORD', value: 'password' }], volumes: [{ hostPath: '/foo/bar', containerPath: '/bar/baz' }], @@ -171,6 +171,9 @@ end describe '#docker_run_string' do + before do + subject.links = [] + end context 'when the service specifies exposed ports' do it 'generates a docker command with --expose' do expect(subject.send(:docker_run_string)).to include '--expose 3306' @@ -243,18 +246,17 @@ context 'when the service specifies docker links' do it 'translates docker links to environment variables' do - subject.links = [{ name: 'db', alias: 'db_1', protocol: 'tcp', port: 3306 }] + subject.links = [{ name: 'db', alias: 'db_1', exposed_ports: [{ protocol: 'tcp', containerPort: 3306, hostPort: 3306 }] }] expect(subject.send(:docker_run_string)).to include '-e DB_1_SERVICE_HOST=`/usr/bin/etcdctl get app/DB/DB_SERVICE_HOST`' - expect(subject.send(:docker_run_string)).to include '-e DB_1_SERVICE_PORT=`/usr/bin/etcdctl get app/DB/DB_SERVICE_PORT`' - expect(subject.send(:docker_run_string)).to include '-e DB_1_PORT=tcp://`/usr/bin/etcdctl get app/DB/DB_SERVICE_HOST`:`/usr/bin/etcdctl get app/DB/DB_SERVICE_PORT`' + expect(subject.send(:docker_run_string)).to include '-e DB_1_PORT=tcp://`/usr/bin/etcdctl get app/DB/DB_SERVICE_HOST`:3306' expect(subject.send(:docker_run_string)).to include '-e DB_1_PORT_3306_TCP_PROTO=tcp' expect(subject.send(:docker_run_string)).to include '-e DB_1_PORT_3306_TCP_ADDR=`/usr/bin/etcdctl get app/DB/DB_SERVICE_HOST`' - expect(subject.send(:docker_run_string)).to include '-e DB_1_PORT_3306_TCP=tcp://`/usr/bin/etcdctl get app/DB/DB_SERVICE_HOST`:`/usr/bin/etcdctl get app/DB/DB_SERVICE_PORT`' - expect(subject.send(:docker_run_string)).to include '-e DB_1_PORT_3306_TCP_PORT=`/usr/bin/etcdctl get app/DB/DB_SERVICE_PORT`' + expect(subject.send(:docker_run_string)).to include '-e DB_1_PORT_3306_TCP=tcp://`/usr/bin/etcdctl get app/DB/DB_SERVICE_HOST`:3306' + expect(subject.send(:docker_run_string)).to include '-e DB_1_PORT_3306_TCP_PORT=3306' end it 'sanitizes the link names when creating the env vars' do - subject.links = [{ name: 'db_@:.-', alias: 'db_1', protocol: 'tcp', port: 3306 }] + subject.links = [{ name: 'db_@:.-', alias: 'db_1', exposed_ports: [{ protocol: 'tcp', containerPort: 3306, hostPort: 3306 }] }] expect(subject.send(:docker_run_string)).to include '-e DB_1_SERVICE_HOST=`/usr/bin/etcdctl get app/DB-----/DB-----_SERVICE_HOST`' end end @@ -272,6 +274,9 @@ end describe '#service_def' do + before do + subject.links = [{ name: 'db', alias: 'db', exposed_ports: [{ hostPort: 1, containerPort: 2, protocol: 'tcp' }] }] + end context 'when the service has links' do it 'assigns dependencies to the unit_block' do expect(subject.send(:service_def)['Unit']['After']).to eq('db.service') diff --git a/spec/support/fixtures/post-services.json b/spec/support/fixtures/post-services.json index 55db0c2..012f255 100644 --- a/spec/support/fixtures/post-services.json +++ b/spec/support/fixtures/post-services.json @@ -16,7 +16,7 @@ "environment": [ {"variable": "MYSQL_ROOT_PASSWORD", "value": "pass@word01"} ], - "ports": [ {"hostPort": 3306, "containerPort": 3306} ], + "ports": [ {"hostPort": 1111, "containerPort": 3306} ], "deployment": { "count": 3 } } ]