-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
📖 Good practices Doc page created #3478
📖 Good practices Doc page created #3478
Conversation
## What is "Reconciliation" in Operators? | ||
|
||
When you create a project using Kubebuilder, see the scaffolded code generated under `cmd/main.go`. This code initializes a [Manager][controller-runtime-manager], and the project relies on the [controller-runtime][controller-runtime] framework. The Manager manages [Controllers][controllers], which offer a reconcile function that synchronizes resources until the desired state is achieved within the cluster. | ||
|
||
Reconciliation is an ongoing loop that executes necessary operations to maintain the desired state, adhering to Kubernetes principles, such as the [control loop][k8s-control-loop]. For further information, check out the [Operator patterns][k8s-operator-pattern] documentation from Kubernetes to better understand those concepts. |
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.
The reconciliation is the control loop code implementation that we do in the controllers which leverage in controller- runtime. But the text does not seems explain it properly. Can we remove it?
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 think to replace it with new defination would be the good idea?I didnt find any reconcilation defination in kubebuilder documentation. I know users coming here will have basic idea about it. But adding it would increase the chances of understanding things better. WDYT?
</aside> | ||
|
||
[docs]: ./cronjob-tutorial/gvks.html | ||
|
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.
can we remove the spaces?
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.
The changes look good to me apart from a nit.
Will leave it for @camilamacedo86 to lgtm before merging.
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.
we should only get merged PRs which are with the commits squashed to ensure that the history will be properly tracked.
Since I know that you @Sajiyah-Salat are working in this one for a while and you were facing issues to squash I am moving forward using the option to automatically merge with squash which is not ideal and we can tackle the nits in a fallow up.
Thank you for ALL your hard work on this one 🥇
/lgtm
/test pull-kubebuilder-e2e-k8s-1-26-0 |
|
||
## Why You Should Adopt Status Conditions | ||
|
||
We recommend you manage your solutions using Status Conditionals following the [K8s Api conventions][k8s-aoi-convetions] because: |
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.
It is missing the link
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 will add it. Ah! the name is wrong as well. Will fix this soon!
/test pull-kubebuilder-e2e-k8s-1-26-0 |
New changes are detected. LGTM label has been removed. |
when I just check out to my branch there were bunch of files deleted which was not done by me. I was trying to squash the changes but there were uncommit files so I just commit them and these resolve conflict occur. What should I do? |
504e37d
to
68a29e4
Compare
@Sajiyah-Salat looks like the PR has conflicts. What it means is - you have made changes in some files, and before you merge it to master, someone else has also made changes in the same files. This means, we need to resolve those conflicts manually. Here is some documentation on how to do so: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/addressing-merge-conflicts/resolving-a-merge-conflict-using-the-command-line. Just to highlight, the overall steps would be:
Feel free to let us know if you have any problems while solving conflicts. |
I resolved the conflicts files should I wait for the test to pass or should I squash the commits first? |
The star is added by me which says includes all the files of project dir. The log was long enough stating all the files differently. When I resolved the conflicts by undo the deleted files. I got into this trouble. |
4f4d807
to
c811b20
Compare
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: camilamacedo86, Sajiyah-Salat, varshaprasad96 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
@Sajiyah-Salat: The following tests failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
Description
Adds new section within Good Practices information
Partial Fixes for #3203
This pr is a duplicate from #3267 pr as I have messed the squashing and rebasing part.