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

Proposal: Target cluster dependent candidate/delegate pods #96

Open
everpeace opened this issue Dec 17, 2020 · 4 comments
Open

Proposal: Target cluster dependent candidate/delegate pods #96

everpeace opened this issue Dec 17, 2020 · 4 comments
Labels
enhancement New feature or request

Comments

@everpeace
Copy link

Currently, users can't create target dependent pod spec (e.g. nodeSelector or resources ).

For example, let's assume you would like to throw a job using V100 GPU with 32GB memory. But node labels are different in each target cluster because operating organization are different or similar reasons. For example,gpu-model: V100-32GB in "oregon" cluster, but, gpu-model: V100, gpu-memory: 32GB in "EU" and so on.

Supporting this situation, let me propose to introduce TargetPodTransformation CRD (there could be more appropriate names).

kind: TargetPodTransformation
metadata:
  name: use-v100-32G
transformations:
- targetReference: 
    kind: Target
    name: eu 
  patch: |
    # user can define inline strategic merge patch here
- targetRefernece: 
    kind: ClusterTarget
    name: oregon
  patch: |
    # user can define inline strategic merge patch here
---
kind: Pod
metadata:
  annotations: 
    multicluster.admiralty.io/elect: ""
    # user can define transformation by annotation
    multicluster.admiralty.io/targetpodtransformation: "use-v100-32G"
spec:
  # common spec here

Then, proxy scheduler can create target cluster dependent PodShaperons by using specified transformation.

What do you think??

@adrienjt
Copy link
Contributor

adrienjt commented Dec 17, 2020

Thank you @everpeace for submitting this proposal, I like it! You're not the only one requesting this feature, and this is an elegant solution.

  1. I'd add a way to refer to groups of targets by labels.
  2. Should the patch be scoped to the pod spec or should it be scoped to the whole object, including labels and annotations?
  3. Since you're introducing a CRD, I assume that you'd reuse transformations across different pod specs, is that right? Otherwise pod annotations would probably be enough.
  4. Would it make sense to have some transformations applied to all pods, or selected pods, without requiring the multicluster.admiralty.io/targetpodtransformation pod annotation? In other words, would it make sense to (optionally?) invert the dependency, with a podReference or podSelector in the CRD?

What do you think?

@everpeace
Copy link
Author

@adrienjt Thanks for your quick reply and sorry for my late reply.

  1. Since you're introducing a CRD, I assume that you'd reuse transformations across different pod specs, is that right? Otherwise pod annotations would probably be enough.

Yes. That's right.

  1. In other words, would it make sense to (optionally?) invert the dependency, with a podReference or podSelector in the CRD?

In my supposed use-case, that aims to absorb differences among target clusters, inverting the dependency might not make sense. For example, if use-v100-32G transformation was defined like below:

# perhaps we would define ClusterTargetPodTransformation (cluster scoped)
kind: TargetPodTransformation
metadata:
  name: use-v100-32G
transformations:
  # this defines podSpec to use V100 GPU(32GB) in each cluster(Target)

This transformation could be reusable from many pods. So, I feel specifying the transformation from pod is more natural instead of specifying pods from transformation.

  1. Should the patch be scoped to the pod spec or should it be scoped to the whole object, including labels and annotations?

I think it is worth to discuss!! I imagined podSpec was only the target at the first time but users sometimes might want to patch labels/annotations.

  1. I'd add a way to refer to groups of targets by labels.

As you might know, the result of patching strategic merge patches depends on the order of applying patches to the pod. So, label selector would not fit here because it just cannot define the order of transformations.

@adrienjt
Copy link
Contributor

Re 3. & 4. 👍

I also like the ClusterTargetPodTransformation.

Re 2. Okay, let's allow metadata patches then. It's a low-cost flexibility, both for the developer and the user. It might feel more natural also, cf. kubectl patch.

Re 1. I meant using labels to select which targets a transformation applies to, not which transformations a pod uses (I agree that a pod should refer to a transformation by name), e.g.:

kind: TargetPodTransformation
metadata:
  name: use-v100-32G
transformations:
- targetSelector: 
    region: eu
  patch: |
    # user can define inline strategic merge patch here

@everpeace
Copy link
Author

Re 1. I meant using labels to select which targets a transformation applies to

I misunderstood. targetSelector sounds great to me !!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants