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.

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

Jump to bottom

kubemark: replace deprecated --log-file parameter with runner #106150

Merged
merged 1 commit into from Nov 5, 2021
Merged

kubemark: replace deprecated --log-file parameter with runner #106150

merged 1 commit into from Nov 5, 2021

Conversation

pohly
Copy link
Contributor

@pohly pohly commented Nov 4, 2021 •
edited

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

The --log-file parameter will be deprecated as of Kubernetes 1.23 and should be
avoided. The replacement for distroless images is the image with go-runner, a
tool that handles output redirection.

Which issue(s) this PR fixes:

Part of: #106103

Does this PR introduce a user-facing change?

kubemark is now built as a portable, static binary.

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

- KEP: https://github.com/kubernetes/enhancements/issues/2845

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Nov 4, 2021
@k8s-ci-robot
Copy link
Contributor

@pohly: This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

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.

@k8s-ci-robot k8s-ci-robot added needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. area/release-eng Issues or PRs related to the Release Engineering subproject area/test sig/scalability Categorizes an issue or PR as relevant to SIG Scalability. sig/testing Categorizes an issue or PR as relevant to SIG Testing. and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Nov 4, 2021
pohly
pohly commented Nov 4, 2021
View reviewed changes
cluster/images/kubemark/Dockerfile
@@ -12,8 +12,13 @@
# See the License for the specific language governing permissions and
# limitations under the License.

# The line below points to distroless/base as of 2021-06-29. The SHA should be
# kept in sycn with distroless_base definition in the WORKSPACE file.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have no idea what "the WORKSPACE file" is. I hope the revised comment is okay.

"Update periodically" is vague, but I don't have any better recommendation.

@marseel
Copy link
Member

marseel commented Nov 4, 2021

/test pull-kubernetes-kubemark-e2e-gce-big

@pohly
Copy link
Contributor Author

pohly commented Nov 4, 2021

Building the kubemark image looks okay in https://storage.googleapis.com/kubernetes-jenkins/pr-logs/pull/106150/pull-kubernetes-kubemark-e2e-gce-big/1456277296036974592/build-log.txt:

Step 1/2 : FROM k8s.gcr.io/build-image/go-runner:v2.3.1-go1.17.2-bullseye.0
v2.3.1-go1.17.2-bullseye.0: Pulling from build-image/go-runner
ec52731e9273: Already exists
a5732a474047: Already exists
Digest: sha256:7b035982301e211bf9cad67f3d1ccf97d7778f88cc18c8ac286e619bb970d5ba
Status: Downloaded newer image for k8s.gcr.io/build-image/go-runner:v2.3.1-go1.17.2-bullseye.0
 ---> dc2689354f85
Step 2/2 : COPY kubemark /kubemark
 ---> 9710a2cb5665
Successfully built 9710a2cb5665
Successfully tagged gcr.io/k8s-infra-e2e-boskos-scale-11/kubemark:huv8wj
docker push gcr.io/k8s-infra-e2e-boskos-scale-11/kubemark:huv8wj
The push refers to repository [gcr.io/k8s-infra-e2e-boskos-scale-11/kubemark]

But then the hollow-nodes don't come up:

Waiting for all hollow-nodes to become Running..........................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................
�[0;31m Timeout waiting for all hollow-nodes to become Running. �[0m
Found only 1 ready hollow-nodes while waiting for 500.

gs://sig-scalability-logs/pull-kubernetes-kubemark-e2e-gce-big/1456277296036974592/e2e-106150-ac87c-minion-group-40jf/npd-hollow-node-w7wnp.log contains:

Flag --system-log-monitors has been deprecated, replaced by --config.system-log-monitor. NPD will panic if both --system-log-monitors and --config.system-log-monitor are set.
I1104 15:42:58.096986       7 log_monitor.go:79] Finish parsing log monitor config file /config/kernel.monitor: {WatcherConfig:{Plugin:filelog PluginConfig:map[message:dummy timestamp:dummy timestampFormat:dummy] LogPath:/dev/null Lookback:10m Delay:} BufferSize:10 Source:kernel-monitor DefaultConditions:[{Type:KernelDeadlock Status: Transition:0001-01-01 00:00:00 +0000 UTC Reason:KernelHasNoDeadlock Message:kernel has no deadlock}] Rules:[] EnableMetricsReporting:0x219a95c}
I1104 15:42:58.136220       7 log_watchers.go:40] Use log watcher of plugin "filelog"
I1104 15:42:58.143285       7 k8s_exporter.go:54] Waiting for kube-apiserver to be ready (timeout 5m0s)...
E1104 15:42:58.156731       7 k8s_exporter.go:111] Can't get node object: nodes "hollow-node-w7wnp" not found
...
E1104 15:47:58.179093       7 k8s_exporter.go:111] Can't get node object: nodes "hollow-node-w7wnp" not found
W1104 15:47:58.179114       7 k8s_exporter.go:56] kube-apiserver did not become ready: timed out on waiting for kube-apiserver to return the node object: timed out waiting for the condition
I1104 15:47:58.179222       7 node_problem_detector.go:63] K8s exporter started.
I1104 15:47:58.179289       7 node_problem_detector.go:67] Prometheus exporter started.
I1104 15:47:58.179305       7 log_monitor.go:111] Start log monitor /config/kernel.monitor
I1104 15:47:58.179380       7 log_watcher.go:80] Start watching filelog
I1104 15:47:58.179393       7 problem_detector.go:76] Problem detector started
I1104 15:47:58.179438       7 log_monitor.go:235] Initialize condition generated: [{Type:KernelDeadlock Status:False Transition:2021-11-04 15:47:58.179428193 +0000 UTC m=+300.109031446 Reason:KernelHasNoDeadlock Message:kernel has no deadlock}]
E1104 15:47:59.182064       7 manager.go:162] failed to update node conditions: nodes "hollow-node-w7wnp" not found
...

