diff --git a/bundle/manifests/numaresources-operator.clusterserviceversion.yaml b/bundle/manifests/numaresources-operator.clusterserviceversion.yaml index a62cb30d5c..182cecffe7 100644 --- a/bundle/manifests/numaresources-operator.clusterserviceversion.yaml +++ b/bundle/manifests/numaresources-operator.clusterserviceversion.yaml @@ -66,7 +66,7 @@ metadata: } ] capabilities: Basic Install - createdAt: "2026-07-06T08:55:09Z" + createdAt: "2026-07-14T08:24:59Z" features.operators.openshift.io/cnf: "true" features.operators.openshift.io/cni: "false" features.operators.openshift.io/csi: "false" @@ -381,22 +381,22 @@ spec: - apiGroups: - "" resources: - - configmaps - - serviceaccounts - verbs: - - '*' - - apiGroups: - - "" - resources: - - events + - bindings + - pods/binding verbs: - create - - patch - apiGroups: - "" resources: + - configmaps + - namespaces - nodes + - persistentvolumeclaims + - persistentvolumes + - replicationcontrollers + - services verbs: + - get - list - watch - apiGroups: @@ -404,22 +404,72 @@ spec: resources: - pods verbs: + - delete - get - list - watch + - apiGroups: + - "" + resources: + - pods/status + verbs: + - patch + - update + - apiGroups: + - "" + - events.k8s.io + resources: + - events + verbs: + - create + - patch + - update - apiGroups: - apiextensions.k8s.io resources: - customresourcedefinitions verbs: - - '*' + - create + - list + - watch + - apiGroups: + - apiextensions.k8s.io + resourceNames: + - noderesourcetopologies.topology.node.k8s.io + resources: + - customresourcedefinitions + verbs: + - get + - update - apiGroups: - apps resources: - - daemonsets - - deployments + - statefulsets + verbs: + - get + - list + - watch + - apiGroups: + - apps + - extensions + resources: + - replicasets + verbs: + - get + - list + - watch + - apiGroups: + - authentication.k8s.io + resources: + - tokenreviews + verbs: + - create + - apiGroups: + - authorization.k8s.io + resources: + - subjectaccessreviews verbs: - - '*' + - create - apiGroups: - config.openshift.io resources: @@ -441,6 +491,36 @@ spec: - clusterversions verbs: - list + - apiGroups: + - coordination.k8s.io + resources: + - leasecandidates + verbs: + - create + - delete + - deletecollection + - get + - list + - patch + - update + - watch + - apiGroups: + - coordination.k8s.io + resources: + - leases + verbs: + - create + - apiGroups: + - coordination.k8s.io + resourceNames: + - numa-scheduler-leader + resources: + - leases + verbs: + - get + - list + - update + - watch - apiGroups: - machineconfiguration.openshift.io resources: @@ -461,7 +541,12 @@ spec: resources: - machineconfigs verbs: - - '*' + - create + - delete + - get + - list + - update + - watch - apiGroups: - nodetopology.openshift.io resources: @@ -487,21 +572,91 @@ spec: - get - patch - update + - apiGroups: + - policy + resources: + - poddisruptionbudgets + verbs: + - get + - list + - watch + - apiGroups: + - rbac.authorization.k8s.io + resourceNames: + - secondary-scheduler + resources: + - clusterrolebindings + verbs: + - delete - apiGroups: - rbac.authorization.k8s.io resources: - clusterrolebindings - clusterroles - - rolebindings - - roles verbs: - - '*' + - create + - list + - watch + - apiGroups: + - rbac.authorization.k8s.io + resourceNames: + - rte + resources: + - clusterrolebindings + - clusterroles + verbs: + - get + - update + - apiGroups: + - rbac.authorization.k8s.io + resourceNames: + - topology-aware-scheduler + resources: + - clusterrolebindings + - clusterroles + verbs: + - get + - update + - apiGroups: + - resource.k8s.io + resources: + - deviceclasses + - resourceclaims + - resourceslices + verbs: + - get + - list + - watch - apiGroups: - security.openshift.io resources: - securitycontextconstraints verbs: - - '*' + - create + - list + - watch + - apiGroups: + - security.openshift.io + resourceNames: + - resource-topology-exporter + - resource-topology-exporter-v2 + resources: + - securitycontextconstraints + verbs: + - get + - update + - apiGroups: + - storage.k8s.io + resources: + - csidrivers + - csinodes + - csistoragecapacities + - storageclasses + - volumeattachments + verbs: + - get + - list + - watch - apiGroups: - topology.node.k8s.io resources: @@ -511,6 +666,7 @@ spec: - get - list - update + - watch serviceAccountName: numaresources-controller-manager deployments: - label: @@ -649,15 +805,150 @@ spec: - apiGroups: - "" resources: + - configmaps + verbs: + - create + - delete + - get + - list + - update + - watch + - apiGroups: + - "" + resources: + - endpoints + verbs: + - create + - apiGroups: + - "" + resourceNames: + - numa-scheduler-leader + resources: + - endpoints + verbs: + - get + - update + - apiGroups: + - "" + resourceNames: + - rte + resources: + - serviceaccounts + verbs: + - get + - update + - apiGroups: + - "" + resourceNames: + - secondary-scheduler + resources: + - serviceaccounts + verbs: + - get + - update + - apiGroups: + - "" + resources: + - serviceaccounts + - services + verbs: + - create + - list + - watch + - apiGroups: + - "" + resourceNames: + - numaresources-rte-metrics-service + resources: - services verbs: - - '*' + - get + - update + - apiGroups: + - apps + resources: + - daemonsets + verbs: + - create + - delete + - get + - list + - update + - watch + - apiGroups: + - apps + resources: + - deployments + verbs: + - create + - list + - watch + - apiGroups: + - apps + resourceNames: + - secondary-scheduler + resources: + - deployments + verbs: + - get + - update + - apiGroups: + - networking.k8s.io + resources: + - networkpolicies + verbs: + - create + - list + - watch + - apiGroups: + - networking.k8s.io + resourceNames: + - rte-default-deny-all + - rte-egress-to-api-server + resources: + - networkpolicies + verbs: + - get + - update - apiGroups: - networking.k8s.io + resourceNames: + - scheduler-default-deny-all + - scheduler-egress-to-api-server resources: - networkpolicies verbs: - - '*' + - get + - update + - apiGroups: + - rbac.authorization.k8s.io + resources: + - rolebindings + - roles + verbs: + - create + - list + - watch + - apiGroups: + - rbac.authorization.k8s.io + resourceNames: + - rte + resources: + - rolebindings + - roles + verbs: + - get + - update + - apiGroups: + - rbac.authorization.k8s.io + resourceNames: + - topology-aware-scheduler-leader-elect + resources: + - rolebindings + - roles + verbs: + - get + - update serviceAccountName: numaresources-controller-manager strategy: deployment installModes: diff --git a/cmd/main.go b/cmd/main.go index 10aa384cf6..bdd2bdff1a 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -38,7 +38,6 @@ import ( securityv1 "github.com/openshift/api/security/v1" ctrltls "github.com/openshift/controller-runtime-common/pkg/tls" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" k8sruntime "k8s.io/apimachinery/pkg/runtime" utilruntime "k8s.io/apimachinery/pkg/util/runtime" clientgoscheme "k8s.io/client-go/kubernetes/scheme" @@ -48,6 +47,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/cache" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/healthz" + "sigs.k8s.io/controller-runtime/pkg/manager" metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" "sigs.k8s.io/controller-runtime/pkg/webhook" @@ -68,6 +68,7 @@ import ( "github.com/openshift-kni/numaresources-operator/pkg/images" rtemetricsmanifests "github.com/openshift-kni/numaresources-operator/pkg/metrics/manifests/monitor" schedmanifests "github.com/openshift-kni/numaresources-operator/pkg/numaresourcesscheduler/manifests/sched" + schedstate "github.com/openshift-kni/numaresources-operator/pkg/numaresourcesscheduler/objectstate/sched" rtestate "github.com/openshift-kni/numaresources-operator/pkg/objectstate/rte" rteupdate "github.com/openshift-kni/numaresources-operator/pkg/objectupdate/rte" schedupdate "github.com/openshift-kni/numaresources-operator/pkg/objectupdate/sched" @@ -289,6 +290,7 @@ func main() { if !ok { namespace = defaultNamespace } + klog.InfoS("namespace detected", "namespace", namespace) rteManifests, err := rtemanifests.GetManifests(discoveredCluster.Platform, discoveredCluster.ShortVersion, namespace, false, true) if err != nil { @@ -340,12 +342,16 @@ func main() { webhookTLSOpts := append(webhookTLSOpts(params.enableHTTP2), tlsConfig) + cacheNamespaces := map[string]cache.Config{ + namespace: {}, + } + if discoveredCluster.Platform == platform.HyperShift { + cacheNamespaces[controller.HyperShiftKubeletConfigConfigMapNamespace] = cache.Config{} + } + mgr, err := ctrl.NewManager(restConfig, ctrl.Options{ Cache: cache.Options{ - DefaultNamespaces: map[string]cache.Config{ - namespace: {}, - metav1.NamespaceNone: {}, - }, + DefaultNamespaces: cacheNamespaces, }, Scheme: scheme, Metrics: metricsserver.Options{ @@ -393,7 +399,6 @@ func main() { if err = (&controller.NUMAResourcesOperatorReconciler{ Client: mgr.GetClient(), Scheme: mgr.GetScheme(), - Recorder: mgr.GetEventRecorderFor("numaresources-controller"), APIManifests: apiManifests, RTEManifests: rtestate.Manifests{ Core: rteManifestsRendered, @@ -413,7 +418,6 @@ func main() { if err = (&controller.KubeletConfigReconciler{ Client: mgr.GetClient(), Scheme: mgr.GetScheme(), - Recorder: mgr.GetEventRecorderFor("kubeletconfig-controller"), Namespace: namespace, Platform: discoveredCluster.Platform, }).SetupWithManager(mgr); err != nil { @@ -421,6 +425,19 @@ func main() { exitWithCancel(cancel, 1) } + // intentionally done unconditionally to catch the unlikely case someone managed + // to disable the scheduler pre-upgrade (very hard task anyway, but still) to + // ensure no dangerous leftovers + if err = mgr.Add(manager.RunnableFunc(func(ctx context.Context) error { + if err := schedstate.Cleanup(ctx, mgr.GetClient()); err != nil { + klog.ErrorS(err, "best-effort scheduler cleanup failed") + } + return nil + })); err != nil { + klog.ErrorS(err, "unable to add scheduler cleanup runnable") + exitWithCancel(cancel, 1) + } + if params.enableScheduler { schedMf, err := schedmanifests.GetManifests(namespace) if err != nil { diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index b18c4080c2..a10fb3cd95 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -7,22 +7,22 @@ rules: - apiGroups: - "" resources: - - configmaps - - serviceaccounts - verbs: - - '*' -- apiGroups: - - "" - resources: - - events + - bindings + - pods/binding verbs: - create - - patch - apiGroups: - "" resources: + - configmaps + - namespaces - nodes + - persistentvolumeclaims + - persistentvolumes + - replicationcontrollers + - services verbs: + - get - list - watch - apiGroups: @@ -30,22 +30,72 @@ rules: resources: - pods verbs: + - delete - get - list - watch +- apiGroups: + - "" + resources: + - pods/status + verbs: + - patch + - update +- apiGroups: + - "" + - events.k8s.io + resources: + - events + verbs: + - create + - patch + - update +- apiGroups: + - apiextensions.k8s.io + resources: + - customresourcedefinitions + verbs: + - create + - list + - watch - apiGroups: - apiextensions.k8s.io + resourceNames: + - noderesourcetopologies.topology.node.k8s.io resources: - customresourcedefinitions verbs: - - '*' + - get + - update - apiGroups: - apps resources: - - daemonsets - - deployments + - statefulsets + verbs: + - get + - list + - watch +- apiGroups: + - apps + - extensions + resources: + - replicasets verbs: - - '*' + - get + - list + - watch +- apiGroups: + - authentication.k8s.io + resources: + - tokenreviews + verbs: + - create +- apiGroups: + - authorization.k8s.io + resources: + - subjectaccessreviews + verbs: + - create - apiGroups: - config.openshift.io resources: @@ -67,6 +117,36 @@ rules: - clusterversions verbs: - list +- apiGroups: + - coordination.k8s.io + resources: + - leasecandidates + verbs: + - create + - delete + - deletecollection + - get + - list + - patch + - update + - watch +- apiGroups: + - coordination.k8s.io + resources: + - leases + verbs: + - create +- apiGroups: + - coordination.k8s.io + resourceNames: + - numa-scheduler-leader + resources: + - leases + verbs: + - get + - list + - update + - watch - apiGroups: - machineconfiguration.openshift.io resources: @@ -87,7 +167,12 @@ rules: resources: - machineconfigs verbs: - - '*' + - create + - delete + - get + - list + - update + - watch - apiGroups: - nodetopology.openshift.io resources: @@ -113,21 +198,91 @@ rules: - get - patch - update +- apiGroups: + - policy + resources: + - poddisruptionbudgets + verbs: + - get + - list + - watch +- apiGroups: + - rbac.authorization.k8s.io + resourceNames: + - secondary-scheduler + resources: + - clusterrolebindings + verbs: + - delete - apiGroups: - rbac.authorization.k8s.io resources: - clusterrolebindings - clusterroles - - rolebindings - - roles verbs: - - '*' + - create + - list + - watch +- apiGroups: + - rbac.authorization.k8s.io + resourceNames: + - rte + resources: + - clusterrolebindings + - clusterroles + verbs: + - get + - update +- apiGroups: + - rbac.authorization.k8s.io + resourceNames: + - topology-aware-scheduler + resources: + - clusterrolebindings + - clusterroles + verbs: + - get + - update +- apiGroups: + - resource.k8s.io + resources: + - deviceclasses + - resourceclaims + - resourceslices + verbs: + - get + - list + - watch +- apiGroups: + - security.openshift.io + resources: + - securitycontextconstraints + verbs: + - create + - list + - watch - apiGroups: - security.openshift.io + resourceNames: + - resource-topology-exporter + - resource-topology-exporter-v2 resources: - securitycontextconstraints verbs: - - '*' + - get + - update +- apiGroups: + - storage.k8s.io + resources: + - csidrivers + - csinodes + - csistoragecapacities + - storageclasses + - volumeattachments + verbs: + - get + - list + - watch - apiGroups: - topology.node.k8s.io resources: @@ -137,6 +292,7 @@ rules: - get - list - update + - watch --- apiVersion: rbac.authorization.k8s.io/v1 kind: Role @@ -147,12 +303,147 @@ rules: - apiGroups: - "" resources: + - configmaps + verbs: + - create + - delete + - get + - list + - update + - watch +- apiGroups: + - "" + resources: + - endpoints + verbs: + - create +- apiGroups: + - "" + resourceNames: + - numa-scheduler-leader + resources: + - endpoints + verbs: + - get + - update +- apiGroups: + - "" + resourceNames: + - rte + resources: + - serviceaccounts + verbs: + - get + - update +- apiGroups: + - "" + resourceNames: + - secondary-scheduler + resources: + - serviceaccounts + verbs: + - get + - update +- apiGroups: + - "" + resources: + - serviceaccounts - services verbs: - - '*' + - create + - list + - watch +- apiGroups: + - "" + resourceNames: + - numaresources-rte-metrics-service + resources: + - services + verbs: + - get + - update +- apiGroups: + - apps + resources: + - daemonsets + verbs: + - create + - delete + - get + - list + - update + - watch +- apiGroups: + - apps + resources: + - deployments + verbs: + - create + - list + - watch +- apiGroups: + - apps + resourceNames: + - secondary-scheduler + resources: + - deployments + verbs: + - get + - update +- apiGroups: + - networking.k8s.io + resources: + - networkpolicies + verbs: + - create + - list + - watch +- apiGroups: + - networking.k8s.io + resourceNames: + - rte-default-deny-all + - rte-egress-to-api-server + resources: + - networkpolicies + verbs: + - get + - update - apiGroups: - networking.k8s.io + resourceNames: + - scheduler-default-deny-all + - scheduler-egress-to-api-server resources: - networkpolicies verbs: - - '*' + - get + - update +- apiGroups: + - rbac.authorization.k8s.io + resources: + - rolebindings + - roles + verbs: + - create + - list + - watch +- apiGroups: + - rbac.authorization.k8s.io + resourceNames: + - rte + resources: + - rolebindings + - roles + verbs: + - get + - update +- apiGroups: + - rbac.authorization.k8s.io + resourceNames: + - topology-aware-scheduler-leader-elect + resources: + - rolebindings + - roles + verbs: + - get + - update diff --git a/hack/verify-generated.sh b/hack/verify-generated.sh index 92e93ffc4f..fab2c7956a 100755 --- a/hack/verify-generated.sh +++ b/hack/verify-generated.sh @@ -2,7 +2,7 @@ # compare the CSV file against the latest revision # but ignore the createdAt field which is always changing. -if [[ "$(git diff --quiet -I'^ createdAt: ' bundle)" -eq 0 ]]; then +if git diff --quiet -I'^ createdAt: ' bundle; then git checkout --quiet bundle fi diff --git a/internal/controller/kubeletconfig_controller.go b/internal/controller/kubeletconfig_controller.go index d97c2e8860..3d9b669854 100644 --- a/internal/controller/kubeletconfig_controller.go +++ b/internal/controller/kubeletconfig_controller.go @@ -30,7 +30,6 @@ import ( "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" - "k8s.io/client-go/tools/record" "k8s.io/klog/v2" kubeletconfigv1beta1 "k8s.io/kubelet/config/v1beta1" @@ -71,7 +70,6 @@ const ( type KubeletConfigReconciler struct { client.Client Scheme *runtime.Scheme - Recorder record.EventRecorder Namespace string Platform platform.Platform // garbageCollectionFor is a list of objects @@ -88,10 +86,9 @@ type kubeletConfigHandler struct { } // Namespace Scoped - +//+kubebuilder:rbac:groups="",resources=configmaps,verbs=create;delete;update,namespace="numaresources" // Cluster Scoped -//+kubebuilder:rbac:groups="",resources=configmaps,verbs=* -//+kubebuilder:rbac:groups="",resources=events,verbs=create;patch +//+kubebuilder:rbac:groups="",resources=configmaps,verbs=get;list;watch //+kubebuilder:rbac:groups=machineconfiguration.openshift.io,resources=kubeletconfigs,verbs=get;list;watch //+kubebuilder:rbac:groups=machineconfiguration.openshift.io,resources=kubeletconfigs/finalizers,verbs=update @@ -120,17 +117,15 @@ func (r *KubeletConfigReconciler) Reconcile(ctx context.Context, req ctrl.Reques if err != nil { var klErr *InvalidKubeletConfig if errors.As(err, &klErr) { - r.Recorder.Event(instance, "Normal", "ProcessSkip", "ignored kubelet config "+klErr.ObjectName) + klog.InfoS("ignored kubelet config", "object", klErr.ObjectName) return ctrl.Result{}, nil } klog.ErrorS(err, "failed to reconcile configmap", "controller", "kubeletconfig") - - r.Recorder.Event(instance, "Warning", "ProcessFailed", "Failed to update RTE config from kubelet config "+req.NamespacedName.String()) return ctrl.Result{}, err } - r.Recorder.Event(instance, "Normal", "ProcessOK", fmt.Sprintf("Updated RTE config %s/%s from kubelet config %s", cm.Namespace, cm.Name, req.NamespacedName.String())) + klog.InfoS("updated RTE config from kubelet config", "configmap", klog.KRef(cm.Namespace, cm.Name), "kubeletconfig", req.NamespacedName.String()) return ctrl.Result{}, nil } diff --git a/internal/controller/kubeletconfig_controller_test.go b/internal/controller/kubeletconfig_controller_test.go index fae8a52f72..2e682e11ff 100644 --- a/internal/controller/kubeletconfig_controller_test.go +++ b/internal/controller/kubeletconfig_controller_test.go @@ -23,7 +23,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/kubernetes/scheme" - "k8s.io/client-go/tools/record" kubeletconfigv1beta1 "k8s.io/kubelet/config/v1beta1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -45,17 +44,12 @@ import ( type reconcilerBuilderFunc func(...runtime.Object) (*KubeletConfigReconciler, error) -const ( - bufferSize = 1024 -) - func NewFakeKubeletConfigReconciler(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.OpenShift, }, nil } @@ -66,7 +60,6 @@ func NewFakeKubeletConfigReconcilerForHyperShift(initObjects ...runtime.Object) Client: fakeClient, Scheme: scheme.Scheme, Namespace: testNamespace, - Recorder: record.NewFakeRecorder(bufferSize), Platform: platform.HyperShift, }, nil } @@ -146,22 +139,7 @@ var _ = Describe("Test KubeletConfig Reconcile", func() { Expect(cm.Labels).To(HaveKeyWithValue(rteconfig.LabelOperatorName, nro.Name)) Expect(cm.Labels).To(HaveKeyWithValue(rteconfig.LabelNodeGroupName+"/"+rteconfig.LabelNodeGroupKindMachineConfigPool, poolName)) }) - It("should send events when NRO present and operation successful", func() { - reconciler, err := newFakeReconciler(nro, mcp1, mcoKc1, cmKc1) - Expect(err).ToNot(HaveOccurred()) - - result, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: key}) - Expect(err).ToNot(HaveOccurred()) - Expect(result).To(Equal(reconcile.Result{})) - - // verify creation event - fakeRecorder, ok := reconciler.Recorder.(*record.FakeRecorder) - Expect(ok).To(BeTrue()) - event := <-fakeRecorder.Events - Expect(event).To(ContainSubstring("ProcessOK")) - }) - - It("should send events when NRO present and operation failure", func() { + It("should fail when NRO present and operation failure", func() { brokenMcoKc := testobjs.NewKubeletConfigWithData("broken", label1, mcp1.Spec.MachineConfigSelector, []byte("")) // on HyperShift we can mimic this behavior by not having a ConfigMap with a KubeletConfig // present on the cluster at all @@ -171,12 +149,6 @@ var _ = Describe("Test KubeletConfig Reconcile", func() { key := client.ObjectKeyFromObject(brokenMcoKc) _, err = reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: key}) Expect(err).To(HaveOccurred()) - - // verify creation event - fakeRecorder, ok := reconciler.Recorder.(*record.FakeRecorder) - Expect(ok).To(BeTrue()) - event := <-fakeRecorder.Events - Expect(event).To(ContainSubstring("ProcessFailed")) }) It("should skip invalid kubeletconfig", func() { @@ -189,13 +161,6 @@ var _ = Describe("Test KubeletConfig Reconcile", func() { key := client.ObjectKeyFromObject(invalidMcoKc) _, err = reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: key}) Expect(err).ToNot(HaveOccurred()) - - // verify creation event - fakeRecorder, ok := reconciler.Recorder.(*record.FakeRecorder) - Expect(ok).To(BeTrue()) - event := <-fakeRecorder.Events - Expect(event).To(ContainSubstring("ProcessSkip")) - Expect(event).To(ContainSubstring(invalidMcoKc.Name)) }) It("should ignore non-matching kubeketconfigs", func() { @@ -209,22 +174,12 @@ var _ = Describe("Test KubeletConfig Reconcile", func() { result, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: key}) Expect(err).ToNot(HaveOccurred()) Expect(result).To(Equal(reconcile.Result{})) - - // verify creation event - fakeRecorder, ok := reconciler.Recorder.(*record.FakeRecorder) - Expect(ok).To(BeTrue()) - event := <-fakeRecorder.Events - Expect(event).To(ContainSubstring("ProcessSkip")) - Expect(event).To(ContainSubstring(ctrlPlaneKc.Name)) }) It("should process matching kubeletconfig, then ignore non-matching kubeketconfig", func() { reconciler, err := newFakeReconciler(nro, mcp1) Expect(err).ToNot(HaveOccurred()) - fakeRecorder, ok := reconciler.Recorder.(*record.FakeRecorder) - Expect(ok).To(BeTrue()) - var reconciledObj client.Object reconciledObj = mcoKc1 if clusterPlatform == platform.HyperShift { @@ -243,10 +198,6 @@ var _ = Describe("Test KubeletConfig Reconcile", func() { 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(reconciledObj.GetName())) ctrlPlaneKc := testobjs.NewKubeletConfigAutoresizeControlPlane() err = reconciler.Client.Create(context.TODO(), ctrlPlaneKc) @@ -261,11 +212,6 @@ var _ = Describe("Test KubeletConfig Reconcile", func() { result, err = reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: key}) Expect(err).ToNot(HaveOccurred()) Expect(result).To(Equal(reconcile.Result{})) - - // verify creation event - event = <-fakeRecorder.Events - Expect(event).To(ContainSubstring("ProcessSkip")) - Expect(event).To(ContainSubstring(ctrlPlaneKc.Name)) }) }) }, diff --git a/internal/controller/numaresourcesoperator_controller.go b/internal/controller/numaresourcesoperator_controller.go index ce4463d7d4..d842b7b8bf 100644 --- a/internal/controller/numaresourcesoperator_controller.go +++ b/internal/controller/numaresourcesoperator_controller.go @@ -34,7 +34,6 @@ import ( "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/sets" - "k8s.io/client-go/tools/record" "k8s.io/klog/v2" ctrl "sigs.k8s.io/controller-runtime" @@ -100,7 +99,6 @@ type NUMAResourcesOperatorReconciler struct { Namespace string Images images.Data ImagePullPolicy corev1.PullPolicy - Recorder record.EventRecorder ForwardMCPConds bool OverridableRTEImage bool // RTEMetricsTLS is derived from apiserver.config.openshift.io/cluster TLS profile at operator @@ -109,24 +107,31 @@ type NUMAResourcesOperatorReconciler struct { } // Namespace Scoped -//+kubebuilder:rbac:groups="",resources=services,verbs=*,namespace="numaresources" -//+kubebuilder:rbac:groups=networking.k8s.io,resources=networkpolicies,verbs=*,namespace="numaresources" +//+kubebuilder:rbac:groups="",resources=services,verbs=create;list;watch,namespace="numaresources" +//+kubebuilder:rbac:groups="",resources=services,resourceNames=numaresources-rte-metrics-service,verbs=get;update,namespace="numaresources" +//+kubebuilder:rbac:groups=networking.k8s.io,resources=networkpolicies,verbs=create;list;watch,namespace="numaresources" +//+kubebuilder:rbac:groups=networking.k8s.io,resources=networkpolicies,resourceNames=rte-default-deny-all;rte-egress-to-api-server,verbs=get;update,namespace="numaresources" +//+kubebuilder:rbac:groups=apps,resources=daemonsets,verbs=create;delete;get;list;update;watch,namespace="numaresources" +//+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=roles;rolebindings,verbs=create;list;watch,namespace="numaresources" +//+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=roles;rolebindings,resourceNames=rte,verbs=get;update,namespace="numaresources" +//+kubebuilder:rbac:groups="",resources=serviceaccounts,verbs=create;list;watch,namespace="numaresources" +//+kubebuilder:rbac:groups="",resources=serviceaccounts,resourceNames=rte,verbs=get;update,namespace="numaresources" // Cluster Scoped //+kubebuilder:rbac:groups=config.openshift.io,resources=apiservers,verbs=get;list;watch //+kubebuilder:rbac:groups=config.openshift.io,resources=clusterversions,verbs=list //+kubebuilder:rbac:groups=config.openshift.io,resources=clusteroperators,verbs=get //+kubebuilder:rbac:groups=config.openshift.io,resources=infrastructures,verbs=get -//+kubebuilder:rbac:groups=machineconfiguration.openshift.io,resources=machineconfigs,verbs=* +//+kubebuilder:rbac:groups=machineconfiguration.openshift.io,resources=machineconfigs,verbs=create;delete;get;list;update;watch //+kubebuilder:rbac:groups=machineconfiguration.openshift.io,resources=machineconfigpools,verbs=get;list;watch -//+kubebuilder:rbac:groups=security.openshift.io,resources=securitycontextconstraints,verbs=* -//+kubebuilder:rbac:groups=apiextensions.k8s.io,resources=customresourcedefinitions,verbs=* -//+kubebuilder:rbac:groups=apps,resources=daemonsets,verbs=* -//+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=roles,verbs=* -//+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=rolebindings,verbs=* -//+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterroles,verbs=* -//+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterrolebindings,verbs=* -//+kubebuilder:rbac:groups="",resources=serviceaccounts,verbs=* +//+kubebuilder:rbac:groups=security.openshift.io,resources=securitycontextconstraints,verbs=create;list;watch +//+kubebuilder:rbac:groups=security.openshift.io,resources=securitycontextconstraints,resourceNames=resource-topology-exporter;resource-topology-exporter-v2,verbs=get;update +//+kubebuilder:rbac:groups=apiextensions.k8s.io,resources=customresourcedefinitions,verbs=create;list;watch +//+kubebuilder:rbac:groups=apiextensions.k8s.io,resources=customresourcedefinitions,resourceNames=noderesourcetopologies.topology.node.k8s.io,verbs=get;update +//+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterroles,verbs=create +//+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterroles,resourceNames=rte,verbs=get;update +//+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterrolebindings,verbs=create +//+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterrolebindings,resourceNames=rte,verbs=get;update //+kubebuilder:rbac:groups="",resources=pods,verbs=get;list;watch //+kubebuilder:rbac:groups="",resources=nodes,verbs=list //+kubebuilder:rbac:groups=nodetopology.openshift.io,resources=numaresourcesoperators,verbs=get;list;watch @@ -232,19 +237,14 @@ func (r *NUMAResourcesOperatorReconciler) degradeStatus(ctx context.Context, ini func (r *NUMAResourcesOperatorReconciler) reconcileResourceAPI(ctx context.Context, instance *nropv1.NUMAResourcesOperator, trees []nodegroupv1.Tree) intreconcile.Step { if annotations.IsNRTAPIDefinitionCluster(instance.Annotations) { - // should never happen, so let's be vocal. Very vocal. - r.Recorder.Eventf(instance, corev1.EventTypeWarning, "SkipCRDInstall", "Skipped to install Node Resource Topology CRD: caused by annotation") + klog.InfoS("Skipped NRT CRD install: caused by annotation") return intreconcile.StepSuccess() } - applied, err := r.syncNodeResourceTopologyAPI(ctx) + _, err := r.syncNodeResourceTopologyAPI(ctx) if err != nil { - r.Recorder.Eventf(instance, corev1.EventTypeWarning, "FailedCRDInstall", "Failed to install Node Resource Topology CRD: %v", err) err = fmt.Errorf("FailedAPISync: %w", err) return intreconcile.StepFailed(err) } - if applied { - r.Recorder.Eventf(instance, corev1.EventTypeNormal, "SuccessfulCRDInstall", "Node Resource Topology CRD installed") - } return intreconcile.StepSuccess() } @@ -253,11 +253,9 @@ func (r *NUMAResourcesOperatorReconciler) reconcileResourceMachineConfig(ctx con // before checking additional components for updates waitByPool, pausedMCPNames, err := r.syncMachineConfigs(ctx, instance, existing, trees) if err != nil { - r.Recorder.Eventf(instance, corev1.EventTypeWarning, "FailedMCSync", "Failed to set up machine configuration for worker nodes: %v", err) err = fmt.Errorf("failed to sync machine configs: %w", err) return intreconcile.StepFailed(err) } - r.Recorder.Eventf(instance, corev1.EventTypeNormal, "SuccessfulMCSync", "Enabled machine configuration for worker nodes") // MCO needs to update the SELinux context removal and other stuff, and need to trigger a reboot. // It can take a while. @@ -277,7 +275,6 @@ func (r *NUMAResourcesOperatorReconciler) reconcileResourceMachineConfig(ctx con func (r *NUMAResourcesOperatorReconciler) reconcileResourceDaemonSet(ctx context.Context, instance *nropv1.NUMAResourcesOperator, existing *rtestate.ExistingManifests, trees []nodegroupv1.Tree) ([]poolDaemonSet, intreconcile.Step) { daemonSetsInfoPerPool, err := r.syncNUMAResourcesOperatorResources(ctx, instance, existing, trees) if err != nil { - r.Recorder.Eventf(instance, corev1.EventTypeWarning, "FailedRTECreate", "Failed to create Resource-Topology-Exporter DaemonSets: %v", err) err = fmt.Errorf("FailedRTESync: %w", err) return nil, intreconcile.StepFailed(err) } @@ -286,8 +283,6 @@ func (r *NUMAResourcesOperatorReconciler) reconcileResourceDaemonSet(ctx context return nil, intreconcile.StepSuccess() } - r.Recorder.Eventf(instance, corev1.EventTypeNormal, "SuccessfulRTECreate", "Created Resource-Topology-Exporter DaemonSets") - dssWithReadyStatus, dsNamePending, err := r.syncDaemonSetsStatuses(ctx, r.Client, daemonSetsInfoPerPool) instance.Status.DaemonSets = dssWithReadyStatus instance.Status.RelatedObjects = relatedobjects.ResourceTopologyExporter(r.Namespace, dssWithReadyStatus) diff --git a/internal/controller/numaresourcesoperator_controller_test.go b/internal/controller/numaresourcesoperator_controller_test.go index 02598fcac6..a958138f4f 100644 --- a/internal/controller/numaresourcesoperator_controller_test.go +++ b/internal/controller/numaresourcesoperator_controller_test.go @@ -37,7 +37,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/kubernetes/scheme" - "k8s.io/client-go/tools/record" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -88,8 +87,6 @@ func NewFakeNUMAResourcesOperatorReconciler(plat platform.Platform, platVersion if err != nil { return nil, err } - recorder := record.NewFakeRecorder(bufferSize) - return &NUMAResourcesOperatorReconciler{ Client: fakeClient, Scheme: scheme.Scheme, @@ -103,7 +100,6 @@ func NewFakeNUMAResourcesOperatorReconciler(plat platform.Platform, platVersion Images: images.Data{ Builtin: testImageSpec, }, - Recorder: recorder, }, nil } diff --git a/internal/controller/numaresourcesscheduler_controller.go b/internal/controller/numaresourcesscheduler_controller.go index 8b6588fe15..138b167abd 100644 --- a/internal/controller/numaresourcesscheduler_controller.go +++ b/internal/controller/numaresourcesscheduler_controller.go @@ -78,15 +78,24 @@ type NUMAResourcesSchedulerReconciler struct { } // Namespace Scoped -//+kubebuilder:rbac:groups=networking.k8s.io,resources=networkpolicies,verbs=*,namespace="numaresources" +//+kubebuilder:rbac:groups=networking.k8s.io,resources=networkpolicies,verbs=create;list;watch,namespace="numaresources" +//+kubebuilder:rbac:groups=networking.k8s.io,resources=networkpolicies,resourceNames=scheduler-default-deny-all;scheduler-egress-to-api-server,verbs=get;update,namespace="numaresources" +//+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=roles;rolebindings,verbs=create;list;watch,namespace="numaresources" +//+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=roles;rolebindings,resourceNames=topology-aware-scheduler-leader-elect,verbs=get;update,namespace="numaresources" +//+kubebuilder:rbac:groups=apps,resources=deployments,verbs=create;list;watch,namespace="numaresources" +//+kubebuilder:rbac:groups=apps,resources=deployments,resourceNames=secondary-scheduler,verbs=get;update,namespace="numaresources" +//+kubebuilder:rbac:groups="",resources=serviceaccounts,verbs=create;list;watch,namespace="numaresources" +//+kubebuilder:rbac:groups="",resources=serviceaccounts,resourceNames=secondary-scheduler,verbs=get;update,namespace="numaresources" +//+kubebuilder:rbac:groups="",resources=configmaps,verbs=create;get;list;update;watch,namespace="numaresources" +//+kubebuilder:rbac:groups="",resources=endpoints,verbs=create,namespace="numaresources" +//+kubebuilder:rbac:groups="",resources=endpoints,resourceNames=numa-scheduler-leader,verbs=get;update,namespace="numaresources" // Cluster Scoped -//+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterroles,verbs=* -//+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterrolebindings,verbs=* -//+kubebuilder:rbac:groups=apps,resources=deployments,verbs=* -//+kubebuilder:rbac:groups="",resources=serviceaccounts,verbs=* -//+kubebuilder:rbac:groups="",resources=configmaps,verbs=* -//+kubebuilder:rbac:groups="",resources=nodes,verbs=list;watch +//+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterroles,verbs=create;list;watch +//+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterroles,resourceNames=topology-aware-scheduler,verbs=get;update +//+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterrolebindings,verbs=create;list;watch +//+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterrolebindings,resourceNames=topology-aware-scheduler,verbs=get;update +//+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterrolebindings,resourceNames=secondary-scheduler,verbs=delete //+kubebuilder:rbac:groups=nodetopology.openshift.io,resources=numaresourcesschedulers,verbs=get;list;watch //+kubebuilder:rbac:groups=nodetopology.openshift.io,resources=numaresourcesschedulers/status,verbs=get;update;patch //+kubebuilder:rbac:groups=nodetopology.openshift.io,resources=numaresourcesschedulers/finalizers,verbs=update @@ -373,12 +382,14 @@ func (r *NUMAResourcesSchedulerReconciler) syncNUMASchedulerResources(ctx contex existing := schedstate.FromClient(ctx, r.Client, r.SchedulerManifests) for _, objState := range existing.State(r.SchedulerManifests) { - if err := controllerutil.SetControllerReference(instance, objState.Desired, r.Scheme); err != nil { - return schedStatus, fmt.Errorf("failed to set controller reference to %s %s: %w", objState.Desired.GetNamespace(), objState.Desired.GetName(), err) + if objState.IsCreateOrUpdate() { + if err := controllerutil.SetControllerReference(instance, objState.Desired, r.Scheme); err != nil { + return schedStatus, fmt.Errorf("failed to set controller reference to %s %s: %w", objState.Desired.GetNamespace(), objState.Desired.GetName(), err) + } } - obj, _, err := apply.ApplyObject(ctx, r.Client, objState) + obj, _, err := apply.ApplyState(ctx, r.Client, objState) if err != nil { - return schedStatus, fmt.Errorf("could not apply (%s) %s/%s: %w", objState.Desired.GetObjectKind().GroupVersionKind(), objState.Desired.GetNamespace(), objState.Desired.GetName(), err) + return schedStatus, fmt.Errorf("could not apply scheduler object: %w", err) } if nname, ok := schedstate.DeploymentNamespacedNameFromObject(obj); ok { diff --git a/internal/controller/operand_rbac_rte.go b/internal/controller/operand_rbac_rte.go new file mode 100644 index 0000000000..0153fcb196 --- /dev/null +++ b/internal/controller/operand_rbac_rte.go @@ -0,0 +1,38 @@ +/* + * Copyright 2026 Red Hat, Inc. + * + * 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. + */ + +// RTE operand RBAC escalation permissions. +// +// The operator creates and updates the "rte" ClusterRole (sourced from +// vendor/github.com/k8stopologyawareschedwg/deployer/pkg/manifests/yaml/rte/clusterrole.yaml). +// Kubernetes RBAC prevents a subject from granting permissions it does not +// already hold, so the operator's own ClusterRole must include every +// permission present in the RTE operand's ClusterRole. +// +// These annotations exist solely to satisfy that escalation requirement. +// The operator itself never exercises these permissions (some overlap +// with the operator's own needs, such as nodes list, but they are listed +// here explicitly so the escalation rationale is self-documenting). +// +// Future improvement: ship the operand ClusterRoles as static manifests +// in the OLM bundle (pre-deploy) instead of creating them in the reconciler. +// The operator would then only create ClusterRoleBindings, which does not +// require escalation privileges, and all annotations below can be removed. + +package controller + +//+kubebuilder:rbac:groups=topology.node.k8s.io,resources=noderesourcetopologies,verbs=get;list;create;update +//+kubebuilder:rbac:groups="",resources=nodes,verbs=watch;list diff --git a/internal/controller/operand_rbac_sched.go b/internal/controller/operand_rbac_sched.go new file mode 100644 index 0000000000..d4f75cdf4c --- /dev/null +++ b/internal/controller/operand_rbac_sched.go @@ -0,0 +1,53 @@ +/* + * Copyright 2026 Red Hat, Inc. + * + * 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. + */ + +// Scheduler operand RBAC escalation permissions. +// +// The operator creates and updates the "topology-aware-scheduler" ClusterRole +// (see pkg/numaresourcesscheduler/manifests/yaml/clusterrole.nrt.yaml). +// Kubernetes RBAC prevents a subject from granting permissions it does not +// already hold, so the operator's own ClusterRole must include every +// permission present in the scheduler operand's ClusterRole. +// +// These annotations exist solely to satisfy that escalation requirement. +// The operator itself never exercises these permissions. +// +// Future improvement: ship the operand ClusterRoles as static manifests +// in the OLM bundle (pre-deploy) instead of creating them in the reconciler. +// The operator would then only create ClusterRoleBindings, which does not +// require escalation privileges, and all annotations below can be removed. + +package controller + +//+kubebuilder:rbac:groups="",resources=nodes,verbs=get;list;watch +//+kubebuilder:rbac:groups="",resources=pods,verbs=delete +//+kubebuilder:rbac:groups="",resources=pods/status,verbs=patch;update +//+kubebuilder:rbac:groups="",resources=pods/binding;bindings,verbs=create +//+kubebuilder:rbac:groups="",resources=replicationcontrollers;services;namespaces,verbs=get;list;watch +//+kubebuilder:rbac:groups="",resources=persistentvolumeclaims;persistentvolumes,verbs=get;list;watch +//+kubebuilder:rbac:groups="",resources=events,verbs=create;patch;update +//+kubebuilder:rbac:groups=events.k8s.io,resources=events,verbs=create;patch;update +//+kubebuilder:rbac:groups=apps;extensions,resources=replicasets,verbs=get;list;watch +//+kubebuilder:rbac:groups=apps,resources=statefulsets,verbs=get;list;watch +//+kubebuilder:rbac:groups=policy,resources=poddisruptionbudgets,verbs=get;list;watch +//+kubebuilder:rbac:groups=authentication.k8s.io,resources=tokenreviews,verbs=create +//+kubebuilder:rbac:groups=authorization.k8s.io,resources=subjectaccessreviews,verbs=create +//+kubebuilder:rbac:groups=coordination.k8s.io,resources=leases,verbs=create +//+kubebuilder:rbac:groups=coordination.k8s.io,resources=leases,resourceNames=numa-scheduler-leader,verbs=get;list;update;watch +//+kubebuilder:rbac:groups=coordination.k8s.io,resources=leasecandidates,verbs=create;delete;deletecollection;get;list;patch;update;watch +//+kubebuilder:rbac:groups=storage.k8s.io,resources=storageclasses;csinodes;csidrivers;csistoragecapacities;volumeattachments,verbs=get;list;watch +//+kubebuilder:rbac:groups=resource.k8s.io,resources=deviceclasses;resourceclaims;resourceslices,verbs=get;list;watch +//+kubebuilder:rbac:groups=topology.node.k8s.io,resources=noderesourcetopologies,verbs=get;list;watch diff --git a/pkg/numaresourcesscheduler/manifests/manifests.go b/pkg/numaresourcesscheduler/manifests/manifests.go index 051474b877..561eb7083d 100644 --- a/pkg/numaresourcesscheduler/manifests/manifests.go +++ b/pkg/numaresourcesscheduler/manifests/manifests.go @@ -62,10 +62,6 @@ func ClusterRole() (*rbacv1.ClusterRole, error) { return cr, nil } -func ClusterRoleBindingK8S(namespace string) (*rbacv1.ClusterRoleBinding, error) { - return loadClusterRoleBinding("clusterrolebinding.yaml", namespace) -} - func ClusterRoleBindingNRT(namespace string) (*rbacv1.ClusterRoleBinding, error) { return loadClusterRoleBinding("clusterrolebinding.nrt.yaml", namespace) } diff --git a/pkg/numaresourcesscheduler/manifests/manifests_test.go b/pkg/numaresourcesscheduler/manifests/manifests_test.go index ff97e49c26..c074461382 100644 --- a/pkg/numaresourcesscheduler/manifests/manifests_test.go +++ b/pkg/numaresourcesscheduler/manifests/manifests_test.go @@ -27,9 +27,6 @@ func TestLoad(t *testing.T) { if obj, err := ClusterRole(); obj == nil || err != nil { t.Errorf("ClusterRole() failed: err=%v", err) } - if obj, err := ClusterRoleBindingK8S(""); obj == nil || err != nil { - t.Errorf("ClusterRoleBindingK8S() failed: err=%v", err) - } if obj, err := ClusterRoleBindingNRT(""); obj == nil || err != nil { t.Errorf("ClusterRoleBindingNRT() failed: err=%v", err) } diff --git a/pkg/numaresourcesscheduler/manifests/sched/sched.go b/pkg/numaresourcesscheduler/manifests/sched/sched.go index c3dd8587f8..f926c054c5 100644 --- a/pkg/numaresourcesscheduler/manifests/sched/sched.go +++ b/pkg/numaresourcesscheduler/manifests/sched/sched.go @@ -33,7 +33,6 @@ type Manifests struct { ServiceAccount *corev1.ServiceAccount ConfigMap *corev1.ConfigMap ClusterRole *rbacv1.ClusterRole - ClusterRoleBindingK8S *rbacv1.ClusterRoleBinding ClusterRoleBindingNRT *rbacv1.ClusterRoleBinding Role *rbacv1.Role RoleBinding *rbacv1.RoleBinding @@ -47,7 +46,6 @@ func (mf Manifests) ToObjects() []client.Object { mf.ServiceAccount, mf.ConfigMap, mf.ClusterRole, - mf.ClusterRoleBindingK8S, mf.ClusterRoleBindingNRT, mf.Role, mf.RoleBinding, @@ -62,7 +60,6 @@ func (mf Manifests) Clone() Manifests { ServiceAccount: mf.ServiceAccount.DeepCopy(), ConfigMap: mf.ConfigMap.DeepCopy(), ClusterRole: mf.ClusterRole.DeepCopy(), - ClusterRoleBindingK8S: mf.ClusterRoleBindingK8S.DeepCopy(), ClusterRoleBindingNRT: mf.ClusterRoleBindingNRT.DeepCopy(), Role: mf.Role.DeepCopy(), RoleBinding: mf.RoleBinding.DeepCopy(), @@ -91,11 +88,6 @@ func GetManifests(namespace string) (Manifests, error) { return mf, err } - mf.ClusterRoleBindingK8S, err = manifests.ClusterRoleBindingK8S(namespace) - if err != nil { - return mf, err - } - mf.ClusterRoleBindingNRT, err = manifests.ClusterRoleBindingNRT(namespace) if err != nil { return mf, err diff --git a/pkg/numaresourcesscheduler/manifests/yaml/clusterrole.nrt.yaml b/pkg/numaresourcesscheduler/manifests/yaml/clusterrole.nrt.yaml index a714ce87c6..fb4049bcf2 100644 --- a/pkg/numaresourcesscheduler/manifests/yaml/clusterrole.nrt.yaml +++ b/pkg/numaresourcesscheduler/manifests/yaml/clusterrole.nrt.yaml @@ -3,10 +3,159 @@ kind: ClusterRole metadata: name: topology-aware-scheduler rules: - - apiGroups: ["topology.node.k8s.io"] - resources: ["noderesourcetopologies"] - verbs: ["get", "list", "watch"] - - apiGroups: ["storage.k8s.io"] - resources: ["storageclasses"] - verbs: ["get", "list", "watch"] - +- apiGroups: + - "" + - events.k8s.io + resources: + - events + verbs: + - create + - patch + - update +- apiGroups: + - "" + resources: + - nodes + verbs: + - get + - list + - watch +- apiGroups: + - "" + resources: + - pods + verbs: + - delete + - get + - list + - watch +- apiGroups: + - "" + resources: + - bindings + - pods/binding + verbs: + - create +- apiGroups: + - "" + resources: + - pods/status + verbs: + - patch + - update +- apiGroups: + - "" + resources: + - replicationcontrollers + - services + - namespaces + verbs: + - get + - list + - watch +- apiGroups: + - apps + - extensions + resources: + - replicasets + verbs: + - get + - list + - watch +- apiGroups: + - apps + resources: + - statefulsets + verbs: + - get + - list + - watch +- apiGroups: + - policy + resources: + - poddisruptionbudgets + verbs: + - get + - list + - watch +- apiGroups: + - "" + resources: + - persistentvolumeclaims + - persistentvolumes + verbs: + - get + - list + - watch +- apiGroups: + - coordination.k8s.io + resources: + - leases + verbs: + - create +- apiGroups: + - coordination.k8s.io + resourceNames: + - numa-scheduler-leader + resources: + - leases + verbs: + - get + - list + - update + - watch +- apiGroups: + - coordination.k8s.io + resources: + - leasecandidates + verbs: + - create + - delete + - deletecollection + - get + - list + - patch + - update + - watch +- apiGroups: + - authentication.k8s.io + resources: + - tokenreviews + verbs: + - create +- apiGroups: + - authorization.k8s.io + resources: + - subjectaccessreviews + verbs: + - create +- apiGroups: + - storage.k8s.io + resources: + - csinodes + - storageclasses + - csidrivers + - csistoragecapacities + - volumeattachments + verbs: + - get + - list + - watch +- apiGroups: + - resource.k8s.io + resources: + - deviceclasses + - resourceclaims + - resourceslices + verbs: + - get + - list + - watch +- apiGroups: + - topology.node.k8s.io + resources: + - noderesourcetopologies + verbs: + - get + - list + - watch diff --git a/pkg/numaresourcesscheduler/manifests/yaml/clusterrolebinding.yaml b/pkg/numaresourcesscheduler/manifests/yaml/clusterrolebinding.yaml deleted file mode 100644 index fedb062ae2..0000000000 --- a/pkg/numaresourcesscheduler/manifests/yaml/clusterrolebinding.yaml +++ /dev/null @@ -1,12 +0,0 @@ -apiVersion: rbac.authorization.k8s.io/v1 -kind: ClusterRoleBinding -metadata: - name: secondary-scheduler -roleRef: - apiGroup: rbac.authorization.k8s.io - kind: ClusterRole - name: system:kube-scheduler -subjects: - - kind: ServiceAccount - name: secondary-scheduler - namespace: placeholder diff --git a/pkg/numaresourcesscheduler/objectstate/sched/sched.go b/pkg/numaresourcesscheduler/objectstate/sched/sched.go index 73204b5c00..e05496df2e 100644 --- a/pkg/numaresourcesscheduler/objectstate/sched/sched.go +++ b/pkg/numaresourcesscheduler/objectstate/sched/sched.go @@ -18,11 +18,13 @@ package sched import ( "context" + "fmt" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" networkingv1 "k8s.io/api/networking/v1" rbacv1 "k8s.io/api/rbac/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/klog/v2" "sigs.k8s.io/controller-runtime/pkg/client" @@ -41,14 +43,30 @@ const ( SchedulerConfigFileName = "config.yaml" SchedulerConfigMapVolumeName = "etckubernetes" SchedulerPluginName = "NodeResourceTopologyMatch" + + legacyClusterRoleBindingK8SName = "secondary-scheduler" ) +// Cleanup removes leftover resources from previous operator versions. +func Cleanup(ctx context.Context, cli client.Client) error { + crb := &rbacv1.ClusterRoleBinding{} + crb.Name = legacyClusterRoleBindingK8SName + err := cli.Delete(ctx, crb) + if err != nil { + if apierrors.IsNotFound(err) { + return nil // expected steady state. Stay silent + } + return fmt.Errorf("failed to delete legacy ClusterRoleBinding %q: %w", legacyClusterRoleBindingK8SName, err) + } + klog.Infof("deleted legacy ClusterRoleBinding %q", legacyClusterRoleBindingK8SName) + return nil +} + type ExistingManifests struct { Existing schedmanifests.Manifests serviceAccountError error configMapError error clusterRoleError error - clusterRoleBindingK8SError error clusterRoleBindingNRTError error roleError error roleBindingError error @@ -80,13 +98,6 @@ func (em ExistingManifests) State(mf schedmanifests.Manifests) []objectstate.Obj Compare: compare.Object, Merge: merge.MetadataForUpdate, }, - { - Existing: em.Existing.ClusterRoleBindingK8S, - Error: em.clusterRoleBindingK8SError, - Desired: mf.ClusterRoleBindingK8S.DeepCopy(), - Compare: compare.Object, - Merge: merge.MetadataForUpdate, - }, { Existing: em.Existing.ClusterRoleBindingNRT, Error: em.clusterRoleBindingNRTError, @@ -152,10 +163,6 @@ func FromClient(ctx context.Context, cli client.Client, mf schedmanifests.Manife ret.Existing.ClusterRole = cro } - crbK8S := &rbacv1.ClusterRoleBinding{} - if ret.clusterRoleBindingK8SError = cli.Get(ctx, client.ObjectKeyFromObject(mf.ClusterRoleBindingK8S), crbK8S); ret.clusterRoleBindingK8SError == nil { - ret.Existing.ClusterRoleBindingK8S = crbK8S - } crbNRT := &rbacv1.ClusterRoleBinding{} if ret.clusterRoleBindingNRTError = cli.Get(ctx, client.ObjectKeyFromObject(mf.ClusterRoleBindingNRT), crbNRT); ret.clusterRoleBindingNRTError == nil { ret.Existing.ClusterRoleBindingNRT = crbNRT diff --git a/pkg/numaresourcesscheduler/objectstate/sched/sched_test.go b/pkg/numaresourcesscheduler/objectstate/sched/sched_test.go index e9ef6c64e5..dfb7672092 100644 --- a/pkg/numaresourcesscheduler/objectstate/sched/sched_test.go +++ b/pkg/numaresourcesscheduler/objectstate/sched/sched_test.go @@ -17,14 +17,18 @@ limitations under the License. package sched import ( + "context" "reflect" "testing" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" + rbacv1 "k8s.io/api/rbac/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes/scheme" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" nropv1 "github.com/openshift-kni/numaresources-operator/api/v1" ) @@ -88,6 +92,52 @@ profiles: kubeconfigpath: "" # needs to be empty string` ) +func TestCleanup(t *testing.T) { + type testCase struct { + name string + objects []client.Object + } + + testCases := []testCase{ + { + name: "CRB exists and is deleted", + objects: []client.Object{ + &rbacv1.ClusterRoleBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: "secondary-scheduler", + }, + }, + }, + }, + { + name: "CRB does not exist", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + builder := fake.NewClientBuilder().WithScheme(scheme.Scheme) + for _, obj := range tc.objects { + builder = builder.WithObjects(obj) + } + cli := builder.Build() + + ctx := context.Background() + + cleanErr := Cleanup(ctx, cli) + if cleanErr != nil { + t.Errorf("unexpected error: %v", cleanErr) + } + crb := &rbacv1.ClusterRoleBinding{} + crb.Name = "secondary-scheduler" + getErr := cli.Get(ctx, client.ObjectKeyFromObject(crb), crb) + if getErr == nil { + t.Errorf("ClusterRoleBinding should not exist after Cleanup") + } + }) + } +} + func TestSchedulerNameFromObject(t *testing.T) { type testCase struct { name string diff --git a/pkg/objectupdate/sched/sched.go b/pkg/objectupdate/sched/sched.go index 388e168739..5086b34468 100644 --- a/pkg/objectupdate/sched/sched.go +++ b/pkg/objectupdate/sched/sched.go @@ -39,8 +39,6 @@ import ( objtls "github.com/openshift-kni/numaresources-operator/pkg/objectupdate/tls" ) -// these should be provided by a deployer API - const ( MainContainerName = "secondary-scheduler" )