Refactor operator in preparation for leader-election #6
Conversation
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
camh-
reviewed
Sep 6, 2021
Reformat markdown files with prettier 2.3.2. Format all the things because automating what can be automated is grand. Sadly all go alternatives investigated for markdown formatting fall short of prettier.
juliaogris
changed the title
added 6 commits
September 6, 2021 20:41
Reformat transflect deployment yaml file for the ClusterRole to read more compact. In a follow-up commit we will add a further CluterRole rule for `leases.coordination.k8s.io`. It is easier to take these in a more compact format me thinks.
Refactor operator main and move probes server into separate file to keep the main file lean. This is in preparation for adding leader election code which will require a bit more rework.
Update inline code comments comment for readability.
Rework operator initialisation so that on `start` call non-constant operator fields and structures, such as informers, stopper channel get re-initialised. This is a preparatory step for adding leader-election, where these structures need to be re-initialised when leadership / execution is re-gained.
Change function order for readability. This a pure code reorganisation within the operator.go file on a function level without any further additions. This is another step towards adding leader election. Readability has been optimised for the final code listing with leader-election, however to keep the leader-election change small, pure code movements have been moved out to this commit. Signed-off-by: Julia Ogris <juliao@squareup.com>
Reorganise Replicaset/Filter processing in opertor.go, for new workloads retrieved from the workqueue. This is again purely a refactor without any functional additions.
jvmakine
approved these changes
Sep 6, 2021
camh-
reviewed
Sep 6, 2021
cmd/transflect-operator/operator.go
Outdated
|
|
||
| log.Debug().Msg("Starting: informer event handlers registered") | ||
| // Run workers and informers | ||
| o.runWorkers(42) | ||
| o.rsInformer.Informer().Run(o.stopper) | ||
| o.deployInformer.Informer().Run(o.stopper) |
There was a problem hiding this comment.
These two calls to Run() are not done asynchronously. From the Run docs:
Run starts and runs the shared informer, returning after it stops.
So the deployInformer does not start until the rsInformer stops.
There was a problem hiding this comment.
yesss!! thanks for the eagle eye yet again.
Fix informer call to start in separate go routine so that the deployment informer is actually executed concurrently with the replicaset informer. oups.
This was referenced Sep 7, 2021
juliaogris
pushed a commit
that referenced
this pull request
Sep 9, 2021
Add leader-election to operator so that several operators may be ready in a cluster, but only one at a time executes. Add cleanup on cancelled context to probes server and signal handler, to ensure quick leaser release and handover. This change leans heavily on k8s client-go leader-election example. https://github.com/kubernetes/client-go/blob/a31b18a6ac98bbff0dd1055707dfde1b0a98ff68/examples/leader-election/main.go There are no tests for the leader election, but I have manually tested it. This PR is a re-creation of #7 which got deleted as it was a stacked PR with #6 and not rebased before merge/delete of base branch. issue: #8 This merges the following commits: * Add leader-election to operator Makefile | 2 +- README.md | 1 + cmd/transflect-operator/main.go | 33 +++++----- cmd/transflect-operator/operator.go | 97 ++++++++++++++++++++++++----- cmd/transflect-operator/probes.go | 6 +- deployment/transflect.yaml | 7 +++ go.mod | 1 + go.sum | 2 + 8 files changed, 117 insertions(+), 32 deletions(-) Pull-Request: #9
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Refactor, reformat, rename and reorganise markdown, yaml and go code
files in preparation for leader election. This PR was created as all
non-functional changes for the leader-election implementation so that
leader-election is purely functional and can be more easily reviewed
with care (see #7 ).
Issue: #8