Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: adjust the operator log level programmatically #1022

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,17 @@ import (
"sort"

corev1 "k8s.io/api/core/v1"

operatorv1 "github.com/openshift/api/operator/v1"
)

func NormalizeSpec(spec *NUMAResourcesOperatorSpec) {
defaultLog := operatorv1.Normal
if spec.LogLevel == "" {
spec.LogLevel = defaultLog
}
}

func (nodeGroup NodeGroup) NormalizeConfig() NodeGroupConfig {
conf := DefaultNodeGroupConfig()
if nodeGroup.Config == nil {
Expand Down
9 changes: 8 additions & 1 deletion api/numaresourcesoperator/v1/numaresourcesoperator_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ type NUMAResourcesOperatorSpec struct {
// Optional Resource Topology Exporter image URL
//+operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Optional RTE image URL",xDescriptors={"urn:alm:descriptor:com.tectonic.ui:text"}
ExporterImage string `json:"imageSpec,omitempty"`
// Valid values are: "Normal", "Debug", "Trace", "TraceAll".
// RTE log verbosity. Valid values are: "Normal", "Debug", "Trace", "TraceAll".
// Defaults to "Normal".
// +optional
// +kubebuilder:default=Normal
Expand All @@ -45,6 +45,10 @@ type NUMAResourcesOperatorSpec struct {
// +optional
//+operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Optional ignore pod namespace/name glob patterns"
PodExcludes []NamespacedName `json:"podExcludes,omitempty"`
// Operator verbosity value, set the same way as the commandline
// +optional
//+operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Operator verbosity"
Verbose *int `json:"verbose,omitempty"`
}

// +kubebuilder:validation:Enum=Disabled;Enabled;EnabledExclusiveResources
Expand Down Expand Up @@ -178,6 +182,9 @@ type NUMAResourcesOperatorStatus struct {
// RelatedObjects list of objects of interest for this operator
//+operator-sdk:csv:customresourcedefinitions:type=status,displayName="Related Objects"
RelatedObjects []configv1.ObjectReference `json:"relatedObjects,omitempty"`
// Verbose is the current log verbosity of the operator.
//+operator-sdk:csv:customresourcedefinitions:type=status,displayName="Operator log verbosity"
Verbose int `json:"verbose"`
}

// MachineConfigPool defines the observed state of each MachineConfigPool selected by node groups
Expand Down
5 changes: 5 additions & 0 deletions api/numaresourcesoperator/v1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ spec:
logLevel:
default: Normal
description: |-
Valid values are: "Normal", "Debug", "Trace", "TraceAll".
RTE log verbosity. Valid values are: "Normal", "Debug", "Trace", "TraceAll".
Defaults to "Normal".
enum:
- ""
Expand Down Expand Up @@ -213,6 +213,9 @@ spec:
type: string
type: object
type: array
verbose:
description: Operator verbosity value, set the same way as the commandline
type: integer
type: object
status:
description: NUMAResourcesOperatorStatus defines the observed state of
Expand Down Expand Up @@ -548,6 +551,11 @@ spec:
- resource
type: object
type: array
verbose:
description: Verbose is the current log verbosity of the operator.
type: integer
required:
- verbose
type: object
type: object
served: true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ metadata:
}
]
capabilities: Basic Install
createdAt: "2025-01-07T13:18:55Z"
createdAt: "2025-01-09T09:47:29Z"
olm.skipRange: '>=4.18.0 <4.19.0'
operatorframework.io/cluster-monitoring: "true"
operators.operatorframework.io/builder: operator-sdk-v1.36.1
Expand All @@ -89,7 +89,7 @@ spec:
x-descriptors:
- urn:alm:descriptor:com.tectonic.ui:text
- description: |-
Valid values are: "Normal", "Debug", "Trace", "TraceAll".
RTE log verbosity. Valid values are: "Normal", "Debug", "Trace", "TraceAll".
Defaults to "Normal".
displayName: RTE log verbosity
path: logLevel
Expand Down Expand Up @@ -131,6 +131,9 @@ spec:
level
displayName: Optional ignore pod namespace/name glob patterns
path: podExcludes
- description: Operator verbosity value, set the same way as the commandline
displayName: Operator verbosity
path: verbose
statusDescriptors:
- description: Conditions show the current state of the NUMAResourcesOperator
Operator
Expand Down Expand Up @@ -168,6 +171,9 @@ spec:
- description: RelatedObjects list of objects of interest for this operator
displayName: Related Objects
path: relatedObjects
- description: Verbose is the current log verbosity of the operator.
displayName: Operator log verbosity
path: verbose
version: v1
- description: NUMAResourcesOperator is the Schema for the numaresourcesoperators
API
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ spec:
logLevel:
default: Normal
description: |-
Valid values are: "Normal", "Debug", "Trace", "TraceAll".
RTE log verbosity. Valid values are: "Normal", "Debug", "Trace", "TraceAll".
Defaults to "Normal".
enum:
- ""
Expand Down Expand Up @@ -213,6 +213,9 @@ spec:
type: string
type: object
type: array
verbose:
description: Operator verbosity value, set the same way as the commandline
type: integer
type: object
status:
description: NUMAResourcesOperatorStatus defines the observed state of
Expand Down Expand Up @@ -548,6 +551,11 @@ spec:
- resource
type: object
type: array
verbose:
description: Verbose is the current log verbosity of the operator.
type: integer
required:
- verbose
type: object
type: object
served: true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ spec:
x-descriptors:
- urn:alm:descriptor:com.tectonic.ui:text
- description: |-
Valid values are: "Normal", "Debug", "Trace", "TraceAll".
RTE log verbosity. Valid values are: "Normal", "Debug", "Trace", "TraceAll".
Defaults to "Normal".
displayName: RTE log verbosity
path: logLevel
Expand Down Expand Up @@ -70,6 +70,9 @@ spec:
level
displayName: Optional ignore pod namespace/name glob patterns
path: podExcludes
- description: Operator verbosity value, set the same way as the commandline
displayName: Operator verbosity
path: verbose
statusDescriptors:
- description: Conditions show the current state of the NUMAResourcesOperator
Operator
Expand Down Expand Up @@ -107,6 +110,9 @@ spec:
- description: RelatedObjects list of objects of interest for this operator
displayName: Related Objects
path: relatedObjects
- description: Verbose is the current log verbosity of the operator.
displayName: Operator log verbosity
path: verbose
version: v1
- description: NUMAResourcesOperator is the Schema for the numaresourcesoperators
API
Expand Down
33 changes: 33 additions & 0 deletions controllers/numaresourcesoperator_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ import (
"github.com/openshift-kni/numaresources-operator/pkg/status/conditioninfo"
"github.com/openshift-kni/numaresources-operator/pkg/validation"

intkloglevel "github.com/openshift-kni/numaresources-operator/internal/kloglevel"
intreconcile "github.com/openshift-kni/numaresources-operator/internal/reconcile"
)

Expand All @@ -91,6 +92,7 @@ type NUMAResourcesOperatorReconciler struct {
ImagePullPolicy corev1.PullPolicy
Recorder record.EventRecorder
ForwardMCPConds bool
Verbose int
}

// TODO: narrow down
Expand Down Expand Up @@ -145,11 +147,18 @@ func (r *NUMAResourcesOperatorReconciler) Reconcile(ctx context.Context, req ctr
return r.degradeStatus(ctx, instance, status.ConditionTypeIncorrectNUMAResourcesOperatorResourceName, err)
}

nropv1.NormalizeSpec(&instance.Spec)
// can't be API normalization because the value is dynamic
normalizeSpecVerboseFromRuntime(&instance.Spec, r.Verbose)

if annotations.IsPauseReconciliationEnabled(instance.Annotations) {
klog.V(2).InfoS("Pause reconciliation enabled", "object", req.NamespacedName)
return ctrl.Result{}, nil
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

else  {
    instance.Status.OperatorLogLeve = intkloglevel.Get() // needs adjustment, but this is the idea
}


// validation step. This is not expected to fail often, hence log messages, if any, needs to be loud
// regardless by verbosiness value, including the value dynamically set.

if err := validation.NodeGroups(instance.Spec.NodeGroups, r.Platform); err != nil {
return r.degradeStatus(ctx, instance, validation.NodeGroupsError, err)
}
Expand Down Expand Up @@ -222,6 +231,20 @@ func (r *NUMAResourcesOperatorReconciler) degradeStatus(ctx context.Context, ins
return ctrl.Result{}, nil
}

func (r *NUMAResourcesOperatorReconciler) UpdateVerbosity(ctx context.Context, verbose *int) int {
// do as early as possible, but always after validation so we don't miss log messages
if verb := *verbose; verb != r.Verbose {
err := intkloglevel.Set(klog.Level(verb))
if err != nil {
klog.InfoS("cannot updated log level dynamically", "desiredLevel", verb)
} else {
r.Verbose = verb
}
}

return r.Verbose
}

func (r *NUMAResourcesOperatorReconciler) reconcileResourceAPI(ctx context.Context, instance *nropv1.NUMAResourcesOperator, trees []nodegroupv1.Tree) intreconcile.Step {
applied, err := r.syncNodeResourceTopologyAPI(ctx)
if err != nil {
Expand Down Expand Up @@ -288,6 +311,9 @@ func (r *NUMAResourcesOperatorReconciler) reconcileResourceDaemonSet(ctx context
}

func (r *NUMAResourcesOperatorReconciler) reconcileResource(ctx context.Context, instance *nropv1.NUMAResourcesOperator, trees []nodegroupv1.Tree) intreconcile.Step {
// this must be the first, and must be done as early as possible. Should not make the reconciliation attempt fail (best-effort)
instance.Status.Verbose = r.UpdateVerbosity(ctx, instance.Spec.Verbose)

if step := r.reconcileResourceAPI(ctx, instance, trees); step.EarlyStop() {
updateStatusConditionsIfNeeded(instance, step.ConditionInfo)
return step
Expand Down Expand Up @@ -777,3 +803,10 @@ func getTreesByNodeGroup(ctx context.Context, cli client.Client, nodeGroups []nr
return nil, fmt.Errorf("unsupported platform")
}
}

func normalizeSpecVerboseFromRuntime(spec *nropv1.NUMAResourcesOperatorSpec, verb int) {
if spec.Verbose != nil {
return
}
spec.Verbose = &verb
}
3 changes: 2 additions & 1 deletion internal/api/features/_topics.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
"tmpol",
"schedattrwatch",
"ngpoolname",
"nganns"
"nganns",
"opverbctl"
]
}
4 changes: 3 additions & 1 deletion main.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ func main() {

klogV, err := intkloglevel.Get()
if err != nil {
klog.V(1).ErrorS(err, "setting up the logger")
klog.ErrorS(err, "setting up the logger")
os.Exit(1)
}

Expand Down Expand Up @@ -282,6 +282,7 @@ func main() {
os.Exit(1)
}

// the only way this can change from the command line is to restart, so fetching once is the best option
if err = (&controllers.NUMAResourcesOperatorReconciler{
Client: mgr.GetClient(),
Scheme: mgr.GetScheme(),
Expand All @@ -296,6 +297,7 @@ func main() {
ImagePullPolicy: pullPolicy,
Namespace: namespace,
ForwardMCPConds: params.enableMCPCondsForward,
Verbose: int(klogV),
}).SetupWithManager(mgr); err != nil {
klog.ErrorS(err, "unable to create controller", "controller", "NUMAResourcesOperator")
os.Exit(1)
Expand Down
1 change: 1 addition & 0 deletions pkg/status/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ const (

const (
ConditionTypeIncorrectNUMAResourcesOperatorResourceName = "IncorrectNUMAResourcesOperatorResourceName"
ConditionTypeInternalError = "Internal Error"
)

const (
Expand Down
Loading