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

scheduler: handle scheduling everywhere #32173

Conversation

danilo-gemoli
Copy link
Contributor

Follow #32085 up.
Enable scheduling in the following components:

  • deck
  • horologium
  • gangway
  • gerrit
  • tide
  • pubsub
  • mkpj

Even if I changed a lot of files the diff is always the same:

- pjutil.NewProwJob(*prowJobSpec, labels, annotations)
+ pjutil.NewProwJob(*prowJobSpec, labels, annotations, pjutil.RequireScheduling(config.Scheduler.Enabled))

Enable scheduling in the following components:
- deck
- horologium
- gangway
- gerrit
- tide
- pubsub
- mkpj
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. area/prow Issues or PRs related to prow labels Mar 6, 2024
@k8s-ci-robot k8s-ci-robot requested review from chases2 and cjwagner March 6, 2024 10:31
@k8s-ci-robot k8s-ci-robot added area/prow/deck Issues or PRs related to prow's deck component area/prow/gerrit Issues or PRs related to prow's gerrit component area/prow/horologium Issues or PRs related to prow's horologium component area/prow/mkpj Issues or PRs related to prow's mkpj component area/prow/pubsub Issues or PRs related to prow's pubsub reporter component area/prow/tide Issues or PRs related to prow's tide component sig/testing Categorizes an issue or PR as relevant to SIG Testing. labels Mar 6, 2024
@danilo-gemoli
Copy link
Contributor Author

/cc @droslean
/cc @jmguzik

@k8s-ci-robot k8s-ci-robot requested review from droslean and jmguzik March 6, 2024 10:31
@danilo-gemoli
Copy link
Contributor Author

/retest

@droslean
Copy link
Member

droslean commented Mar 6, 2024

@danilo-gemoli
I would need some extra context on why we need to enable the scheduling on those components. First, I believe that the scheduling should be configurable and not enabled it by default. Also, how are those components related to the scheduling?

@jmguzik
Copy link
Contributor

jmguzik commented Mar 6, 2024

@danilo-gemoli I would need some extra context on why we need to enable the scheduling on those components. First, I believe that the scheduling should be configurable and not enabled it by default. Also, how are those components related to the scheduling?

@droslean I think @danilo-gemoli meant that we enable option to have scheduling in these components, but let's wait for clarification.

@danilo-gemoli
Copy link
Contributor Author

Yeah, the scheduler isn't enabled by default in any of those.

pjutil.NewProwJob(*prowJobSpec, labels, annotations, pjutil.RequireScheduling(config.Scheduler.Enabled))

enables scheduling only if a user adds the following Prow configuration:

scheduler:
  enabled: true

by default config.Scheduler.Enabled is false, therefore pjutil.RequireScheduling(config.Scheduler.Enabled) enables nothing.
On the other hand, when someone wants the scheduling to take place (enabling the scheduler with the config above), we need to ensure that ProwJobs are being created in scheduling state. Each of those component I've modified is able to create a ProwJob:

  • deck: reruns PJs on demand via UI
  • horologium: creates periodics
  • gangway: creates PJs on demand via API
  • gerrit: creates PJs when on code change in Gerrit
  • tide: retests PJs in batch before a PR merges in GitHub
  • pubsub: creates PJs on demand via API
  • mkpj: creates PJs on demand via CLI

With #32085 we have covered trigger firstly, but that alone is not sufficient.

@danilo-gemoli
Copy link
Contributor Author

Alright it may be misleading
/retitle scheduler: handle scheduling everywhere

@k8s-ci-robot k8s-ci-robot changed the title scheduler: enable scheduling everywhere scheduler: handle scheduling everywhere Mar 6, 2024
@droslean
Copy link
Member

droslean commented Mar 6, 2024

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 6, 2024
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: danilo-gemoli, droslean

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 6, 2024
@k8s-ci-robot k8s-ci-robot merged commit 0b904de into kubernetes:master Mar 6, 2024
7 checks passed
@danilo-gemoli danilo-gemoli deleted the feat/create-prowjobs-in-scheduling-state branch March 13, 2024 09:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/prow/deck Issues or PRs related to prow's deck component area/prow/gerrit Issues or PRs related to prow's gerrit component area/prow/horologium Issues or PRs related to prow's horologium component area/prow/mkpj Issues or PRs related to prow's mkpj component area/prow/pubsub Issues or PRs related to prow's pubsub reporter component area/prow/tide Issues or PRs related to prow's tide component area/prow Issues or PRs related to prow cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants