Skip to content
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

[Docs] Document how to record events #2897

Closed
AlmogBaku opened this issue Aug 26, 2022 · 23 comments
Closed

[Docs] Document how to record events #2897

AlmogBaku opened this issue Aug 26, 2022 · 23 comments
Assignees
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. kind/documentation Categorizes issue or PR as related to documentation. kind/feature Categorizes issue or PR as related to a new feature. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@AlmogBaku
Copy link
Member

What do you want to happen?

Following up on: #2684 (comment)

As discussed in the meeting, we think it might be a good idea to add a section about event recording to the documentation, similar to what we had in v1.

Extra Labels

No response

@AlmogBaku AlmogBaku added the kind/feature Categorizes issue or PR as related to a new feature. label Aug 26, 2022
@AlmogBaku
Copy link
Member Author

AlmogBaku commented Aug 26, 2022

Also, it's probably better to recommend using the controller-runtime recorder over the raw client recorder: https://github.com/kubernetes-sigs/controller-runtime/blob/master/pkg/recorder/recorder.go

@k8s-ci-robot
Copy link
Contributor

@AlmogBaku: The label(s) /label triage/accepted cannot be applied. These labels are supported: api-review, tide/merge-method-merge, tide/merge-method-rebase, tide/merge-method-squash, team/katacoda, refactor

In response to this:

/label triage/accepted

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.

@camilamacedo86 camilamacedo86 added good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. kind/documentation Categorizes issue or PR as related to documentation. triage/accepted Indicates an issue or PR is ready to be actively worked on. labels Sep 11, 2022
@camilamacedo86
Copy link
Member

Following is some relevant info which would be important we cover in the doc:

(IMO) we could add a warning with:

Events should be raised in certain circumstances only and following this guideline acctually is not a good practice. See what the Kubernetes APIs convention describes when using them here: https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#events
Note that is NOT recommended to emit events for all Operations. If authors raise too many events it brings bad UX experiences for those that are consuming the solutions on the cluster and they will not have attention to what they actually must be notified.

Also, we can share how to implement the solution to raise an event:

Then, we can also create a note to let users know that the new deploy-image plugin raises events and can be used as an example:

What is the purpose of this plugin?
The deploy Image plugin allows users to scaffold API/Controllers to deploy and manage an Operand (image) on the cluster following the guidelines and best practices. It abstracts the complexities of achieving this goal while allowing users to customize the generated code. (More info).
Demo: https://www.youtube.com/watch?v=UwPuRjjnMjY

Note that we have a doc in the old book version. So, we might use this content as a base and just ensure that we have the info properly updated: https://book-v1.book.kubebuilder.io/beyond_basics/creating_events.html

By last, I think we could add it under the Reference section.

@mjlshen
Copy link
Contributor

mjlshen commented Sep 12, 2022

I would like to work on this, /assign

@camilamacedo86
Copy link
Member

Hi @mjlshen,

Are you working on this one? If you need help please feel free to ping your questions into the kubebuilder slack channel.

@mjlshen
Copy link
Contributor

mjlshen commented Nov 1, 2022

Hi @camilamacedo86! Sorry, yes I started - then got sidetracked - will pick this up again

@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

  • Mark this PR as fresh with /remove-lifecycle stale
  • Close this PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 8, 2023
@mjlshen
Copy link
Contributor

mjlshen commented Feb 8, 2023

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 8, 2023
@camilamacedo86
Copy link
Member

/remove-lifecycle stale

@camilamacedo86 camilamacedo86 added the lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. label Feb 8, 2023
@Sajiyah-Salat
Copy link
Contributor

Hello @mjlshen can I take this on?

@ashutosh887
Copy link
Contributor

I would like to work on this @camilamacedo86

/assign

@ashutosh887
Copy link
Contributor

@AlmogBaku Please guide me how should I proceed?

@Sajiyah-Salat
Copy link
Contributor

Hey @ashutosh887 if you want we can work together.
/assign

@AlmogBaku
Copy link
Member Author

take a look at how I do at Raptor.ml

In general, you should inject the event recorder to the Reconciler when initializing it, then record events

@Sajiyah-Salat
Copy link
Contributor

hello @AlmogBaku, in v1 docs it is given perfectly. I dont have depth so I dont know which things need to be updated. morever I have questions regarding to place a record event file. i think I should just open a pr with v1 doc record event file and after your reviews and suggestion we can make it ready for current version?

@Sajiyah-Salat
Copy link
Contributor

hello @AlmogBaku, in v1 docs it is given perfectly. I dont have depth so I dont know which things need to be updated. morever I have questions regarding to place a record event file. i think I should just open a pr with v1 doc record event file and after your reviews and suggestion we can make it ready for current version?

/cc @camilamacedo86

@camilamacedo86
Copy link
Member

camilamacedo86 commented Mar 7, 2023

Yes, we can start by adding the content from v1
Then, as part of this task you should follow up the instructions to ensure that is right and/or update what is required. Also, we should let the users know and point they out that deploy image plugin scaffold will use it and can be checked as an example, see: https://github.com/kubernetes-sigs/kubebuilder/blob/master/testdata/project-v4-with-deploy-image/cmd/main.go#L95

@ashutosh887
Copy link
Contributor

Sure I've started working on it @camilamacedo86
Thanks

@Sajiyah-Salat
Copy link
Contributor

Hello @ashutosh887 are you still working on this. If yes, please link the pr.

@ashutosh887
Copy link
Contributor

Yes I'm willing to take this forward @Sajiyah-Salat

@Sajiyah-Salat
Copy link
Contributor

Have you made a pr

@Sajiyah-Salat
Copy link
Contributor

Following is some relevant info which would be important we cover in the doc:

(IMO) we could add a warning with:

Events should be raised in certain circumstances only and following this guideline acctually is not a good practice. See what the Kubernetes APIs convention describes when using them here: https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#events Note that is NOT recommended to emit events for all Operations. If authors raise too many events it brings bad UX experiences for those that are consuming the solutions on the cluster and they will not have attention to what they actually must be notified.

Also, we can share how to implement the solution to raise an event:

Then, we can also create a note to let users know that the new deploy-image plugin raises events and can be used as an example:

What is the purpose of this plugin?
The deploy Image plugin allows users to scaffold API/Controllers to deploy and manage an Operand (image) on the cluster following the guidelines and best practices. It abstracts the complexities of achieving this goal while allowing users to customize the generated code. (More info).
Demo: https://www.youtube.com/watch?v=UwPuRjjnMjY

Note that we have a doc in the old book version. So, we might use this content as a base and just ensure that we have the info properly updated: https://book-v1.book.kubebuilder.io/beyond_basics/creating_events.html

By last, I think we could add it under the Reference section.

The video you suggested is of v1. do you think we should add that?

@camilamacedo86
Copy link
Member

It is done : https://book.kubebuilder.io/reference/raising-events

Closing this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. kind/documentation Categorizes issue or PR as related to documentation. kind/feature Categorizes issue or PR as related to a new feature. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants