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

dra-evolution: simplify CEL syntax #23

Open
pohly opened this issue Jun 5, 2024 · 6 comments
Open

dra-evolution: simplify CEL syntax #23

pohly opened this issue Jun 5, 2024 · 6 comments
Assignees
Labels
lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness.

Comments

@pohly
Copy link
Contributor

pohly commented Jun 5, 2024

#14 uses device.attributes[<attribute name>] and device.<type>Attributes[<attribute name>], with defaults (nil for attributes, null value for <type>Attributes. This works and has been prototyped in dra-evolution/pkg/schedule.

There are different ideas how that syntax could be simplified. See #14 (comment) (long discussion!) and https://docs.google.com/document/d/1i0xVx884vsWzVaVb72LtEqjsVILmNkeZimAyz4Bj-m0/edit?usp=sharing (summary of different options).

/assign @johnbelamaric @thockin

@johnbelamaric
Copy link
Contributor

Based on the discussion in the doc linked in the issue description, I believe for 1.31 we have settle on:

device.attributes["example.com/foo"]

Last point, though:

Do we need device., or would just attributes be OK? Originally we had device. because we wanted to do short names like device.model which is very readable. We also wanted to leave space in case we ever wanted things like node..

Now, we are qualifying attribute names with a driver domain. While we did discuss inferring the qualifying domain from the device's driver, this will limit some tricks we probably want to play - see #27 (comment).

Given this I think we may want to always require the attribute names to be fully qualified, rather than allowing short names at all. In that case, I see no reason why we shouldn't just use

attributes["example.com/foo"]

which is a little more compact.

@pohly
Copy link
Contributor Author

pohly commented Jun 11, 2024

My thinking was that we might have CEL expressions on the future for a collection of devices (the constraints), in which case an array called devices might make its appearance and mirror the role of the singular device.

We also have device.driverName, which is not an attribute. It seemed nicer to group that and the attributes together.

Another reason is that for someone not familiar with DRA, device.attributes gives some hint what these attributes are about.

The downside, as you said, is that it is longer.

@johnbelamaric
Copy link
Contributor

Ok, I think this is changeable at implementation time anyway. We should discuss further but not gate the KEP on it.

@pohly
Copy link
Contributor Author

pohly commented Jun 17, 2024

As mentioned in kubernetes/enhancements#4709 (comment)), we can also consider abbreviations (dev.attribs).

@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue as fresh with /remove-lifecycle stale
  • Close this issue with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 15, 2024
@pohly
Copy link
Contributor Author

pohly commented Sep 16, 2024

/remove-lifecycle stale
/lifecycle frozen

@k8s-ci-robot k8s-ci-robot added lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Sep 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness.
Projects
None yet
Development

No branches or pull requests

5 participants