-
Notifications
You must be signed in to change notification settings - Fork 1.8k
pkg/sdk: add informer pkg and RegisterInformer and NewKubeInformer api #8
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
pkg/sdk: add informer pkg and RegisterInformer and NewKubeInformer api #8
Conversation
| dp.Run(ctx) | ||
| } | ||
|
|
||
| /* |
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.
put this into doc.go or create example_test.go
| ```main.go | ||
| func main() { | ||
| sdk.RegisterInformer("play-informer", informer.NewKubeInformer(&Play{})) | ||
| sdk.RegisterInformer("pod-informer", informer.NewKubeInformer(&Pod{})) |
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.
why not just sdk.watch("watch-pod", &Pod{})?
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.
There will be two kinds of states to watch: k8s state and application state.
We need to provide an abstraction to extend inform events mechanism.
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.
@xiang90 if I understand how informer works. it requires the restClient that's associated with the watched resource.
e.g:
source := cache.NewListWatchFromClient(
v.vaultsCRCli.VaultV1alpha1().RESTClient(),
api.VaultServicePlural,
v.namespace,
fields.Everything())
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 do not quite understand. can you provide me an example?
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.
nvm, i am thinking about a different issue.
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 hide all the inform stuff for users if possible.
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 do not quite understand. can you provide me an example?
Sure.
For example, we might have an informer that watches application API and triggers custom events, e.g. application is unhealthy or overloaded, which cannot be observed via k8s api.
we should hide all the inform stuff for users if possible.
Yes. We will provide a KubeInformer() which is the same mechanism you mentioned above.
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.
@xiang90 I am thinking about this watch interface:
type Watchable struct {
RESTClient rest.Interface
Resource string
Namespace string
}
type Client interface {
Watcher(...Watchable) Watcher
}But the Watchable is not elegant since it needs to know the RESTClient that's associating with the watched resource.
I thought about sdk.watch("watch-pod", &Pod{})?. I am not sure how to create informer internally just with the &Pod{} struct.
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.
For example, we might have an informer that watches application API and triggers custom events, e.g. application is unhealthy or overloaded, which cannot be observed via k8s api.
OK. let us worry about this later by creating a new func. WatchApplication or whatever. I do not want to overload one func.
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.
watch(whatever, withNamespace(x), withFields(y)...). the options work exactly like what etcd/cleintv3 does and other projects in go do. that is the suggested way for go.
| sdk.RegisterActor("kube-apply", actor.KubeResourceApply) | ||
| sdk.RegisterActor("kube-delete", actor.KubeResourceDelete) | ||
|
|
||
| sdk.RegisterHandle(stub.NewHandler()) |
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.
remove Register. use handleFunc or Serve. see examples here: https://golang.org/pkg/net/http/#HandleFunc
|
|
||
| type Action struct { | ||
| Object Object | ||
| Actor string |
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.
this should be just action (apply, delete, create, patch etc..). also type define the string.
|
|
||
| type Event struct { | ||
| Object Object | ||
| ObjectExist bool |
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.
should this also be action?
|
|
||
| import "context" | ||
|
|
||
| type Object interface{} |
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.
should this be k8s runtime.object?
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.
let us not worry about user defined event object for now.
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.
Sounds good.
| sdkTypes "github.com/coreos/operator-sdk/pkg/sdk/types" | ||
| ) | ||
|
|
||
| type Dispatcher interface { |
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 hope to see this looks like https://golang.org/pkg/net/http/#ServeMux
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.
HandleMux
|
Going to remove the Dispatcher and Action type related code for now as that was just added to give the general idea. |
|
On second thought, going to leave this PR as it is for the benefit of discussing the overall design. Going to push a new PR specifically for the informer implementation. |
|
Closing in favour of #9 |
#8) * Infer the image type (SQLite/FBC) based on the image label and handle FBC scenarios Signed-off-by: rashmigottipati <[email protected]>
An initial implementation of what the sdk Informer API
NewKubeInformer()andRegisterInformer()could look like.The main focus of this PR is the sdk Informer.
I've only added the other interfaces for Handler and Dispatcher to make sense of how the sdk passes Events from Informer->Dispatcher->Handler.
Going to add a diagram in the design doc to convey the overall structure.