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

Use --zap-time-encoding flag for log timestamp format #963

Merged
merged 2 commits into from
Jan 19, 2022

Conversation

creydr
Copy link
Collaborator

@creydr creydr commented Jan 18, 2022

Is this a BUG FIX or a FEATURE ?:

Uncomment only one, leave it on its own line:

/kind bug

/kind enhancement

What this PR does / why we need it:
Since #843 a more readable format for log timestamps is used. Since v0.11, controller-runtime allows to define the encoding for log timestamps via the zap-time-encoding flag (kubernetes-sigs/controller-runtime#1688). This PR makes use of the flag to give the user the possibility to specify the required format (e.g. in case the currently hard coded ISO8601 format is not feasible).

Special notes for your reviewer:
To use v0.11 of controller-runtime I had to update the k8s dependencies to 1.23. In addition the go.mod files were adjusted to "use" golang 1.17, which leads to a 2nd require block for indirect dependencies (see https://go.dev/doc/go1.17).

Depends-On: #965 and #964

Release note:

Use --zap-time-encoding flag to specify the format for timestamp encoding of log messages

@kubevirt-bot kubevirt-bot added kind/enhancement release-note Denotes a PR that will be considered when it comes time to generate release notes. dco-signoff: no Indicates the PR's author has not DCO signed all their commits. labels Jan 18, 2022
@creydr creydr force-pushed the use-controller-runtime-log-flags branch from 9bcc429 to e8ec1a0 Compare January 18, 2022 12:00
@kubevirt-bot kubevirt-bot added dco-signoff: yes Indicates the PR's author has DCO signed all their commits. and removed dco-signoff: no Indicates the PR's author has not DCO signed all their commits. labels Jan 18, 2022
@creydr creydr force-pushed the use-controller-runtime-log-flags branch from e8ec1a0 to 89e92d8 Compare January 18, 2022 12:21
api/go.mod Outdated
Comment on lines 13 to 37

require (
github.com/fsnotify/fsnotify v1.4.9 // indirect
github.com/go-logr/logr v0.4.0 // indirect
github.com/go-task/slim-sprig v0.0.0-20210107165309-348f09dbbbc0 // indirect
github.com/gogo/protobuf v1.3.2 // indirect
github.com/google/go-cmp v0.5.6 // indirect
github.com/google/gofuzz v1.1.0 // indirect
github.com/json-iterator/go v1.1.11 // indirect
github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
github.com/modern-go/reflect2 v1.0.1 // indirect
github.com/nxadm/tail v1.4.8 // indirect
golang.org/x/net v0.0.0-20210805182204-aaa1db679c0d // indirect
golang.org/x/sys v0.0.0-20211013075003-97ac67df715c // indirect
golang.org/x/text v0.3.6 // indirect
golang.org/x/tools v0.1.7 // indirect
google.golang.org/protobuf v1.27.1 // indirect
gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect
gopkg.in/inf.v0 v0.9.1 // indirect
gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7 // indirect
gopkg.in/yaml.v2 v2.4.0 // indirect
k8s.io/klog/v2 v2.9.0 // indirect
sigs.k8s.io/structured-merge-diff/v4 v4.1.2 // indirect
)
Copy link
Member

Choose a reason for hiding this comment

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

This wasn't generated with go mod tidy right ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Actutally I ran a go mod tidy on them

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Reran it on api/go.mod without changes

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Reran it on ./go.mod, with a minor change (moves go.uber.org/zap to indirect dependencies). Will push it...

@creydr creydr force-pushed the use-controller-runtime-log-flags branch from 89e92d8 to 991749f Compare January 18, 2022 12:39
qinqon added a commit to qinqon/project-infra that referenced this pull request Jan 18, 2022
Use golang 1.17 for knmstate to check PR [1]

[1] nmstate/kubernetes-nmstate#963

Signed-off-by: Quique Llorente <ellorent@redhat.com>
qinqon added a commit to qinqon/project-infra that referenced this pull request Jan 18, 2022
Use golang 1.17 for knmstate to check PR [1]

[1] nmstate/kubernetes-nmstate#963

Signed-off-by: Quique Llorente <ellorent@redhat.com>
@qinqon
Copy link
Member

qinqon commented Jan 18, 2022

/hold

We have to bump prow jobs first with kubevirt/project-infra#1866

@kubevirt-bot kubevirt-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 18, 2022
qinqon added a commit to qinqon/project-infra that referenced this pull request Jan 18, 2022
Use golang specified at go.mod for to check PR [1]

[1] nmstate/kubernetes-nmstate#963

Signed-off-by: Quique Llorente <ellorent@redhat.com>
qinqon added a commit to qinqon/project-infra that referenced this pull request Jan 18, 2022
Use golang specified at go.mod for to check PR [1]

[1] nmstate/kubernetes-nmstate#963

Signed-off-by: Quique Llorente <ellorent@redhat.com>
qinqon added a commit to qinqon/project-infra that referenced this pull request Jan 18, 2022
Use golang specified at go.mod for to check PR [1]

[1] nmstate/kubernetes-nmstate#963

Signed-off-by: Quique Llorente <ellorent@redhat.com>
qinqon added a commit to qinqon/project-infra that referenced this pull request Jan 18, 2022
Use golang specified at go.mod for to check PR [1]

[1] nmstate/kubernetes-nmstate#963

Signed-off-by: Quique Llorente <ellorent@redhat.com>
qinqon added a commit to qinqon/project-infra that referenced this pull request Jan 18, 2022
Use golang specified at go.mod for to check PR [1]

[1] nmstate/kubernetes-nmstate#963

Signed-off-by: Quique Llorente <ellorent@redhat.com>
qinqon added a commit to qinqon/project-infra that referenced this pull request Jan 18, 2022
Use golang specified at go.mod for to check PR [1]

[1] nmstate/kubernetes-nmstate#963

Signed-off-by: Quique Llorente <ellorent@redhat.com>
qinqon added a commit to qinqon/project-infra that referenced this pull request Jan 18, 2022
Use golang specified at go.mod for to check PR [1]

[1] nmstate/kubernetes-nmstate#963

Signed-off-by: Quique Llorente <ellorent@redhat.com>
qinqon added a commit to qinqon/project-infra that referenced this pull request Jan 18, 2022
Use golang specified at go.mod for to check PR [1]

[1] nmstate/kubernetes-nmstate#963

Signed-off-by: Quique Llorente <ellorent@redhat.com>
qinqon added a commit to qinqon/project-infra that referenced this pull request Jan 18, 2022
Use golang specified at go.mod for to check PR [1]

[1] nmstate/kubernetes-nmstate#963

Signed-off-by: Quique Llorente <ellorent@redhat.com>
@creydr creydr force-pushed the use-controller-runtime-log-flags branch 3 times, most recently from b5de4b2 to d01afc8 Compare January 18, 2022 17:15
@kubevirt-bot kubevirt-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 19, 2022
Signed-off-by: Christoph Stäbler <cstabler@redhat.com>
Since v0.11 controller-runtime supports the zap-time-encoding flag to
specify the time encoding of log timestamps. This flag is used to give
the user the possibilty to specify the required format.
By default nmstate-kubernetes uses the ISO8601 format.

Signed-off-by: Christoph Stäbler <cstabler@redhat.com>
@creydr creydr force-pushed the use-controller-runtime-log-flags branch from d01afc8 to 28b0907 Compare January 19, 2022 07:14
@kubevirt-bot kubevirt-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 19, 2022
@creydr creydr requested a review from qinqon January 19, 2022 09:13
@creydr
Copy link
Collaborator Author

creydr commented Jan 19, 2022

/test pull-kubernetes-nmstate-e2e-handler-k8s-future

@creydr
Copy link
Collaborator Author

creydr commented Jan 19, 2022

/hold cancel
as #965 and #964 got merged

@kubevirt-bot kubevirt-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 19, 2022
@kubevirt-bot
Copy link
Collaborator

@creydr: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
pull-kubernetes-nmstate-e2e-handler-k8s-future 28b0907 link false /test pull-kubernetes-nmstate-e2e-handler-k8s-future

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@qinqon
Copy link
Member

qinqon commented Jan 19, 2022

/lgtm
/approve

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Jan 19, 2022
@kubevirt-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: qinqon

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kubevirt-bot kubevirt-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 19, 2022
@kubevirt-bot kubevirt-bot merged commit 0b82fa3 into nmstate:main Jan 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. kind/enhancement lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XXL
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants