Skip to content
Snippets Groups Projects

Admission Controller: Initial implementation pass

9 unresolved threads

What does this MR do and why?

Admission Controller: Initial implementation pass in service of #132770

Adds: * Admission Controller integration * API endpoint handling response from Admission Controller * Build model interface for Admission Controller response * Requisite handling in the build and runner matchers

@ayufan @ajwalker from #132770

Related to #378322

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
12 Gitlab::AppLogger.info "#{self.class}: Cleaning preparing timed-out builds"
13
14 drop(
15 preparing_builds(BUILD_PREPRARING_OUTDATED_TIMEOUT.ago),
16 failure_reason: :stuck_or_timeout_failure
17 )
18
19 drop_stuck(
20 preparing_builds(BUILD_PREPRARING_STUCK_TIMEOUT.ago),
21 failure_reason: :stuck_or_timeout_failure
22 )
23 end
24
25 private
26
27 # rubocop: disable CodeReuse/ActiveRecord
  • 1 # frozen_string_literal: true
    2
    3 module Ci
    4 class BuildPrerequisiteWorker # rubocop:disable Scalability/IdempotentWorker
  • 1 # frozen_string_literal: true
    2
    3 module Ci
    4 class BuildPrerequisiteWorker # rubocop:disable Scalability/IdempotentWorker
    5 include ApplicationWorker
    6
    7 data_consistency :always # rubocop:disable SidekiqLoadBalancing/WorkerDataConsistency
  • added backend label

  • @johnwparent Hey :wave: Thank you for working on this :bow: If this MR is ready to be reviewed , feel free to Remove the "Draft: " from the MR title and assign it to the suggested reviewer from the roulette :slight_smile:

    Thank you :pray:

  • @johnwparent, it seems we're waiting on an action from you for approximately two weeks.

    This message was generated automatically. You're welcome to improve it.

  • 🤖 GitLab Bot 🤖 marked this merge request as ready

    marked this merge request as ready

  • requested review from @ayufan and @ajwalker

  • @ayufan @ajwalker, this Community contribution is ready for review.

    • Do you have capacity and domain expertise to review this? If not, find one or more reviewers and assign to them.
    • If you've reviewed it, add the workflowin dev label if these changes need more work before the next review.

    This message was generated automatically. You're welcome to improve it.

  • added idle label

  • removed idle label

  • John Parent added 7519 commits

    added 7519 commits

    • 679feb25...b5e58841 - 7515 commits from branch gitlab-org:master
    • ced27a49 - Admission Controller: Initial implementation pass
    • 88866a9d - Internationalization from gettext
    • 21f5a725 - Update feature category for FailureReason spec
    • fda6ef1d - Make cop happy

    Compare with previous version

  • Contributor
    6 Warnings
    :warning: f2517208: The commit subject must start with a capital letter. For more information, take a look at our Commit message guidelines.
    :warning: 9691440a: The commit subject must start with a capital letter. For more information, take a look at our Commit message guidelines.
    :warning: 74882e30: The commit subject must contain at least 3 words. For more information, take a look at our Commit message guidelines.
    :warning: 74882e30: The commit subject must start with a capital letter. For more information, take a look at our Commit message guidelines.
    :warning: There were no new or modified feature flag YAML files detected in this MR.

    If the changes here are already controlled under an existing feature flag, please add
    the feature flagexists. Otherwise, if you think the changes here don't need
    to be under a feature flag, please add the label feature flagskipped, and
    add a short comment about why we skipped the feature flag.

    For guidance on when to use a feature flag, please see the documentation.

    :warning: This merge request does not refer to an existing milestone.
    1 Message
    :book: CHANGELOG missing:

    If this merge request needs a changelog entry, add the Changelog trailer to the commit message you want to add to the changelog.

    If this merge request doesn't need a CHANGELOG entry, feel free to ignore this message.

    Reviewer roulette

    Changes that require review have been detected!

    Please refer to the table below for assigning reviewers and maintainers suggested by Danger in the specified category:

    Category Reviewer Maintainer
    backend @syasonik profile link current availability (UTC-4) @ahuntsman profile link current availability (UTC-5)
    groupimport and integrate (backend) @georgekoltsov profile link current availability (UTC+1) Maintainer review is optional for groupimport and integrate (backend)
    ~"Verify" Reviewer review is optional for ~"Verify" @pedropombeiro profile link current availability (UTC+2)

    Please check reviewer's status!

    • available Reviewer is available!
    • unavailable Reviewer is unavailable!

    Feel free to override these selections if you think someone else would be better-suited or use the GitLab Review Workload Dashboard to find other available reviewers.

    To read more on how to use the reviewer roulette, please take a look at the Engineering workflow and code review guidelines. Please consider assigning a reviewer or maintainer who is a domain expert in the area of the merge request.

    Once you've decided who will review this merge request, assign them as a reviewer! Danger does not automatically notify them for you.

    Sidekiq queue changes

    This merge request contains changes to Sidekiq queues. Please follow the documentation on changing a queue's urgency.

    These queues were added:

    • pipeline_processing:ci_build_prerequisite

    If needed, you can retry the :repeat: danger-review job that generated this comment.

    Generated by :no_entry_sign: Danger

  • added idle label

  • removed idle label

  • Patrick Rice mentioned in merge request !135151

    mentioned in merge request !135151

  • 29 requires :admission, type: String, values: [:accepted, :rejected], desc: 'Job admission status'
    30 optional :reason, type: String, desc: 'Reason for admission status'
    31 optional :accepted, type: Array, desc: 'Admission acceptance requirements' do
    32 optional :tags, type: Array, desc: 'Tag modifications for job selection on runners' do
    33 optional :additions, type: Array, desc: 'Tags to be added to current job tag list'
    34 optional :removals, type: Array, desc: 'Tags to be removed from current job tag list'
    35 at_least_one_of :additions, :removals
    36 end
    37 optional :runners, type: Array, desc: 'Runner ids over which to scope runner selection of job' do
    38 optional :allowed_ids, type: Array, desc: 'The only runner ids for which a job is able to match'
    39 optional :denied_ids, type: Array, desc: 'Runner ids for which the job cannot match'
    40 at_least_one_of :allowed_ids, :denied_ids
    41 end
    42 end
    43 end
    44 post do
  • 9 9 tag_list
    10 10 build_ids
    11 11 project
    12 runner_allowlist
  • 488 def admission_controller_enabled_present?
    489 # TODO (john.parent): bool below should be check for
    490 # configured admission controller webhook when that
    491 # change is introduced
    492 Feature.enabled?(:admission_controller_hook) && false
    493 end
    494
    495 def admission_failed!(reason: :unknown_admission_failure)
    496 unless reason == :unknown_admission_failure
    497 update_admission_reason(reason, :rejection)
    498 end
    499 # TODO (john.parent@kitware.com): Find better exit code
    500 drop_with_exit_code!(:admission_rejected, 1)
    501 end
    502
    503 def admission_accepted!(params: :no_context, reason: :no_reason)
  • added idle label

  • Since some changes have been requested I'm moving this back to workflowin dev :slight_smile:

    @johnwparent feel free to ask for another review once ready with @gitlab-bot ready @ayufan :slight_smile:

  • removed idle label

  • John Parent mentioned in merge request !135150

    mentioned in merge request !135150

  • John Parent added 3 commits

    added 3 commits

    Compare with previous version

  • @ayufan @ajwalker, this Community contribution is ready for review.

    • Do you have capacity and domain expertise to review this? If not, find one or more reviewers and assign to them.
    • If you've reviewed it, add the workflowin dev label if these changes need more work before the next review.

    This message was generated automatically. You're welcome to improve it.

  • @ayufan, @ajwalker, this Community contribution was recently assigned to you for review.

    • Do you still have capacity to review this? We are mindful of your time, so if you are not able to take this on, please re-assign to one or more other reviewers.
    • Add the workflowin dev label if the merge request needs action from the author.

    This message was generated automatically. You're welcome to improve it.

  • Lee Tickett requested review from @ayufan

    requested review from @ayufan

  • Darren Eastman mentioned in merge request !153643 (merged)

    mentioned in merge request !153643 (merged)

  • added idle label

  • Hi @ajwalker @ayufan :wave: Just a friendly reminder to make sure this is on your tracks :slight_smile:

  • removed idle label

  • added idle label

  • Security policy violations have been resolved.

    Edited by GitLab Security Bot
  • removed idle label

  • requested review from @panoskanell

  • Panos Kanellidis removed review request for @panoskanell

    removed review request for @panoskanell

  • Kamil Trzciński requested review from @tianwenchen and removed review request for @ayufan and @ajwalker

    requested review from @tianwenchen and removed review request for @ayufan and @ajwalker

  • The MR !135151 is a smaller one which has been actively working on. I guess I will assign myself to this MR once !135151 is merged.

  • Tianwen Chen removed review request for @tianwenchen

    removed review request for @tianwenchen

  • Hi @tianwenchen :wave:

    We noticed this MR is marked as workflowready for review but no reviewer is assigned. workflowin dev has automatically been applied to this MR based on the likelihood the review is finished. If additional reviews are still required, please assign a reviewer and reapply workflowready for review.

    @johnwparent you may also request a review by commenting @gitlab-bot ready. You can also assign reviewers directly using @gitlab-bot ready @user1 @user2 if you know the relevant reviewer(s), such as those who were involved in a related issue.

    This message was generated automatically. You're welcome to improve it.

  • added idle label

  • @johnwparent, it seems we're waiting on an action from you for approximately two weeks.

    This message was generated automatically. You're welcome to improve it.

  • 🤖 GitLab Bot 🤖 added stale label and removed idle label

    added stale label and removed idle label

  • Hi @nicolewilliams! :wave:

    To provide a better contribution experience, we're identifying older merge requests with no human activity in the past 120 days. Our goal is to bring these merge requests to completion or close them, enabling us to spend more time on active merge requests.

    Review this merge request and determine if you should:

    • Work on the provided feedback. Add @gitlab-bot ready when you need a review or are looking for more feedback.
    • Don't know how to proceed? Ask for help from a merge request coach by adding @gitlab-bot help in a comment.
    • Close the merge request.

    Please see the handbook for additional details on this

    (Improve this message?)

  • Please register or sign in to reply
    Loading