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

feat: support vc-scheduler loading custom plugins (#1215) #1218

Merged
merged 11 commits into from
Dec 25, 2020
Merged

feat: support vc-scheduler loading custom plugins (#1215) #1218

merged 11 commits into from
Dec 25, 2020

Conversation

zen-xu
Copy link
Contributor

@zen-xu zen-xu commented Dec 23, 2020

Signed-off-by: Xu ZhengYu zen-xu@outlook.com

@volcano-sh-bot volcano-sh-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Dec 23, 2020
Signed-off-by: Xu ZhengYu <zen-xu@outlook.com>
Signed-off-by: ZhengYu, Xu <zen-xu@outlook.com>
cmd/scheduler/app/options/options.go Outdated Show resolved Hide resolved
cmd/scheduler/app/options/options.go Show resolved Hide resolved
cmd/scheduler/app/server.go Show resolved Hide resolved
Signed-off-by: Xu ZhengYu <zen-xu@outlook.com>
Signed-off-by: Xu ZhengYu <zen-xu@outlook.com>
We will use musl-gcc to build scheduler, so it can run in `alpine` when CGO enabled

Signed-off-by: Xu ZhengYu <zen-xu@outlook.com>
Signed-off-by: Xu ZhengYu <zen-xu@outlook.com>
Signed-off-by: Xu ZhengYu <zen-xu@outlook.com>
@volcano-sh-bot volcano-sh-bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Dec 24, 2020
@zen-xu zen-xu closed this Dec 24, 2020
@zen-xu zen-xu reopened this Dec 24, 2020
cmd/scheduler/app/server.go Outdated Show resolved Hide resolved
# Build plugin

## In docker
Plugin must use `musl-gcc` to build. The best choose is build it in the docker.
Copy link
Member

Choose a reason for hiding this comment

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

hm... IMO, the compiler of plugin and scheduler should be the same; as some users may build scheduler themself.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

because we use alpine as base image,which only has musl-libc

Copy link
Member

Choose a reason for hiding this comment

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

yes, that's ok in our case. But if someone want to build scheduler themself, it's better to highlight that; or add add a reference about the limitation of Plugin.

@k82cn
Copy link
Member

k82cn commented Dec 24, 2020

LGTM overall :)

Signed-off-by: Xu ZhengYu <zen-xu@outlook.com>
Signed-off-by: Xu ZhengYu <zen-xu@outlook.com>
Signed-off-by: Xu ZhengYu <zen-xu@outlook.com>
Signed-off-by: Xu ZhengYu <zen-xu@outlook.com>
@volcano-sh-bot volcano-sh-bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Dec 25, 2020
@k82cn
Copy link
Member

k82cn commented Dec 25, 2020

/lgtm
/approve

@volcano-sh-bot volcano-sh-bot added the lgtm Indicates that a PR is ready to be merged. label Dec 25, 2020
@volcano-sh-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: k82cn, zen-xu

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 Dec 25, 2020
@volcano-sh-bot volcano-sh-bot merged commit d5ee644 into volcano-sh:master Dec 25, 2020
done

# scheduler need cgo enabled to support plugin
CC=${CC} CGO_ENABLED=1 $(GOBIN)/gox -osarch=${REL_OSARCH} -ldflags ${LD_FLAGS} -output ${BIN_DIR}/${REL_OSARCH}/vc-scheduler ./cmd/scheduler;
Copy link
Member

Choose a reason for hiding this comment

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

hi @zen-xu , it will cause some problem using gcc with CGO enabled, l think it will be better to specify the musl-gcc in Makefile.

FYI: l just ran make images directly to build images and there's something wrong.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

what's the wrong message?

Copy link
Member

Choose a reason for hiding this comment

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

standard_init_linux.go:211: exec user process caused "no such file or directory"

It should be a cross compilation problem with CGO enabled

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, I know the reason

Copy link
Contributor Author

@zen-xu zen-xu Dec 28, 2020

Choose a reason for hiding this comment

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

Since CI use ubuntu, so I think scheduler should also use ubuntu.

Copy link
Member

@shinytang6 shinytang6 Dec 28, 2020

Choose a reason for hiding this comment

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

Makefile is not only used by CI, but users on different platforms may use it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you use gcc build the scheduler, you should promise your container has gnu-libc
If you use musl-gcc build the scheduler, you should promise your container has musl-libc

Copy link
Member

Choose a reason for hiding this comment

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

But this is a strong assumption. The compilation method needs to be changed according to the environment in which the image is deployed. E.g. I am currently on ubuntu, and I just want to compile vc-scheduler image(with alpine as basic image) through make images, but it doesn't work and you don't even tell me to use musl-gcc

Copy link
Contributor Author

@zen-xu zen-xu Dec 28, 2020

Choose a reason for hiding this comment

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

I have written a design doc, But has not been merged yet.

Copy link
Member

Choose a reason for hiding this comment

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

okay, now l got it.. but it is still confusing if someone doesn't use the Custom Plugin need to know that)

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. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants