[YUNIKORN-2685] Use the newer WaitForCondition() in shim test

[mean-world]

What is this PR for?

shim completly use WaitForCondition function from yunikorn-core
also remove E2E test about Dao.allocation.partition based on YUNIKORN-2593

What type of PR is it?

• - Bug Fix
• - Improvement
• - Feature
• - Documentation
• - Hot Fix
• - Refactoring

Todos

• - Task

What is the Jira issue?

https://issues.apache.org/jira/browse/YUNIKORN-2685

How should this be tested?

Screenshots (if appropriate)

Questions:

• - The licenses files need update.
• - There is breaking changes for older versions.
• - It needs documentation.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 67.30%. Comparing base (24efbed) to head (c831256).

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #858      +/-   ##
==========================================
+ Coverage   67.16%   67.30%   +0.13%     
==========================================
  Files          70       70              
  Lines        7639     7631       -8     
==========================================
+ Hits         5131     5136       +5     
+ Misses       2289     2277      -12     
+ Partials      219      218       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[pbacsko]

cc @wilfred-s @craigcondit do we ever import stuff from the core directly? We do it for the deadlock detection, but apart from that, I haven't seen this practice anywhere.

[chenyulin0719]

@mean-world Sorry I wasn't aware that there is a similar function in shim's utils.

For me, I would vote to align the implementation of WaitForCondition(), but keep two separate function in shim and core.
I don't think code reuse is necessary here.

@pbacsko, @chia7712 WDYT?

[chia7712]

do we ever import stuff from the core directly?

there are use case already.

yunikorn-k8shim/pkg/cache/task_test.go

Line 203 in 24efbed

err = common.WaitFor(100*time.Millisecond, 3*time.Second, func() bool {

yunikorn-k8shim/pkg/client/clients_test.go

Line 61 in 24efbed

err := common.WaitForCondition(func() bool {

yunikorn-k8shim/pkg/client/clients_test.go

Line 68 in 24efbed

err = common.WaitForCondition(func() bool {

The core and k8shim are different repo, so it will be a issue in aligning the version, but reusing the utils methods is always good to me.

[craigcondit]

We shouldn't share code between core and shim. The scheduler interface is meant to be the common point. There are exceptions for some low-level functionality like locks and logging, but that's out of necessity. Tightly coupling the two projects just creates friction and makes refactoring more difficult.

The other examples just given should probably be fixed to not use core code.

[chia7712]

The other examples just given should probably be fixed to not use core code.

that sounds good to me. Maybe we can have another jira to cleanup the usage of core code.

[pbacsko]

We're on the same page. This PR can be closed and a new ticket should eliminate the existiting core imports where it's possible.

@mean-world please create a new JIRA and close this one.

[mean-world]

We're on the same page. This PR can be closed and a new ticket should eliminate the existiting core imports where it's possible.

@mean-world please create a new JIRA and close this one.

I already create new JIRA
https://issues.apache.org/jira/browse/YUNIKORN-2690