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.

Sign up for GitHub

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

Jump to bottom

Add one click install script #345

Merged
merged 6 commits into from
Jul 19, 2019
Merged

Add one click install script #345

merged 6 commits into from
Jul 19, 2019

Conversation

hzxuzhonghu
Copy link
Collaborator

Added local-up-volcano.sh, developers can
Install kind cluster and then install volcano by

$ ./hack/local-up-volcano.sh

Install install volcano into a existing cluster

$ export INSTALL_MODE="xx" // set this env to a value not equals kind
$ ./hack/local-up-volcano.sh

@hzxuzhonghu
Copy link
Collaborator Author

/assign @TommyLike

@volcano-sh-bot volcano-sh-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jul 17, 2019
k82cn
k82cn reviewed Jul 17, 2019
View reviewed changes
hack/local-up-volcano.sh Outdated
# TODO: add a graceful way waiting for all crd ready
kubectl apply -f ${RELEASE_FOLDER}/${YAML_FILENAME} --kubeconfig ${KUBECONFIG}
sleep 5
kubectl apply -f ${VK_ROOT}/installer/helm/chart/volcano/templates/default-queue.yaml --kubeconfig ${KUBECONFIG}
Copy link
Member

Choose a reason for hiding this comment

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

just have a retry should be enough :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I want to make the default-queue install into the scheduler if possible.

@k82cn
Copy link
Member

k82cn commented Jul 17, 2019

/approve

@volcano-sh-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hzxuzhonghu, k82cn

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 17, 2019
@TommyLike
Copy link
Contributor

@hzxuzhonghu It's better to update/add document along with this change, otherwise we need another one after this.

TommyLike
TommyLike reviewed Jul 17, 2019
View reviewed changes
hack/local-up-volcano.sh

prepare

if [ "${INSTALL_MODE}" == "kind" ]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

For the clusters other than kind cluster, how can user deploy the images into the cluster?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

good question: we can not know their registry. If they use one node development env, the image is on the host. But if it is a multi-node cluster, we can not do that for users.

Copy link
Contributor

Choose a reason for hiding this comment

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

Need to mention this in the document or just support kind cluster, since for the other cases, they can deploy volcano via the installation guide.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

SG

@hzxuzhonghu
Copy link
Collaborator Author

It's better to update/add document along with this change, otherwise we need another one after this.

Yes, but i am not sure where to put it. Any suggestion?

@TommyLike
Copy link
Contributor

It's better to update/add document along with this change, otherwise we need another one after this.

Yes, but i am not sure where to put it. Any suggestion?

Add a development section or individual document would be ok.

kevin-wangzefeng
kevin-wangzefeng reviewed Jul 18, 2019
View reviewed changes
Copy link
Member

@kevin-wangzefeng kevin-wangzefeng left a comment

Choose a reason for hiding this comment

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

I can see some functions repeated those in run-e2e-kind.sh, better move them to a common file to reduce duplicated code.

@hzxuzhonghu
Copy link
Collaborator Author

I can see some functions repeated those in run-e2e-kind.sh, better move them to a common file to reduce duplicated code.

Will do

@hzxuzhonghu
Copy link
Collaborator Author

hzxuzhonghu commented Jul 18, 2019 •
edited
Loading

@hzxuzhonghu It's better to update/add document along with this change, otherwise we need another one after this.

@SrinivasChilveri Could you help

EDIT: this is done

@hzxuzhonghu hzxuzhonghu mentioned this pull request Jul 18, 2019
Merged
@hzxuzhonghu
Copy link
Collaborator Author

ping @TommyLike ptal

TommyLike
TommyLike reviewed Jul 18, 2019
View reviewed changes
pkg/scheduler/cache/cache.go
Weight: 1,
},
}
if _, err := kbClient.SchedulingV1alpha2().Queues().Create(&defaultQue); err != nil && !apierrors.IsAlreadyExists(err) {
Copy link
Contributor

Choose a reason for hiding this comment

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

What if I have a existing default-queue from the previous version of volcano?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Does not matter

@TommyLike
Copy link
Contributor

ping @TommyLike ptal

https://github.com/volcano-sh/volcano/blob/master/hack/publish.sh#L52 and https://github.com/volcano-sh/volcano/blob/master/installer/README.md both need update since the default-queue is not required now.

@hzxuzhonghu
Copy link
Collaborator Author

This can be a follow up.

@TommyLike
Copy link
Contributor

/lgtm

@volcano-sh-bot volcano-sh-bot added the lgtm Indicates that a PR is ready to be merged. label Jul 19, 2019
@volcano-sh-bot volcano-sh-bot merged commit 4492e66 into volcano-sh:master Jul 19, 2019
@hzxuzhonghu hzxuzhonghu deleted the one-click-install branch July 19, 2019 02:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@k82cn k82cn k82cn left review comments

@kevin-wangzefeng kevin-wangzefeng kevin-wangzefeng left review comments

@TommyLike TommyLike TommyLike left review comments

@quinton-hoole quinton-hoole Awaiting requested review from quinton-hoole

Assignees

@TommyLike TommyLike

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/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@hzxuzhonghu @k82cn @volcano-sh-bot @TommyLike @kevin-wangzefeng