Refactors InferencePool v1 Status and FailureModes #1427
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -35,17 +35,18 @@ | |
| // +optional | ||
| metav1.ObjectMeta `json:"metadata,omitempty"` | ||
|
|
||
| // Spec defines the desired state of the InferencePool. | ||
| // | ||
| // +required | ||
| Spec InferencePoolSpec `json:"spec,omitzero"` | ||
|
|
||
| // Status defines the observed state of the InferencePool. | ||
| // | ||
| // +optional | ||
| Status InferencePoolStatus `json:"status,omitempty"` | ||
|
Check failure on line 46 in api/v1/inferencepool_types.go
|
||
| } | ||
|
|
||
| // InferencePoolList contains a list of InferencePools. | ||
| // | ||
| // +kubebuilder:object:root=true | ||
| type InferencePoolList struct { | ||
|
|
@@ -54,7 +55,7 @@ | |
| Items []InferencePool `json:"items"` | ||
| } | ||
|
|
||
| // InferencePoolSpec defines the desired state of the InferencePool. | ||
| type InferencePoolSpec struct { | ||
| // Selector determines which Pods are members of this inference pool. | ||
| // It matches Pods by their labels only within the same namespace; cross-namespace | ||
|
|
@@ -65,19 +66,22 @@ | |
| // this configuration into a Service resource. | ||
| // | ||
| // +required | ||
| Selector LabelSelector `json:"selector,omitzero"` | ||
|
|
||
| // TargetPorts defines a list of ports that are exposed by this InferencePool. | ||
| // Currently, the list may only include a single port definition. | ||
| // | ||
| // +kubebuilder:validation:MinItems=1 | ||
| // +kubebuilder:validation:MaxItems=1 | ||
| // +listType=atomic | ||
| // +required | ||
| TargetPorts []Port `json:"targetPorts,omitzero"` | ||
|
|
||
| // EndpointPickerRef is a reference to the Endpoint Picker extension and its | ||
| // associated configuration. | ||
| // | ||
| // +required | ||
| EndpointPickerRef EndpointPickerRef `json:"endpointPickerRef,omitzero"` | ||
| } | ||
|
|
||
| // Port defines the network port that will be exposed by this InferencePool. | ||
|
|
@@ -86,21 +90,22 @@ | |
| // The number must be in the range 1 to 65535. | ||
| // | ||
| // +required | ||
| Number PortNumber `json:"number,omitzero"` | ||
| } | ||
|
|
||
| // EndpointPickerRef specifies a reference to an Endpoint Picker extension and its | ||
| // associated configuration. | ||
| type EndpointPickerRef struct { | ||
| // Group is the group of the referent API object. When unspecified, the default value | ||
| // is "", representing the Core API group. | ||
| // | ||
| // +optional | ||
| // +kubebuilder:default="" | ||
| Group *Group `json:"group,omitempty"` | ||
|
|
||
| // Kind is the Kubernetes resource kind of the referent. | ||
| // | ||
| // Required if the referent is ambiguous, e.g. service with multiple ports. | ||
| // | ||
| // Defaults to "Service" when not specified. | ||
| // | ||
|
|
@@ -112,100 +117,105 @@ | |
| // | ||
| // +optional | ||
| // +kubebuilder:default=Service | ||
| Kind *Kind `json:"kind,omitempty"` | ||
|
|
||
| // Name is the name of the referent API object. | ||
| // | ||
| // +required | ||
| Name ObjectName `json:"name,omitzero"` | ||
|
|
||
| // PortNumber is the port number of the Endpoint Picker extension service. When unspecified, | ||
| // implementations SHOULD infer a default value of 9002 when the kind field is "Service" or | ||
| // unspecified (defaults to "Service"). | ||
| // | ||
| // +optional | ||
| PortNumber *PortNumber `json:"portNumber,omitempty"` | ||
|
|
||
| // FailureMode configures how the parent handles the case when the Endpoint Picker extension | ||
| // is non-responsive. When unspecified, defaults to "FailClose". | ||
| // | ||
| // +optional | ||
| // +kubebuilder:default="FailClose" | ||
| FailureMode *EndpointPickerFailureMode `json:"failureMode,omitempty"` | ||
| } | ||
|
|
||
| // EndpointPickerFailureMode defines the options for how the parent handles the case when the | ||
| // Endpoint Picker extension is non-responsive. | ||
| // | ||
| // +kubebuilder:validation:Enum=FailOpen;FailClose | ||
| type EndpointPickerFailureMode string | ||
|
|
||
| const ( | ||
| // EndpointPickerFailOpen specifies that the parent should forward the request to an endpoint | ||
| // of its picking when the Endpoint Picker extension fails. | ||
| EndpointPickerFailOpen EndpointPickerFailureMode = "FailOpen" | ||
| // EndpointPickerFailClose specifies that the parent should drop the request when the Endpoint | ||
| // Picker extension fails. | ||
| EndpointPickerFailClose EndpointPickerFailureMode = "FailClose" | ||
| ) | ||
|
|
||
| // InferencePoolStatus defines the observed state of the InferencePool. | ||
| type InferencePoolStatus struct { | ||
| // Parents is a list of parent resources, typically Gateways, that are associated with | ||
| // the InferencePool, and the status of the InferencePool with respect to each parent. | ||
| // | ||
| // A controller that manages the InferencePool, must add an entry for each parent it manages | ||
| // and remove the parent entry when the controller no longer considers the InferencePool to | ||
| // be associated with that parent. | ||
| // | ||
| // A maximum of 32 parents will be represented in this list. When the list is empty, | ||
| // it indicates that the InferencePool is not associated with any parents. | ||
| // | ||
| // +kubebuilder:validation:MaxItems=32 | ||
| // +optional | ||
| // +listType=atomic | ||
| Parents []ParentStatus `json:"parents,omitempty"` | ||
| } | ||
|
|
||
| // ParentStatus defines the observed state of InferencePool from a Parent, i.e. Gateway. | ||
| type ParentStatus struct { | ||
| // Conditions is a list of status conditions that provide information about the observed | ||
| // state of the InferencePool. This field is required to be set by the controller that | ||
| // manages the InferencePool. | ||
| // | ||
| // Supported condition types are: | ||
| // | ||
| // * "Accepted" | ||
| // * "ResolvedRefs" | ||
| // | ||
| // +optional | ||
| // +listType=map | ||
| // +listMapKey=type | ||
| // +patchStrategy=merge | ||
| // +patchMergeKey=type | ||
| // +kubebuilder:validation:MaxItems=8 | ||
| Conditions []metav1.Condition `json:"conditions,omitempty"` | ||
|
|
||
| // ParentRef is used to identify the parent resource that this status | ||
| // is associated with. It is used to match the InferencePool with the parent | ||
| // resource, such as a Gateway. | ||
| // | ||
| // +required | ||
| ParentRef ParentReference `json:"parentRef,omitzero"` | ||
| } | ||
|
|
||
| // InferencePoolConditionType is a type of status condition for the InferencePool. | ||
| type InferencePoolConditionType string | ||
|
|
||
| // InferencePoolReason is the reason for a type of InferencePool status condition. | ||
| type InferencePoolReason string | ||
|
|
||
| const ( | ||
| // InferencePoolConditionAccepted is a type of condition that indicates whether | ||
| // the InferencePool has been accepted or rejected by a Parent, and why. | ||
| // | ||
| // Possible reasons for this condition to be True are: | ||
| // | ||
| // * "SupportedByParent" | ||
|
There was a problem hiding this comment. We've always used There was a problem hiding this comment. I have always found the There was a problem hiding this comment. @robscott While on the topic of status condition reasons, PTAL at the following: // This reason is used with the "Accepted" when a controller has not yet
// reconciled the InferencePool.
InferencePoolReasonPending InferencePoolReason = "Pending"Now that the default status condition has been removed, I question whether this reason is still applicable. If a controller has not yet reconciled the resource, something else is required to set There was a problem hiding this comment.
+1, let's remove it.
The problem is when you have multiple reasons that a condition could be false. Take HTTPRoute for an example. To follow your example, the positive condition would be something like "AllowedByListenersAndHasMatchingHostnameAndParentAndNoUnsupportedValues". I think the odds that we add more reasons to be false in the future are quite here, so I'd rather stick with a simpler reason for the positive state here and follow what GW API has done. |
||
| // | ||
| // Possible reasons for this condition to be False are: | ||
| // | ||
| // * "Accepted" | ||
| // * "HTTPRouteNotAccepted" | ||
| // | ||
| // Possible reasons for this condition to be Unknown are: | ||
|
|
@@ -216,28 +226,26 @@ | |
| // prefer to use the reasons listed above to improve interoperability. | ||
| InferencePoolConditionAccepted InferencePoolConditionType = "Accepted" | ||
|
|
||
| // InferencePoolReasonAccepted is a reason used with the "Accepted" condition | ||
| // when the InferencePool is accepted by a Parent because the Parent supports | ||
| // InferencePool as a backend. | ||
| InferencePoolReasonAccepted InferencePoolReason = "Accepted" | ||
|
|
||
| // InferencePoolReasonNotSupportedByParent is a reason used with the "Accepted" | ||
| // condition when the InferencePool has not been accepted by a Parent because | ||
| // the Parent does not support InferencePool as a backend. | ||
| InferencePoolReasonNotSupportedByParent InferencePoolReason = "NotSupportedByParent" | ||
|
|
||
| // InferencePoolReasonHTTPRouteNotAccepted is an optional reason used with the | ||
| // "Accepted" condition when the InferencePool is referenced by an HTTPRoute that | ||
| // has been rejected by the Parent. The user should inspect the status of the | ||
| // referring HTTPRoute for the specific reason. | ||
| InferencePoolReasonHTTPRouteNotAccepted InferencePoolReason = "HTTPRouteNotAccepted" | ||
| ) | ||
|
|
||
| const ( | ||
| // InferencePoolConditionResolvedRefs is a type of condition that indicates whether | ||
| // the controller was able to resolve all the object references for the InferencePool. | ||
| // | ||
| // Possible reasons for this condition to be True are: | ||
| // | ||
|
|
@@ -251,39 +259,46 @@ | |
| // prefer to use the reasons listed above to improve interoperability. | ||
| InferencePoolConditionResolvedRefs InferencePoolConditionType = "ResolvedRefs" | ||
|
|
||
| // InferencePoolReasonResolvedRefs is a reason used with the "ResolvedRefs" | ||
| // condition when the condition is true. | ||
| InferencePoolReasonResolvedRefs InferencePoolReason = "ResolvedRefs" | ||
|
|
||
| // InferencePoolReasonInvalidExtensionRef is a reason used with the "ResolvedRefs" | ||
| // condition when the Extension is invalid in some way. This can include an | ||
| // unsupported kind or API group, or a reference to a resource that cannot be found. | ||
| InferencePoolReasonInvalidExtensionRef InferencePoolReason = "InvalidExtensionRef" | ||
| ) | ||
|
|
||
| // ParentReference identifies an API object. It is used to associate the InferencePool with a | ||
| // parent resource, such as a Gateway. | ||
| type ParentReference struct { | ||
| // Group is the group of the referent API object. When unspecified, the referent is assumed | ||
| // to be in the "gateway.networking.k8s.io" API group. | ||
| // | ||
| // +optional | ||
| // +kubebuilder:default="gateway.networking.k8s.io" | ||
| Group *Group `json:"group,omitempty"` | ||
|
|
||
| // Kind is the kind of the referent API object. When unspecified, the referent is assumed | ||
| // to be a "Gateway" kind. | ||
| // | ||
| // +optional | ||
| // +kubebuilder:default=Gateway | ||
| Kind *Kind `json:"kind,omitempty"` | ||
|
|
||
| // Name is the name of the referent API object. | ||
| // | ||
| // +required | ||
| Name ObjectName `json:"name,omitzero"` | ||
|
|
||
| // Namespace is the namespace of the referenced object. When unspecified, the local | ||
| // namespace is inferred. | ||
| // | ||
| // Note that when a namespace different than the local namespace is specified, | ||
| // a ReferenceGrant object is required in the referent namespace to allow that | ||
| // namespace's owner to accept the reference. See the ReferenceGrant | ||
| // documentation for details: https://gateway-api.sigs.k8s.io/api-types/referencegrant/ | ||
| // | ||
| // +optional | ||
| Namespace *Namespace `json:"namespace,omitempty"` | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed
+kubebuilder:validation:MinProperties=1since the struct contains only one field, and the field is optional.