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

openmpi: namespaced resource names should be prefixed with component name #698

Conversation

everpeace
Copy link
Contributor

@everpeace everpeace commented Apr 20, 2018

openmpi package currently creates fixed named pods which are openmpi-master, openmpi-worker-n, and openmpi-assets. This prevents users from creating multiple openmpi components in one namespace.

I think prefixing component name to namespaced resource names would be preferred.

$ COMPONENT=c1
$ ks generate openmpi ${COMPONENT} --image ${IMAGE} --secret ${SECRET} --workers ${WORKERS} --gpus ${GPUS} --exec "${EXEC}"
$ COMPONENT=c2
$ ks generate openmpi ${COMPONENT} --image ${IMAGE} --secret ${SECRET} --workers ${WORKERS} --gpus ${GPUS} --exec "${EXEC}"
$ ks apply default
...

$ k get po,svc,cm --show-all 
NAME             READY     STATUS    RESTARTS   AGE
po/c1-master     1/1       Running   0          12s
po/c1-worker-0   1/1       Running   0          11s
po/c1-worker-1   1/1       Running   0          11s
po/c2-master     1/1       Running   0          12s
po/c2-worker-0   1/1       Running   0          12s
po/c2-worker-1   1/1       Running   0          11s
NAME      TYPE        CLUSTER-IP   EXTERNAL-IP   PORT(S)     AGE
svc/c1    ClusterIP   None         <none>        12345/TCP   13s
svc/c2    ClusterIP   None         <none>        12345/TCP   13s
NAME           DATA      AGE
cm/c1-assets   5         13s
cm/c2-assets   5         13s

This change is Reviewable

…name so that users can create multiple openmpi prototypes in one namespace
@everpeace
Copy link
Contributor Author

/cc @jiezhang @pdmack Please bear with me for asking so many times but could you review this, too? 🙇

@pdmack
Copy link
Member

pdmack commented Apr 20, 2018

@everpeace Going forward, I'll probably need to see sign-off from both you and @jiezhang. This is an area I have no particular insight into, but am happy to add "ok to test" at least.

@jiezhang
Copy link

Why not simply use namespace to scope the resources?

@everpeace
Copy link
Contributor Author

everpeace commented Apr 20, 2018

@jiezhang It is because that there is a case which some users are not allowed to create namespace freely by cluster administrators. As you might know that kubernetes has RBAC and RBAC can do that, right? and also, Namespace is the unit of resource quota in kubernetes.

So, I think it's a natural way for administrators to restrict users creating namespaces and they would probably assign specific namespace to each user/team and set quotas to them when multiple users/teams share a cluster with the fixed number of nodes (please imagine on-premise environment).

@everpeace
Copy link
Contributor Author

@pdmack Thank you for the comment. I understood.

@jiezhang
Copy link

Per the documentation https://kubernetes.io/docs/concepts/overview/working-with-objects/namespaces/ it sounds like namespace is the standard way to separate different users. And it supports resource quota.

We plan to run this package in an external service so that end user doesn’t need to have any k8s setup or permission. So scoping each user/job in its own namespace is feasible for Uber’s use case.

@jlewi What’s your recommendation?

@jiezhang
Copy link

/assign @jlewi

@jlewi
Copy link
Contributor

jlewi commented Apr 23, 2018

In general we want to be able to run multiple jobs in a namespace. So we shouldn't need to create a new namespace for each job.

Can multiple jobs share the same openmpi resources? If not then we should probably bring up a distinct set of resources for each job by making the name unique.

I will send you invites to the org so you can run tests.

@jlewi
Copy link
Contributor

jlewi commented Apr 23, 2018

The approach in this PR. Of giving each resource a unique name based on the name of the job is consistent with what we do for other jobs (e.g. TFJobs).

So I think this is the correct think to do.

/lgtm
/approve

/hold @jiezhang

/ok-to-test

@jiezhang When you are statisfied you can do "/hold cancel"

/assign @jiezhang

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jlewi

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

@k8s-ci-robot k8s-ci-robot merged commit a456bac into kubeflow:master Apr 24, 2018
saffaalvi pushed a commit to StatCan/kubeflow that referenced this pull request Feb 11, 2021
…name so that users can create multiple openmpi prototypes in one namespace (kubeflow#698)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants