[ 
https://issues.apache.org/jira/browse/YUNIKORN-2844?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Peter Bacsko updated YUNIKORN-2844:
-----------------------------------
    Description: 
The current implementation creates an event recorder like that:

{noformat}
func GetRecorder() events.EventRecorder {
        lock.Lock()
        defer lock.Unlock()
        once.Do(func() {
                // note, the initiation of the event recorder requires on a 
workable Kubernetes client,
                // in test mode we should skip this and just use a fake 
recorder instead.
                configs := conf.GetSchedulerConf()
                if !configs.IsTestMode() {
                        k8sClient := client.NewKubeClient(configs.KubeConfig)
                        eventBroadcaster := 
events.NewBroadcaster(&events.EventSinkImpl{
                                Interface: k8sClient.GetClientSet().EventsV1()})
                        eventBroadcaster.StartRecordingToSink(make(<-chan 
struct{}))
                        eventRecorder = 
eventBroadcaster.NewRecorder(scheme.Scheme, constants.SchedulerName)
                }
        })

        return eventRecorder
}
{noformat}

The problem with this approach is that we need to indicate "test mode" in the 
config, which just complicates things. 

We can simplify this code if the recorder is set during Yunikorn initialization 
in eg. {{NewShimScheduler()}}. The plugin code already does this in 
{{NewSchedulerPlugin()}} and calls 
{{events.SetRecorder(handle.EventRecorder())}}.

We should also get rid of the default fake recorder. This uses a buffered 
channel with the size of 1024. This isn't a problem now, but if a new test 
somehow ends up generating a lot of events, message sending will block. It 
might not be obvious to someone to understand why running a unit test just 
starts to block suddenly.

  was:
The current implementation creates an event recorder like that:

{noformat}
func GetRecorder() events.EventRecorder {
        lock.Lock()
        defer lock.Unlock()
        once.Do(func() {
                // note, the initiation of the event recorder requires on a 
workable Kubernetes client,
                // in test mode we should skip this and just use a fake 
recorder instead.
                configs := conf.GetSchedulerConf()
                if !configs.IsTestMode() {
                        k8sClient := client.NewKubeClient(configs.KubeConfig)
                        eventBroadcaster := 
events.NewBroadcaster(&events.EventSinkImpl{
                                Interface: k8sClient.GetClientSet().EventsV1()})
                        eventBroadcaster.StartRecordingToSink(make(<-chan 
struct{}))
                        eventRecorder = 
eventBroadcaster.NewRecorder(scheme.Scheme, constants.SchedulerName)
                }
        })

        return eventRecorder
}
{noformat}

The problem with this approach is that we need to indicate "test mode" in the 
config, which just complicates things. 

We can simplify this code if the recorder is set during Yunikorn initialization 
in {{NewScheduler()}}. The plugin code already does this in 
{{NewSchedulerPlugin()}} and calls 
{{events.SetRecorder(handle.EventRecorder())}}.

We should also get rid of the default fake recorder. This uses a buffered 
channel with the size of 1024. This isn't a problem now, but if a new test 
somehow ends up generating a lot of events, message sending will block. It 
might not be obvious to someone to understand why running a unit test just 
starts to block suddenly.


> Inject event recorder externally
> --------------------------------
>
>                 Key: YUNIKORN-2844
>                 URL: https://issues.apache.org/jira/browse/YUNIKORN-2844
>             Project: Apache YuniKorn
>          Issue Type: Improvement
>          Components: shim - kubernetes
>            Reporter: Peter Bacsko
>            Assignee: Peter Bacsko
>            Priority: Major
>
> The current implementation creates an event recorder like that:
> {noformat}
> func GetRecorder() events.EventRecorder {
>       lock.Lock()
>       defer lock.Unlock()
>       once.Do(func() {
>               // note, the initiation of the event recorder requires on a 
> workable Kubernetes client,
>               // in test mode we should skip this and just use a fake 
> recorder instead.
>               configs := conf.GetSchedulerConf()
>               if !configs.IsTestMode() {
>                       k8sClient := client.NewKubeClient(configs.KubeConfig)
>                       eventBroadcaster := 
> events.NewBroadcaster(&events.EventSinkImpl{
>                               Interface: k8sClient.GetClientSet().EventsV1()})
>                       eventBroadcaster.StartRecordingToSink(make(<-chan 
> struct{}))
>                       eventRecorder = 
> eventBroadcaster.NewRecorder(scheme.Scheme, constants.SchedulerName)
>               }
>       })
>       return eventRecorder
> }
> {noformat}
> The problem with this approach is that we need to indicate "test mode" in the 
> config, which just complicates things. 
> We can simplify this code if the recorder is set during Yunikorn 
> initialization in eg. {{NewShimScheduler()}}. The plugin code already does 
> this in {{NewSchedulerPlugin()}} and calls 
> {{events.SetRecorder(handle.EventRecorder())}}.
> We should also get rid of the default fake recorder. This uses a buffered 
> channel with the size of 1024. This isn't a problem now, but if a new test 
> somehow ends up generating a lot of events, message sending will block. It 
> might not be obvious to someone to understand why running a unit test just 
> starts to block suddenly.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to