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.

Sign up for GitHub

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

Maintenance mode: check if domain is updated via API #229

Closed
wants to merge 1 commit into from
Closed

Maintenance mode: check if domain is updated via API #229

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 6 additions & 2 deletions lib/core/maintenance_mode.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,13 @@ def start_or_stop_maintenance_workload(action)
def switch_domain_workload(to:)
step("Switching workload for domain '#{domain_data['name']}' to '#{to}'") do
cp.set_domain_workload(domain_data, to)
end

step("Waiting for changes to take effect", retry_on_failure: true, wait: 10, max_retry_count: 3) do
Copy link
Member

Choose a reason for hiding this comment

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

@borela should we be using https://github.com/shakacode/uber_task?

@zzaakiirr is this ready to merge?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@justin808 I left a question for @rafaelgomesxyz, I want to get his opinion on approach I took in this PR for checking if domain was updated.

If this is the right way and Rafael approves this PR then we can merge this

refetched_domain_data = cp.fetch_domain(domain_data["name"])
raise "Can't find domain" if refetched_domain_data.nil?

# Give it a bit of time for the domain to update
Kernel.sleep(30)
cp.domain_workload_matches?(refetched_domain_data, to)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rafaelgomesxyz Is this check enough to be sure that domain was updated?

Copy link
Collaborator

Choose a reason for hiding this comment

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

The arbitrary sleep we had before wasn't really to check if the domain value was updated in the workload (after all, this should be true immediately when it first runs, no?).

The sleep was for waiting for DNS changes to take effect, which can sometimes take a bit. So we'd need a different method for checking it (we'd essentially need to check if the domain is working for the switched workload).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So we'd need a different method for checking it

Do you have any ideas how can we check this? Maybe controlplane displays some message while workload change takes effect?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@zzaakiirr A way I can think of is to make a real request to the domain and see if it's displaying the regular page or the maintenance page.

I'm not sure if there's a better way to do it.

end

progress.puts
Expand Down
Loading