Admission Controller: Initial implementation pass
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
Activity
mentioned in merge request !132770 (merged)
added typefeature label
Hey @johnwparent!
Thank you for your contribution to GitLab. Please refer to the contribution documentation for an overview of the process.
When you're ready for a first review, post
@gitlab-bot ready
. If you know a relevant reviewer(s) (for example, someone that was involved in a related issue), you can also assign them directly with@gitlab-bot ready @user1 @user2
.At any time, if you need help, feel free to post
@gitlab-bot help
or initiate a mentor session on Discord. Read more on how to get help.You can comment
@gitlab-bot label <label1> <label2>
to add labels to your MR. Please see the list of allowed labels in thelabel
command documentation.This message was generated automatically. You're welcome to improve it.
added Community contribution workflowin dev labels
assigned to @johnwparent
added linked-issue label
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 Consider removing this inline disabling and adhering to the rubocop rule. If that isn't possible, please provide context as a reply for reviewers. See rubocop best practices.
changed this line in version 2 of the diff
- app/workers/ci/build_prerequisite_worker.rb 0 → 100644
1 # frozen_string_literal: true 2 3 module Ci 4 class BuildPrerequisiteWorker # rubocop:disable Scalability/IdempotentWorker Consider removing this inline disabling and adhering to the rubocop rule. If that isn't possible, please provide context as a reply for reviewers. See rubocop best practices.
changed this line in version 2 of the diff
- app/workers/ci/build_prerequisite_worker.rb 0 → 100644
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 Consider removing this inline disabling and adhering to the rubocop rule. If that isn't possible, please provide context as a reply for reviewers. See rubocop best practices.
changed this line in version 2 of the diff
mentioned in issue gitlab-org/quality/triage-reports#14618 (closed)
added backend label
@johnwparent Hey
Thank you for working on this 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 rouletteThank you
mentioned in issue gitlab-org/quality/triage-reports#14619 (closed)
added devopsverify grouprunner sectionci labels
added [Deprecated] Category:Runner Fleet label
@johnwparent, it seems we're waiting on an action from you for approximately two weeks.
- Do you still have capacity to work on this? If not, you might want to close this MR and/or ask someone to take over.
- Do you need help in getting it ready? At any time, you can:
- If you're actually ready for a review, you can post
@gitlab-bot ready
.
This message was generated automatically. You're welcome to improve it.
added automation:author-reminded label
@gitlab-bot ready @ayufan @ajwalker
added workflowready for review label and removed workflowin dev label
@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.
removed automation:author-reminded label
@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.
Hi @ayufan / @ajwalker
This is just a friendly reminder to make sure this is still on your tracks@johnwparent, as an aside, I'm sure the reviewers would ask you to solve the conflicts that are present. Could you please have a look?
Hi @ayufan / @ajwalker - Just another friendly hello on this issue since it's been marked as idle for a while!
@johnwparent - Feel free to let us know if you don't have time to update the conflicts or provide the notes that Dangerbot is asking for
added automation:reviewers-reminded label
added idle label
mentioned in issue gitlab-org/quality/triage-reports#15261 (closed)
removed idle label
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
Toggle commit list-
679feb25...b5e58841 - 7515 commits from branch
6 Warnings f2517208: The commit subject must start with a capital letter. For more information, take a look at our Commit message guidelines. 9691440a: The commit subject must start with a capital letter. For more information, take a look at our Commit message guidelines. 74882e30: The commit subject must contain at least 3 words. For more information, take a look at our Commit message guidelines. 74882e30: The commit subject must start with a capital letter. For more information, take a look at our Commit message guidelines. 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.
This merge request does not refer to an existing milestone. 1 Message 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
(UTC-4)
@ahuntsman
(UTC-5)
groupimport and integrate (backend) @georgekoltsov
(UTC+1)
Maintainer review is optional for groupimport and integrate (backend) ~"Verify" Reviewer review is optional for ~"Verify" @pedropombeiro
(UTC+2)
Please check reviewer's status!
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
danger-review
job that generated this comment.Generated by
Dangeradded idle label
mentioned in issue gitlab-org/quality/triage-reports#15811 (closed)
mentioned in issue gitlab-org/quality/triage-reports#15884 (closed)
removed idle label
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 changed this line in version 3 of the diff
9 9 tag_list 10 10 build_ids 11 11 project 12 runner_allowlist changed this line in version 3 of the diff
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) changed this line in version 3 of the diff
added idle label
mentioned in issue gitlab-org/quality/triage-reports#16991 (closed)
Since some changes have been requested I'm moving this back to workflowin dev
@johnwparent feel free to ask for another review once ready with
@gitlab-bot ready @ayufan
removed idle label
added workflowin dev label and removed workflowready for review label
removed automation:reviewers-reminded label
mentioned in merge request !135150
added 3 commits
@gitlab-bot ready @ayufan
@ayufan Made suggested updates here and the other MR (will update that one as well), let me know if that was what you were thinking. I think I now need to update the changes here to reflect working with the refactored Prerequsite model before moving on to the next stage of development.
Hi @ayufan
Are you able to advise if the updates to this Leading Organization MR & the related Make Build Preparation an async process (!135151) are on the right track with what you were looking for?Hi @ayufan @ajwalker
Could you review updates in this Leading Organization MR and the related one Make Build Preparation an async process (!135151) to move them forward? Thanks
added workflowready for review label and removed workflowin dev label
@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.
added automation:reviewers-reminded label
added Leading Organization label
mentioned in issue gitlab-org/quality/triage-reports#17263 (closed)
requested review from @ayufan
mentioned in issue gitlab-org/quality/triage-reports#17339 (closed)
mentioned in issue gitlab-org/quality/triage-reports#17439 (closed)
mentioned in issue gitlab-org/quality/triage-reports#17608 (closed)
mentioned in issue gitlab-org/quality/triage-reports#17711 (closed)
mentioned in issue gitlab-org/quality/triage-reports#17791 (closed)
mentioned in issue gitlab-org/quality/triage-reports#17898 (closed)
mentioned in issue gitlab-org/quality/triage-reports#18061 (closed)
mentioned in merge request !153643 (merged)
mentioned in issue gitlab-org/quality/triage-reports#18161 (closed)
mentioned in issue gitlab-org/quality/triage-reports#18244 (closed)
mentioned in issue gitlab-org/quality/triage-reports#18245 (closed)
mentioned in issue gitlab-org/quality/triage-reports#18354 (closed)
mentioned in issue gitlab-org/quality/triage-reports#18433 (closed)
mentioned in issue gitlab-org/quality/triage-reports#18629 (closed)
added idle label
mentioned in issue gitlab-org/quality/triage-reports#18716 (closed)
mentioned in issue gitlab-org/quality/triage-reports#18717 (closed)
removed idle label
mentioned in issue gitlab-org/quality/triage-reports#18798 (closed)
mentioned in issue gitlab-org/quality/triage-reports#18910 (closed)
mentioned in issue gitlab-org/quality/triage-reports#19081 (closed)
added idle label
Security policy violations have been resolved.
Edited by GitLab Security Botmentioned in issue gitlab-org/quality/triage-reports#19193 (closed)
mentioned in issue gitlab-org/quality/triage-reports#19195 (closed)
removed idle label
mentioned in issue gitlab-org/quality/triage-reports#19279 (closed)
requested review from @panoskanell
@johnwparent I have read the MR and the lengthy discussions you've been through on the issue's thread.I have contacted my team to get some more input from them.
@johnwparent I see you have another active MR.
Is this MR still relevant?
There are actually 3 MRs that compose the body of changes needed implement the Admission Controller Blueprint. This MR, !135151, and !135150 comprise the relevant changes. Note this MR and !135150 are still very rough as we were initially unsure about our approach. !135151 is under review now and is the first that should land. !135150 is next, and this MR is the final piece that should go in (as it actually implements the functionality defined in the blueprint).
removed review request for @panoskanell
mentioned in issue gitlab-org/quality/triage-reports#19363 (closed)
mentioned in issue gitlab-org/quality/triage-reports#19534 (closed)
mentioned in issue gitlab-org/quality/triage-reports#19649 (closed)
mentioned in issue gitlab-org/quality/triage-reports#19661 (closed)
requested review from @tianwenchen and removed review request for @ayufan and @ajwalker
mentioned in issue gitlab-org/quality/triage-reports#19798 (closed)
mentioned in issue gitlab-org/quality/triage-reports#19908 (closed)
removed review request for @tianwenchen
Hi @tianwenchen
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 workflowin dev label and removed workflowready for review label
removed automation:reviewers-reminded label
added idle label
@johnwparent, it seems we're waiting on an action from you for approximately two weeks.
- Do you still have capacity to work on this? If not, you might want to close this MR and/or ask someone to take over.
- Do you need help in getting it ready? At any time, you can:
- If you're actually ready for a review, you can post
@gitlab-bot ready
.
This message was generated automatically. You're welcome to improve it.
added automation:author-reminded label
Hi @nicolewilliams!
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
- Work on the provided feedback. Add
added automation:stale-reminded label