Skip to content

Commit

Permalink
Merge pull request #19712 from carbonin/scope_deployments_by_server
Browse files Browse the repository at this point in the history
Differentiate deployment names by server
  • Loading branch information
Fryguy authored Jan 15, 2020
2 parents 9a7ab40 + 1f5e6b0 commit 3d6f538
Show file tree
Hide file tree
Showing 6 changed files with 39 additions and 16 deletions.
6 changes: 5 additions & 1 deletion app/models/miq_worker/container_common.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,15 @@ def container_image_tag
"latest"
end

def deployment_prefix
"#{MiqServer.my_server.compressed_id}-"
end

def worker_deployment_name
@worker_deployment_name ||= begin
deployment_name = abbreviated_class_name.dup.chomp("Worker").sub("Manager", "").sub(/^Miq/, "")
deployment_name << "-#{Array(ems_id).map { |id| ApplicationRecord.split_id(id).last }.join("-")}" if respond_to?(:ems_id)
deployment_name.underscore.dasherize.tr("/", "-")
"#{deployment_prefix}#{deployment_name.underscore.dasherize.tr("/", "-")}"
end
end
end
Expand Down
16 changes: 13 additions & 3 deletions app/models/miq_worker/service_worker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,13 @@ module ServiceWorker
def create_container_objects
orchestrator = ContainerOrchestrator.new

orchestrator.create_service(worker_deployment_name, SERVICE_PORT)
orchestrator.create_service(service_name, service_label, SERVICE_PORT)
orchestrator.create_deployment(worker_deployment_name) do |definition|
configure_worker_deployment(definition)

definition[:spec][:serviceName] = worker_deployment_name
definition[:spec][:serviceName] = service_name
definition[:spec][:template][:metadata][:labels].merge!(service_label)

container = definition[:spec][:template][:spec][:containers].first
container[:ports] = [{:containerPort => SERVICE_PORT}]
container[:env] << {:name => "PORT", :value => container_port.to_s}
Expand All @@ -25,7 +27,7 @@ def create_container_objects
def delete_container_objects
orch = ContainerOrchestrator.new
orch.delete_deployment(worker_deployment_name)
orch.delete_service(worker_deployment_name)
orch.delete_service(service_name)
end

def stop_container
Expand All @@ -40,6 +42,14 @@ def add_readiness_probe(container_definition)
}
end

def service_label
{:service => service_name}
end

def service_name
worker_deployment_name.delete_prefix(deployment_prefix)
end

# Can be overriden by including classes
def container_port
SERVICE_PORT
Expand Down
4 changes: 2 additions & 2 deletions lib/container_orchestrator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ def create_deployment(name)
raise unless e.message =~ /already exists/
end

def create_service(name, port)
definition = service_definition(name, port)
def create_service(name, selector, port)
definition = service_definition(name, selector, port)
yield(definition) if block_given?
kube_connection.create_service(definition)
rescue KubeException => e
Expand Down
4 changes: 2 additions & 2 deletions lib/container_orchestrator/object_definition.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,15 @@ def deployment_definition(name)
}
end

def service_definition(name, port)
def service_definition(name, selector, port)
{
:metadata => {
:name => name,
:labels => {:app => app_name},
:namespace => my_namespace
},
:spec => {
:selector => {:name => name},
:selector => selector,
:ports => [{
:name => "#{name}-#{port}",
:port => port,
Expand Down
6 changes: 4 additions & 2 deletions spec/lib/container_orchestrator_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -106,9 +106,11 @@
ports = definition[:spec][:ports]
expect(ports.first).to eq(:name => "http-80", :port => 80, :targetPort => 80)
expect(ports.last).to eq(:name => "https", :port => 443, :targetPort => 5000)

expect(definition[:spec][:selector][:service]).to eq("http")
end

subject.create_service("http", 80) do |spec|
subject.create_service("http", {:service => "http"}, 80) do |spec|
spec[:spec][:ports] << {:name => "https", :port => 443, :targetPort => 5000}
end
end
Expand All @@ -117,7 +119,7 @@
error = KubeException.new(500, "service already exists", "")
expect(kube_connection_stub).to receive(:create_service).and_raise(error)

expect { subject.create_service("http", 80) }.not_to raise_error
expect { subject.create_service("http", {:service => "http"}, 80) }.not_to raise_error
end
end

Expand Down
19 changes: 13 additions & 6 deletions spec/models/miq_worker/container_common_spec.rb
Original file line number Diff line number Diff line change
@@ -1,11 +1,18 @@
describe MiqWorker::ContainerCommon do
before { EvmSpecHelper.local_miq_server }
let(:compressed_server_id) { MiqServer.my_server.compressed_id }

def deployment_name_for(name)
"#{compressed_server_id}-#{name}"
end

describe "#worker_deployment_name" do
let(:test_cases) do
[
{:subject => MiqGenericWorker.new, :name => "generic"},
{:subject => MiqUiWorker.new, :name => "ui"},
{:subject => ManageIQ::Providers::Openshift::ContainerManager::EventCatcher.new(:queue_name => "ems_2"), :name => "openshift-container-event-catcher-2"},
{:subject => ManageIQ::Providers::Redhat::NetworkManager::MetricsCollectorWorker.new, :name => "redhat-network-metrics-collector"}
{:subject => MiqGenericWorker.new, :name => deployment_name_for("generic")},
{:subject => MiqUiWorker.new, :name => deployment_name_for("ui")},
{:subject => ManageIQ::Providers::Openshift::ContainerManager::EventCatcher.new(:queue_name => "ems_2"), :name => deployment_name_for("openshift-container-event-catcher-2")},
{:subject => ManageIQ::Providers::Redhat::NetworkManager::MetricsCollectorWorker.new, :name => deployment_name_for("redhat-network-metrics-collector")}
]
end

Expand Down Expand Up @@ -33,14 +40,14 @@
it "scales the deployment to the number of configured workers" do
allow(MiqGenericWorker).to receive(:worker_settings).and_return(:count => 2)

expect(orchestrator).to receive(:scale).with("generic", 2)
expect(orchestrator).to receive(:scale).with(deployment_name_for("generic"), 2)
MiqGenericWorker.new.scale_deployment
end

it "deletes the container objects if the worker count is zero" do
allow(MiqGenericWorker).to receive(:worker_settings).and_return(:count => 0)

expect(orchestrator).to receive(:scale).with("generic", 0)
expect(orchestrator).to receive(:scale).with(deployment_name_for("generic"), 0)
worker = MiqGenericWorker.new
expect(worker).to receive(:delete_container_objects)
worker.scale_deployment
Expand Down

0 comments on commit 3d6f538

Please sign in to comment.