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.

Sign up for GitHub

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

Jump to bottom

Update API for 1.31 without partitionable model #36

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Update API for 1.31 without partitionable model #36

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fbc7e07
Update API for 1.31 without partitionable model
johnbelamaric Jun 18, 2024
3a7aa4f
Replace all common things with a single discriminator
johnbelamaric Jun 18, 2024
6187d27
Remove left over comments
johnbelamaric Jun 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 46 additions & 32 deletions dra-evolution/pkg/api/capacity_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,14 +59,11 @@ type ResourcePoolSpec struct {
// vendor of the driver.
DriverName string `json:"driverName" protobuf:"bytes,3,name=driverName"`

// SharedCapacity defines the set of shared capacity consumable by
// devices in this ResourceSlice.
// CommonData represents information that is common across various devices,
// to reduce redundancy in the devices list.
//
// Must not have more than 128 entries.
//
// +listType=atomic
// +optional
SharedCapacity []SharedCapacity `json:"sharedCapacity,omitempty"`
CommonData []CommonDeviceData `json:"commonData,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't include this. If we are ripping out the model that supports partitioning, I think we should should just go back to an explicit list of attributes per device.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need an extension point to insert the things we are debating in #20. That's the primary reason that is here. Adding the common things was just a way to make it have some meaning and not be a stub. With this, an old scheduler an see that there is an entry in this list, but when it unmarshals it, all the fields the scheduler is aware of are nil, so it knows "ooo, I should ignore this slice, I don't know what to do with it".

If we don't have this, then we need some other way for an older scheduler to know that "it doesn't understand this object".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The alternative would be to have a discriminator field, like SliceContent: "SimpleDevices" or something. Then we would add new values to that so old schedulers know if they should handle the slice or not.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My current thinking is that we would not have any extension points directly in the device at this point. Instead every device will have the ability to point to one (and exactly one) DeviceShape/DeviceTemplate. That is what will have all of these extra bells and whistles. The device itself is still just a named list of attributes (and now capacities).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, that works for me. The thing is, we still need a way for old schedulers to know "this slice is not something I understand". So a "SliceType" discriminator perhaps? Or we have to have a field like I have done here, that has a one-of.


// Devices lists all available devices in this pool.
//
Expand All @@ -78,37 +75,70 @@ type ResourcePoolSpec struct {
// them) empty pool.
}

const ResourcePoolMaxSharedCapacity = 128
const ResourcePoolMaxCommonData = 16
const ResourcePoolMaxDevices = 128

type CommonDeviceData struct {
// Name identifies the particular set of common data represented
// here.
//
// +required
Name string `json:"name"`

// Exactly one must be populated
Attributes []DeviceAttribute
Capacity []DeviceCapacity

// Future possible additions:
// - SharedCapacity for use in allocating partitionable devices
// - DeviceShape for encapsulating attributes, capacities, partion schemes
// - DeviceShapeRef for storing those in a secondary object
}

// Device represents one individual hardware instance that can be selected based
// on its attributes.
type Device struct {
// Name is unique identifier among all devices managed by
// the driver on the node. It must be a DNS label.
Name string `json:"name" protobuf:"bytes,1,name=name"`

// CommonData is the list of names of entries from the common data that
// should be included in this device.
//
// +listType=atomic
// +optional
klueska marked this conversation as resolved.
Show resolved Hide resolved
//
CommonData []string `json:"commonData,omitempty"`
klueska marked this conversation as resolved.
Show resolved Hide resolved

// Attributes defines the attributes of this device.
// The name of each attribute must be unique.
//
// Must not have more than 32 entries.
//
// Values in this list whose name conflict with common attributes
// will override the common attribute values.
//
klueska marked this conversation as resolved.
Show resolved Hide resolved
// +listType=atomic
// +optional
//
Attributes []DeviceAttribute `json:"attributes,omitempty" protobuf:"bytes,3,opt,name=attributes"`

// SharedCapacityConsumed defines the set of shared capacity consumed by
// this device.
// Capacity defines the capacity values for this device.
// The name of each capacity must be unique.
//
// Must not have more than 32 entries.
//
// Values in this list whose name conflict with common capacity
// will override the common capacity values.
//
klueska marked this conversation as resolved.
Show resolved Hide resolved
// +listType=atomic
// +optional
SharedCapacityConsumed []SharedCapacity `json:"sharedCapacityConsumed,omitempty"`
//
Capacity []DeviceCapacity `json:"capacity,omitempty"`
}

const ResourcePoolMaxAttributesPerDevice = 32
const ResourcePoolMaxSharedCapacityConsumedPerDevice = 32
const ResourcePoolMaxCapacityPerDevice = 32

// ResourcePoolMaxDevices and ResourcePoolMaxAttributesPerDevice where chosen
// so that with the maximum attribute length of 96 characters the total size of
Expand Down Expand Up @@ -154,40 +184,24 @@ type DeviceAttribute struct {
VersionValue *string `json:"version,omitempty" protobuf:"bytes,5,opt,name=version"`
}

type SharedCapacity struct {
// Name is a unique identifier among all shared capacities managed by the
type DeviceCapacity struct {
// Name is a unique identifier among all capacities managed by the
// driver in the pool.
//
// It is referenced both when defining the total amount of shared capacity
// that is available, as well as by individual devices when declaring
// how much of this shared capacity they consume.
//
// SharedCapacity names must be a C-style identifier (e.g. "the_name") with
// a maximum length of 32.
//
// By limiting these names to a C-style identifier, the same validation can
// be used for both these names and the identifier portion of a
// DeviceAttribute name.
//
// +required
Name string `json:"name"`

// Capacity is the total capacity of the named resource.
// This can either represent the total *available* capacity, or the total
// capacity *consumed*, depending on the context where it is referenced.
//
// +required
Capacity resource.Quantity `json:"capacity"`
johnbelamaric marked this conversation as resolved.
Show resolved Hide resolved
}

// CStyleIdentifierMaxLength is the maximum length of a c-style identifier used for naming.
const CStyleIdentifierMaxLength = 32

// DeviceAttributeMaxIDLength is the maximum length of the identifier in a device attribute name (`<domain>/<ID>`).
const DeviceAttributeMaxIDLength = CStyleIdentifierMaxLength
const DeviceAttributeMaxIDLength = 32

// DeviceAttributeMaxValueLength is the maximum length of a string or version attribute value.
const DeviceAttributeMaxValueLength = 64

// SharedCapacityMaxNameLength is the maximum length of a shared capacity name.
const SharedCapacityMaxNameLength = CStyleIdentifierMaxLength
// DeviceCapacityMaxNameLength is the maximum length of a shared capacity name.
const DeviceCapacityMaxNameLength = DeviceAttributeMaxIDLength