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

KOGITO-8276: Improve the logging adding missing levels #201

Merged
merged 25 commits into from
Aug 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
c7a5d4c
KOGITO-8276: Improve the logging adding missing levels
manstis Jul 25, 2023
d555eb5
Add support for -zap-log-level CLI flag
manstis Jul 25, 2023
de258ea
Fix function invocation.
manstis Jul 25, 2023
2b6d787
Updates following peer review.
manstis Jul 25, 2023
c70f1b4
Updates following peer review x2. Remove use of logger argument.
manstis Jul 26, 2023
a4688db
Remove klog.InitFlags(nil) in Logger wrappers.
manstis Jul 26, 2023
3bbe577
Run make generate-all
manstis Jul 27, 2023
172b6c5
Another refactor removing wrappers and contexts.
manstis Jul 27, 2023
65bcfc0
More updates.
manstis Jul 27, 2023
9022305
Enable Info logging by default.
manstis Jul 27, 2023
51daf6b
Add missing klog dependency.
manstis Jul 28, 2023
f546a34
Fix erroneous logging statements.
manstis Jul 28, 2023
01476e0
Move to structured logging.
manstis Jul 28, 2023
cfb078a
Remove references to k8s.io/klog[v1]
manstis Jul 28, 2023
63d592e
Fix build
manstis Jul 28, 2023
277943a
Fix go.sum
manstis Jul 28, 2023
a202d90
Update container-builder/builder/kubernetes/builder.go
manstis Aug 1, 2023
d0a84fd
Update container-builder/builder/kubernetes/builder.go
manstis Aug 1, 2023
e129a69
Update utils/kubernetes/annotations.go
manstis Aug 1, 2023
88ed716
Update controllers/builder/containerbuilder.go
manstis Aug 1, 2023
31448d1
Update container-builder/client/fastmapper.go
manstis Aug 1, 2023
d1befab
Update container-builder/common/registry_docker.go
manstis Aug 2, 2023
cfbd274
Update controllers/platform/platformutils.go
manstis Aug 2, 2023
b35d405
Update controllers/platform/platformutils.go
manstis Aug 2, 2023
ec17b73
Update controllers/platform/platformutils.go
manstis Aug 2, 2023
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
22 changes: 22 additions & 0 deletions api/const.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/*
* Copyright 2023 Red Hat, Inc. and/or its affiliates.
*
* 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.
*/

package api

