TT Lab
Blog

한국어 English 日本語 中文

Get started
Blog

Blog

Pointer Address in Logs: How a Two-Line Log Fix in CDI Was Merged in 2 Days and 19 Hours

Result: Merged (2026-10-22 20:19, kubevirt/containerized-data-importer#4320). Actually retrieved from GitHub PRs, reviews, comments, timelines, test records, CDI source (the base commit of the PR 9cd4fbdb), Go standard library documentation, and the draft of the contribution ledger. All times are in KST (Korea Standard Time).

Table of Contents

One-Line Summary

PR kubevirt/containerized-data-importer#4320 Log the storage class name, not its pointer, in filesystem overhead
Changes pkg/controller/common/util.go Two log lines in . 2 lines added, 2 lines deleted
Reaction Maintainer approved after 16 hours /approve·/lgtm. When an unrelated e2e failure was explained, a maintainer overrode it
Result PR opened 10-20 00:49 → Approved 17:05 → Overrode 10-22 16:42 → Merged 20:19, approx. 2 days, 19 hours, and 30 minutes

1. What Was the Issue: Two Lines Found While Reading Code

This issue was not found in production logs. While writing the "Reading KubeVirt Source Code" serieson this blog and following CDI's size calculation logic, I jotted it down as the fifth item on the contribution ledger's "Contribution Candidates Found by Reading Code" list. This log is located in the function covered in Part 8that selects filesystem headroom values per storage class.

GetFilesystemOverheadForStorageClass checks for a default class if the storage class is not found, and falls back to global headroom if that is also absent. At these two branching points, it logs at verbosity level 3 (-v=3). The output noted in the PR description was as follows:

Storage class0x7035abda0020not found, trying default storage class

A memory address appears where the storage class name should be, and there are no spaces around it. After the fix, it prints like this:

Storage class "local-path" not found, trying default storage class

The PR description stated that this output was reproduced, but the original output of that reproduction does not remain in the contribution ledger. The two lines above are taken from the example in the PR description. I also did not verify whether CDI in the TT Lab cluster was running at verbosity level 3 (unverified).

2. Cause: Which Line in the Source

Line numbers are based on the base commit of the PR 9cd4fbdb .

storageClassName . *string This appears to be a mistake stemming from the common Kubernetes API pattern of receiving "strings that can be empty" as pointers. Two factors combine to produce the output above.

  1. klog.Info joins arguments using the fmt.Sprint method. Go's fmt.Sprint documentationstates that "spaces are added only when neither operand is a string." Between "Storage class" and the pointer, and between the pointer and "not found…" , spaces are omitted because one side is a string.
  2. %v Printing a pointer with outputs the address rather than the referenced value. Consequently, values like 0x7035abda0020 end up in the name position.

Info Replacing Infof with and providing a format string resolves the spacing issue. Pointers must be dereferenced, but since they might be nil, I used k8s.io/utils/ptr 's ptr.Deref(storageClassName, "") which was already imported in this file.

3. What Was Changed: Diff Summary

 	targetStorageClass, err := GetStorageClassByNameWithK8sFallback(ctx, client, storageClassName)
 	if err != nil || targetStorageClass == nil {
-		klog.V(3).Info("Storage class", storageClassName, "not found, trying default storage class")
+		klog.V(3).Infof("Storage class %q not found, trying default storage class", ptr.Deref(storageClassName, ""))
 ...
 	if targetStorageClass == nil {
-		klog.V(3).Info("Storage class", storageClassName, "not found, continuing with global overhead")
+		klog.V(3).Infof("Storage class %q not found, continuing with global overhead", ptr.Deref(storageClassName, ""))

%q Choosing ensures that even when the name is an empty string, it appears as "" . The other log in the same function, "no default storage class found", was left as is because it takes a single string argument and had no issues. The PR description noted that the change only modifies logs and stated what was verified. go vet ./pkg/controller/common/ was clean, and all 43 tests in go test ./pkg/controller/common/ passed (golang:1.27.2 container, -mod=vendor). No separate issue was opened; I simply noted that it was "found while reading code".

The use of AI was disclosed in the commit trailer Assisted-by: Claude , the DCO sign-off, and a paragraph at the end of the body stating, "Found and fixed with the help of Claude, reviewed, and reproduced the log output above." This commit also appears as an unsigned commit on GitHub. It seems the SSH signature required by rule 3 was missing.

4. Reaction: Approval in 16 Hours, Then a Single e2e Test

Time Day
10-20 00:49 PR opened. Prow assigned the release-note-none·size/XS label and two reviewers
10-20 17:05 Maintainer reviewed with /approve·/lgtm and a word of thanks. lgtm·approved label
10-21 22:23 We ran /retest-required , but the bot replied that "untrusted authors cannot trigger tests directly"
10-22 01:05 Comment detailing the failed test and asking for a retest
10-22 16:42 Maintainer /override pull-containerized-data-importer-e2e-ceph-wffc
10-22 20:19 tide merged

Approval was quick. Since it was just two lines with the before-and-after logs shown side-by-side, there likely wasn't much to question. This maintainer is the same person who reviewed Part 7and Part 6.

What blocked merging after approval was the e2e suite e2e-ceph-wffc . Judging by the name, it appears to be a suite running with Ceph storage and late binding (WaitForFirstConsumer). There was only a single failure, a test named DataImportCron … with snapshot sources [test_id:10033] . Out of 546 tests, 545 passed. It seemed highly unlikely that a change modifying two log strings could break a DataImportCron test involving snapshot sources.

So I first ran the retest command /retest-required , but Prow rejected it, stating it was a command from an external author. In this repository, untrusted authors cannot trigger tests with /ok-to-test or rerun them. A few hours later, I posted a comment noting the failed test name and the 545/546 count, explained in a single sentence why this change had nothing to do with that test, and asked someone to trigger a retest. Instead of retesting, the maintainer overrode the check, and it was merged four hours later.

5. What I Learned: Open Source Contribution Tips

was a dead end, but the bot's response clearly explained what external authors cannot do in this repository. It's the same rule seen in the main KubeVirt repository in Part 6. Titi's Word.

Links

All materials were accessed on 2026-10-29.

Sign in to like

Comments

No comments yet.

Sign in to leave a comment