-
Notifications
You must be signed in to change notification settings - Fork 126
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: First draft of a scheduler #19
Conversation
7c6092c
to
b26f64d
Compare
15d3439
to
bc8054c
Compare
Signed-off-by: RealAnna <[email protected]>
Signed-off-by: RealAnna <[email protected]> scheduler fix Signed-off-by: RealAnna <[email protected]> changed make file Signed-off-by: RealAnna <[email protected]> changed make file Signed-off-by: RealAnna <[email protected]> changed make file Signed-off-by: RealAnna <[email protected]> changed make file Signed-off-by: RealAnna <[email protected]> mit Signed-off-by: RealAnna <[email protected]> mit Signed-off-by: RealAnna <[email protected]> mit Signed-off-by: RealAnna <[email protected]> mit Signed-off-by: RealAnna <[email protected]> sched fin Signed-off-by: RealAnna <[email protected]> sched fin Signed-off-by: RealAnna <[email protected]> sched fin Signed-off-by: RealAnna <[email protected]>
bc8054c
to
2dbf6a6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would remove the as-a-second-scheduler part and make this the standard as currently the keptn scheduler won't replace another one ... Furthermore I'm not sure how many things of the boilerplate are really needed (/hack). The rest looks good like a good start 🚀
lfc-scheduler/Makefile
Outdated
# RELEASE_REGISTRY is the container registry to push | ||
# into. The default is to push to the staging | ||
# registry, not production(k8s.gcr.io). | ||
RELEASE_REGISTRY?=docker.io/annadreal |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be keptnsandbox?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we have one or should we make it? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I commented it out for now, we will make one soon
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think keptnsandbox already exists ...
|
||
rand.Seed(time.Now().UnixNano()) | ||
command := app.NewSchedulerCommand( | ||
app.WithPlugin(klcpermit.Name, klcpermit.New), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think it would make sense to hook in an earlier stage? (https://kubernetes.io/docs/concepts/scheduling-eviction/scheduling-framework/)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my mind pre-deployment checks could take a while and it would not make sense to not discover in the meantime that there are other reasons for the pod not to be scheduled e.g. filters make it impossible to find a compatible node. We discussed having prechecks and scheduling steps in parallel up to resource allocation, but maybe also pre-Score could be an option, just I could not think of a reason why we would like to interfere with the node choice. What do you think could be another approach?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, think it makes sense to leave this in the permit-phase at the moment :-)
Signed-off-by: RealAnna <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should adapt the Helm-Chart readme, then we are good to go 👍
lfc-scheduler/manifests/install/charts/keptn-scheduler/README.md
Outdated
Show resolved
Hide resolved
lfc-scheduler/manifests/install/charts/keptn-scheduler/README.md
Outdated
Show resolved
Hide resolved
lfc-scheduler/manifests/install/charts/keptn-scheduler/README.md
Outdated
Show resolved
Hide resolved
Signed-off-by: RealAnna <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Resolves: #5
edit make file and scheduler image with your own registry
to run in parallel to the default scheduler
helm upgrade --install keptn-scheduler keptn-scheduler/
wait for it to be up
deploy something on the scheduler
eg.
test deployment available on keptn-dev cluster