-
Notifications
You must be signed in to change notification settings - Fork 583
WIP:DNM: Bpfman operator APIs review #2221
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
Changes from 3 commits
f107af4
78534fc
89fb437
5f3305a
b6ea83d
88663fd
797bbdb
7996fc8
c892aa5
b56446d
6a8439c
a21e120
4d6bd6a
e0efe84
637c663
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,136 @@ | ||
| /* | ||
| Copyright 2023 The bpfman Authors. | ||
|
|
||
| Licensed under the Apache License, Version 2.0 (the "License"); | ||
| you may not use this file except in compliance with the License. | ||
| You may obtain a copy of the License at | ||
|
|
||
| http://www.apache.org/licenses/LICENSE-2.0 | ||
|
|
||
| Unless required by applicable law or agreed to in writing, software | ||
| distributed under the License is distributed on an "AS IS" BASIS, | ||
| WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| See the License for the specific language governing permissions and | ||
| limitations under the License. | ||
| */ | ||
|
|
||
| package v1alpha1 | ||
|
|
||
| import ( | ||
| metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
| metav1types "k8s.io/apimachinery/pkg/types" | ||
| "sigs.k8s.io/controller-runtime/pkg/client" | ||
| ) | ||
|
|
||
| // BpfApplicationProgramState defines the desired state of BpfApplication | ||
| // +union | ||
| // +kubebuilder:validation:XValidation:rule="has(self.type) && self.type == 'XDP' ? has(self.xdpInfo) : !has(self.xdpInfo)",message="xdpInfo configuration is required when type is xdp, and forbidden otherwise" | ||
| // +kubebuilder:validation:XValidation:rule="has(self.type) && self.type == 'TC' ? has(self.tcInfo) : !has(self.tcInfo)",message="tcInfo configuration is required when type is tc, and forbidden otherwise" | ||
| // +kubebuilder:validation:XValidation:rule="has(self.type) && self.type == 'TCX' ? has(self.tcxInfo) : !has(self.tcxInfo)",message="tcx configuration is required when type is TCX, and forbidden otherwise" | ||
| // +kubebuilder:validation:XValidation:rule="has(self.type) && self.type == 'Uprobe' ? has(self.uprobeInfo) : !has(self.uprobeInfo)",message="uprobe configuration is required when type is uprobe, and forbidden otherwise" | ||
| // +kubebuilder:validation:XValidation:rule="has(self.type) && self.type == 'UretProbe' ? has(self.uretprobeInfo) : !has(self.upretrobeInfo)",message="uretprobe configuration is required when type is uretprobe, and forbidden otherwise" | ||
| type BpfApplicationProgramState struct { | ||
| BpfProgramStateCommon `json:",inline"` | ||
|
|
||
| // type specifies the bpf program type | ||
|
||
| // +unionDiscriminator | ||
| // +required | ||
| // +kubebuilder:validation:Enum:="XDP";"TC";"TCX";"Uprobe";"UretProbe" | ||
| Type EBPFProgType `json:"type"` | ||
|
|
||
| // xdpInfo defines the desired state of the application's XdpPrograms. | ||
| // +unionMember | ||
| // +optional | ||
| XDPInfo *XdpProgramInfoState `json:"xdpInfo,omitempty"` | ||
msherif1234 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| // tcInfo defines the desired state of the application's TcPrograms. | ||
| // +unionMember | ||
| // +optional | ||
| TCInfo *TcProgramInfoState `json:"tcInfo,omitempty"` | ||
|
|
||
| // tcxInfo defines the desired state of the application's TcxPrograms. | ||
| // +unionMember | ||
| // +optional | ||
| TCXInfo *TcxProgramInfoState `json:"tcxInfo,omitempty"` | ||
|
|
||
| // uprobeInfo defines the desired state of the application's UprobePrograms. | ||
| // +unionMember | ||
| // +optional | ||
| UprobeInfo *UprobeProgramInfoState `json:"uprobeInfo,omitempty"` | ||
|
|
||
| // uretprobeInfo defines the desired state of the application's UretprobePrograms. | ||
| // +unionMember | ||
| // +optional | ||
| UretprobeInfo *UprobeProgramInfoState `json:"uretprobeInfo,omitempty"` | ||
| } | ||
|
|
||
| // BpfApplicationSpec defines the desired state of BpfApplication | ||
| type BpfApplicationStateSpec struct { | ||
| // node is the name of the node for this BpfApplicationStateSpec. | ||
| Node string `json:"node"` | ||
|
||
| // updateCount is the number of times the BpfApplicationState has been updated. Set to 1 | ||
| // when the object is created, then it is incremented prior to each update. | ||
| // This allows us to verify that the API server has the updated object prior | ||
| // to starting a new Reconcile operation. | ||
| UpdateCount int64 `json:"updateCount"` | ||
| // appLoadStatus reflects the status of loading the bpf application on the | ||
| // given node. | ||
| AppLoadStatus AppLoadStatus `json:"appLoadStatus"` | ||
| // programs is a list of bpf programs contained in the parent application. | ||
| // It is a map from the bpf program name to BpfApplicationProgramState | ||
| // elements. | ||
| Programs []BpfApplicationProgramState `json:"programs,omitempty"` | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing SSATags, needs |
||
| } | ||
|
|
||
| // +genclient | ||
| // +kubebuilder:object:root=true | ||
| // +kubebuilder:subresource:status | ||
| // +kubebuilder:resource:scope=Namespaced | ||
|
|
||
| // BpfApplicationState contains the per-node state of a BpfApplication. | ||
| // +kubebuilder:printcolumn:name="Node",type=string,JSONPath=".spec.node" | ||
| // +kubebuilder:printcolumn:name="Status",type=string,JSONPath=`.status.conditions[0].reason` | ||
| // +kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp" | ||
| type BpfApplicationState struct { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I realized mid-review of some of the above fields that this seems to be a separate CRD for conveying the observed state of a |
||
| metav1.TypeMeta `json:",inline"` | ||
| metav1.ObjectMeta `json:"metadata,omitempty"` | ||
|
|
||
| Spec BpfApplicationStateSpec `json:"spec,omitempty"` | ||
| Status BpfAppStatus `json:"status,omitempty"` | ||
| } | ||
|
|
||
| // +kubebuilder:object:root=true | ||
| // BpfApplicationStateList contains a list of BpfApplicationState objects | ||
| type BpfApplicationStateList struct { | ||
| metav1.TypeMeta `json:",inline"` | ||
| metav1.ListMeta `json:"metadata,omitempty"` | ||
| Items []BpfApplicationState `json:"items"` | ||
| } | ||
|
|
||
| func (an BpfApplicationState) GetName() string { | ||
| return an.Name | ||
| } | ||
|
|
||
| func (an BpfApplicationState) GetUID() metav1types.UID { | ||
| return an.UID | ||
| } | ||
|
|
||
| func (an BpfApplicationState) GetAnnotations() map[string]string { | ||
| return an.Annotations | ||
| } | ||
|
|
||
| func (an BpfApplicationState) GetLabels() map[string]string { | ||
| return an.Labels | ||
| } | ||
|
|
||
| func (an BpfApplicationState) GetStatus() *BpfAppStatus { | ||
| return &an.Status | ||
| } | ||
|
|
||
| func (an BpfApplicationState) GetClientObject() client.Object { | ||
| return &an | ||
| } | ||
|
|
||
| func (anl BpfApplicationStateList) GetItems() []BpfApplicationState { | ||
| return anl.Items | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,101 @@ | ||
| /* | ||
| Copyright 2024 The bpfman Authors. | ||
|
|
||
| Licensed under the Apache License, Version 2.0 (the "License"); | ||
| you may not use this file except in compliance with the License. | ||
| You may obtain a copy of the License at | ||
|
|
||
| http://www.apache.org/licenses/LICENSE-2.0 | ||
|
|
||
| Unless required by applicable law or agreed to in writing, software | ||
| distributed under the License is distributed on an "AS IS" BASIS, | ||
| WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| See the License for the specific language governing permissions and | ||
| limitations under the License. | ||
| */ | ||
|
|
||
| package v1alpha1 | ||
|
|
||
| import ( | ||
| metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
| ) | ||
|
|
||
| // BpfApplicationProgram defines the desired state of BpfApplication | ||
| // +union | ||
| // +kubebuilder:validation:XValidation:rule="has(self.type) && self.type == 'XDP' ? has(self.xdpInfo) : !has(self.xdpInfo)",message="xdpInfo configuration is required when type is xdp, and forbidden otherwise" | ||
| // +kubebuilder:validation:XValidation:rule="has(self.type) && self.type == 'TC' ? has(self.tcInfo) : !has(self.tcInfo)",message="tcInfo configuration is required when type is tc, and forbidden otherwise" | ||
| // +kubebuilder:validation:XValidation:rule="has(self.type) && self.type == 'TCX' ? has(self.tcxInfo) : !has(self.tcxInfo)",message="tcxInfo configuration is required when type is tcx, and forbidden otherwise" | ||
| // +kubebuilder:validation:XValidation:rule="has(self.type) && self.type == 'Uprobe' ? has(self.uprobeInfo) : !has(self.uprobeInfo)",message="uprobeInfo configuration is required when type is uprobe, and forbidden otherwise" | ||
| // +kubebuilder:validation:XValidation:rule="has(self.type) && self.type == 'UretProbe' ? has(self.uretprobeInfo) : !has(self.upretrobeInfo)",message="uretprobe configuration is required when type is uretprobe, and forbidden otherwise" | ||
| type BpfApplicationProgram struct { | ||
| // name is the name of the function that is the entry point for the BPF | ||
| // program | ||
| Name string `json:"name"` | ||
msherif1234 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| // type specifies the bpf program type | ||
|
||
| // +unionDiscriminator | ||
| // +required | ||
| // +kubebuilder:validation:Enum:="XDP";"TC";"TCX";"Uprobe";"UretProbe" | ||
| Type EBPFProgType `json:"type"` | ||
|
|
||
| // xdpInfo defines the desired state of the application's XdpPrograms. | ||
| // +unionMember | ||
| // +optional | ||
| XDPInfo *XdpProgramInfo `json:"xdpInfo,omitempty"` | ||
|
|
||
| // tcInfo defines the desired state of the application's TcPrograms. | ||
| // +unionMember | ||
| // +optional | ||
| TCInfo *TcProgramInfo `json:"tcInfo,omitempty"` | ||
|
|
||
| // tcxInfo defines the desired state of the application's TcxPrograms. | ||
| // +unionMember | ||
| // +optional | ||
| TCXInfo *TcxProgramInfo `json:"tcxInfo,omitempty"` | ||
|
|
||
| // uprobeInfo defines the desired state of the application's UprobePrograms. | ||
| // +unionMember | ||
| // +optional | ||
| UprobeInfo *UprobeProgramInfo `json:"uprobeInfo,omitempty"` | ||
|
|
||
| // uretprobeInfo defines the desired state of the application's UretprobePrograms. | ||
| // +unionMember | ||
| // +optional | ||
| UretprobeInfo *UprobeProgramInfo `json:"uretprobeInfo,omitempty"` | ||
| } | ||
|
|
||
| // BpfApplicationSpec defines the desired state of BpfApplication | ||
| type BpfApplicationSpec struct { | ||
| BpfAppCommon `json:",inline"` | ||
|
|
||
| // programs is the list of bpf programs in the BpfApplication that should be | ||
| // loaded. The application can selectively choose which program(s) to run | ||
| // from this list based on the optional attach points provided. | ||
| // +kubebuilder:validation:MinItems:=1 | ||
| Programs []BpfApplicationProgram `json:"programs,omitempty"` | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this meant to be an optional field? What happens if I create a
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Generally for lists we recommend having an upper bound to the number of entries. Is there a reasonable upper bound that can be specified here? Additionally, what other constraints exist here? Can I have multiple program entries with the same There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The purpose of a BpfApplication is to ask bpfman to load and possibly attach eBpf programs. I don't see a use case for a BpfApplication CRD with no programs identified. Regarding the bound, I don't know what an acceptable bound would be. The same goes for links.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In that case, this should probably be a required field. For an upper bound, how many programs would you generally expect a user to specify? What number would surprise you as being a really high number? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed, it should be required. We'll fix that. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I didn't want to arbitrarily limit it. However, we'll try to come up with some upper bounds on what's reasonable.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can always increase it in the future if needed. You can never decrease it though. If you truly can't enforce a reasonable upper bound then that is fine, but usually there is a reasonable one that can be set |
||
| } | ||
|
|
||
| // +genclient | ||
| // +kubebuilder:object:root=true | ||
| // +kubebuilder:subresource:status | ||
| // +kubebuilder:resource:scope=Namespaced | ||
|
|
||
| // BpfApplication is the Schema for the bpfapplications API | ||
| // +kubebuilder:printcolumn:name="NodeSelector",type=string,JSONPath=`.spec.nodeselector` | ||
| // +kubebuilder:printcolumn:name="Status",type=string,JSONPath=`.status.conditions[0].reason` | ||
| // +kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp" | ||
| type BpfApplication struct { | ||
| metav1.TypeMeta `json:",inline"` | ||
| metav1.ObjectMeta `json:"metadata,omitempty"` | ||
|
|
||
| Spec BpfApplicationSpec `json:"spec,omitempty"` | ||
| Status BpfAppStatus `json:"status,omitempty"` | ||
|
Comment on lines
+191
to
+192
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Both these fields should have godoc and either a |
||
| } | ||
|
|
||
| // +kubebuilder:object:root=true | ||
| // BpfApplicationList contains a list of BpfApplications | ||
| type BpfApplicationList struct { | ||
| metav1.TypeMeta `json:",inline"` | ||
| metav1.ListMeta `json:"metadata,omitempty"` | ||
| Items []BpfApplication `json:"items"` | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Field should have godoc and either a |
||
| } | ||
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.
Generally we recommend to not add other fields to unions, and have just the discriminator and the members.
It's been found to be confusing in the past when we have mixed members and non members