Skip to content

✨main.go: switch to klog-based logger #1317

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Sep 27, 2024
Merged

✨main.go: switch to klog-based logger #1317

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
14 changes: 6 additions & 8 deletions cmd/manager/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,18 +28,18 @@ import (
"github.com/containers/image/v5/types"
"github.com/go-logr/logr"
"github.com/spf13/pflag"
"go.uber.org/zap/zapcore"
apiextensionsv1client "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/typed/apiextensions/v1"
k8slabels "k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/selection"
corev1client "k8s.io/client-go/kubernetes/typed/core/v1"
_ "k8s.io/client-go/plugin/pkg/client/auth"
"k8s.io/klog/v2"
"k8s.io/klog/v2/textlogger"
ctrl "sigs.k8s.io/controller-runtime"
crcache "sigs.k8s.io/controller-runtime/pkg/cache"
"sigs.k8s.io/controller-runtime/pkg/client"
crfinalizer "sigs.k8s.io/controller-runtime/pkg/finalizer"
"sigs.k8s.io/controller-runtime/pkg/healthz"
"sigs.k8s.io/controller-runtime/pkg/log/zap"
"sigs.k8s.io/controller-runtime/pkg/metrics/server"

catalogd "github.com/operator-framework/catalogd/api/core/v1alpha1"
Expand Down Expand Up @@ -101,11 +101,8 @@ func main() {
flag.StringVar(&cachePath, "cache-path", "/var/cache", "The local directory path used for filesystem based caching")
flag.BoolVar(&operatorControllerVersion, "version", false, "Prints operator-controller version information")
flag.StringVar(&systemNamespace, "system-namespace", "", "Configures the namespace that gets used to deploy system resources.")
opts := zap.Options{
Development: true,
TimeEncoder: zapcore.RFC3339NanoTimeEncoder,
}
opts.BindFlags(flag.CommandLine)

klog.InitFlags(flag.CommandLine)

pflag.CommandLine.AddGoFlagSet(flag.CommandLine)
features.OperatorControllerFeatureGate.AddFlag(pflag.CommandLine)
Expand All @@ -116,7 +113,8 @@ func main() {
os.Exit(0)
}

ctrl.SetLogger(zap.New(zap.UseFlagOptions(&opts), zap.StacktraceLevel(zapcore.DPanicLevel)))
ctrl.SetLogger(textlogger.NewLogger(textlogger.NewConfig()))

setupLog.Info("starting up the controller", "version info", version.String())

if systemNamespace == "" {
Expand Down
10 changes: 5 additions & 5 deletions config/base/manager/manager.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,12 +90,12 @@ spec:
capabilities:
drop:
- "ALL"
image: gcr.io/kubebuilder/kube-rbac-proxy:v0.13.1
image: gcr.io/kubebuilder/kube-rbac-proxy:v0.15.0
args:
- "--secure-listen-address=0.0.0.0:8443"
- "--upstream=http://127.0.0.1:8080/"
- "--logtostderr=true"
- "--v=0"
- --secure-listen-address=0.0.0.0:8443
- --http2-disable
- --upstream=http://127.0.0.1:8080/
- --logtostderr=true
Comment on lines +93 to +98
Copy link
Member Author

Choose a reason for hiding this comment

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

Making arguments for krp look the same as catalogd (which also requires bumping krp version).

I'm doing this to ensure consistency in the contents and ordering of arguments between catalogd and operator controller so that vendors have an easier time manipulating flags for their specific use cases.

ports:
- containerPort: 8443
protocol: TCP
Expand Down
5 changes: 1 addition & 4 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ require (
github.com/operator-framework/operator-registry v1.47.0
github.com/spf13/pflag v1.0.5
github.com/stretchr/testify v1.9.0
go.uber.org/zap v1.27.0
golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56
gopkg.in/yaml.v2 v2.4.0
helm.sh/helm/v3 v3.16.1
Expand All @@ -33,6 +32,7 @@ require (
k8s.io/cli-runtime v0.31.1
k8s.io/client-go v0.31.1
k8s.io/component-base v0.31.1
k8s.io/klog/v2 v2.130.1
k8s.io/utils v0.0.0-20240711033017-18e509b52bc8
sigs.k8s.io/controller-runtime v0.19.0
sigs.k8s.io/yaml v1.4.0
Expand Down Expand Up @@ -98,7 +98,6 @@ require (
github.com/go-gorp/gorp/v3 v3.1.0 // indirect
github.com/go-jose/go-jose/v4 v4.0.2 // indirect
github.com/go-logr/stdr v1.2.2 // indirect
github.com/go-logr/zapr v1.3.0 // indirect
github.com/go-openapi/analysis v0.23.0 // indirect
github.com/go-openapi/errors v0.22.0 // indirect
github.com/go-openapi/jsonpointer v0.21.0 // indirect
Expand Down Expand Up @@ -224,7 +223,6 @@ require (
go.opentelemetry.io/otel/trace v1.28.0 // indirect
go.opentelemetry.io/proto/otlp v1.3.1 // indirect
go.starlark.net v0.0.0-20230612165344-9532f5667272 // indirect
go.uber.org/multierr v1.11.0 // indirect
golang.org/x/crypto v0.27.0 // indirect
golang.org/x/net v0.29.0 // indirect
golang.org/x/oauth2 v0.22.0 // indirect
Expand All @@ -245,7 +243,6 @@ require (
gopkg.in/warnings.v0 v0.1.2 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
k8s.io/apiserver v0.31.1 // indirect
k8s.io/klog/v2 v2.130.1 // indirect
k8s.io/kube-openapi v0.0.0-20240228011516-70dd3763d340 // indirect
k8s.io/kubectl v0.31.0 // indirect
oras.land/oras-go v1.2.5 // indirect
Expand Down
16 changes: 8 additions & 8 deletions internal/controllers/clusterextension_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,8 @@ func (r *ClusterExtensionReconciler) Reconcile(ctx context.Context, req ctrl.Req
l := log.FromContext(ctx).WithName("operator-controller")
ctx = log.IntoContext(ctx, l)

l.V(1).Info("reconcile starting")
defer l.V(1).Info("reconcile ending")
l.Info("reconcile starting")
defer l.Info("reconcile ending")
Comment on lines +104 to +105
Copy link
Member Author

Choose a reason for hiding this comment

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

With the original zap logger l.V(1).Info was being output by default.

With klogr, we need to put these log messages at a lower verbosity level to keep them coming by default.


existingExt := &ocv1alpha1.ClusterExtension{}
if err := r.Client.Get(ctx, req.NamespacedName, existingExt); err != nil {
Expand Down Expand Up @@ -190,7 +190,7 @@ func checkForUnexpectedFieldChange(a, b ocv1alpha1.ClusterExtension) bool {
func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alpha1.ClusterExtension) (ctrl.Result, error) {
l := log.FromContext(ctx)

l.V(1).Info("handling finalizers")
l.Info("handling finalizers")
finalizeResult, err := r.Finalizers.Finalize(ctx, ext)
if err != nil {
// TODO: For now, this error handling follows the pattern of other error handling.
Expand All @@ -210,7 +210,7 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp
return ctrl.Result{}, nil
}

l.V(1).Info("getting installed bundle")
l.Info("getting installed bundle")
installedBundle, err := r.InstalledBundleGetter.GetInstalledBundle(ctx, ext)
if err != nil {
setInstallStatus(ext, nil)
Expand All @@ -221,7 +221,7 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp
}

// run resolution
l.V(1).Info("resolving bundle")
l.Info("resolving bundle")
resolvedBundle, resolvedBundleVersion, resolvedDeprecation, err := r.Resolver.Resolve(ctx, ext, installedBundle)
if err != nil {
// Note: We don't distinguish between resolution-specific errors and generic errors
Expand Down Expand Up @@ -261,7 +261,7 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp
Ref: resolvedBundle.Image,
},
}
l.V(1).Info("unpacking resolved bundle")
l.Info("unpacking resolved bundle")
unpackResult, err := r.Unpacker.Unpack(ctx, bundleSource)
if err != nil {
// Wrap the error passed to this with the resolution information until we have successfully
Expand All @@ -286,7 +286,7 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp
labels.BundleVersionKey: resolvedBundleVersion.String(),
}

l.V(1).Info("applying bundle contents")
l.Info("applying bundle contents")
// NOTE: We need to be cautious of eating errors here.
// We should always return any error that occurs during an
// attempt to apply content to the cluster. Only when there is
Expand All @@ -309,7 +309,7 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp
setInstallStatus(ext, installStatus)
setInstalledStatusConditionSuccess(ext, fmt.Sprintf("Installed bundle %s successfully", resolvedBundle.Image))

l.V(1).Info("watching managed objects")
l.Info("watching managed objects")
cache, err := r.Manager.Get(ctx, ext)
if err != nil {
// No need to wrap error with resolution information here (or beyond) since the
Expand Down
2 changes: 1 addition & 1 deletion test/upgrade-e2e/post_upgrade_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func TestClusterExtensionAfterOLMUpgrade(t *testing.T) {
defer cancel()
substrings := []string{
"reconcile ending",
fmt.Sprintf(`"ClusterExtension": {"name":"%s"}`, testClusterExtensionName),
fmt.Sprintf(`ClusterExtension=%q`, testClusterExtensionName),
}
found, err := watchPodLogsForSubstring(logCtx, &managerPods.Items[0], "manager", substrings...)
require.NoError(t, err)
Expand Down