This repository was archived by the owner on Mar 11, 2022. It is now read-only.
Emit Kubernetes events when Job completes#212
Merged
seanmalloy merged 9 commits intoKohlsTechnology:masterfrom Dec 19, 2019
Merged
Emit Kubernetes events when Job completes#212seanmalloy merged 9 commits intoKohlsTechnology:masterfrom
seanmalloy merged 9 commits intoKohlsTechnology:masterfrom
Conversation
added 7 commits
December 17, 2019 15:14
- Change the names of the consts used to tag Kubernetes objects, such that it's easier to understand their purpose, and to make them look similar - Change the contents of the finalizer label/tag, to make sure its contents is properly namespaced, to avoid possible conflicts with other user-provided tags.
Add a helper dir for tests, containing an empty .yaml file. Change the existing test using an empty yaml file to use this new directory. It is also expected to be reused in future tests that I plan to submit soon.
The name of the controller will be needed in a subsequent commit, so this commit extracts it into a const, so that it can be safely reused. Also, the label used in the logger is changed to make it consistent with the name of the controller.
…Technology#110) This commit makes eunomia emit Kubernetes events when Jobs it controls end. This is expected to be useful for tracking when a particular GitOps change got applied, and whether it applied successfully. The implementation is based on watching Job resource change events reported by the Kubernetes client-go API. This commit does not reuse the mechanisms established in jobmonitor.go. The reasons for this are multi-fold: - This commit is an exploration of the possibility of using Kubernetes watching mechanisms (Informers), as a next step after the jobmonitor.go code. I believe this approach has a chance to be more robust and in-line with the features exposed by the client-go API. I hope jobmonitor could be refactored to this same mechanism in future as well. - Comparing the two approaches, it seems that the one used in this commit detects more job completion events than the one used in jobmonitor.go. This commit is a partial fix to KohlsTechnology#110. However, it does not yet implement all its requested features: - no Git SHA1 in the Message - this was postponed for the time being, to reduce the scope of the current task - InvolvedObjects field is not used - the Events API as exposed in client-go doesn't seem to provide means of setting this field; instead, this commit places extra data (Job name) in the Annotations field.
Add tests related to emitting Job events.
- Fix a log.Info call, by changing it to log.Error and properly emitting the error contents (previously it was ignored by log.Info) - Fix grouping of imports in jobmonitor.go - Add a TODO for consideration in jobmonitor.go
Member
vinny-sabatini
left a comment
vinny-sabatini
left a comment
There was a problem hiding this comment.
Looks good to me! Just a couple minor comments
added 2 commits
December 18, 2019 15:07
Change the package name to fit the standard Go convention, where both the package and the directory that contains it have the same name. The test which used this package did alias it to 'test' anyway actually. Also: - improve godoc comment for test.Initialize func - fix grouping of imports in controller_test.go
Extract WatchEvents func from events_test.go to the global helper package test, so that it can be more easily used in future tests.
akavel
commented
Dec 18, 2019
Author
There was a problem hiding this comment.
@vinny-sabatini Thanks! If you're ok with the changes, I'd be glad if you could click the tick afterwards 😁
seanmalloy
approved these changes
Dec 19, 2019
This file contains hidden or 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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
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.
Description
This PR makes eunomia emit Kubernetes events when Jobs it controls
end. This is expected to be useful for tracking when a particular GitOps
change got applied, and whether it applied successfully.
The implementation is based on watching Job resource change events
reported by the Kubernetes client-go API. This PR does not reuse the
mechanisms established in jobmonitor.go. The reasons for this are
multi-fold:
watching mechanisms (Informers), as a next step after the
jobmonitor.go code. I believe this approach has a chance to be more
robust and in-line with the features exposed by the client-go API. I
hope jobmonitor could be refactored to this same mechanism in future
as well.
PR detects more job completion events than the one used in
jobmonitor.go.
This PR is a partial fix to #110. However, it does not yet implement
all its requested features:
to reduce the scope of the current task
client-go doesn't seem to provide means of setting this field;
instead, this PR places extra data (Job name) in the Annotations
field.
Partial fix to #110
Type of change
Checklist