gs://sig-scalability-logs/pull-kubernetes-kubemark-e2e-gce-big/1456277296036974592/e2e-106150-ac87c-minion-group-40jf/kubeproxy-hollow-node-w7wnp.log is empty. My expectation is that this should be the output of /kubemark --morph=proxy. I'll try to reproduce this locally.

@pohly
Copy link
Contributor Author

pohly commented Nov 4, 2021

The reason for the failure is that kubemark is not a static binary and doesn't run inside the new base image, probably because of missing or wrong glibc. That's a bit odd because the go-runner image is also based on gcr.io/distroless. I'm investigating...

@pohly pohly changed the title kubemark: replace deprecated --log-file parameter with runner WIP: kubemark: replace deprecated --log-file parameter with runner Nov 4, 2021
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 4, 2021
@pohly
kubemark: static binary, replace deprecated --log-file parameter
c3cd9a3 
The --log-file parameter will be deprecated as of Kubernetes 1.23 and should be
avoided. The replacement for distroless images is the image with go-runner, a
tool that handles output redirection.

For kubemark to run in that image it must be built as static binary.
@pohly
Copy link
Contributor Author

pohly commented Nov 4, 2021 •
edited

go-runner is truly "distroless" - it has no libc. What the kubemark Dockerfile uses is gcr.io/distroless/base which still has the libc. That explains the failure in this PR.

I'm wondering whether kubemark can and/or should be built as static binary. I've made that change in the revised PR.

@pohly
Copy link
Contributor Author

pohly commented Nov 4, 2021

/test pull-kubernetes-kubemark-e2e-gce-big

@pohly pohly changed the title WIP: kubemark: replace deprecated --log-file parameter with runner kubemark: replace deprecated --log-file parameter with runner Nov 4, 2021
@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note-none Denotes a PR that doesn't merit a release note. labels Nov 4, 2021
@pohly
Copy link
Contributor Author

pohly commented Nov 4, 2021

@marseel : can you take another look? After switching to a static build of kubemark it is working, pull-kubernetes-kubemark-e2e-gce-big passed.

@marseel
Copy link
Member

marseel commented Nov 5, 2021

I think it should be fine, but I would like second pair of eyes.
@mborsz
Could you take a look?

@mborsz
Copy link
Member

mborsz commented Nov 5, 2021

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 5, 2021
@marseel
Copy link
Member

marseel commented Nov 5, 2021

/assign @wojtek-t
for approval.

@wojtek-t
Copy link
Member

wojtek-t commented Nov 5, 2021

/lgtm
/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mborsz, pohly, wojtek-t

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 5, 2021
@k8s-ci-robot k8s-ci-robot merged commit aa964e0 into kubernetes:master Nov 5, 2021
@k8s-ci-robot k8s-ci-robot added this to the v1.23 milestone Nov 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mborsz mborsz Awaiting requested review from mborsz

@wojtek-t wojtek-t Awaiting requested review from wojtek-t

Assignees

@mborsz mborsz

@wojtek-t wojtek-t

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/release-eng Issues or PRs related to the Release Engineering subproject area/test cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/scalability Categorizes an issue or PR as relevant to SIG Scalability. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
v1.23
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@pohly @k8s-ci-robot @marseel @mborsz @wojtek-t