const (
ricardozanini marked this conversation as resolved.
Show resolved Hide resolved
// ComponentName just a name to identify this package/component/application
ComponentName = "sonataflow"
)
2 changes: 1 addition & 1 deletion api/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ require (
k8s.io/apiextensions-apiserver v0.27.2 // indirect
k8s.io/client-go v0.27.2 // indirect
k8s.io/component-base v0.27.2 // indirect
k8s.io/klog/v2 v2.90.1 // indirect
k8s.io/klog/v2 v2.100.1 // indirect
k8s.io/kube-openapi v0.0.0-20230501164219-8b0f38b5fd1f // indirect
k8s.io/utils v0.0.0-20230313181309-38a27ef9d749 // indirect
sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect
Expand Down
3 changes: 1 addition & 2 deletions api/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -271,8 +271,7 @@ k8s.io/client-go v0.27.2 h1:vDLSeuYvCHKeoQRhCXjxXO45nHVv2Ip4Fe0MfioMrhE=
k8s.io/client-go v0.27.2/go.mod h1:tY0gVmUsHrAmjzHX9zs7eCjxcBsf8IiNe7KQ52biTcQ=
k8s.io/component-base v0.27.2 h1:neju+7s/r5O4x4/txeUONNTS9r1HsPbyoPBAtHsDCpo=
k8s.io/component-base v0.27.2/go.mod h1:5UPk7EjfgrfgRIuDBFtsEFAe4DAvP3U+M8RTzoSJkpo=
k8s.io/klog/v2 v2.90.1 h1:m4bYOKall2MmOiRaR1J+We67Do7vm9KiQVlT96lnHUw=
k8s.io/klog/v2 v2.90.1/go.mod h1:y1WjHnz7Dj687irZUWR/WLkLc5N1YHtjLdmgWjndZn0=
k8s.io/klog/v2 v2.100.1 h1:7WCHKK6K8fNhTqfBhISHQ97KrnJNFZMcQvKp7gP/tmg=
k8s.io/kube-openapi v0.0.0-20230501164219-8b0f38b5fd1f h1:2kWPakN3i/k81b0gvD5C5FJ2kxm1WrQFanWchyKuqGg=
k8s.io/kube-openapi v0.0.0-20230501164219-8b0f38b5fd1f/go.mod h1:byini6yhqGC14c3ebc/QwanvYwhuMWF6yz2F8uwW8eg=
k8s.io/utils v0.0.0-20230313181309-38a27ef9d749 h1:xMMXJlJbsU8w3V5N2FLDQ8YgU8s1EoULdbQBcAeNJkY=
Expand Down
1 change: 1 addition & 0 deletions config/manager/manager.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ spec:
- /usr/local/bin/manager
args:
- --leader-elect
- '-v=2'
Copy link
Member

Choose a reason for hiding this comment

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

any reason to add it here?

Copy link
Member

Choose a reason for hiding this comment

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

Please see the review history. This is the default verbosity for the manager. We will update the Troubleshooting Guides in the operator section to explain to administrators how to increase the verbosity level.

image: controller:latest
name: manager
securityContext:
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2022 Red Hat, Inc. and/or its affiliates.
* Copyright 2023 Red Hat, Inc. and/or its affiliates.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -14,7 +14,7 @@
* limitations under the License.
*/

package util
package api

const (
// ComponentName just a name to identify this package/component/application
Expand Down
11 changes: 6 additions & 5 deletions container-builder/builder/kaniko_docker_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,12 @@ import (
"testing"
"time"

"github.com/sirupsen/logrus"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/suite"
"k8s.io/klog/v2"

"github.com/kiegroup/kogito-serverless-operator/container-builder/common"
"github.com/kiegroup/kogito-serverless-operator/container-builder/util/log"
)

func TestKanikoTestSuite(t *testing.T) {
Expand All @@ -39,7 +40,7 @@ func (suite *KanikoDockerTestSuite) TestKanikoBuild() {
//registry, err, repos := checkEmptyDockerRegistry(suite)
mydir, err := os.Getwd()
if err != nil {
logrus.Error(err)
klog.V(log.E).ErrorS(err, "error getting working directory.")
}
dockefileDir := mydir + "/../examples/dockerfiles"
assert.Nil(suite.T(), suite.Docker.PullImage("gcr.io/kaniko-project/executor:latest"), "Pull image failed")
Expand All @@ -52,11 +53,11 @@ func (suite *KanikoDockerTestSuite) TestKanikoBuild() {
RegistryFinalImageName: imageName,
ReadBuildOutput: false,
}
logrus.Infof("Start Kaniko build")
klog.V(log.I).InfoS("Start Kaniko build")
start := time.Now()
imageID, error := KanikoBuild(suite.Docker.Connection, config)
timeElapsed := time.Since(start)
logrus.Infof("The Kaniko build took %s", timeElapsed)
klog.V(log.I).InfoS("The Kaniko build took", "duration", timeElapsed)
assert.Nil(suite.T(), error, "ContainerBuild failed")
assert.NotNil(suite.T(), imageID, error, "ContainerBuild failed")
//@TODO investigate when the code will be in the mono repo
Expand All @@ -74,7 +75,7 @@ func checkEmptyDockerRegistry(suite *KanikoDockerTestSuite) (common.RegistryCont
assert.Truef(suite.T(), suite.RegistryID != "", "Registry not started")
registry, err := common.GetRegistryContainer()
if err != nil {
logrus.Error("registry not found")
klog.V(log.E).ErrorS(err, "registry not found")
}
repos, _ := registry.GetRepositories()
assert.True(suite.T(), len(repos) == 0)
Expand Down
13 changes: 7 additions & 6 deletions container-builder/builder/kaniko_integration_test_suite.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,12 @@ package builder
import (
"time"

"github.com/sirupsen/logrus"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/suite"
"k8s.io/klog/v2"

"github.com/kiegroup/kogito-serverless-operator/container-builder/common"
"github.com/kiegroup/kogito-serverless-operator/container-builder/util/log"
)

type KanikoDockerTestSuite struct {
Expand All @@ -46,7 +47,7 @@ func (suite *KanikoDockerTestSuite) SetupSuite() {

pullErr := suite.Docker.PullImage(EXECUTOR_IMAGE)
if pullErr != nil {
logrus.Infof("Pull Kaniko executor Error:%s", pullErr)
klog.V(log.E).ErrorS(pullErr, "Pull Kaniko executor")
}
time.Sleep(4 * time.Second) // Needed on CI
}
Expand All @@ -59,14 +60,14 @@ func (suite *KanikoDockerTestSuite) TearDownSuite() {
suite.LocalRegistry.StopRegistry()
}
purged, err := suite.Docker.PurgeContainer("", common.REGISTRY_IMG)
logrus.Infof("Purged containers %t", purged)
klog.V(log.I).InfoS("Purged containers", "containers", purged)
if err != nil {
logrus.Infof("Purged registry err %t", err)
klog.V(log.E).ErrorS(err, "Purged registry")
}

purgedBuild, err := suite.Docker.PurgeContainer("", EXECUTOR_IMAGE)
if err != nil {
logrus.Infof("Purged container err %t", err)
klog.V(log.E).ErrorS(err, "Purged container")
}
logrus.Infof("Purged container build %t", purgedBuild)
klog.V(log.I).InfoS("Purged container build", "container", purgedBuild)
}
16 changes: 8 additions & 8 deletions container-builder/builder/kaniko_vanilla.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,14 @@ import (
"context"
"os"

"k8s.io/klog/v2"

"github.com/docker/docker/api/types"
"github.com/docker/docker/api/types/container"
"github.com/docker/docker/client"
"github.com/docker/docker/pkg/stdcopy"
"github.com/sirupsen/logrus"

"github.com/kiegroup/kogito-serverless-operator/container-builder/util/log"
)

type KanikoVanillaConfig struct {
Expand Down Expand Up @@ -62,28 +65,25 @@ func KanikoBuild(connection *client.Client, config KanikoVanillaConfig) (string,
}, hostConfig, nil, nil, config.ContainerName)

if err != nil {
logrus.Error(err)
klog.V(log.E).ErrorS(err, "error during KanikoBuild, ContainerCreate")
}

if err := connection.ContainerStart(ctx, resp.ID, types.ContainerStartOptions{}); err != nil {
logrus.Error(err)
}
if err != nil {
logrus.Error(err)
klog.V(log.E).ErrorS(err, "error during KanikoBuild, ContainerStart")
}

statusCh, errCh := connection.ContainerWait(ctx, resp.ID, container.WaitConditionNotRunning)
select {
case err := <-errCh:
if err != nil {
logrus.Error(err)
klog.V(log.E).ErrorS(err, "error during KanikoBuild, ContainerWait")
}
case <-statusCh:
}

out, err := connection.ContainerLogs(ctx, resp.ID, types.ContainerLogsOptions{ShowStdout: true})
if err != nil {
logrus.Error(err)
klog.V(log.E).ErrorS(err, "error during KanikoBuild, ContainerLogs")
}
if config.ReadBuildOutput {
stdcopy.StdCopy(os.Stdout, os.Stderr, out)
Expand Down
7 changes: 0 additions & 7 deletions container-builder/builder/kubernetes/action.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,9 @@ import (

"github.com/kiegroup/kogito-serverless-operator/container-builder/api"
"github.com/kiegroup/kogito-serverless-operator/container-builder/client"
"github.com/kiegroup/kogito-serverless-operator/container-builder/util/log"
)

type Action interface {
log.Injectable
client.Injectable
// Name returns user-friendly name for the action
Name() string
Expand All @@ -37,15 +35,10 @@ type Action interface {

type baseAction struct {
client client.Client
L log.Logger
}

// TODO: implement our client wrapper

func (action *baseAction) InjectClient(client client.Client) {
action.client = client
}

func (action *baseAction) InjectLogger(log log.Logger) {
action.L = log
}
12 changes: 5 additions & 7 deletions container-builder/builder/kubernetes/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,12 @@ import (
"context"
"fmt"

"k8s.io/klog/v2"

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

"github.com/kiegroup/kogito-serverless-operator/container-builder/api"
"github.com/kiegroup/kogito-serverless-operator/container-builder/client"
"github.com/kiegroup/kogito-serverless-operator/container-builder/util"
"github.com/kiegroup/kogito-serverless-operator/container-builder/util/log"
)

Expand All @@ -49,7 +50,6 @@ type containerBuildContext struct {
}

type builder struct {
L log.Logger
Context containerBuildContext
}

Expand Down Expand Up @@ -94,7 +94,6 @@ type schedulerHandler interface {

func FromBuild(build *api.ContainerBuild) ContainerBuilder {
return &builder{
L: log.WithName(util.ComponentName),
Context: containerBuildContext{
ContainerBuild: build,
C: context.TODO(),
Expand Down Expand Up @@ -174,20 +173,19 @@ func (b *builder) Reconcile() (*api.ContainerBuild, error) {
target := b.Context.ContainerBuild.DeepCopy()

for _, a := range actions {
a.InjectLogger(b.L)
a.InjectClient(b.Context.Client)

if a.CanHandle(target) {
b.L.Infof("Invoking action %s", a.Name())
klog.V(log.I).InfoS("Invoking action", "buildAction", a.Name())
newTarget, err := a.Handle(b.Context.C, target)
if err != nil {
b.L.Errorf(err, "Failed to invoke action %s", a.Name())
klog.V(log.E).ErrorS(err, "Failed to invoke action", "buildAction", a.Name())
return nil, err
}

if newTarget != nil {
if newTarget.Status.Phase != target.Status.Phase {
b.L.Info(
klog.V(log.I).InfoS(
"state transition",
"phase-from", target.Status.Phase,
"phase-to", newTarget.Status.Phase,
Expand Down
3 changes: 0 additions & 3 deletions container-builder/builder/kubernetes/builder_kaniko.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@ import (
corev1 "k8s.io/api/core/v1"

"github.com/kiegroup/kogito-serverless-operator/container-builder/api"
"github.com/kiegroup/kogito-serverless-operator/container-builder/util"
"github.com/kiegroup/kogito-serverless-operator/container-builder/util/log"
)

type kanikoScheduler struct {
Expand Down Expand Up @@ -60,7 +58,6 @@ func (k kanikoSchedulerHandler) CreateScheduler(info ContainerBuilderInfo, build
sched := &kanikoScheduler{
&scheduler{
builder: builder{
L: log.WithName(util.ComponentName),
Context: buildCtx,
},
Resources: make([]resource, 0),
Expand Down
10 changes: 7 additions & 3 deletions container-builder/builder/kubernetes/recovery.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ import (
"context"
"time"

"k8s.io/klog/v2"

"github.com/kiegroup/kogito-serverless-operator/container-builder/util/log"

"github.com/jpillora/backoff"

"github.com/kiegroup/kogito-serverless-operator/container-builder/api"
Expand Down Expand Up @@ -86,9 +90,9 @@ func (action *errorRecoveryAction) Handle(ctx context.Context, build *api.Contai
build.Status.Failure.Recovery.Attempt++
build.Status.Failure.Recovery.AttemptTime = metav1.Now()

action.L.Infof("Recovery attempt (%d/%d)",
build.Status.Failure.Recovery.Attempt,
build.Status.Failure.Recovery.AttemptMax,
klog.V(log.I).InfoS("Recovery attempt",
"attempt", build.Status.Failure.Recovery.Attempt,
"attemptMax", build.Status.Failure.Recovery.AttemptMax,
)

return build, nil
Expand Down
9 changes: 5 additions & 4 deletions container-builder/cleaner/docker_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,12 @@ import (
"testing"
"time"

"github.com/sirupsen/logrus"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/suite"
"k8s.io/klog/v2"

"github.com/kiegroup/kogito-serverless-operator/container-builder/common"
"github.com/kiegroup/kogito-serverless-operator/container-builder/util/log"
)

func TestDockerIntegrationTestSuite(t *testing.T) {
Expand All @@ -43,21 +44,21 @@ func (suite *DockerTestSuite) TestImagesOperationsOnDockerRegistryForTest() {

pullErr := suite.Docker.PullImage(common.TEST_IMG + ":" + common.LATEST_TAG)
if pullErr != nil {
logrus.Infof("Pull Error:%s", pullErr)
klog.V(log.E).ErrorS(pullErr, "Pull Error")
}
assert.Nil(suite.T(), pullErr, "Pull image failed")
time.Sleep(2 * time.Second) // Needed on CI
assert.True(suite.T(), suite.LocalRegistry.IsImagePresent(common.TEST_IMG), "Test image not found in the registry after the pull")
tagErr := suite.Docker.TagImage(common.TEST_IMG, common.TEST_IMG_LOCAL_TAG)
if tagErr != nil {
logrus.Infof("Tag Error:%s", tagErr)
klog.V(log.E).ErrorS(tagErr, "Tag Error")
}

assert.Nil(suite.T(), tagErr, "Tag image failed")
time.Sleep(2 * time.Second) // Needed on CI
pushErr := suite.Docker.PushImage(common.TEST_IMG_LOCAL_TAG, common.REGISTRY_CONTAINER_URL_FROM_DOCKER_SOCKET, "", "")
if pushErr != nil {
logrus.Infof("Push Error:%s", pushErr)
klog.V(log.E).ErrorS(pushErr, "Push Error")
}

assert.Nil(suite.T(), pushErr, "Push image in the Docker container failed")
Expand Down
7 changes: 4 additions & 3 deletions container-builder/cleaner/docker_integration_test_suite.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,12 @@
package cleaner

import (
"github.com/sirupsen/logrus"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/suite"
"k8s.io/klog/v2"

"github.com/kiegroup/kogito-serverless-operator/container-builder/common"
"github.com/kiegroup/kogito-serverless-operator/container-builder/util/log"
)

type DockerTestSuite struct {
Expand Down Expand Up @@ -52,7 +53,7 @@ func (suite *DockerTestSuite) TearDownSuite() {
}
purged, err := suite.Docker.PurgeContainer("", common.REGISTRY_IMG)
if err != nil {
logrus.Errorf("Error during purged container in TearDown Suite %t", err)
klog.V(log.E).ErrorS(err, "Error during purged container in TearDown Suite.")
}
logrus.Infof("Purged containers %t", purged)
klog.V(log.I).InfoS("Purged container", "containers", purged)
}
Loading
Loading