From 26b385dd1ddab6d4c14d34cf3d464a5bfc4abd14 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 23 Aug 2023 07:59:51 -0230 Subject: [PATCH] Add NodeModulesConfig status management functions (#514) (#698) Use the plural form for the NodeModulesConfig CRD, per convention. Add new CRD fields to be used by the NodeModulesConfig reconciler. Upstream-Commit: 1767d02cf6724f663a350c879d0f9c993ef8f6c9 Co-authored-by: Quentin Barrand --- api/v1beta1/nodemodulesconfig_types.go | 16 +- api/v1beta1/zz_generated.deepcopy.go | 13 +- ...dule-management.clusterserviceversion.yaml | 3 + .../kmm.sigs.x-k8s.io_nodemodulesconfigs.yaml | 304 ++++++++++++++++++ ...kmm.sigs.x-k8s.io_nodemodulesconfigs.yaml} | 119 ++++++- config/crd/kustomization.yaml | 1 + internal/nmc/helper.go | 42 +++ internal/nmc/helper_test.go | 74 +++++ 8 files changed, 558 insertions(+), 14 deletions(-) create mode 100644 bundle/manifests/kmm.sigs.x-k8s.io_nodemodulesconfigs.yaml rename config/crd/bases/{kmm.sigs.x-k8s.io_nodemodulesconfig.yaml => kmm.sigs.x-k8s.io_nodemodulesconfigs.yaml} (59%) diff --git a/api/v1beta1/nodemodulesconfig_types.go b/api/v1beta1/nodemodulesconfig_types.go index 946531e63..f9d1f7e5f 100644 --- a/api/v1beta1/nodemodulesconfig_types.go +++ b/api/v1beta1/nodemodulesconfig_types.go @@ -28,7 +28,6 @@ type ModuleConfig struct { //+optional InTreeModuleToRemove string `json:"inTreeModuleToRemove,omitempty"` Modprobe ModprobeSpec `json:"modprobe"` - KmodLoaded bool `json:"kmodLoaded"` } type NodeModuleSpec struct { @@ -47,9 +46,14 @@ type NodeModulesConfigSpec struct { } type NodeModuleStatus struct { - Name string `json:"name"` - Namespace string `json:"namespace"` - KmodLoaded bool `json:"kmodLoaded"` + //+optional + Config *ModuleConfig `json:"config,omitempty"` + InProgress bool `json:"inProgress"` + //+optional + LastTransitionTime *metav1.Time `json:"lastTransitionTime,omitempty"` + Name string `json:"name"` + Namespace string `json:"namespace"` + ServiceAccountName string `json:"serviceAccountName"` } // NodeModuleConfigStatus is the most recently observed status of the KMM modules on node. @@ -67,8 +71,8 @@ type NodeModulesConfigStatus struct { // +kubebuilder:subresource:status // NodeModulesConfig keeps spec and state of the KMM modules on a node. -// +kubebuilder:resource:path=nodemodulesconfig,scope=Cluster -// +kubebuilder:resource:path=nodemodulesconfig,scope=Cluster,shortName=nmc +// +kubebuilder:resource:path=nodemodulesconfigs,scope=Cluster +// +kubebuilder:resource:path=nodemodulesconfigs,scope=Cluster,shortName=nmc // +operator-sdk:csv:customresourcedefinitions:displayName="Node Modules Config" type NodeModulesConfig struct { metav1.TypeMeta `json:",inline"` diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index 09c7bf4ed..10a01799c 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -477,6 +477,15 @@ func (in *NodeModuleSpec) DeepCopy() *NodeModuleSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *NodeModuleStatus) DeepCopyInto(out *NodeModuleStatus) { *out = *in + if in.Config != nil { + in, out := &in.Config, &out.Config + *out = new(ModuleConfig) + (*in).DeepCopyInto(*out) + } + if in.LastTransitionTime != nil { + in, out := &in.LastTransitionTime, &out.LastTransitionTime + *out = (*in).DeepCopy() + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NodeModuleStatus. @@ -576,7 +585,9 @@ func (in *NodeModulesConfigStatus) DeepCopyInto(out *NodeModulesConfigStatus) { if in.Modules != nil { in, out := &in.Modules, &out.Modules *out = make([]NodeModuleStatus, len(*in)) - copy(*out, *in) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } } } diff --git a/bundle/manifests/kernel-module-management.clusterserviceversion.yaml b/bundle/manifests/kernel-module-management.clusterserviceversion.yaml index b29664a10..68b0f2354 100644 --- a/bundle/manifests/kernel-module-management.clusterserviceversion.yaml +++ b/bundle/manifests/kernel-module-management.clusterserviceversion.yaml @@ -63,6 +63,9 @@ spec: kind: Module name: modules.kmm.sigs.x-k8s.io version: v1beta1 + - kind: NodeModulesConfig + name: nodemodulesconfigs.kmm.sigs.x-k8s.io + version: v1beta1 - description: PreflightValidation initiates a preflight validations for all Modules on the current Kubernetes cluster. displayName: Preflight Validation diff --git a/bundle/manifests/kmm.sigs.x-k8s.io_nodemodulesconfigs.yaml b/bundle/manifests/kmm.sigs.x-k8s.io_nodemodulesconfigs.yaml new file mode 100644 index 000000000..dafa13c70 --- /dev/null +++ b/bundle/manifests/kmm.sigs.x-k8s.io_nodemodulesconfigs.yaml @@ -0,0 +1,304 @@ +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.12.0 + creationTimestamp: null + labels: + app.kubernetes.io/component: kmm + app.kubernetes.io/name: kmm + app.kubernetes.io/part-of: kmm + name: nodemodulesconfigs.kmm.sigs.x-k8s.io +spec: + group: kmm.sigs.x-k8s.io + names: + kind: NodeModulesConfig + listKind: NodeModulesConfigList + plural: nodemodulesconfigs + shortNames: + - nmc + singular: nodemodulesconfig + scope: Cluster + versions: + - name: v1beta1 + schema: + openAPIV3Schema: + description: NodeModulesConfig keeps spec and state of the KMM modules on + a node. + properties: + apiVersion: + description: 'APIVersion defines the versioned schema of this representation + of an object. Servers should convert recognized schemas to the latest + internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources' + type: string + kind: + description: 'Kind is a string value representing the REST resource this + object represents. Servers may infer this from the endpoint the client + submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' + type: string + metadata: + type: object + spec: + description: 'NodeModulesConfigSpec describes the desired state of modules + on the node More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#spec-and-status' + properties: + modules: + description: Modules list the spec of all the modules that need to + be executed on the node + items: + properties: + config: + properties: + containerImage: + type: string + inTreeModuleToRemove: + type: string + insecurePull: + description: When InsecurePull is true, the container image + can be pulled without TLS. + type: boolean + kernelVersion: + type: string + modprobe: + properties: + args: + description: 'Args is an optional list of arguments + to be passed to modprobe before the name of the kernel + module. The resulting commands will be: `modprobe + ${Args} module_name`.' + properties: + load: + description: Load is an optional list of arguments + to be used when loading the kernel module. + items: + type: string + minItems: 1 + type: array + unload: + description: Unload is an optional list of arguments + to be used when unloading the kernel module. + items: + type: string + minItems: 1 + type: array + type: object + dirName: + default: /opt + description: DirName is the root directory for modules. + It adds `-d ${DirName}` to the modprobe command-line. + type: string + firmwarePath: + description: FirmwarePath is the path of the firmware(s). + The firmware(s) will be copied to the host for the + kernel to find them. + type: string + moduleName: + description: ModuleName is the name of the Module to + be loaded. + type: string + modulesLoadingOrder: + description: 'ModulesLoadingOrder defines the dependency + between kernel modules loading, in case it was not + created by depmod (independent kernel modules). The + list order should be: upmost module, then the module + it depends on and so on. Example: if moduleA depends + on first loading moduleB, and moduleB depends on first + loading moduleC the entry should look: ModulesLoadingOrder: + - moduleA - moduleB - moduleC In order to load all + 3 modules, moduleA shoud be defined in the ModuleName + parameter of this struct' + items: + type: string + type: array + parameters: + description: 'Parameters is an optional list of kernel + module parameters to be provided to modprobe. They + should be in the form of key=value and will be separated + by spaces in the modprobe command. The resulting loading + command will be: `modprobe module_name ${Parameters}`.' + items: + type: string + type: array + rawArgs: + description: 'If RawArgs are specified, they are passed + straight to the modprobe binary; all other properties + in this object are ignored. The resulting commands + will be: `modprobe ${RawArgs}`.' + properties: + load: + description: Load is an optional list of arguments + to be used when loading the kernel module. + items: + type: string + minItems: 1 + type: array + unload: + description: Unload is an optional list of arguments + to be used when unloading the kernel module. + items: + type: string + minItems: 1 + type: array + type: object + required: + - moduleName + type: object + required: + - containerImage + - insecurePull + - kernelVersion + - modprobe + type: object + name: + type: string + namespace: + type: string + required: + - config + - name + - namespace + type: object + type: array + type: object + status: + description: 'NodeModuleConfigStatus is the most recently observed status + of the KMM modules on node. It is populated by the system and is read-only. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#spec-and-status' + properties: + modules: + description: Modules contain observations about each Module's node + state status + items: + properties: + config: + properties: + containerImage: + type: string + inTreeModuleToRemove: + type: string + insecurePull: + description: When InsecurePull is true, the container image + can be pulled without TLS. + type: boolean + kernelVersion: + type: string + modprobe: + properties: + args: + description: 'Args is an optional list of arguments + to be passed to modprobe before the name of the kernel + module. The resulting commands will be: `modprobe + ${Args} module_name`.' + properties: + load: + description: Load is an optional list of arguments + to be used when loading the kernel module. + items: + type: string + minItems: 1 + type: array + unload: + description: Unload is an optional list of arguments + to be used when unloading the kernel module. + items: + type: string + minItems: 1 + type: array + type: object + dirName: + default: /opt + description: DirName is the root directory for modules. + It adds `-d ${DirName}` to the modprobe command-line. + type: string + firmwarePath: + description: FirmwarePath is the path of the firmware(s). + The firmware(s) will be copied to the host for the + kernel to find them. + type: string + moduleName: + description: ModuleName is the name of the Module to + be loaded. + type: string + modulesLoadingOrder: + description: 'ModulesLoadingOrder defines the dependency + between kernel modules loading, in case it was not + created by depmod (independent kernel modules). The + list order should be: upmost module, then the module + it depends on and so on. Example: if moduleA depends + on first loading moduleB, and moduleB depends on first + loading moduleC the entry should look: ModulesLoadingOrder: + - moduleA - moduleB - moduleC In order to load all + 3 modules, moduleA shoud be defined in the ModuleName + parameter of this struct' + items: + type: string + type: array + parameters: + description: 'Parameters is an optional list of kernel + module parameters to be provided to modprobe. They + should be in the form of key=value and will be separated + by spaces in the modprobe command. The resulting loading + command will be: `modprobe module_name ${Parameters}`.' + items: + type: string + type: array + rawArgs: + description: 'If RawArgs are specified, they are passed + straight to the modprobe binary; all other properties + in this object are ignored. The resulting commands + will be: `modprobe ${RawArgs}`.' + properties: + load: + description: Load is an optional list of arguments + to be used when loading the kernel module. + items: + type: string + minItems: 1 + type: array + unload: + description: Unload is an optional list of arguments + to be used when unloading the kernel module. + items: + type: string + minItems: 1 + type: array + type: object + required: + - moduleName + type: object + required: + - containerImage + - insecurePull + - kernelVersion + - modprobe + type: object + inProgress: + type: boolean + lastTransitionTime: + format: date-time + type: string + name: + type: string + namespace: + type: string + serviceAccountName: + type: string + required: + - inProgress + - name + - namespace + - serviceAccountName + type: object + type: array + type: object + type: object + served: true + storage: true + subresources: + status: {} +status: + acceptedNames: + kind: "" + plural: "" + conditions: null + storedVersions: null diff --git a/config/crd/bases/kmm.sigs.x-k8s.io_nodemodulesconfig.yaml b/config/crd/bases/kmm.sigs.x-k8s.io_nodemodulesconfigs.yaml similarity index 59% rename from config/crd/bases/kmm.sigs.x-k8s.io_nodemodulesconfig.yaml rename to config/crd/bases/kmm.sigs.x-k8s.io_nodemodulesconfigs.yaml index 7b002186e..7a9019359 100644 --- a/config/crd/bases/kmm.sigs.x-k8s.io_nodemodulesconfig.yaml +++ b/config/crd/bases/kmm.sigs.x-k8s.io_nodemodulesconfigs.yaml @@ -4,13 +4,13 @@ kind: CustomResourceDefinition metadata: annotations: controller-gen.kubebuilder.io/version: v0.12.0 - name: nodemodulesconfig.kmm.sigs.x-k8s.io + name: nodemodulesconfigs.kmm.sigs.x-k8s.io spec: group: kmm.sigs.x-k8s.io names: kind: NodeModulesConfig listKind: NodeModulesConfigList - plural: nodemodulesconfig + plural: nodemodulesconfigs shortNames: - nmc singular: nodemodulesconfig @@ -55,8 +55,6 @@ spec: type: boolean kernelVersion: type: string - kmodLoaded: - type: boolean modprobe: properties: args: @@ -145,7 +143,6 @@ spec: - containerImage - insecurePull - kernelVersion - - kmodLoaded - modprobe type: object name: @@ -169,16 +166,124 @@ spec: state status items: properties: - kmodLoaded: + config: + properties: + containerImage: + type: string + inTreeModuleToRemove: + type: string + insecurePull: + description: When InsecurePull is true, the container image + can be pulled without TLS. + type: boolean + kernelVersion: + type: string + modprobe: + properties: + args: + description: 'Args is an optional list of arguments + to be passed to modprobe before the name of the kernel + module. The resulting commands will be: `modprobe + ${Args} module_name`.' + properties: + load: + description: Load is an optional list of arguments + to be used when loading the kernel module. + items: + type: string + minItems: 1 + type: array + unload: + description: Unload is an optional list of arguments + to be used when unloading the kernel module. + items: + type: string + minItems: 1 + type: array + type: object + dirName: + default: /opt + description: DirName is the root directory for modules. + It adds `-d ${DirName}` to the modprobe command-line. + type: string + firmwarePath: + description: FirmwarePath is the path of the firmware(s). + The firmware(s) will be copied to the host for the + kernel to find them. + type: string + moduleName: + description: ModuleName is the name of the Module to + be loaded. + type: string + modulesLoadingOrder: + description: 'ModulesLoadingOrder defines the dependency + between kernel modules loading, in case it was not + created by depmod (independent kernel modules). The + list order should be: upmost module, then the module + it depends on and so on. Example: if moduleA depends + on first loading moduleB, and moduleB depends on first + loading moduleC the entry should look: ModulesLoadingOrder: + - moduleA - moduleB - moduleC In order to load all + 3 modules, moduleA shoud be defined in the ModuleName + parameter of this struct' + items: + type: string + type: array + parameters: + description: 'Parameters is an optional list of kernel + module parameters to be provided to modprobe. They + should be in the form of key=value and will be separated + by spaces in the modprobe command. The resulting loading + command will be: `modprobe module_name ${Parameters}`.' + items: + type: string + type: array + rawArgs: + description: 'If RawArgs are specified, they are passed + straight to the modprobe binary; all other properties + in this object are ignored. The resulting commands + will be: `modprobe ${RawArgs}`.' + properties: + load: + description: Load is an optional list of arguments + to be used when loading the kernel module. + items: + type: string + minItems: 1 + type: array + unload: + description: Unload is an optional list of arguments + to be used when unloading the kernel module. + items: + type: string + minItems: 1 + type: array + type: object + required: + - moduleName + type: object + required: + - containerImage + - insecurePull + - kernelVersion + - modprobe + type: object + inProgress: type: boolean + lastTransitionTime: + format: date-time + type: string name: type: string namespace: type: string + serviceAccountName: + type: string required: - - kmodLoaded + - inProgress - name - namespace + - serviceAccountName type: object type: array type: object diff --git a/config/crd/kustomization.yaml b/config/crd/kustomization.yaml index 726146955..eee50a0d7 100644 --- a/config/crd/kustomization.yaml +++ b/config/crd/kustomization.yaml @@ -6,6 +6,7 @@ kind: Kustomization # It should be run by config/default resources: - bases/kmm.sigs.x-k8s.io_modules.yaml +- bases/kmm.sigs.x-k8s.io_nodemodulesconfigs.yaml - bases/kmm.sigs.x-k8s.io_preflightvalidations.yaml - bases/kmm.sigs.x-k8s.io_preflightvalidationsocp.yaml #+kubebuilder:scaffold:crdkustomizeresource diff --git a/internal/nmc/helper.go b/internal/nmc/helper.go index 3f5031bb7..80ade6f4b 100644 --- a/internal/nmc/helper.go +++ b/internal/nmc/helper.go @@ -74,3 +74,45 @@ func (h *helper) GetModuleEntry(nmc *kmmv1beta1.NodeModulesConfig, modNamespace, } return nil, 0 } + +func findModuleStatus(statuses []kmmv1beta1.NodeModuleStatus, moduleNamespace, moduleName string) *kmmv1beta1.NodeModuleStatus { + for i := 0; i < len(statuses); i++ { + s := statuses[i] + + if s.Namespace == moduleNamespace && s.Name == moduleName { + return &statuses[i] + } + } + + return nil +} + +func RemoveModuleStatus(statuses *[]kmmv1beta1.NodeModuleStatus, modNamespace, modName string) { + if statuses == nil || len(*statuses) == 0 { + return + } + + newStatuses := make([]kmmv1beta1.NodeModuleStatus, 0, len(*statuses)-1) + + for _, s := range *statuses { + if s.Namespace != modNamespace || s.Name != modName { + newStatuses = append(newStatuses, s) + } + } + + *statuses = newStatuses +} + +func SetModuleStatus(statuses *[]kmmv1beta1.NodeModuleStatus, status kmmv1beta1.NodeModuleStatus) { + if statuses == nil { + return + } + + s := findModuleStatus(*statuses, status.Namespace, status.Name) + + if s != nil { + *s = status + } else { + *statuses = append(*statuses, status) + } +} diff --git a/internal/nmc/helper_test.go b/internal/nmc/helper_test.go index d1c0fae50..4811316d9 100644 --- a/internal/nmc/helper_test.go +++ b/internal/nmc/helper_test.go @@ -234,3 +234,77 @@ var _ = Describe("GetModuleEntry", func() { Expect(index).To(Equal(0)) }) }) + +var _ = Describe("RemoveModuleStatus", func() { + const ( + name = "test-name" + namespace = "test-namespace" + ) + + It("should do nothing if the list is nil", func() { + RemoveModuleStatus(nil, namespace, name) + }) + + It("should do nothing if the list is empty", func() { + statuses := make([]kmmv1beta1.NodeModuleStatus, 0) + + RemoveModuleStatus(&statuses, namespace, name) + }) + + It("should remove a status if it exists in the list", func() { + statuses := []kmmv1beta1.NodeModuleStatus{ + {Namespace: namespace, Name: name}, + } + + RemoveModuleStatus(&statuses, namespace, name) + + Expect(statuses).To(BeEmpty()) + }) +}) + +var _ = Describe("SetModuleStatus", func() { + const ( + name = "test-name" + namespace = "test-namespace" + ) + + now := metav1.Now() + + s := kmmv1beta1.NodeModuleStatus{ + Config: &kmmv1beta1.ModuleConfig{ + KernelVersion: "some-kver", + ContainerImage: "some-kernel-image", + InsecurePull: true, + InTreeModuleToRemove: "intree", + }, + LastTransitionTime: &now, + Name: name, + Namespace: namespace, + ServiceAccountName: "sa", + } + + It("should do nothing if the slice is nil", func() { + SetModuleStatus(nil, kmmv1beta1.NodeModuleStatus{}) + }) + + It("should add an entry nothing if the list is empty", func() { + statuses := make([]kmmv1beta1.NodeModuleStatus, 0) + + SetModuleStatus(&statuses, s) + + Expect(statuses).To(HaveLen(1)) + Expect(statuses[0]).To(BeComparableTo(s)) + }) + + It("should update an entry if it already exists", func() { + statuses := []kmmv1beta1.NodeModuleStatus{s} + + new := s + new.InProgress = true + + SetModuleStatus(&statuses, new) + + Expect(statuses).To(HaveLen(1)) + Expect(statuses[0]).To(BeComparableTo(new)) + }) +})