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

fix: undefined: buildJmpDirective on arm64 machine #2805

Merged
merged 1 commit into from Jul 24, 2023

Conversation

lowang-bh
Copy link
Member

@lowang-bh lowang-bh commented Apr 19, 2023

buildJmpDirective not define on arm64 machine, eg, macbook m1/m2
update gomonkey to v2.2.0 or higher.

fix #2804

@volcano-sh-bot volcano-sh-bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Apr 19, 2023
Copy link
Member

@hwdef hwdef left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please make the CI happy

@lowang-bh
Copy link
Member Author

same error, it looks have nothing to do with this commit:

              Status: "Failure",
              Message: "Operation cannot be fulfilled on queues.scheduling.volcano.sh \"reclaim-q4\": the object has been modified; please apply your changes to the latest version and try again",
              Reason: "Conflict",

@hwdef
Copy link
Member

hwdef commented Apr 23, 2023

same error, it looks have nothing to do with this commit:

              Status: "Failure",
              Message: "Operation cannot be fulfilled on queues.scheduling.volcano.sh \"reclaim-q4\": the object has been modified; please apply your changes to the latest version and try again",
              Reason: "Conflict",

@wangyang0616 Cloud you please check this error?

@lowang-bh
Copy link
Member Author

lowang-bh commented Apr 23, 2023

without this commit, there are build errors on master branch when run unit-test even in docker:

➜  volcano git:(master) ✗ docker run --rm -it -v ./:/go/src/volcano.sh/ -w /go/src/volcano.sh/ golang:1.19.4 bash
root@c8a0428409b9:/go/src/volcano.sh# make unit-test
go clean -testcache
go test -p 8 -race $(find pkg cmd -type f -name '*_test.go' | sed -r 's|/[^/]+$||' | sort | uniq | sed "s|^|volcano.sh/volcano/|")
# github.com/agiledragon/gomonkey/v2
vendor/github.com/agiledragon/gomonkey/v2/patch.go:163:10: undefined: buildJmpDirective
ok  	volcano.sh/volcano/cmd/controller-manager/app/options	0.073s
ok  	volcano.sh/volcano/cmd/scheduler/app/options	0.034s
ok  	volcano.sh/volcano/pkg/cli/job	0.220s
ok  	volcano.sh/volcano/pkg/cli/queue	0.176s
ok  	volcano.sh/volcano/pkg/cli/util	0.265s
ok  	volcano.sh/volcano/pkg/cli/vcancel	0.098s
ok  	volcano.sh/volcano/pkg/cli/vresume	0.190s
ok  	volcano.sh/volcano/pkg/cli/vsuspend	0.177s
ok  	volcano.sh/volcano/pkg/controllers/apis	0.049s
ok  	volcano.sh/volcano/pkg/controllers/cache	0.044s
ok  	volcano.sh/volcano/pkg/controllers/garbagecollector	0.138s
FAIL	volcano.sh/volcano/pkg/controllers/job [build failed]
ok  	volcano.sh/volcano/pkg/controllers/job/helpers	0.132s
ok  	volcano.sh/volcano/pkg/controllers/job/plugins/distributed-framework/mpi	0.103s
ok  	volcano.sh/volcano/pkg/controllers/job/plugins/distributed-framework/pytorch	0.135s
ok  	volcano.sh/volcano/pkg/controllers/job/plugins/distributed-framework/tensorflow	0.128s
ok  	volcano.sh/volcano/pkg/controllers/job/plugins/ssh	0.142s
ok  	volcano.sh/volcano/pkg/controllers/podgroup	0.200s
ok  	volcano.sh/volcano/pkg/controllers/queue	0.193s
ok  	volcano.sh/volcano/pkg/controllers/util	0.210s
ok  	volcano.sh/volcano/pkg/scheduler	0.226s
FAIL	volcano.sh/volcano/pkg/scheduler/actions/allocate [build failed]
ok  	volcano.sh/volcano/pkg/scheduler/actions/preempt	0.147s
ok  	volcano.sh/volcano/pkg/scheduler/actions/reclaim	0.158s
ok  	volcano.sh/volcano/pkg/scheduler/actions/shuffle	0.097s
ok  	volcano.sh/volcano/pkg/scheduler/api	0.160s
ok  	volcano.sh/volcano/pkg/scheduler/api/devices/nvidia/gpushare	0.173s
ok  	volcano.sh/volcano/pkg/scheduler/api/helpers	0.217s
ok  	volcano.sh/volcano/pkg/scheduler/cache	0.159s
ok  	volcano.sh/volcano/pkg/scheduler/capabilities/volumebinding	29.356s
ok  	volcano.sh/volcano/pkg/scheduler/framework	0.284s
ok  	volcano.sh/volcano/pkg/scheduler/metrics/source	0.052s
ok  	volcano.sh/volcano/pkg/scheduler/plugins/binpack	0.117s
ok  	volcano.sh/volcano/pkg/scheduler/plugins/cdp	0.185s
FAIL	volcano.sh/volcano/pkg/scheduler/plugins/drf [build failed]
ok  	volcano.sh/volcano/pkg/scheduler/plugins/numaaware/policy	0.184s
ok  	volcano.sh/volcano/pkg/scheduler/plugins/numaaware/provider/cpumanager	0.135s
FAIL	volcano.sh/volcano/pkg/scheduler/plugins/predicates [build failed]
FAIL	volcano.sh/volcano/pkg/scheduler/plugins/proportion [build failed]
ok  	volcano.sh/volcano/pkg/scheduler/plugins/task-topology	0.135s
ok  	volcano.sh/volcano/pkg/scheduler/plugins/tdm	0.187s
ok  	volcano.sh/volcano/pkg/scheduler/util	0.150s
ok  	volcano.sh/volcano/pkg/webhooks/admission/jobs/mutate	0.225s
ok  	volcano.sh/volcano/pkg/webhooks/admission/jobs/plugins/mpi	0.121s
ok  	volcano.sh/volcano/pkg/webhooks/admission/jobs/validate	0.100s
ok  	volcano.sh/volcano/pkg/webhooks/admission/pods/mutate	0.188s
ok  	volcano.sh/volcano/pkg/webhooks/admission/pods/validate	0.095s
ok  	volcano.sh/volcano/pkg/webhooks/admission/queues/mutate	0.071s
ok  	volcano.sh/volcano/pkg/webhooks/admission/queues/validate	0.069s
FAIL
make: *** [Makefile:104: unit-test] Error 2
root@c8a0428409b9:/go/src/volcano.sh#

