Define the XMesh API #4030
Define the XMesh API #4030
Conversation
k8s-ci-robot
added
release-note
k8s-ci-robot
added
do-not-merge/hold
kflynn
force-pushed
the
flynn/api-3949
branch
2 times, most recently
from
afbf562 to
b48f5ee
Compare
kflynn
force-pushed
the
flynn/api-3949
branch
2 times, most recently
from
14f5bd5 to
7bba59c
Compare
…s environment Signed-off-by: Flynn <[email protected]>
Signed-off-by: Flynn <[email protected]>
Co-authored-by: Mike Morris <[email protected]> Signed-off-by: Flynn <[email protected]>
Co-authored-by: Mike Morris <[email protected]> Signed-off-by: Flynn <[email protected]>
Signed-off-by: Flynn <[email protected]>
…conflating status and condition. Signed-off-by: Flynn <[email protected]>
Signed-off-by: Flynn <[email protected]>
There was a problem hiding this comment.
Thanks @kflynn! Would like to see LGTM from another mesh lead here before this merges.
/cc @howardjohn @LiorLieberman @mikemorris
/approve
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: kflynn, robscott The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
| // | ||
| // +kubebuilder:validation:MaxLength=64 | ||
| // +optional | ||
| Description *string `json:"description,omitempty"` |
There was a problem hiding this comment.
any examples of descriptions we anticipate to have here? 🤔
There was a problem hiding this comment.
We have the field in Gateway, so I added it here for parallelism. 🙂 Probably not a bad idea to have an example in user-facing docs.
| MeshReasonAccepted MeshConditionReason = "Accepted" | ||
|
|
||
| // This reason is used with the "Accepted" condition when the Mesh was not | ||
| // accepted because the parametersRef field refers to |
There was a problem hiding this comment.
is this expected to be an exhaustive-list, or just examples?
There was a problem hiding this comment.
Parallel with Gateway, it's somewhere in between. This is the preferred set. It is possible to use other values, but a controller should have a very reason for doing so.
| // +listType=map | ||
| // +listMapKey=type | ||
| // +kubebuilder:validation:MaxItems=8 | ||
| // +kubebuilder:default={{type: "Accepted", status: "Unknown", reason:"Pending", message:"Waiting for controller", lastTransitionTime: "1970-01-01T00:00:00Z"},{type: "Programmed", status: "Unknown", reason:"Pending", message:"Waiting for controller", lastTransitionTime: "1970-01-01T00:00:00Z"}} |
There was a problem hiding this comment.
nit: just confirming my understanding, this default kicks in when MeshStatus is not empty, (e.g with supportedFeatures) and Conditions are empty. And the defaults on XMesh.status kicks in when Status is not set at all. is that correct?
There was a problem hiding this comment.
Again, parallel with Gateway: when the object is created, the API server will give it an Accepted: Unknown status with reason Pending. Eventually it'll be picked up by a controller, and the controller will update the Accepted status. (It's well known, I think, that I hate having things change the spec behind the user's back, but status is different. 🙂)
There was a problem hiding this comment.
My question is more on the double defaults we have, one here and one here
There was a problem hiding this comment.
That is also parallel to Gateway. 😂 I think it might be worth revisiting both of them...
|
/lgtm |
k8s-ci-robot
added
the
lgtm
Signed-off-by: Flynn [email protected]
/kind feature
This API definition is mostly copied straight from the GEP. The GEP has been tweaked to match places where it wasn't an exact copy.
Note also that this includes a tweak to
hack/update-protosto not rely on$GOPATHbeing in the environment (because on my machine, that was resulting in an unintended change to the version toprotoc-gen-gowhich was breaking tests).