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

Admission webhook - hierarchical queues can not have hierarchy parts that are a substring [0:i] of other queue hierarchy. #3759

Open
PigNatovsky opened this issue Oct 1, 2024 · 3 comments
Assignees
Labels
kind/bug Categorizes issue or PR as related to a bug.

Comments

@PigNatovsky
Copy link

Description

If I create two queues like these:

---
apiVersion: scheduling.volcano.sh/v1beta1
kind: Queue
metadata:
  name: root-namewithsuffix
  annotations:
    "volcano.sh/hierarchy": "root/namewithsuffix"
    "volcano.sh/hierarchy-weights": "1/1"
spec:
  reclaimable: true
---
apiVersion: scheduling.volcano.sh/v1beta1
kind: Queue
metadata:
  name: root-namewith
  annotations:
    "volcano.sh/hierarchy": "root/namewith"
    "volcano.sh/hierarchy-weights": "1/1"
spec:
  reclaimable: true

Admission webhook throws:

Error from server: error when creating "./local/queues-hdrf.yaml": admission webhook "validatequeue.volcano.sh" denied the request: requestBody.metadata.annotations: Invalid value: "root/namewith": root/namewith is not allowed to be in the sub path of root/namewithsuffix of queue root-namewithsuffix

Steps to reproduce the issue

  1. Create queue with an annotation: "volcano.sh/hierarchy": "root/namewithsuffix"
  2. Try to create a queue with an annotation: "volcano.sh/hierarchy": "root/namewith"

Describe the results you received and expected

As mentioned, the result is a queue revocation done by the admission webhook. My expectation is that I should be able to create queues like root/someproject and root/someprojectdev.

What version of Volcano are you using?

1.9.0

Any other relevant information

I can pick up this issue, because currently I'm working on admission webhook in our internal env.

@PigNatovsky PigNatovsky added the kind/bug Categorizes issue or PR as related to a bug. label Oct 1, 2024
@PigNatovsky
Copy link
Author

/assign

@PigNatovsky
Copy link
Author

PigNatovsky commented Oct 1, 2024

I think that issue is here:

strings.HasPrefix(hierarchyInTree, hierarchy) {

We're just checking if our annotation hierarchy is a substring of other hierarchy, but it's not always a good check, because hierarchy may be at the same length, but with name that contains substring.

@PigNatovsky
Copy link
Author

PigNatovsky commented Oct 1, 2024

Simple solution: just add a "/" at the end of hierarchy before check.

Example 1:
Queue in the cluster: "root/somename/suffixdev" (because first one was a dev env for this project)
New queue: "root/somename/suffix" (time for a prd queue)

hierarchyInTree: "root/somename/suffixdev"
hierarchy: "root/somename/suffix"

How it works now?
strings.HasPrefix("root/somename/suffixdev", "root/somename/suffix") gives true which throws an error.

How it will work?
strings.HasPrefix("root/somename/suffixdev", "root/somename/suffix/") gives false and accepts queue.

Example 2 (from code comment):
Queue in the cluster: "root/sci/dev"
New queue: "root/sci"

hierarchyInTree: "root/sci/dev"
hierarchy: "root/sci"

How it works now?
strings.HasPrefix("root/sci/dev", "root/sci") gives true and throws an error.

How it will work?
strings.HasPrefix("root/sci/dev", "root/sci") gives true and throws an error (and it should).

@PigNatovsky PigNatovsky changed the title Admission webhook - hierarchical queues can not have hierarchy parts that are a substring [0:i] of other queue name. Admission webhook - hierarchical queues can not have hierarchy parts that are a substring [0:i] of other queue hierarchy. Oct 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

No branches or pull requests

1 participant