Add latest changes from gitlab-org/gitlab@master

app/workers/container_registry/migration/enqueuer_worker.rb

@@ -6,7 +6,7 @@ module ContainerRegistry
       include ApplicationWorker
       include CronjobQueue # rubocop:disable Scalability/CronWorkerContext
       include Gitlab::Utils::StrongMemoize
-      include Gitlab::ExclusiveLeaseHelpers
+      include ExclusiveLeaseGuard
 
       DEFAULT_LEASE_TIMEOUT = 30.minutes.to_i.freeze
 
@@ -18,15 +18,11 @@ module ContainerRegistry
 
       def perform
         re_enqueue = false
+        try_obtain_lease do
+          break unless runnable?
 
-        took_lease = with_a_lease_on_repository do
+          re_enqueue = handle_aborted_migration || handle_next_migration
-          if runnable?
-            re_enqueue = handle_aborted_migration || handle_next_migration
-          end
         end
-
-        log_extra_metadata_on_done(:lease_already_taken, true) unless took_lease
-
         re_enqueue_if_capacity if re_enqueue
       end
 
@@ -37,20 +33,6 @@ module ContainerRegistry
 
       private
 
-      def with_a_lease_on_repository
-        repository = next_aborted_repository || next_repository
-
-        if repository
-          in_lock(lease_key_for(repository), retries: 0, ttl: DEFAULT_LEASE_TIMEOUT) { yield }
-        else
-          log_extra_metadata_on_done(:no_container_repository_found, true)
-        end
-
-        true
-      rescue FailedToObtainLockError
-        false
-      end
-
       def handle_aborted_migration
         return unless next_aborted_repository
 
@@ -183,8 +165,14 @@ module ContainerRegistry
         log_extra_metadata_on_done(:container_repository_migration_state, repository.migration_state)
       end
 
-      def lease_key_for(repository)
+      # used by ExclusiveLeaseGuard
-        "container_registry:migration:enqueuer_worker:for:#{repository.id}"
+      def lease_key
+        'container_registry:migration:enqueuer_worker'
+      end
+
+      # used by ExclusiveLeaseGuard
+      def lease_timeout
+        DEFAULT_LEASE_TIMEOUT
       end
     end
   end

doc/api/features.md

@@ -129,11 +129,12 @@ POST /features/:name
 | `feature_group` | string | no | A Feature group name |
 | `user` | string | no | A GitLab username |
 | `group` | string | no | A GitLab group's path, for example `gitlab-org` |
+| `namespace` | string | no | A GitLab group or user namespace's path, for example `gitlab-org` or username path |
 | `project` | string | no | A projects path, for example `gitlab-org/gitlab-foss` |
 | `force` | boolean | no | Skip feature flag validation checks, such as a YAML definition |
 
 You can enable or disable a feature for a `feature_group`, a `user`,
-a `group`, and a `project` in a single API call.
+a `group`, a `namespace` and a `project` in a single API call.
 
 ```shell
 curl --data "value=30" --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/api/v4/features/new_library"

doc/development/code_review.md

@@ -219,7 +219,9 @@ should be confident that:
 
 The best way to do this, and to avoid unnecessary back-and-forth with reviewers,
 is to perform a self-review of your own merge request, following the
-[Code Review](#reviewing-a-merge-request) guidelines.
+[Code Review](#reviewing-a-merge-request) guidelines. During this self-review,
+try to include comments in the MR on lines
+where decisions or trade-offs were made, or where a contextual explanation might aid the reviewer in more easily understanding the code.
 
 To reach the required level of confidence in their solution, an author is expected
 to involve other people in the investigation and implementation processes as

doc/user/group/roadmap/index.md

@@ -1,5 +1,4 @@
 ---
-type: reference
 stage: Plan
 group: Product Planning
 info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments
@@ -7,7 +6,6 @@ info: To determine the technical writer assigned to the Stage/Group associated w
 
 # Roadmap **(PREMIUM)**
 
-> - Introduced in GitLab 10.5.
 > - [Moved](https://gitlab.com/gitlab-org/gitlab/-/issues/198062) from GitLab Ultimate to GitLab Premium in 12.9.
 > - In [GitLab 12.9](https://gitlab.com/gitlab-org/gitlab/-/issues/5164) and later, the epic bars show epics' title, progress, and completed weight percentage.
 > - Milestones appear in roadmaps in [GitLab 12.10](https://gitlab.com/gitlab-org/gitlab/-/issues/6802), and later.
@@ -27,10 +25,10 @@ You can expand epics that contain child epics to show their child epics in the r
 You can select the chevron (**{chevron-down}**) next to the epic title to expand and collapse the
 child epics.
 
-On top of the milestone bars, you can see their title.
+On top of the milestone bars, you can see their title. When you point to a
-When you hover over a milestone bar or title, a popover appears with its title, start date, and due
+milestone bar or title, a popover appears with its title, start date, and due
-date. You can also select the chevron (**{chevron-down}**) next to the **Milestones** heading to
+date. You can also select the chevron (**{chevron-down}**) next to the **Milestones**
-toggle the list of the milestone bars.
+heading to toggle the list of the milestone bars.
 
 ![roadmap view](img/roadmap_view_v14_3.png)
 
@@ -41,8 +39,8 @@ toggle the list of the milestone bars.
 > - Filtering by epic [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/218623) in GitLab 13.11.
 > - Filtering by milestone [feature flag removed](https://gitlab.com/gitlab-org/gitlab/-/issues/323917) in GitLab 14.5.
 
-WARNING:
+NOTE:
-Filtering roadmaps by milestone might not be available to you. Check the **version history** note above for details.
+Filtering roadmaps by milestone might not be available to you. Be sure to review this section's version history for details.
 
 When you want to explore a roadmap, there are several ways to make it easier by sorting epics or
 filtering them by what's important for you.
@@ -52,8 +50,9 @@ You can sort epics in the Roadmap view by:
 - Start date
 - Due date
 
-Each option contains a button that toggles the sort order between **ascending** and **descending**.
+Each option contains a button that toggles the sort order between **ascending**
-The sort option and order persist when browsing Epics, including the [epics list view](../epics/index.md).
+and **descending**. The sort option and order persist when browsing Epics, including
+the [epics list view](../epics/index.md).
 
 You can also filter epics in the Roadmap view by the epics':
 
@@ -66,7 +65,7 @@ You can also filter epics in the Roadmap view by the epics':
 
 ![roadmap date range in weeks](img/roadmap_filters_v13_11.png)
 
-Roadmaps can also be [visualized inside an epic](../epics/index.md#roadmap-in-epics).
+You can also [visualize roadmaps inside of an epic](../epics/index.md#roadmap-in-epics).
 
 ### Roadmap settings
 
@@ -78,38 +77,40 @@ When you enable the roadmap settings sidebar, you can use it to refine epics sho
 You can configure the following:
 
 - Select date range.
-- Turn milestones on or off and select whether to show all, group, subgroup, or project milestones.
+- Turn milestones on or off, and select whether to show all, group, subgroup, or
+  project milestones.
 - Show all, open, or closed epics.
 - Turn progress tracking for child issues on or off and select whether
   to use issue weights or counts.
 
-  The progress tracking setting is not saved in user preferences but is saved or shared using URL parameters.
+The progress tracking setting isn't saved in user preferences, but is saved or
+shared using URL parameters.
 
 ## Timeline duration
 
-> - Introduced in GitLab 11.0.
+> [Moved](https://gitlab.com/gitlab-org/gitlab/-/issues/198062) from GitLab Ultimate to GitLab Premium in 12.9.
-> - [Moved](https://gitlab.com/gitlab-org/gitlab/-/issues/198062) from GitLab Ultimate to GitLab Premium in 12.9.
 
 ### Date range presets
 
-> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/204994) in GitLab 14.3. [Deployed behind the `roadmap_daterange_filter` flag](../../../administration/feature_flags.md), disabled by default.
+> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/204994) in GitLab 14.3 [with a flag](../../../administration/feature_flags.md) named `roadmap_daterange_filter`. Disabled by default.
 > - [Enabled on GitLab.com and self-managed](https://gitlab.com/gitlab-org/gitlab/-/issues/323917) in GitLab 14.3.
-> - [Feature flag `roadmap_daterange_filter` removed](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/72419) in GitLab 14.5.
+> - Generally available in GitLab 14.5. [Feature flag `roadmap_daterange_filter`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/72419) removed.
 
-Roadmap provides three date range options, each with predetermined timeline duration:
+Roadmap provides these date range options, each with a predetermined timeline duration:
 
-- **This quarter**: includes weeks present in current quarter.
+- **This quarter**: Includes the weeks present in the current quarter.
-- **This year**: includes weeks or months present in current year.
+- **This year**: Includes the weeks or months present in the current year.
-- **Within 3 years**: includes weeks, months, or quarters present in the previous 18 months and
+- **Within 3 years**: Includes the weeks, months, or quarters present both in
-  upcoming 18 months (that is, three years in total).
+  the previous 18 months and the upcoming 18 months (three years in total).
 
 ### Layout presets
 
-Depending on selected [date range preset](#date-range-presets), Roadmap supports the following layout presets:
+Depending on selected [date range preset](#date-range-presets), Roadmap supports
+these layout presets:
 
-- **Quarters**: only available when the "Within 3 years" date range is selected.
+- **Quarters**: Available only when the **Within 3 years** date range is selected.
-- **Months**: available when either "This year" or "Within 3 years" date range is selected.
+- **Months**: Available when either **This year** or **Within 3 years** date range is selected.
-- **Weeks** (default): available for all the date range presets.
+- **Weeks** (default): Available for all the date range presets.
 
 ### Quarters
 
@@ -125,12 +126,11 @@ the timeline header represent the month of the quarter.
 
 ![roadmap date range in months](img/roadmap_timeline_months.png)
 
-In the **Months** preset, roadmap shows epics and milestones which have start or due dates
+In the **Months** preset, roadmap shows epics and milestones which have start or
-**falling within** or
+due dates **falling within** or **going through** currently selected date range
-**going through** currently selected date range preset, where **today**
+preset, where **today** is shown by the vertical red line in the timeline. The
-is shown by the vertical red line in the timeline. The sub-headers underneath the month name on
+sub-headers underneath the month name on the timeline header represent the date
-the timeline header represent the date on starting day (Sunday) of the week. This preset is
+on the start day (Sunday) of the week. This preset is selected by default.
-selected by default.
 
 ### Weeks
 

lib/api/features.rb

@@ -70,12 +70,14 @@ module API
         optional :feature_group, type: String, desc: 'A Feature group name'
         optional :user, type: String, desc: 'A GitLab username'
         optional :group, type: String, desc: "A GitLab group's path, such as 'gitlab-org'"
+        optional :namespace, type: String, desc: "A GitLab group or user namespace path, such as 'gitlab-org'"
         optional :project, type: String, desc: 'A projects path, like gitlab-org/gitlab-ce'
         optional :force, type: Boolean, desc: 'Skip feature flag validation checks, ie. YAML definition'
 
         mutually_exclusive :key, :feature_group
         mutually_exclusive :key, :user
         mutually_exclusive :key, :group
+        mutually_exclusive :key, :namespace
         mutually_exclusive :key, :project
       end
       post ':name' do

metrics_server/metrics_server.rb

@@ -38,7 +38,8 @@ class MetricsServer # rubocop:disable Gitlab/NamespacedClass
     def spawn(target, metrics_dir:, **options)
       return spawn_ruby_server(target, metrics_dir: metrics_dir, **options) unless new_metrics_server?
 
-      settings = settings_for(target)
+      name = settings_key(target)
+      settings = ::Settings.monitoring[name]
       path = options[:path]&.then { |p| Pathname.new(p) } || Pathname.new('')
       cmd = path.join('gitlab-metrics-exporter').to_path
       env = {
@@ -48,6 +49,13 @@ class MetricsServer # rubocop:disable Gitlab/NamespacedClass
         'GME_SERVER_PORT' => settings['port'].to_s
       }
 
+      if settings['log_enabled']
+        env['GME_LOG_FILE'] = File.join(Rails.root, 'log', "#{name}.log")
+        env['GME_LOG_LEVEL'] = 'info'
+      else
+        env['GME_LOG_LEVEL'] = 'quiet'
+      end
+
       Process.spawn(env, cmd, err: $stderr, out: $stdout, pgroup: true).tap do |pid|
         Process.detach(pid)
       end
@@ -102,14 +110,12 @@ class MetricsServer # rubocop:disable Gitlab/NamespacedClass
       raise "Target must be one of [puma,sidekiq]" unless %w(puma sidekiq).include?(target)
     end
 
-    def settings_for(target)
+    def settings_key(target)
-      settings_key =
+      case target
-        case target
+      when 'puma' then 'web_exporter'
-        when 'puma' then 'web_exporter'
+      when 'sidekiq' then 'sidekiq_exporter'
-        when 'sidekiq' then 'sidekiq_exporter'
+      else ensure_valid_target!(target)
-        else ensure_valid_target!(target)
+      end
-        end
-      ::Settings.monitoring[settings_key]
     end
   end
 

spec/metrics_server/metrics_server_spec.rb

@@ -97,18 +97,38 @@ RSpec.describe MetricsServer do # rubocop:disable RSpec/FilePath
         end
 
         context 'for Golang server' do
+          let(:log_enabled) { false }
+          let(:settings) do
+            {
+              'web_exporter' => {
+                'enabled' => true,
+                'address' => 'localhost',
+                'port' => '8083',
+                'log_enabled' => log_enabled
+              },
+              'sidekiq_exporter' => {
+                'enabled' => true,
+                'address' => 'localhost',
+                'port' => '8082',
+                'log_enabled' => log_enabled
+              }
+            }
+          end
+
           let(:expected_port) { target == 'puma' ? '8083' : '8082' }
           let(:expected_env) do
             {
               'GME_MMAP_METRICS_DIR' => metrics_dir,
               'GME_PROBES' => 'self,mmap',
               'GME_SERVER_HOST' => 'localhost',
-              'GME_SERVER_PORT' => expected_port
+              'GME_SERVER_PORT' => expected_port,
+              'GME_LOG_LEVEL' => 'quiet'
             }
           end
 
           before do
             stub_env('GITLAB_GOLANG_METRICS_SERVER', '1')
+            allow(::Settings).to receive(:monitoring).and_return(settings)
           end
 
           it 'spawns a new server process and returns its PID' do
@@ -133,6 +153,24 @@ RSpec.describe MetricsServer do # rubocop:disable RSpec/FilePath
 
             described_class.spawn(target, metrics_dir: metrics_dir, path: '/path/to/gme/')
           end
+
+          context 'when logs are enabled' do
+            let(:log_enabled) { true }
+            let(:expected_log_file) { target == 'puma' ? 'web_exporter.log' : 'sidekiq_exporter.log' }
+
+            it 'sets log related environment variables' do
+              expect(Process).to receive(:spawn).with(
+                expected_env.merge(
+                  'GME_LOG_LEVEL' => 'info',
+                  'GME_LOG_FILE' => File.join(Rails.root, 'log', expected_log_file)
+                ),
+                'gitlab-metrics-exporter',
+                hash_including(pgroup: true)
+              ).and_return(99)
+
+              described_class.spawn(target, metrics_dir: metrics_dir)
+            end
+          end
         end
       end
     end

spec/requests/api/features_spec.rb

@@ -310,6 +310,55 @@ RSpec.describe API::Features, stub_feature_flags: false do
           'definition' => known_feature_flag_definition_hash
         )
       end
+
+      describe 'mutually exclusive parameters' do
+        shared_examples 'fails to set the feature flag' do
+          it 'returns an error' do
+            expect(response).to have_gitlab_http_status(:bad_request)
+            expect(json_response['error']).to match(/key, \w+ are mutually exclusive/)
+          end
+        end
+
+        context 'when key and feature_group are provided' do
+          before do
+            post api("/features/#{feature_name}", admin), params: { value: '0.01', key: 'percentage_of_actors', feature_group: 'some-value' }
+          end
+
+          it_behaves_like 'fails to set the feature flag'
+        end
+
+        context 'when key and user are provided' do
+          before do
+            post api("/features/#{feature_name}", admin), params: { value: '0.01', key: 'percentage_of_actors', user: 'some-user' }
+          end
+
+          it_behaves_like 'fails to set the feature flag'
+        end
+
+        context 'when key and group are provided' do
+          before do
+            post api("/features/#{feature_name}", admin), params: { value: '0.01', key: 'percentage_of_actors', group: 'somepath' }
+          end
+
+          it_behaves_like 'fails to set the feature flag'
+        end
+
+        context 'when key and namespace are provided' do
+          before do
+            post api("/features/#{feature_name}", admin), params: { value: '0.01', key: 'percentage_of_actors', namespace: 'somepath' }
+          end
+
+          it_behaves_like 'fails to set the feature flag'
+        end
+
+        context 'when key and project are provided' do
+          before do
+            post api("/features/#{feature_name}", admin), params: { value: '0.01', key: 'percentage_of_actors', project: 'somepath' }
+          end
+
+          it_behaves_like 'fails to set the feature flag'
+        end
+      end
     end
 
     context 'when the feature exists' do

spec/workers/container_registry/migration/enqueuer_worker_spec.rb

@@ -253,22 +253,12 @@ RSpec.describe ContainerRegistry::Migration::EnqueuerWorker, :aggregate_failures
     end
 
     context 'when no repository qualifies' do
-      before do
+      include_examples 'an idempotent worker' do
-        allow(ContainerRepository).to receive(:ready_for_import).and_return(ContainerRepository.none)
+        before do
-      end
+          allow(ContainerRepository).to receive(:ready_for_import).and_return(ContainerRepository.none)
-
-      context 'idempotency' do
-        include_examples 'an idempotent worker' do
-          it_behaves_like 'no action'
         end
-      end
 
-      context 'logging' do
+        it_behaves_like 'no action'
-        it 'logs no_container_repository_found' do
-          expect_log_extra_metadata(no_container_repository_found: true)
-
-          subject
-        end
       end
     end
 
@@ -334,7 +324,7 @@ RSpec.describe ContainerRegistry::Migration::EnqueuerWorker, :aggregate_failures
     end
 
     context 'with the exclusive lease taken' do
-      let(:lease_key) { "container_registry:migration:enqueuer_worker:for:#{container_repository.id}" }
+      let(:lease_key) { worker.send(:lease_key) }
 
       before do
         stub_exclusive_lease_taken(lease_key, timeout: 30.minutes)
@@ -343,23 +333,6 @@ RSpec.describe ContainerRegistry::Migration::EnqueuerWorker, :aggregate_failures
       it 'does not perform' do
         expect(worker).not_to receive(:runnable?)
         expect(worker).not_to receive(:re_enqueue_if_capacity)
-        expect_log_extra_metadata(lease_already_taken: true)
-
-        subject
-      end
-    end
-
-    context 'with exclusive lease taken for a different repository' do
-      let(:lease_key) { "container_registry:migration:enqueuer_worker:for:test" }
-
-      before do
-        Gitlab::ExclusiveLease.new(lease_key, timeout: 30.minutes).try_obtain
-      end
-
-      it 'does perform' do
-        expect(worker).to receive(:runnable?).and_return(true)
-        expect(worker).to receive(:handle_aborted_migration).and_return(container_repository)
-        expect(worker).to receive(:re_enqueue_if_capacity)
 
         subject
       end