Skip to content

Commit

Permalink
WIP: consume kloglevel at runtime
Browse files Browse the repository at this point in the history
Signed-off-by: Francesco Romani <[email protected]>
  • Loading branch information
ffromani committed Nov 20, 2024
1 parent 00e2403 commit 0dd5ad0
Show file tree
Hide file tree
Showing 11 changed files with 99 additions and 7 deletions.
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 @@ -163,6 +167,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 @@ -205,6 +205,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 @@ -540,6 +543,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: "2024-11-14T15:38:47Z"
createdAt: "2024-11-20T12:47:01Z"
olm.skipRange: '>=4.17.0 <4.18.0'
operators.operatorframework.io/builder: operator-sdk-v1.36.1
operators.operatorframework.io/project_layout: go.kubebuilder.io/v3
Expand All @@ -88,7 +88,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 @@ -130,6 +130,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 @@ -167,6 +170,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 @@ -205,6 +205,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 @@ -540,6 +543,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 @@ -27,7 +27,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 @@ -69,6 +69,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 @@ -106,6 +109,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 @@ -69,6 +69,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 @@ -92,6 +93,7 @@ type NUMAResourcesOperatorReconciler struct {
ImagePullPolicy corev1.PullPolicy
Recorder record.EventRecorder
ForwardMCPConds bool
Verbose int
}

// TODO: narrow down
Expand Down Expand Up @@ -146,6 +148,13 @@ 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)

// 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); err != nil {
return r.degradeStatus(ctx, instance, validation.NodeGroupsError, err)
}
Expand Down Expand Up @@ -219,6 +228,20 @@ func (r *NUMAResourcesOperatorReconciler) degradeStatus(ctx context.Context, ins
return ctrl.Result{}, nil
}

func (r *NUMAResourcesOperatorReconciler) reconcileInternal(ctx context.Context, instance *nropv1.NUMAResourcesOperator) {
// do as early as possible, but always after validation so we don't miss log messages
if verb := *instance.Spec.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
}
}

instance.Status.Verbose = 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 @@ -284,6 +307,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)
r.reconcileInternal(ctx, instance)

if step := r.reconcileResourceAPI(ctx, instance, trees); step.EarlyStop() {
updateStatusConditionsIfNeeded(instance, step.ConditionInfo)
return step
Expand Down Expand Up @@ -745,3 +771,10 @@ func getTreesByNodeGroup(ctx context.Context, cli client.Client, nodeGroups []nr
}
return nodegroupv1.FindTrees(mcps, nodeGroups)
}

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 @@ -17,6 +17,7 @@
"byres",
"tmpol",
"schedattrwatch",
"ngpoolname"
"ngpoolname",
"opverbctl"
]
}
8 changes: 8 additions & 0 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,13 @@ 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
verb, err := intkloglevel.Get()
if err != nil {
klog.ErrorS(err, "unable to acquire verbosiness level", "controller", "NUMAResourcesOperator")
os.Exit(1)
}

if err = (&controllers.NUMAResourcesOperatorReconciler{
Client: mgr.GetClient(),
Scheme: mgr.GetScheme(),
Expand All @@ -270,6 +277,7 @@ func main() {
ImagePullPolicy: pullPolicy,
Namespace: namespace,
ForwardMCPConds: params.enableMCPCondsForward,
Verbose: int(verb),
}).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"
)

func IsUpdatedNUMAResourcesOperator(oldStatus, newStatus *nropv1.NUMAResourcesOperatorStatus) bool {
Expand Down

0 comments on commit 0dd5ad0

Please sign in to comment.