-
Notifications
You must be signed in to change notification settings - Fork 112
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
Restructure controllers pkg #593
Restructure controllers pkg #593
Conversation
aa658a3
to
7fc15f0
Compare
This introduce reconciler pkg, this package provides more clarity between controller and reconciler logic. It also allows each resource to have an internal resources package to deal with all related resources involved during that resource reconciliation.
7fc15f0
to
6cad175
Compare
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.
Did a first pass. I added comments where I have questions.
Fix all sanity check linters Add all missing license headers
Do not include markdown files under .github dir
6cad175
to
c7c4fde
Compare
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 like it. :-)
/approve
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: SaschaSchwarze0 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 |
/lgtm |
Changes
This came up while working on #558 . This does NOT fix #558, this is a general code structure enhancement around controllers definition and reconcile logic.
The goal of this PR is to make the code definition more clear, for future contributors and future enhancements.
This PR provides:
/pkg/controller
to/pkg/reconciler
. This provides more clarity on where the reconcile logic is.resources
, to allocate all logic related to resources that required to be manage during reconciliations. This is only added at the moment for the buildrun one, at/pkg/reconciler/buildrun/resources
./pkg/controller
package to host the manager definition based in all controllers definitions.The above is based on how
Tekton/Knative
organize their project structures regarding reconcile and controllers logic.Submitter Checklist
See the contributor guide
for details on coding conventions, github and prow interactions, and the code review process.
Release Notes
Note: We still have room for improvement on the code organization, mainly on the BuildRun reconciler logic. But adding an initial PR to avoid adding a larger one.