-
Notifications
You must be signed in to change notification settings - Fork 118
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
feat: Add periodic job support #1474
Closed
Closed
Changes from 59 commits
Commits
Show all changes
61 commits
Select commit
Hold shift + click to select a range
44074c2
Add periodics controller
dippynark ee32402
Improve comments
dippynark a7cca17
Add comments to reconiliation logic
dippynark 2783e6a
Add a
dippynark 89bd2ec
Rename to strobe
dippynark 086be29
Merge branch 'main' into add-periodics-controller
dippynark 098567b
Implement reconciliation and update docs
dippynark f954770
Merge branch 'main' into add-periodics-controller
dippynark 40472e3
Implement maximum concurrency
dippynark 9160b1f
Update docs
dippynark c45ca3b
Update Helm chart
dippynark b2b7532
Add strobe Docker build
dippynark 11c7c20
Fix indent
dippynark 36ebbf0
Fix RBAC
dippynark 17bca56
Fix spelling
dippynark af48ed3
Add release build
dippynark 03c89e9
Update docs
dippynark d32e121
Check for nil
dippynark ecb22d3
Remove print
dippynark 9e4813f
Add comment
dippynark 281bcac
Use UTC time
dippynark 173ecb5
Use periodicJobFirstObserved
dippynark b894164
Improve ordering and comments
dippynark 222fd39
Improve comments
dippynark 021aa21
Use NamespacedName
dippynark 51b9f92
Fix earliestScheduleTime
dippynark 75c58cb
Fix LighthouseJob generation and add tests
dippynark ffa809a
Reorder
dippynark d1db0d8
Add cron parse comment
dippynark a9c642c
Only enqueue jobs at next schedule time
dippynark b1e50ce
Check if job has already been triggered
dippynark 7283ce9
Recover from downtime
dippynark 3f54b19
Update docs
dippynark 28d69d0
Update docs
dippynark 30c4fda
Improve recovery comment
dippynark c745145
Improve recovery comment
dippynark 252a2b8
Improve recovery comment
dippynark 12c117f
Use 0
dippynark e59242f
Use >0
dippynark 5400640
Merge branch 'main' into add-periodics-controller
dippynark 8dca4b4
Use rolling update
dippynark 8026c48
Use Unix time
dippynark a0c24ee
Print Unix time properly
dippynark d75422a
Improve time hashing and explanation
dippynark 04d671c
Improve time hashing and explanation
dippynark d922423
Fix name suffix
dippynark 59012ad
Improve lock explanation
dippynark 3001c1b
Improve lock explanation
dippynark 4897cb9
Add generateLighthouseJob test
dippynark 0611afc
Add comment
dippynark 17bf289
Use actualLighthouseJob
dippynark 122752b
Fix test file name
dippynark 454b8b4
Remove the
dippynark c0a2d13
Use common pipelineRunSpec
dippynark 6826cd5
Merge branch 'main' into add-periodics-controller
dippynark d007a2e
Merge branch 'main' into add-periodics-controller
dippynark d463bb0
Merge branch 'main' into add-periodics-controller
dippynark 9ca5a16
Merge branch 'main' into add-periodics-controller
dippynark 27c7bdd
Merge branch 'main' into add-periodics-controller
dippynark e4adcf7
Merge branch 'main' into add-periodics-controller
dippynark 29c0b9a
Merge branch 'main' into add-periodics-controller
dippynark File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,53 @@ | ||
{{- if .Values.strobe.enabled }} | ||
apiVersion: apps/v1 | ||
kind: Deployment | ||
metadata: | ||
name: {{ template "strobe.name" . }} | ||
labels: | ||
chart: "{{ .Chart.Name }}-{{ .Chart.Version | replace "+" "_" }}" | ||
app: {{ template "strobe.name" . }} | ||
spec: | ||
replicas: {{ .Values.strobe.replicaCount }} | ||
minReadySeconds: 10 | ||
selector: | ||
matchLabels: | ||
app: {{ template "strobe.name" . }} | ||
template: | ||
metadata: | ||
labels: | ||
app: {{ template "strobe.name" . }} | ||
{{- if .Values.podAnnotations }} | ||
annotations: | ||
{{ toYaml .Values.podAnnotations | indent 8 }} | ||
{{- end }} | ||
spec: | ||
serviceAccountName: {{ template "strobe.name" . }} | ||
containers: | ||
- name: {{ template "strobe.name" . }} | ||
image: {{ tpl .Values.strobe.image.repository . }}:{{ tpl .Values.strobe.image.tag . }} | ||
imagePullPolicy: {{ tpl .Values.strobe.image.pullPolicy . }} | ||
args: | ||
- "--namespace={{ .Release.Namespace }}" | ||
{{- if hasKey .Values "env" }} | ||
env: | ||
{{- range $pkey, $pval := .Values.env }} | ||
- name: {{ $pkey }} | ||
value: {{ quote $pval }} | ||
{{- end }} | ||
{{- end }} | ||
resources: | ||
{{ toYaml .Values.strobe.resources | indent 10 }} | ||
terminationGracePeriodSeconds: {{ .Values.strobe.terminationGracePeriodSeconds }} | ||
{{- with .Values.strobe.nodeSelector }} | ||
nodeSelector: | ||
{{ toYaml . | indent 8 }} | ||
{{- end }} | ||
{{- with .Values.strobe.affinity }} | ||
affinity: | ||
{{ toYaml . | indent 8 }} | ||
{{- end }} | ||
{{- with .Values.strobe.tolerations }} | ||
tolerations: | ||
{{ toYaml . | indent 8 }} | ||
{{- end }} | ||
{{- end }} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
{{- if .Values.strobe.enabled }} | ||
kind: RoleBinding | ||
apiVersion: rbac.authorization.k8s.io/v1 | ||
metadata: | ||
name: {{ template "strobe.name" . }} | ||
roleRef: | ||
apiGroup: rbac.authorization.k8s.io | ||
kind: Role | ||
name: {{ template "strobe.name" . }} | ||
subjects: | ||
- kind: ServiceAccount | ||
name: {{ template "strobe.name" . }} | ||
{{- end }} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
{{- if .Values.strobe.enabled }} | ||
kind: Role | ||
apiVersion: rbac.authorization.k8s.io/v1 | ||
metadata: | ||
name: {{ template "strobe.name" . }} | ||
rules: | ||
- apiGroups: | ||
- "" | ||
resources: | ||
- configmaps | ||
verbs: | ||
- get | ||
- list | ||
- watch | ||
- apiGroups: | ||
- lighthouse.jenkins.io | ||
resources: | ||
- lighthousejobs | ||
verbs: | ||
- list | ||
- create | ||
- apiGroups: | ||
- lighthouse.jenkins.io | ||
resources: | ||
- lighthousejobs/status | ||
verbs: | ||
- update | ||
{{- end }} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
{{- if .Values.strobe.enabled }} | ||
kind: ServiceAccount | ||
apiVersion: v1 | ||
metadata: | ||
name: {{ template "strobe.name" . }} | ||
{{- end }} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
# Strobe | ||
|
||
Strobe is a controller that implements the periodic jobs defined in the | ||
Lighthouse config ConfigMap: | ||
|
||
```yaml | ||
periodics: | ||
- name: hello-world | ||
cron: "*/1 * * * *" | ||
agent: tekton-pipeline | ||
pipeline_run_spec: | ||
pipelineSpec: | ||
tasks: | ||
- name: hello-world | ||
taskSpec: | ||
steps: | ||
- image: busybox | ||
script: echo 'Hello World!' | ||
``` | ||
|
||
This is done by watching the ConfigMap and processing each periodic job by name. | ||
Inspiration is taken from the [Kubernetes CronJob | ||
controller](https://github.com/kubernetes/kubernetes/blob/v1.25.2/pkg/controller/cronjob/cronjob_controllerv2.go). | ||
|
||
Note that if Strobe misses a schedule time for a particular periodic job due to | ||
crashing or being restarted it will attempt to schedule a job only for the last | ||
missed time. |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure if it should be defined in a configmap. Normally in jx, configmap would be defined/configured in the cluster git repo, which should be normally off limits for repository owners. It should be in the same repo, so if a repository author wants to define a periodics, they should be able to define a new periodics job in the triggers yaml, wdyt?
May be a block like pre-submit/post-submit?
Also the lifecycle of lighthouse configmap and a build job are very different.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ankitm123 By triggers yaml do you mean the TriggerConfig resource? https://github.com/dippynark/kfmt/blob/main/.lighthouse/triggers.yaml
How would the controller get access to this TriggerConfig in all registered repositories? Following through the webhook code, this is done for presubmit and postsubmit jobs when a SCM event is received for a specific repository:
lighthouse/pkg/webhook/events.go
Line 132 in 6a37f27
lighthouse/pkg/webhook/create_agent.go
Line 76 in 6a37f27
lighthouse/pkg/triggerconfig/inrepo/calculate.go
Lines 12 to 13 in 6a37f27
But of course periodic jobs don't have any associated SCM events since they are triggered on a timer. One option could be to add a new webhook plugin which persists periodic job configuration by syncing TriggerConfig periodic jobs to a ConfigMap which the controller then picks up? But I guess that work could be done in a subsequent PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also sorry to reference other repositories, but would it be possible to get a review for this PR please? jenkins-x/go-scm#321
We depend on the poller component being able to see if a particular job has ever succeeded (rather than just the latest run) which is the reason for the change. We are using a custom fork using that PR at the moment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the approval! I have created another PR to bump the go-scm version used by Lighthouse: #1493
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose the periodics would be registered similar to how pipelines are cached (but periodics will store instead). It being registered within the cluster repo goes away from the jenkins-x ideaollogy of being repo and developer centric as it would make you have to jump through 2 repos to add a feature to your repo