With this fix:

➜  volcano git:(fix_arm64_error) ✗ docker run --rm -it -v ./:/go/src/volcano.sh/ -w /go/src/volcano.sh/ golang:1.19.4 bash
root@9d6923527e0d:/go/src/volcano.sh# make unit-test
go clean -testcache
go test -p 8 -race $(find pkg cmd -type f -name '*_test.go' | sed -r 's|/[^/]+$||' | sort | uniq | sed "s|^|volcano.sh/volcano/|")
ok  	volcano.sh/volcano/cmd/controller-manager/app/options	0.063s
ok  	volcano.sh/volcano/cmd/scheduler/app/options	0.061s
ok  	volcano.sh/volcano/pkg/cli/job	0.175s
ok  	volcano.sh/volcano/pkg/cli/queue	0.146s
ok  	volcano.sh/volcano/pkg/cli/util	0.158s
ok  	volcano.sh/volcano/pkg/cli/vcancel	0.178s
ok  	volcano.sh/volcano/pkg/cli/vresume	0.330s
ok  	volcano.sh/volcano/pkg/cli/vsuspend	0.095s
ok  	volcano.sh/volcano/pkg/controllers/apis	0.031s
ok  	volcano.sh/volcano/pkg/controllers/cache	0.039s
ok  	volcano.sh/volcano/pkg/controllers/garbagecollector	0.091s
ok  	volcano.sh/volcano/pkg/controllers/job	0.625s
ok  	volcano.sh/volcano/pkg/controllers/job/helpers	0.164s
ok  	volcano.sh/volcano/pkg/controllers/job/plugins/distributed-framework/mpi	0.134s
ok  	volcano.sh/volcano/pkg/controllers/job/plugins/distributed-framework/pytorch	0.171s
ok  	volcano.sh/volcano/pkg/controllers/job/plugins/distributed-framework/tensorflow	0.239s
ok  	volcano.sh/volcano/pkg/controllers/job/plugins/ssh	0.185s
ok  	volcano.sh/volcano/pkg/controllers/podgroup	0.227s
ok  	volcano.sh/volcano/pkg/controllers/queue	0.151s
ok  	volcano.sh/volcano/pkg/controllers/util	0.129s
ok  	volcano.sh/volcano/pkg/scheduler	0.124s
ok  	volcano.sh/volcano/pkg/scheduler/actions/allocate	0.417s
ok  	volcano.sh/volcano/pkg/scheduler/actions/preempt	0.158s
ok  	volcano.sh/volcano/pkg/scheduler/actions/reclaim	0.226s
ok  	volcano.sh/volcano/pkg/scheduler/actions/shuffle	0.204s
ok  	volcano.sh/volcano/pkg/scheduler/api	0.293s
ok  	volcano.sh/volcano/pkg/scheduler/api/devices/nvidia/gpushare	0.215s
ok  	volcano.sh/volcano/pkg/scheduler/api/helpers	0.249s
ok  	volcano.sh/volcano/pkg/scheduler/cache	0.154s
ok  	volcano.sh/volcano/pkg/scheduler/capabilities/volumebinding	30.640s
ok  	volcano.sh/volcano/pkg/scheduler/framework	0.142s
ok  	volcano.sh/volcano/pkg/scheduler/metrics/source	0.055s
ok  	volcano.sh/volcano/pkg/scheduler/plugins/binpack	0.204s
ok  	volcano.sh/volcano/pkg/scheduler/plugins/cdp	0.126s
ok  	volcano.sh/volcano/pkg/scheduler/plugins/drf	0.158s
ok  	volcano.sh/volcano/pkg/scheduler/plugins/numaaware/policy	0.136s
ok  	volcano.sh/volcano/pkg/scheduler/plugins/numaaware/provider/cpumanager	0.186s
ok  	volcano.sh/volcano/pkg/scheduler/plugins/predicates	0.187s
ok  	volcano.sh/volcano/pkg/scheduler/plugins/proportion	0.179s
ok  	volcano.sh/volcano/pkg/scheduler/plugins/task-topology	0.111s
ok  	volcano.sh/volcano/pkg/scheduler/plugins/tdm	0.160s
ok  	volcano.sh/volcano/pkg/scheduler/util	0.131s
ok  	volcano.sh/volcano/pkg/webhooks/admission/jobs/mutate	0.136s
ok  	volcano.sh/volcano/pkg/webhooks/admission/jobs/plugins/mpi	0.200s
ok  	volcano.sh/volcano/pkg/webhooks/admission/jobs/validate	0.241s
ok  	volcano.sh/volcano/pkg/webhooks/admission/pods/mutate	0.119s
ok  	volcano.sh/volcano/pkg/webhooks/admission/pods/validate	0.095s
ok  	volcano.sh/volcano/pkg/webhooks/admission/queues/mutate	0.057s
ok  	volcano.sh/volcano/pkg/webhooks/admission/queues/validate	0.078s
root@9d6923527e0d:/go/src/volcano.sh# exit

Copy link
Member

@hwdef hwdef left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@volcano-sh-bot volcano-sh-bot added the lgtm Indicates that a PR is ready to be merged. label Apr 28, 2023
@lowang-bh
Copy link
Member Author

@Yikun Please help to review this pr, thanks.

@hwdef
Copy link
Member

hwdef commented May 8, 2023

/approve

@Yikun
Copy link
Member

Yikun commented May 8, 2023

/lgtm

@volcano-sh-bot volcano-sh-bot removed the lgtm Indicates that a PR is ready to be merged. label May 10, 2023
Copy link
Member

@hwdef hwdef left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@volcano-sh-bot volcano-sh-bot added the lgtm Indicates that a PR is ready to be merged. label May 10, 2023
@lowang-bh
Copy link
Member Author

Could this PR be merged now? The unit-test is dependent on it. @hwdef @Yikun

@hwdef
Copy link
Member

hwdef commented May 26, 2023

Could this PR be merged now? The unit-test is dependent on it. @hwdef @Yikun

I don't have permission to merge, currently there is only one person with permission to merge in Volcano, he is @william-wang

@lowang-bh
Copy link
Member Author

/priority important-soon

@lowang-bh
Copy link
Member Author

/assign @william-wang

@lowang-bh
Copy link
Member Author

image image

@volcano-sh-bot volcano-sh-bot removed the lgtm Indicates that a PR is ready to be merged. label Jun 3, 2023
Copy link
Member

@hwdef hwdef left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@volcano-sh-bot volcano-sh-bot added the lgtm Indicates that a PR is ready to be merged. label Jun 3, 2023
Signed-off-by: lowang_bh <lhui_wang@163.com>
@volcano-sh-bot volcano-sh-bot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jun 19, 2023
@lowang-bh
Copy link
Member Author

lowang-bh commented Jun 19, 2023

@william-wang would you please merge this with high priority, please? Developing on mac depend on it.
cc @k82cn

@hwdef
Copy link
Member

hwdef commented Jun 20, 2023

please make CI happy

@lowang-bh
Copy link
Member Author

trigger CI

@lowang-bh lowang-bh closed this Jun 21, 2023
@lowang-bh lowang-bh reopened this Jun 21, 2023
@hwdef
Copy link
Member

hwdef commented Jun 21, 2023

I think this CI is broken.

@lowang-bh
Copy link
Member Author

trigger ci

@lowang-bh lowang-bh closed this Jun 29, 2023
@lowang-bh lowang-bh reopened this Jun 29, 2023
@lowang-bh
Copy link
Member Author

lowang-bh commented Jul 23, 2023

@william-wang Can this fix be merge now? It has been hung up for a few months till now.

Copy link
Member

@hwdef hwdef left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@volcano-sh-bot volcano-sh-bot added the lgtm Indicates that a PR is ready to be merged. label Jul 23, 2023
@volcano-sh-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hwdef, Thor-wl

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@volcano-sh-bot volcano-sh-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 24, 2023
@volcano-sh-bot volcano-sh-bot merged commit 74375cc into volcano-sh:master Jul 24, 2023
30 of 33 checks passed
@lowang-bh lowang-bh deleted the fix_arm64_error branch July 25, 2023 14:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

gomonkey@v2.1.0 doesn't support arm64, need to upgrade to v2.2.0 or higher
6 participants