# HG changeset patch # User John Cai <jcai@gitlab.com> # Date 1633705074 14400 # Fri Oct 08 10:57:54 2021 -0400 # Node ID 9568b5c999bc3bc1a810f53a72ce6268c925d8cf # Parent 6c560bbc963af081786dfa9f7f10d2f99348fe9c add list-untracked-repositories command Add a praefect command called list-untracked-repositories that will call the praefect binary. This command lists all repositories that exist on disk on gitaly nodes but praefect knows nothing about. Changelog: added diff --git a/files/gitlab-ctl-commands/lib/praefect.rb b/files/gitlab-ctl-commands/lib/praefect.rb --- a/files/gitlab-ctl-commands/lib/praefect.rb +++ b/files/gitlab-ctl-commands/lib/praefect.rb @@ -9,8 +9,9 @@ gitlab-ctl praefect command [options] COMMANDS: - remove-repository Remove repository from Gitaly cluster - track-repository Tells Gitaly cluster to track a repository + remove-repository Remove repository from Gitaly cluster + track-repository Tells Gitaly cluster to track a repository + list-untracked-repositories Lists repositories that exist on disk but are untracked by Praefect EOS def self.parse_options!(args) @@ -31,16 +32,24 @@ opts.banner = "Usage: gitlab-ctl praefect remove-repository [options]" parse_common_options!(options, opts) + parse_repository_options!(options, opts) end, 'track-repository' => OptionParser.new do |opts| opts.banner = "Usage: gitlab-ctl praefect track-repository [options]" parse_common_options!(options, opts) + parse_repository_options!(options, opts) opts.on('--authoritative-storage STORAGE-NAME', 'The storage to use as the primary for this repository (optional)') do |authoritative_storage| options[:authoritative_storage] = authoritative_storage end + end, + + 'list-untracked-repositories' => OptionParser.new do |opts| + opts.banner = "Usage: gitlab-ctl praefect untracked-repositories [options]" + + parse_common_options!(options, opts) end } @@ -48,6 +57,7 @@ command = args.shift + # common arguments raise OptionParser::ParseError, "Praefect command is not specified." \ if command.nil? || command.empty? @@ -56,12 +66,14 @@ commands[command].parse!(args) - # common arguments - raise OptionParser::ParseError, "Option --virtual-storage-name must be specified" \ - unless options.key?(:virtual_storage_name) + # repository arguments + if ['remove-repository', 'track-repository'].include?(command) + raise OptionParser::ParseError, "Option --virtual-storage-name must be specified" \ + unless options.key?(:virtual_storage_name) - raise OptionParser::ParseError, "Option --repository-relative-path must be specified" \ - unless options.key?(:repository_relative_path) + raise OptionParser::ParseError, "Option --repository-relative-path must be specified" \ + unless options.key?(:repository_relative_path) + end options[:command] = command options @@ -76,7 +88,9 @@ option_parser.on('--dir DIR', 'Directory in which Praefect is installed') do |dir| options[:dir] = dir end + end + def self.parse_repository_options!(options, option_parser) option_parser.on('--virtual-storage-name NAME', 'Name of the virtual storage where the repository resides (mandatory)') do |virtual_storage_name| options[:virtual_storage_name] = virtual_storage_name end @@ -97,8 +111,12 @@ # common arguments command = [EXEC_PATH, "-config", config_file_path, options[:command]] - command += ["-virtual-storage", options[:virtual_storage_name]] - command += ["-repository", options[:repository_relative_path]] + + # repository arguments + if ['remove-repository', 'track-repository'].include?(options[:command]) + command += ["-virtual-storage", options[:virtual_storage_name]] + command += ["-repository", options[:repository_relative_path]] + end # command specific arguments command += ["-authoritative-storage", options[:authoritative_storage]] if options[:command] == 'track-repository' && options.key?(:authoritative_storage) diff --git a/spec/gitlab-ctl-commands/lib/praefect_spec.rb b/spec/gitlab-ctl-commands/lib/praefect_spec.rb --- a/spec/gitlab-ctl-commands/lib/praefect_spec.rb +++ b/spec/gitlab-ctl-commands/lib/praefect_spec.rb @@ -9,6 +9,12 @@ allow(Kernel).to receive(:exit) { |code| raise "Kernel.exit(#{code})" } end + shared_examples 'unknown option is specified' do + it 'throws an error' do + expect { Praefect.parse_options!(%W(praefect #{command} --unknown)) }.to raise_error(OptionParser::InvalidOption, /unknown/) + end + end + it 'throws an error when command is not specified' do expect { Praefect.parse_options!(%w(praefect)) }.to raise_error(OptionParser::ParseError, /Praefect command is not specified/) end @@ -17,49 +23,48 @@ expect { Praefect.parse_options!(%w(praefect unknown-command)) }.to raise_error(OptionParser::ParseError, /Unknown Praefect command: unknown-command/) end - shared_examples 'parses common required options' do - it 'throws an error when unknown option is specified' do - expect { Praefect.parse_options!(%W(praefect #{command} --unknown)) }.to raise_error(OptionParser::InvalidOption, /unknown/) - end - + shared_examples 'parses repository options' do it 'throws an error when an argument for --virtual-storage-name is not specified' do - expect { Praefect.parse_options!(%W(praefect #{command} --virtual-storage-name)) }.to raise_error(OptionParser::MissingArgument, /virtual-storage-name/) + expect { Praefect.parse_options!(%W(praefect #{command} --dir dir --virtual-storage-name)) }.to raise_error(OptionParser::MissingArgument, /virtual-storage-name/) end it 'throws an error when --virtual-storage-name is not specified' do - expect { Praefect.parse_options!(%W(praefect #{command})) }.to raise_error(OptionParser::ParseError, /Option --virtual-storage-name must be specified/) + expect { Praefect.parse_options!(%W(praefect #{command} --dir dir)) }.to raise_error(OptionParser::ParseError, /Option --virtual-storage-name must be specified/) end it 'throws an error when an argument for --repository-relative-path is not specified' do - expect { Praefect.parse_options!(%W(praefect #{command} --repository-relative-path)) }.to raise_error(OptionParser::MissingArgument, /repository-relative-path/) + expect { Praefect.parse_options!(%W(praefect #{command} --dir dir --repository-relative-path)) }.to raise_error(OptionParser::MissingArgument, /repository-relative-path/) end it 'throws an error when --repository-relative-path is not specified' do - expect { Praefect.parse_options!(%W(praefect #{command} --virtual-storage-name name)) }.to raise_error(OptionParser::ParseError, /Option --repository-relative-path must be specified/) + expect { Praefect.parse_options!(%W(praefect #{command} --dir dir --virtual-storage-name name)) }.to raise_error(OptionParser::ParseError, /Option --repository-relative-path must be specified/) end it 'successfully parses correct params' do - expected_options = { command: command, virtual_storage_name: 'name', repository_relative_path: 'path' } + expected_options = { command: command, dir: 'dir', virtual_storage_name: 'name', repository_relative_path: 'path' } - expect(Praefect.parse_options!(%W(praefect #{command} --virtual-storage-name name --repository-relative-path path))).to eq(expected_options) + expect(Praefect.parse_options!(%W(praefect #{command} --dir dir --virtual-storage-name name --repository-relative-path path))).to eq(expected_options) end end context 'when command is remove-repository' do let(:command) { 'remove-repository' } - it_behaves_like 'parses common required options' + it_behaves_like 'parses repository options' + it_behaves_like 'unknown option is specified' end context 'when command is track-repository' do let(:command) { 'track-repository' } - it_behaves_like 'parses common required options' + it_behaves_like 'parses repository options' + it_behaves_like 'unknown option is specified' it 'successfully parses authoritative-storage' do - expected_options = { command: command, virtual_storage_name: 'name', repository_relative_path: 'path', authoritative_storage: 'storage-1' } + expected_options = { command: command, dir: 'dir', virtual_storage_name: 'name', repository_relative_path: 'path', authoritative_storage: 'storage-1' } expect(Praefect.parse_options!(%W(praefect #{command} + --dir dir --virtual-storage-name name --repository-relative-path path --authoritative-storage storage-1))).to eq(expected_options) @@ -88,6 +93,8 @@ end describe '#execute' do + let(:repository_options) { { virtual_storage_name: 'storage-name', repository_relative_path: 'repository-path' } } + context 'when package is not installed correctly' do it 'aborts the execution if a path does not exit' do allow(File).to receive(:exists?).twice.and_return(false) @@ -103,7 +110,6 @@ expect(Kernel).not_to receive(:abort) args = [ Praefect::EXEC_PATH, '-config', 'dir/config.toml', command, - '-virtual-storage', 'storage-name', '-repository', 'repository-path' ] args += command_args @@ -112,8 +118,6 @@ expect(Kernel).not_to receive(:exit!) common_options = { - virtual_storage_name: 'storage-name', - repository_relative_path: 'repository-path', command: command, dir: 'dir' } @@ -123,26 +127,38 @@ end context 'remove-repository command' do - let(:command) { 'remove-repository' } + let(:command) { 'list-untracked-repositories' } let(:command_args) { [] } let(:command_options) { {} } it_behaves_like 'executes the command' end - context 'track-repository command' do - let(:command) { 'track-repository' } - let(:command_args) { [] } - let(:command_options) { {} } + context 'repository commands' do + let(:command_args) { ['-virtual-storage', 'storage-name', '-repository', 'repository-path'] } + let(:command_options) { repository_options } - it_behaves_like 'executes the command' - - context 'with authoritative-storage' do - let(:command_options) { { authoritative_storage: 'storage' } } - let(:command_args) { ['-authoritative-storage', 'storage'] } + context 'remove-repository command' do + let(:command) { 'remove-repository' } it_behaves_like 'executes the command' end + + context 'track-repository command' do + let(:command) { 'track-repository' } + let(:command_args) { ['-virtual-storage', 'storage-name', '-repository', 'repository-path'] } + let(:command_options) { repository_options } + + it_behaves_like 'executes the command' + + context 'with authoritative-storage' do + let(:command) { 'track-repository' } + let(:command_args) { ['-virtual-storage', 'storage-name', '-repository', 'repository-path', '-authoritative-storage', 'storage-1'] } + let(:command_options) { repository_options.merge(authoritative_storage: 'storage-1') } + + it_behaves_like 'executes the command' + end + end end end end