From 39989308b70c58a0e6c993719992372995e77e84 Mon Sep 17 00:00:00 2001 From: Talor Itzhak Date: Tue, 10 Sep 2024 10:39:20 +0300 Subject: [PATCH 1/7] vendor: bump deployer to v0.20.2 Latest version allow detecting HyperShift platform. Signed-off-by: Talor Itzhak --- go.mod | 2 +- go.sum | 4 +-- .../pkg/deployer/platform/detect/detect.go | 28 ++++++++++++++++++- .../pkg/deployer/platform/platform.go | 3 ++ vendor/modules.txt | 2 +- 5 files changed, 34 insertions(+), 5 deletions(-) diff --git a/go.mod b/go.mod index 282484edb..b012004ca 100644 --- a/go.mod +++ b/go.mod @@ -9,7 +9,7 @@ require ( github.com/go-logr/logr v1.4.2 github.com/google/go-cmp v0.6.0 github.com/jaypipes/ghw v0.12.0 - github.com/k8stopologyawareschedwg/deployer v0.20.1 + github.com/k8stopologyawareschedwg/deployer v0.20.2 github.com/k8stopologyawareschedwg/noderesourcetopology-api v0.1.2 github.com/k8stopologyawareschedwg/podfingerprint v0.2.2 github.com/k8stopologyawareschedwg/resource-topology-exporter v0.16.1 diff --git a/go.sum b/go.sum index 237ea6c59..2d1f2ef43 100644 --- a/go.sum +++ b/go.sum @@ -1177,8 +1177,8 @@ github.com/julienschmidt/httprouter v1.2.0/go.mod h1:SYymIcj16QtmaHHD7aYtjjsJG7V github.com/julienschmidt/httprouter v1.3.0/go.mod h1:JR6WtHb+2LUe8TCKY3cZOxFyyO8IZAc4RVcycCCAKdM= github.com/jung-kurt/gofpdf v1.0.0/go.mod h1:7Id9E/uU8ce6rXgefFLlgrJj/GYY22cpxn+r32jIOes= github.com/jung-kurt/gofpdf v1.0.3-0.20190309125859-24315acbbda5/go.mod h1:7Id9E/uU8ce6rXgefFLlgrJj/GYY22cpxn+r32jIOes= -github.com/k8stopologyawareschedwg/deployer v0.20.1 h1:1T3eKxZr+HiCFRvinmvXSHphT0DhQ/ZnOKjTAMNXIMc= -github.com/k8stopologyawareschedwg/deployer v0.20.1/go.mod h1:hnPU2dPLclkKXU28H+RkRrS21LFpmMTAU55mISsPuMk= +github.com/k8stopologyawareschedwg/deployer v0.20.2 h1:2h00RgsjvZvo5aeQdE0XTTDJXMDFgRhM/DHeXClzXQQ= +github.com/k8stopologyawareschedwg/deployer v0.20.2/go.mod h1:hnPU2dPLclkKXU28H+RkRrS21LFpmMTAU55mISsPuMk= github.com/k8stopologyawareschedwg/noderesourcetopology-api v0.1.2 h1:uAwqOtyrFYggq3pVf3hs1XKkBxrQ8dkgjWz3LCLJsiY= github.com/k8stopologyawareschedwg/noderesourcetopology-api v0.1.2/go.mod h1:LBzS4n6GX1C69tzSd5EibZ9cGOXFuHP7GxEMDYVe1sM= github.com/k8stopologyawareschedwg/podfingerprint v0.2.2 h1:iFHPfZInM9pz2neye5RdmORMp1hPmte1EGJYpOOzZVg= diff --git a/vendor/github.com/k8stopologyawareschedwg/deployer/pkg/deployer/platform/detect/detect.go b/vendor/github.com/k8stopologyawareschedwg/deployer/pkg/deployer/platform/detect/detect.go index 4519e6ad4..12d9deb4a 100644 --- a/vendor/github.com/k8stopologyawareschedwg/deployer/pkg/deployer/platform/detect/detect.go +++ b/vendor/github.com/k8stopologyawareschedwg/deployer/pkg/deployer/platform/detect/detect.go @@ -64,13 +64,18 @@ func Platform(ctx context.Context) (platform.Platform, error) { if err != nil { return platform.Unknown, err } - return PlatformFromLister(ctx, ocpCli.ConfigV1.ClusterVersions()) + return PlatformFromClients(ctx, ocpCli.ConfigV1.ClusterVersions(), ocpCli.ConfigV1.Infrastructures()) } type ClusterVersionsLister interface { List(ctx context.Context, opts metav1.ListOptions) (*ocpconfigv1.ClusterVersionList, error) } +type InfrastructuresGetter interface { + Get(ctx context.Context, name string, options metav1.GetOptions) (*ocpconfigv1.Infrastructure, error) +} + +// PlatformFromLister is deprecated, use PlatformFromClients instead func PlatformFromLister(ctx context.Context, cvLister ClusterVersionsLister) (platform.Platform, error) { vers, err := cvLister.List(ctx, metav1.ListOptions{}) if err != nil { @@ -85,6 +90,27 @@ func PlatformFromLister(ctx context.Context, cvLister ClusterVersionsLister) (pl return platform.Kubernetes, nil } +func PlatformFromClients(ctx context.Context, cvLister ClusterVersionsLister, infraGetter InfrastructuresGetter) (platform.Platform, error) { + vers, err := cvLister.List(ctx, metav1.ListOptions{}) + if err != nil { + if errors.IsNotFound(err) { + return platform.Kubernetes, nil + } + return platform.Unknown, err + } + if len(vers.Items) > 0 { + infra, err := infraGetter.Get(ctx, "cluster", metav1.GetOptions{}) + if err != nil { + return platform.Unknown, err + } + if infra.Status.ControlPlaneTopology == ocpconfigv1.ExternalTopologyMode { + return platform.HyperShift, nil + } + return platform.OpenShift, nil + } + return platform.Kubernetes, nil +} + func Version(ctx context.Context, plat platform.Platform) (platform.Version, error) { if plat == platform.OpenShift { return OpenshiftVersion(ctx) diff --git a/vendor/github.com/k8stopologyawareschedwg/deployer/pkg/deployer/platform/platform.go b/vendor/github.com/k8stopologyawareschedwg/deployer/pkg/deployer/platform/platform.go index a0b69ccf4..2ce73633b 100644 --- a/vendor/github.com/k8stopologyawareschedwg/deployer/pkg/deployer/platform/platform.go +++ b/vendor/github.com/k8stopologyawareschedwg/deployer/pkg/deployer/platform/platform.go @@ -24,6 +24,7 @@ const ( Unknown = Platform("Unknown") Kubernetes = Platform("Kubernetes") OpenShift = Platform("OpenShift") + HyperShift = Platform("HyperShift") ) func (p Platform) String() string { @@ -37,6 +38,8 @@ func ParsePlatform(plat string) (Platform, bool) { return Kubernetes, true case "openshift": return OpenShift, true + case "hypershift": + return HyperShift, true default: return Unknown, false } diff --git a/vendor/modules.txt b/vendor/modules.txt index 028f62582..e9ab8947f 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -175,7 +175,7 @@ github.com/josharian/intern # github.com/json-iterator/go v1.1.12 ## explicit; go 1.12 github.com/json-iterator/go -# github.com/k8stopologyawareschedwg/deployer v0.20.1 +# github.com/k8stopologyawareschedwg/deployer v0.20.2 ## explicit; go 1.21 github.com/k8stopologyawareschedwg/deployer/pkg/assets/rte github.com/k8stopologyawareschedwg/deployer/pkg/assets/selinux From 85134bdc97e307d3b1077a430e4e398780fb60ea Mon Sep 17 00:00:00 2001 From: Talor Itzhak Date: Sun, 15 Sep 2024 19:18:45 +0300 Subject: [PATCH 2/7] main: add platform field Add the platform field to the reconciler so it would be able to detect whether it run on OCP or HCP. Signed-off-by: Talor Itzhak --- controllers/kubeletconfig_controller.go | 2 ++ main.go | 1 + 2 files changed, 3 insertions(+) diff --git a/controllers/kubeletconfig_controller.go b/controllers/kubeletconfig_controller.go index 47ea5ccef..83c6ab852 100644 --- a/controllers/kubeletconfig_controller.go +++ b/controllers/kubeletconfig_controller.go @@ -39,6 +39,7 @@ import ( "github.com/pkg/errors" + "github.com/k8stopologyawareschedwg/deployer/pkg/deployer/platform" nropv1 "github.com/openshift-kni/numaresources-operator/api/numaresourcesoperator/v1" "github.com/openshift-kni/numaresources-operator/internal/machineconfigpools" "github.com/openshift-kni/numaresources-operator/pkg/apply" @@ -58,6 +59,7 @@ type KubeletConfigReconciler struct { Scheme *runtime.Scheme Recorder record.EventRecorder Namespace string + Platform platform.Platform } //+kubebuilder:rbac:groups="",resources=configmaps,verbs=* diff --git a/main.go b/main.go index 8ca257c5f..2d7947b0d 100644 --- a/main.go +++ b/main.go @@ -273,6 +273,7 @@ func main() { Scheme: mgr.GetScheme(), Recorder: mgr.GetEventRecorderFor("kubeletconfig-controller"), Namespace: namespace, + Platform: clusterPlatform, }).SetupWithManager(mgr); err != nil { klog.ErrorS(err, "unable to create controller", "controller", "KubeletConfig") os.Exit(1) From ff197eb0ffa5a1ef0e04c5fa453a4a87b50a341f Mon Sep 17 00:00:00 2001 From: Talor Itzhak Date: Sun, 15 Sep 2024 19:21:46 +0300 Subject: [PATCH 3/7] controller: watch for change on HCP (predicates) Add the functionality to watch for ConfigMap changes when running on HCP. On HCP the controller should not watch for the bare KubeletConfig object, but for the ConfigMap that hold it. Signed-off-by: Talor Itzhak --- controllers/kubeletconfig_controller.go | 35 ++++++++++++++++++------- main.go | 2 +- 2 files changed, 26 insertions(+), 11 deletions(-) diff --git a/controllers/kubeletconfig_controller.go b/controllers/kubeletconfig_controller.go index 83c6ab852..b1d8538da 100644 --- a/controllers/kubeletconfig_controller.go +++ b/controllers/kubeletconfig_controller.go @@ -50,7 +50,8 @@ import ( ) const ( - kubeletConfigRetryPeriod = 30 * time.Second + kubeletConfigRetryPeriod = 30 * time.Second + HypershiftKubeletConfigConfigMapLabel = "hypershift.openshift.io/kubeletconfig-config" ) // KubeletConfigReconciler reconciles a KubeletConfig object @@ -107,18 +108,32 @@ func (r *KubeletConfigReconciler) Reconcile(ctx context.Context, req ctrl.Reques return ctrl.Result{}, nil } -func (r *KubeletConfigReconciler) SetupWithManager(mgr ctrl.Manager) error { - // we have nothing to do in case of deletion - p := predicate.Funcs{ - DeleteFunc: func(e event.DeleteEvent) bool { - kubelet := e.Object.(*mcov1.KubeletConfig) - klog.InfoS("KubeletConfig object got deleted", "KubeletConfig", kubelet.Name) +func (r *KubeletConfigReconciler) SetupWithManager(mgr ctrl.Manager, clusterPlatform platform.Platform) error { + var o client.Object + var p predicate.Funcs + if clusterPlatform == platform.OpenShift { + o = &mcov1.KubeletConfig{} + // we have nothing to do in case of deletion + p = predicate.Funcs{ + DeleteFunc: func(e event.DeleteEvent) bool { + kubelet := e.Object.(*mcov1.KubeletConfig) + klog.InfoS("KubeletConfig object got deleted", "KubeletConfig", kubelet.Name) + return false + }, + } + } + if clusterPlatform == platform.HyperShift { + o = &corev1.ConfigMap{} + p = predicate.NewPredicateFuncs(func(o client.Object) bool { + kubelet := o.(*corev1.ConfigMap) + if _, ok := kubelet.Labels[HypershiftKubeletConfigConfigMapLabel]; ok { + return true + } return false - }, + }) } - return ctrl.NewControllerManagedBy(mgr). - For(&mcov1.KubeletConfig{}, builder.WithPredicates(p)). + For(o, builder.WithPredicates(p)). Owns(&corev1.ConfigMap{}). Complete(r) } diff --git a/main.go b/main.go index 2d7947b0d..ae117c3b9 100644 --- a/main.go +++ b/main.go @@ -274,7 +274,7 @@ func main() { Recorder: mgr.GetEventRecorderFor("kubeletconfig-controller"), Namespace: namespace, Platform: clusterPlatform, - }).SetupWithManager(mgr); err != nil { + }).SetupWithManager(mgr, clusterPlatform); err != nil { klog.ErrorS(err, "unable to create controller", "controller", "KubeletConfig") os.Exit(1) } From d1a983bdb45ce7d51a78bb5d1ea1160f8be3a67f Mon Sep 17 00:00:00 2001 From: Talor Itzhak Date: Sun, 15 Sep 2024 19:30:07 +0300 Subject: [PATCH 4/7] controller: introduce kubeletConfigHandler object The business logic of kubelet controller is similar when running on OCP or HCP. However there are slighty different parameters that some of the existing functions in the reconcile loop accepts. We introduce the `kubeletConfigHandler` so we could maintain a common API for the functions for both OCP and HCP platforms. Signed-off-by: Talor Itzhak --- controllers/kubeletconfig_controller.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/controllers/kubeletconfig_controller.go b/controllers/kubeletconfig_controller.go index b1d8538da..28576fd27 100644 --- a/controllers/kubeletconfig_controller.go +++ b/controllers/kubeletconfig_controller.go @@ -63,6 +63,13 @@ type KubeletConfigReconciler struct { Platform platform.Platform } +type kubeletConfigHandler struct { + ownerObject client.Object + mcoKc *mcov1.KubeletConfig + // mcp or nodePool name + poolName string +} + //+kubebuilder:rbac:groups="",resources=configmaps,verbs=* //+kubebuilder:rbac:groups="",resources=events,verbs=create;patch //+kubebuilder:rbac:groups=machineconfiguration.openshift.io,resources=kubeletconfigs,verbs=get;list;watch From 661ba18fb187a302dac109c087890d0a5f552097 Mon Sep 17 00:00:00 2001 From: Talor Itzhak Date: Tue, 17 Sep 2024 13:25:00 +0300 Subject: [PATCH 5/7] controller: build KubeletConfigHandler for platform The `makeKCHandlerForPlatform` provides a common handler that can be used on both HCP and OCP platform. This way allow us to narrow down the changes in the logic in to a centeral close-scoped function in the code, without having the need to branch the logic for HCP and OCP in other areas in the Reconcile loop. Signed-off-by: Talor Itzhak --- controllers/kubeletconfig_controller.go | 116 +++++++++++++++++------- 1 file changed, 85 insertions(+), 31 deletions(-) diff --git a/controllers/kubeletconfig_controller.go b/controllers/kubeletconfig_controller.go index 28576fd27..19cbe07f6 100644 --- a/controllers/kubeletconfig_controller.go +++ b/controllers/kubeletconfig_controller.go @@ -24,6 +24,7 @@ import ( corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" + serializer "k8s.io/apimachinery/pkg/runtime/serializer/json" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/tools/record" "k8s.io/klog/v2" @@ -50,8 +51,13 @@ import ( ) const ( - kubeletConfigRetryPeriod = 30 * time.Second + kubeletConfigRetryPeriod = 30 * time.Second +) + +const ( HypershiftKubeletConfigConfigMapLabel = "hypershift.openshift.io/kubeletconfig-config" + HyperShiftNodePoolLabel = "hypershift.openshift.io/nodePool" + HyperShiftConfigMapConfigKey = "config" ) // KubeletConfigReconciler reconciles a KubeletConfig object @@ -96,7 +102,6 @@ func (r *KubeletConfigReconciler) Reconcile(ctx context.Context, req ctrl.Reques // KubeletConfig changes are expected to be sporadic, yet are important enough // to be made visible at kubernetes level. So we generate events to handle them - cm, err := r.reconcileConfigMap(ctx, instance, req.NamespacedName) if err != nil { var klErr *InvalidKubeletConfig @@ -159,38 +164,11 @@ func (e *InvalidKubeletConfig) Unwrap() error { } func (r *KubeletConfigReconciler) reconcileConfigMap(ctx context.Context, instance *nropv1.NUMAResourcesOperator, kcKey client.ObjectKey) (*corev1.ConfigMap, error) { - mcoKc := &mcov1.KubeletConfig{} - if err := r.Client.Get(ctx, kcKey, mcoKc); err != nil { - return nil, err - } - - mcps, err := machineconfigpools.GetListByNodeGroupsV1(ctx, r.Client, instance.Spec.NodeGroups) + kcHandler, err := r.makeKCHandlerForPlatform(ctx, instance, kcKey) if err != nil { return nil, err } - - mcp, err := machineconfigpools.FindBySelector(mcps, mcoKc.Spec.MachineConfigPoolSelector) - if err != nil { - klog.ErrorS(err, "cannot find a matching mcp for MCO KubeletConfig", "name", kcKey.Name) - var notFound *machineconfigpools.NotFound - if errors.As(err, ¬Found) { - return nil, &InvalidKubeletConfig{ - ObjectName: kcKey.Name, - Err: notFound, - } - } - return nil, err - } - - klog.V(3).InfoS("matched MCP to MCO KubeletConfig", "kubeletconfig name", kcKey.Name, "MCP name", mcp.Name) - - // nothing we care about, and we can't do much anyway - if mcoKc.Spec.KubeletConfig == nil { - klog.InfoS("detected KubeletConfig with empty payload, ignoring", "name", kcKey.Name) - return nil, &InvalidKubeletConfig{ObjectName: kcKey.Name} - } - - kubeletConfig, err := kubeletconfig.MCOKubeletConfToKubeletConf(mcoKc) + kubeletConfig, err := kubeletconfig.MCOKubeletConfToKubeletConf(kcHandler.mcoKc) if err != nil { klog.ErrorS(err, "cannot extract KubeletConfiguration from MCO KubeletConfig", "name", kcKey.Name) return nil, err @@ -230,6 +208,73 @@ func (r *KubeletConfigReconciler) syncConfigMap(ctx context.Context, mcoKc *mcov return rendered, nil } +func (r *KubeletConfigReconciler) makeKCHandlerForPlatform(ctx context.Context, instance *nropv1.NUMAResourcesOperator, kcKey client.ObjectKey) (*kubeletConfigHandler, error) { + switch r.Platform { + case platform.OpenShift: + mcoKc := &mcov1.KubeletConfig{} + if err := r.Client.Get(ctx, kcKey, mcoKc); err != nil { + return nil, err + } + + mcps, err := machineconfigpools.GetListByNodeGroupsV1(ctx, r.Client, instance.Spec.NodeGroups) + if err != nil { + return nil, err + } + + mcp, err := machineconfigpools.FindBySelector(mcps, mcoKc.Spec.MachineConfigPoolSelector) + if err != nil { + klog.ErrorS(err, "cannot find a matching mcp for MCO KubeletConfig", "name", kcKey.Name) + var notFound *machineconfigpools.NotFound + if errors.As(err, ¬Found) { + return nil, &InvalidKubeletConfig{ + ObjectName: kcKey.Name, + Err: notFound, + } + } + return nil, err + } + + klog.V(3).InfoS("matched MCP to MCO KubeletConfig", "kubeletconfig name", kcKey.Name, "MCP name", mcp.Name) + + // nothing we care about, and we can't do much anyway + if mcoKc.Spec.KubeletConfig == nil { + klog.InfoS("detected KubeletConfig with empty payload, ignoring", "name", kcKey.Name) + return nil, &InvalidKubeletConfig{ObjectName: kcKey.Name} + } + return &kubeletConfigHandler{ + ownerObject: mcoKc, + mcoKc: mcoKc, + poolName: mcp.Name, + }, nil + + case platform.HyperShift: + cmKc := &corev1.ConfigMap{} + if err := r.Client.Get(ctx, kcKey, cmKc); err != nil { + return nil, err + } + + nodePoolName := cmKc.Labels[HyperShiftNodePoolLabel] + kcData := cmKc.Data[HyperShiftConfigMapConfigKey] + mcoKc := &mcov1.KubeletConfig{} + err := decodeKCFrom([]byte(kcData), r.Scheme, mcoKc) + if err != nil { + return nil, err + } + + // nothing we care about, and we can't do much anyway + if mcoKc.Spec.KubeletConfig == nil { + klog.InfoS("detected KubeletConfig with empty payload, ignoring", "name", kcKey.Name) + return nil, &InvalidKubeletConfig{ObjectName: kcKey.Name} + } + return &kubeletConfigHandler{ + ownerObject: cmKc, + mcoKc: mcoKc, + poolName: nodePoolName, + }, nil + } + return nil, fmt.Errorf("unsupported platform: %s", r.Platform) +} + func podExcludesListToMap(podExcludes []nropv1.NamespacedName) map[string]string { ret := make(map[string]string) for _, pe := range podExcludes { @@ -237,3 +282,12 @@ func podExcludesListToMap(podExcludes []nropv1.NamespacedName) map[string]string } return ret } + +func decodeKCFrom(data []byte, scheme *runtime.Scheme, mcoKc *mcov1.KubeletConfig) error { + yamlSerializer := serializer.NewSerializerWithOptions( + serializer.DefaultMetaFactory, scheme, scheme, + serializer.SerializerOptions{Yaml: true, Pretty: true, Strict: true}) + + _, _, err := yamlSerializer.Decode(data, nil, mcoKc) + return err +} From f1362e915321f6b525d30b6c6cf590b593d9afb1 Mon Sep 17 00:00:00 2001 From: Talor Itzhak Date: Mon, 16 Sep 2024 12:05:55 +0300 Subject: [PATCH 6/7] controller: refactor syncConfigMap func Change the signature to accept kubeletConfigHandler object. Signed-off-by: Talor Itzhak --- controllers/kubeletconfig_controller.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/controllers/kubeletconfig_controller.go b/controllers/kubeletconfig_controller.go index 19cbe07f6..1dac17801 100644 --- a/controllers/kubeletconfig_controller.go +++ b/controllers/kubeletconfig_controller.go @@ -174,11 +174,11 @@ func (r *KubeletConfigReconciler) reconcileConfigMap(ctx context.Context, instan return nil, err } - return r.syncConfigMap(ctx, mcoKc, kubeletConfig, instance, mcp.Name) + return r.syncConfigMap(ctx, kubeletConfig, instance, kcHandler) } -func (r *KubeletConfigReconciler) syncConfigMap(ctx context.Context, mcoKc *mcov1.KubeletConfig, kubeletConfig *kubeletconfigv1beta1.KubeletConfiguration, instance *nropv1.NUMAResourcesOperator, mcpName string) (*corev1.ConfigMap, error) { - generatedName := objectnames.GetComponentName(instance.Name, mcpName) +func (r *KubeletConfigReconciler) syncConfigMap(ctx context.Context, kubeletConfig *kubeletconfigv1beta1.KubeletConfiguration, instance *nropv1.NUMAResourcesOperator, kcHandler *kubeletConfigHandler) (*corev1.ConfigMap, error) { + generatedName := objectnames.GetComponentName(instance.Name, kcHandler.poolName) klog.V(3).InfoS("generated configMap name", "generatedName", generatedName) podExcludes := podExcludesListToMap(instance.Spec.PodExcludes) @@ -192,13 +192,13 @@ func (r *KubeletConfigReconciler) syncConfigMap(ctx context.Context, mcoKc *mcov rendered := rteconfig.CreateConfigMap(r.Namespace, generatedName, data) cfgManifests := cfgstate.Manifests{ - Config: rteconfig.AddSoftRefLabels(rendered, instance.Name, mcpName), + Config: rteconfig.AddSoftRefLabels(rendered, instance.Name, kcHandler.poolName), } existing := cfgstate.FromClient(ctx, r.Client, r.Namespace, generatedName) for _, objState := range existing.State(cfgManifests) { // the owner should be the KubeletConfig object and not the NUMAResourcesOperator CR // this means that when KubeletConfig will get deleted, the ConfigMap gets deleted as well - if err := controllerutil.SetControllerReference(mcoKc, objState.Desired, r.Scheme); err != nil { + if err := controllerutil.SetControllerReference(kcHandler.ownerObject, objState.Desired, r.Scheme); err != nil { return nil, errors.Wrapf(err, "Failed to set controller reference to %s %s", objState.Desired.GetNamespace(), objState.Desired.GetName()) } if _, _, err := apply.ApplyObject(ctx, r.Client, objState); err != nil { From c9d46909b4ec05cf3980bac0a1870f19fd47e67c Mon Sep 17 00:00:00 2001 From: Talor Itzhak Date: Tue, 17 Sep 2024 13:27:28 +0300 Subject: [PATCH 7/7] unit-tests: modify the controller tests for HCP Put the kubeletconfig controller tests inside a DescribeTable where one entry is for OCP and another for HCP. Signed-off-by: Talor Itzhak --- controllers/kubeletconfig_controller_test.go | 86 ++++++++++++++----- .../numaresourcesoperator_controller_test.go | 1 - internal/objects/objects.go | 27 ++++++ 3 files changed, 90 insertions(+), 24 deletions(-) diff --git a/controllers/kubeletconfig_controller_test.go b/controllers/kubeletconfig_controller_test.go index 019842a5b..a8cda7fd2 100644 --- a/controllers/kubeletconfig_controller_test.go +++ b/controllers/kubeletconfig_controller_test.go @@ -32,6 +32,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client/fake" "sigs.k8s.io/controller-runtime/pkg/reconcile" + "github.com/k8stopologyawareschedwg/deployer/pkg/deployer/platform" nropv1 "github.com/openshift-kni/numaresources-operator/api/numaresourcesoperator/v1" "github.com/openshift-kni/numaresources-operator/pkg/objectnames" rteconfig "github.com/openshift-kni/numaresources-operator/rte/pkg/config" @@ -39,6 +40,8 @@ import ( testobjs "github.com/openshift-kni/numaresources-operator/internal/objects" ) +type reconcilerBuilderFunc func(...runtime.Object) (*KubeletConfigReconciler, error) + const ( bufferSize = 1024 ) @@ -50,15 +53,30 @@ func NewFakeKubeletConfigReconciler(initObjects ...runtime.Object) (*KubeletConf Scheme: scheme.Scheme, Namespace: testNamespace, Recorder: record.NewFakeRecorder(bufferSize), + Platform: platform.OpenShift, + }, nil +} + +func NewFakeKubeletConfigReconcilerForHyperShift(initObjects ...runtime.Object) (*KubeletConfigReconciler, error) { + fakeClient := fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(initObjects...).Build() + return &KubeletConfigReconciler{ + Client: fakeClient, + Scheme: scheme.Scheme, + Namespace: testNamespace, + Recorder: record.NewFakeRecorder(bufferSize), + Platform: platform.HyperShift, }, nil } var _ = Describe("Test KubeletConfig Reconcile", func() { - Context("with KubeletConfig objects already present in the cluster", func() { + DescribeTableSubtree("On different platforms with KubeletConfig objects already present in the cluster", func(newFakeReconciler reconcilerBuilderFunc, clusterPlatform platform.Platform) { var nro *nropv1.NUMAResourcesOperator var mcp1 *machineconfigv1.MachineConfigPool var mcoKc1 *machineconfigv1.KubeletConfig var label1 map[string]string + var key client.ObjectKey + var poolName string + cmKc1 := &corev1.ConfigMap{} BeforeEach(func() { label1 = map[string]string{ @@ -70,23 +88,30 @@ var _ = Describe("Test KubeletConfig Reconcile", func() { }) kubeletConfig := &kubeletconfigv1beta1.KubeletConfiguration{} mcoKc1 = testobjs.NewKubeletConfig("test1", label1, mcp1.Spec.MachineConfigSelector, kubeletConfig) + key = client.ObjectKeyFromObject(mcoKc1) + poolName = mcp1.Name + + if clusterPlatform == platform.HyperShift { + poolName = "test-hostedcluster1" + label1[HyperShiftNodePoolLabel] = poolName + cmKc1 = testobjs.NewKubeletConfigConfigMap("test1", label1, mcoKc1) + key = client.ObjectKeyFromObject(cmKc1) + } }) Context("on the first iteration", func() { It("without NRO present, should wait", func() { - reconciler, err := NewFakeKubeletConfigReconciler(mcp1, mcoKc1) + reconciler, err := newFakeReconciler(mcp1, mcoKc1, cmKc1) Expect(err).ToNot(HaveOccurred()) - key := client.ObjectKeyFromObject(mcoKc1) result, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: key}) Expect(err).ToNot(HaveOccurred()) Expect(result).To(Equal(reconcile.Result{RequeueAfter: kubeletConfigRetryPeriod})) }) It("with NRO present, should create configmap", func() { - reconciler, err := NewFakeKubeletConfigReconciler(nro, mcp1, mcoKc1) + reconciler, err := newFakeReconciler(nro, mcp1, mcoKc1, cmKc1) Expect(err).ToNot(HaveOccurred()) - key := client.ObjectKeyFromObject(mcoKc1) result, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: key}) Expect(err).ToNot(HaveOccurred()) Expect(result).To(Equal(reconcile.Result{})) @@ -94,15 +119,14 @@ var _ = Describe("Test KubeletConfig Reconcile", func() { cm := &corev1.ConfigMap{} key = client.ObjectKey{ Namespace: testNamespace, - Name: objectnames.GetComponentName(nro.Name, mcp1.Name), + Name: objectnames.GetComponentName(nro.Name, poolName), } Expect(reconciler.Client.Get(context.TODO(), key, cm)).ToNot(HaveOccurred()) }) It("with NRO present, the created configmap should have the linking labels", func() { - reconciler, err := NewFakeKubeletConfigReconciler(nro, mcp1, mcoKc1) + reconciler, err := newFakeReconciler(nro, mcp1, mcoKc1, cmKc1) Expect(err).ToNot(HaveOccurred()) - key := client.ObjectKeyFromObject(mcoKc1) result, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: key}) Expect(err).ToNot(HaveOccurred()) Expect(result).To(Equal(reconcile.Result{})) @@ -110,17 +134,16 @@ var _ = Describe("Test KubeletConfig Reconcile", func() { cm := &corev1.ConfigMap{} key = client.ObjectKey{ Namespace: testNamespace, - Name: objectnames.GetComponentName(nro.Name, mcp1.Name), + Name: objectnames.GetComponentName(nro.Name, poolName), } Expect(reconciler.Client.Get(context.TODO(), key, cm)).ToNot(HaveOccurred()) Expect(cm.Labels).To(HaveKeyWithValue(rteconfig.LabelOperatorName, nro.Name)) - Expect(cm.Labels).To(HaveKeyWithValue(rteconfig.LabelNodeGroupName+"/"+rteconfig.LabelNodeGroupKindMachineConfigPool, mcp1.Name)) + Expect(cm.Labels).To(HaveKeyWithValue(rteconfig.LabelNodeGroupName+"/"+rteconfig.LabelNodeGroupKindMachineConfigPool, poolName)) }) - It("should send events when NRO present and operation succesfull", func() { - reconciler, err := NewFakeKubeletConfigReconciler(nro, mcp1, mcoKc1) + It("should send events when NRO present and operation successful", func() { + reconciler, err := newFakeReconciler(nro, mcp1, mcoKc1, cmKc1) Expect(err).ToNot(HaveOccurred()) - key := client.ObjectKeyFromObject(mcoKc1) result, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: key}) Expect(err).ToNot(HaveOccurred()) Expect(result).To(Equal(reconcile.Result{})) @@ -134,7 +157,9 @@ var _ = Describe("Test KubeletConfig Reconcile", func() { It("should send events when NRO present and operation failure", func() { brokenMcoKc := testobjs.NewKubeletConfigWithData("broken", label1, mcp1.Spec.MachineConfigSelector, []byte("")) - reconciler, err := NewFakeKubeletConfigReconciler(nro, mcp1, brokenMcoKc) + // on HyperShift we can mimic this behavior by not having a ConfigMap with a KubeletConfig + // present on the cluster at all + reconciler, err := newFakeReconciler(nro, mcp1, brokenMcoKc) Expect(err).ToNot(HaveOccurred()) key := client.ObjectKeyFromObject(brokenMcoKc) @@ -150,7 +175,9 @@ var _ = Describe("Test KubeletConfig Reconcile", func() { It("should skip invalid kubeletconfig", func() { invalidMcoKc := testobjs.NewKubeletConfigWithoutData("payloadless", label1, mcp1.Spec.MachineConfigSelector) - reconciler, err := NewFakeKubeletConfigReconciler(nro, mcp1, invalidMcoKc) + // adding a CM for when this test emulates HyperShift platform + invalidCmMcoKc := testobjs.NewKubeletConfigConfigMap("payloadless", label1, invalidMcoKc) + reconciler, err := newFakeReconciler(nro, mcp1, invalidMcoKc, invalidCmMcoKc) Expect(err).ToNot(HaveOccurred()) key := client.ObjectKeyFromObject(invalidMcoKc) @@ -167,8 +194,9 @@ var _ = Describe("Test KubeletConfig Reconcile", func() { It("should ignore non-matching kubeketconfigs", func() { ctrlPlaneKc := testobjs.NewKubeletConfigAutoresizeControlPlane() - - reconciler, err := NewFakeKubeletConfigReconciler(nro, mcp1, mcoKc1, ctrlPlaneKc) + // adding a CM for when this test emulates HyperShift platform + ctrlPlaneCmKc := testobjs.NewKubeletConfigConfigMap(ctrlPlaneKc.Name, label1, ctrlPlaneKc) + reconciler, err := newFakeReconciler(nro, mcp1, mcoKc1, ctrlPlaneKc, ctrlPlaneCmKc) Expect(err).ToNot(HaveOccurred()) key := client.ObjectKeyFromObject(ctrlPlaneKc) @@ -185,16 +213,20 @@ var _ = Describe("Test KubeletConfig Reconcile", func() { }) It("should process matching kubeletconfig, then ignore non-matching kubeketconfig", func() { - reconciler, err := NewFakeKubeletConfigReconciler(nro, mcp1) + reconciler, err := newFakeReconciler(nro, mcp1) Expect(err).ToNot(HaveOccurred()) fakeRecorder, ok := reconciler.Recorder.(*record.FakeRecorder) Expect(ok).To(BeTrue()) - err = reconciler.Client.Create(context.TODO(), mcoKc1) + var reconciledObj client.Object + reconciledObj = mcoKc1 + if clusterPlatform == platform.HyperShift { + reconciledObj = cmKc1 + } + err = reconciler.Client.Create(context.TODO(), reconciledObj) Expect(err).ToNot(HaveOccurred()) - key := client.ObjectKeyFromObject(mcoKc1) result, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: key}) Expect(err).ToNot(HaveOccurred()) Expect(result).To(Equal(reconcile.Result{})) @@ -202,18 +234,23 @@ var _ = Describe("Test KubeletConfig Reconcile", func() { cm := &corev1.ConfigMap{} key = client.ObjectKey{ Namespace: testNamespace, - Name: objectnames.GetComponentName(nro.Name, mcp1.Name), + Name: objectnames.GetComponentName(nro.Name, poolName), } Expect(reconciler.Client.Get(context.TODO(), key, cm)).ToNot(HaveOccurred()) // verify creation event event := <-fakeRecorder.Events Expect(event).To(ContainSubstring("ProcessOK")) - Expect(event).To(ContainSubstring(mcoKc1.Name)) + Expect(event).To(ContainSubstring(reconciledObj.GetName())) ctrlPlaneKc := testobjs.NewKubeletConfigAutoresizeControlPlane() err = reconciler.Client.Create(context.TODO(), ctrlPlaneKc) Expect(err).ToNot(HaveOccurred()) + // adding a CM for when this test emulates HyperShift platform + ctrlPlaneCmKc := testobjs.NewKubeletConfigConfigMap(ctrlPlaneKc.Name, label1, ctrlPlaneKc) + err = reconciler.Client.Create(context.TODO(), ctrlPlaneCmKc) + Expect(err).ToNot(HaveOccurred()) + key = client.ObjectKeyFromObject(ctrlPlaneKc) result, err = reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: key}) Expect(err).ToNot(HaveOccurred()) @@ -225,5 +262,8 @@ var _ = Describe("Test KubeletConfig Reconcile", func() { Expect(event).To(ContainSubstring(ctrlPlaneKc.Name)) }) }) - }) + }, + Entry("OpenShift Platform", NewFakeKubeletConfigReconciler, platform.OpenShift), + Entry("HyperShift Platform", NewFakeKubeletConfigReconcilerForHyperShift, platform.HyperShift), + ) }) diff --git a/controllers/numaresourcesoperator_controller_test.go b/controllers/numaresourcesoperator_controller_test.go index c07ac6534..5962b75a0 100644 --- a/controllers/numaresourcesoperator_controller_test.go +++ b/controllers/numaresourcesoperator_controller_test.go @@ -66,7 +66,6 @@ func NewFakeNUMAResourcesOperatorReconciler(plat platform.Platform, platVersion if err != nil { return nil, err } - rteManifests, err := rtemanifests.GetManifests(plat, platVersion, testNamespace, true) if err != nil { return nil, err diff --git a/internal/objects/objects.go b/internal/objects/objects.go index 55e61ff50..a1f0b2427 100644 --- a/internal/objects/objects.go +++ b/internal/objects/objects.go @@ -17,6 +17,7 @@ package objects import ( + "bytes" "encoding/json" "time" @@ -24,6 +25,8 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + serializer "k8s.io/apimachinery/pkg/runtime/serializer/json" + "k8s.io/client-go/kubernetes/scheme" kubeletconfigv1beta1 "k8s.io/kubelet/config/v1beta1" nropv1 "github.com/openshift-kni/numaresources-operator/api/numaresourcesoperator/v1" @@ -149,6 +152,30 @@ func NewKubeletConfigAutoresizeControlPlane() *machineconfigv1.KubeletConfig { return ctrlPlaneKc } +func NewKubeletConfigConfigMap(name string, labels map[string]string, config *machineconfigv1.KubeletConfig) *corev1.ConfigMap { + yamlSerializer := serializer.NewSerializerWithOptions( + serializer.DefaultMetaFactory, scheme.Scheme, scheme.Scheme, + serializer.SerializerOptions{Yaml: true, Pretty: true, Strict: true}) + + buff := &bytes.Buffer{} + // supervised testing environment + // no need to check for error + _ = yamlSerializer.Encode(config, buff) + return &corev1.ConfigMap{ + TypeMeta: metav1.TypeMeta{ + Kind: "ConfigMap", + APIVersion: corev1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Labels: labels, + }, + Data: map[string]string{ + "config": buff.String(), + }, + } +} + func NewNamespace(name string) *corev1.Namespace { return &corev1.Namespace{ TypeMeta: metav1.TypeMeta{