Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Synchronize provider with server when zone changes #18272

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 22 additions & 8 deletions app/models/embedded_ansible_worker/runner.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
class EmbeddedAnsibleWorker::Runner < MiqWorker::Runner
attr_accessor :provider_server_last_synchronized
def prepare
ObjectSpace.garbage_collect
# Overriding prepare so we can set started when we're ready
Expand All @@ -22,6 +23,7 @@ def heartbeat
end

def do_work
synchronize_provider_with_server unless provider_in_sync_with_server?
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this new method name looks better... @carbonin what do you think?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the same problem is still there.

The name only mentions that we're checking the properties, not that we are also returning true based on some timeframe. It doesn't really matter though. I'm not going to hold this up for a better name though.

embedded_ansible.start if !embedded_ansible.alive? && !embedded_ansible.running?
provider.authentication_check if embedded_ansible.alive? && !provider.authentication_status_ok?
update_job_data_retention
Expand All @@ -38,14 +40,7 @@ def setup_ansible
end

def update_embedded_ansible_provider
server = MiqServer.my_server(true)

provider.name = "Embedded Ansible"
provider.zone = server.zone
provider.url = provider_url
provider.verify_ssl = 0

provider.save!
synchronize_provider_with_server

api_connection = embedded_ansible.api_connection
worker.remove_demo_data(api_connection)
Expand All @@ -57,6 +52,25 @@ def update_embedded_ansible_provider
provider.authentication_check
end

def synchronize_provider_with_server
server = MiqServer.my_server(true)

provider.name = "Embedded Ansible"
provider.zone = server.zone
provider.url = provider_url
provider.verify_ssl = 0

provider.save!
end

def provider_in_sync_with_server?
now = Time.now.utc
return true if @provider_server_last_synchronized.kind_of?(Time) && (@provider_server_last_synchronized + worker_settings[:sync_provider_with_server_interval]) >= now

@provider_server_last_synchronized = now
provider.reload.zone == server.reload.zone && provider.url == provider_url
end

# Base class methods we override since we don't have a separate process. We might want to make these opt-in features in the base class that this subclass can choose to opt-out.
def set_process_title; end
def set_database_application_name; end
Expand Down
1 change: 1 addition & 0 deletions config/settings.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1141,6 +1141,7 @@
:starting_timeout: 20.minutes
:poll: 10.seconds
:memory_threshold: 0.megabytes
:sync_provider_with_server_interval: 1.minute
:agent_coordinator_worker:
:heartbeat_timeout: 30.minutes
:poll: 30.seconds
Expand Down
40 changes: 39 additions & 1 deletion spec/models/embedded_ansible_worker/runner_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@
end

it "starts embedded ansible if it is not alive and not running" do
allow(runner).to receive(:provider_in_sync_with_server?).and_return(true)
allow(embedded_ansible_instance).to receive(:alive?).and_return(false)
allow(embedded_ansible_instance).to receive(:running?).and_return(false)

Expand All @@ -109,7 +110,7 @@
end

context "with a provider" do
let(:provider) { FactoryGirl.create(:provider_embedded_ansible, :with_authentication) }
let!(:provider) { FactoryGirl.create(:provider_embedded_ansible, :with_authentication) }
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was added because all examples were already calling provider and my new example wasn't, but still needed it created.


it "runs an authentication check if embedded ansible is alive and the credentials are not valid" do
auth = provider.authentications.first
Expand Down Expand Up @@ -139,6 +140,43 @@
expect(embedded_ansible_instance).to receive(:set_job_data_retention)
runner.do_work
end

it "updates provider zone if appliance zone changed" do
allow(embedded_ansible_instance).to receive(:alive?).and_return(true)
miq_server.update(:zone => FactoryGirl.create(:zone))

runner.do_work
expect(provider.reload.zone).to eq(miq_server.zone)
end

it "updates provider URL if appliance hostname changes" do
allow(embedded_ansible_instance).to receive(:alive?).and_return(true)
miq_server.update(:hostname => "example42.com")

runner.do_work
expect(provider.reload.url).to include("example42.com")
end

it "provider zone change is delayed 1 minute after appliance's zone changes" do
allow(embedded_ansible_instance).to receive(:alive?).and_return(true)
runner.sync_worker_settings

# provider zone is checked
runner.do_work

original_zone = miq_server.zone
miq_server.update(:zone => FactoryGirl.create(:zone))

# zone was just checked, doesn't do anything yet
runner.do_work
expect(provider.reload.zone).to eq(original_zone)

# wait at least 1 minute and try again, it now matches the server zone
Timecop.travel(65.seconds) do
runner.do_work
expect(provider.reload.zone).to eq(miq_server.zone)
end
end
end
end

Expand Down