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

Feature proposal: Uploading master.key #231

Closed
brendon opened this issue Aug 19, 2019 · 7 comments · Fixed by #232
Closed

Feature proposal: Uploading master.key #231

brendon opened this issue Aug 19, 2019 · 7 comments · Fixed by #232

Comments

@brendon
Copy link
Contributor

brendon commented Aug 19, 2019

I currently have the following in my deploy configuration:

append :linked_files, "config/master.key"

namespace :deploy do
  namespace :check do
    before :linked_files, :set_master_key do
      on roles(:app), in: :sequence, wait: 10 do
        unless test("[ -f #{shared_path}/config/master.key ]")
          upload! 'config/master.key', "#{shared_path}/config/master.key"
        end
      end
    end
  end
end

I was wondering if you'd be interested in integrating this into the capistrano-rails gem? (obviously with more options around whether this should happen or not per if the file actually exists locally.)

@mattbrictson
Copy link
Member

Hi @brendon , thanks for asking. I don't use the Rails encrypted secrets feature, so I am not comfortable making this a built in part of capistrano-rails. However if you would like to submit a PR to update the Recommendations section of the README, I will gladly accept it.

@brendon
Copy link
Contributor Author

brendon commented Aug 21, 2019

Thanks @mattbrictson. I've done up the PR :)

@gurgeous
Copy link
Contributor

yes but why is there a wait in there

@brendon
Copy link
Contributor Author

brendon commented Jun 21, 2023

Isn't that just a max-timeout?

@gurgeous
Copy link
Contributor

My comment was largely for humor given that the PR is ancient. Impressed that you are swinging back around to participate in my snark. :) From old release notes:

https://github.com/capistrano/capistrano/blob/master/docs/_posts/2013-06-01-release-announcement.markdown

on :all, in: :sequence, wait: 15 do
  # This takes all servers, in sequence and waits 15 seconds between
  # each server, this might be perfect if you are afraid about
  # overloading a shared resource, or want to defer the asset compilation
  # over your cluster owing to worries about load
end

This matches the behavior we are seeing. Looks like this is a feature of sshkit when running things in sequence:

https://github.com/capistrano/sshkit/blob/master/lib/sshkit/runners/sequential.rb

      def initialize(hosts, options = nil, &block)
        super(hosts, options, &block)
        @wait_interval = @options[:wait] || 2
      end

      def execute
        last_host = hosts.pop

        hosts.each do |host|
          run_backend(host, &block)
          sleep wait_interval
        end
        ...

Think of the countless seconds spent waiting for keys to slowly deploy!

mattbrictson pushed a commit that referenced this issue Jun 21, 2023
This will save gajillions of seconds for capistrano enthusiasts everywere
@brendon
Copy link
Contributor Author

brendon commented Jun 21, 2023

Thanks @gurgeous, I bet the :wait got picked up from some other rule at some point and I managed to propagate it to here. In fact I think it came from this:

  task :stop do
    on roles(:app), in: :sequence, wait: 10 do
      execute :touch, release_path.join('tmp/stop.txt')
      execute :sudo, 'service delayedjob stop'
      execute :sudo, 'service faye stop'
    end
  end

Still doesn't make much sense and I suspect that whoever I got the code off of originally added the :wait for the same mistaken reason. It certainly doesn't wait 10 seconds between each of those execute's.

Thanks for the heads up! :D

I should add, I only have one server so that explains why I never noticed the potential for problems there :)

@gurgeous
Copy link
Contributor

Ha! Yeah, we only noticed when we switched from one box to four a few days ago :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants