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

Go 1.21 compatibility: Set verbosity after flag definition #3384

Merged
merged 1 commit into from
Oct 7, 2023

Conversation

frebib
Copy link
Contributor

@frebib frebib commented Aug 20, 2023

This seems to be related to a change in the klog library where the "v" flag isn't defined until after InitFlags() is called. Defer the call of setting the default verbosity until after we do that instead of during init(). Fixes startup:

$ GO_FLAGS=-trimpath build/build.sh && _output/cadvisor -logtostderr
>> building cadvisor
go: downloading github.com/prometheus/common v0.38.0
panic: flag v set at github.com/google/cadvisor/cmd/cadvisor.go:105 before being defined

goroutine 1 [running]:
flag.(*FlagSet).Var(0xc00017a150, {0x1874460, 0x2213bc8}, {0x1866490, 0x1}, {0x15f0596, 0x22})
        flag/flag.go:1031 +0x33a
k8s.io/klog/v2.InitFlags.func1(0xc0001ab2f0?)
        k8s.io/klog/v2@v2.80.1/klog.go:439 +0x31
flag.(*FlagSet).VisitAll(0xc00032fa50?, 0xc0005e1c90)
        flag/flag.go:458 +0x42
k8s.io/klog/v2.InitFlags(0x7fbe00b2f5b8?)
        k8s.io/klog/v2@v2.80.1/klog.go:438 +0x45
main.main()
        github.com/google/cadvisor/cmd/cadvisor.go:109 +0x36

Possibly broken due to this? kubernetes/klog@28f7906

This seems to be related to a change in the klog library where the "v"
flag isn't defined until after InitFlags() is called. Defer the call of
setting the default verbosity until after we do that instead of during
init(). Fixes startup:

    $ GO_FLAGS=-trimpath build/build.sh && _output/cadvisor -logtostderr
    >> building cadvisor
    go: downloading github.com/prometheus/common v0.38.0
    panic: flag v set at github.com/google/cadvisor/cmd/cadvisor.go:105 before being defined

    goroutine 1 [running]:
    flag.(*FlagSet).Var(0xc00017a150, {0x1874460, 0x2213bc8}, {0x1866490, 0x1}, {0x15f0596, 0x22})
            flag/flag.go:1031 +0x33a
    k8s.io/klog/v2.InitFlags.func1(0xc0001ab2f0?)
            k8s.io/klog/v2@v2.80.1/klog.go:439 +0x31
    flag.(*FlagSet).VisitAll(0xc00032fa50?, 0xc0005e1c90)
            flag/flag.go:458 +0x42
    k8s.io/klog/v2.InitFlags(0x7fbe00b2f5b8?)
            k8s.io/klog/v2@v2.80.1/klog.go:438 +0x45
    main.main()
            github.com/google/cadvisor/cmd/cadvisor.go:109 +0x36

Signed-off-by: Joe Groocock <me@frebib.net>
@k8s-ci-robot
Copy link
Collaborator

Hi @frebib. Thanks for your PR.

I'm waiting for a google member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

@iwankgb
Copy link
Collaborator

iwankgb commented Aug 20, 2023

/ok-to-test

Copy link
Collaborator

@iwankgb iwankgb left a comment

Choose a reason for hiding this comment

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

I'm puzzled:

➜  cadvisor git:(master) GO_FLAGS=-trimpath build/build.sh
>> building cadvisor
go: downloading google.golang.org/api v0.126.0
go: downloading cloud.google.com/go/compute v1.23.0
go: downloading github.com/opencontainers/runc v1.1.9
go: downloading golang.org/x/net v0.10.0
go: downloading golang.org/x/crypto v0.9.0
go: downloading golang.org/x/oauth2 v0.8.0
go: downloading google.golang.org/grpc v1.57.0
go: downloading google.golang.org/protobuf v1.31.0
go: downloading google.golang.org/genproto/googleapis/rpc v0.0.0-20230803162519-f966b187b2e5
go: downloading google.golang.org/genproto v0.0.0-20230807174057-1744710a1577
go: downloading github.com/google/s2a-go v0.1.4
go: downloading github.com/googleapis/enterprise-certificate-proxy v0.2.3
go: downloading github.com/googleapis/gax-go/v2 v2.11.0
go: downloading github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da

What version of Go are you using?

@iwankgb
Copy link
Collaborator

iwankgb commented Aug 20, 2023

make build works too:

➜  cadvisor git:(master) make build 
>> building assets
>> building binaries
>> building cadvisor

Tested on:

  • fbd519b
  • go version go1.20.7 linux/amd64

@frebib
Copy link
Contributor Author

frebib commented Aug 20, 2023

@iwankgb The issue manifests when running the built binary, not the compilation.

What version of Go are you using?

go version go1.21.0 linux/amd64
Same issue on fbd519b, 0.47.3 and 0.47.0

I think I must have a newer/different version of a library that causes this to happen. Try a go get -u or build in a container to ensure your Go module cache isn't interfering? I was a little perplexed that nobody had reported this yet, which makes me think it's possibly a very new change in some dependency that's causing this? I did even go back and try an old version (0.47.0) that I know definitely worked before and the same issue occurred. The first failure I saw with this error was today in https://drone.spritsail.io/spritsail/cadvisor/11/1/3 (hopefully you can see that)

@iwankgb
Copy link
Collaborator

iwankgb commented Aug 21, 2023

I downloaded 0.47.2 from releases page:

➜  ~ ./cadvisor-v0.47.2-linux-amd64 -v=2
I0821 11:36:54.535791   19489 cadvisor.go:124] enabled metrics: app,cpu,cpuLoad,disk,diskIO,memory,network,oom_event,percpu,perf_event
I0821 11:36:54.535808   19489 storagedriver.go:55] Caching stats in memory for 2m0s
I0821 11:36:54.552245   19489 fs.go:133] Filesystem UUIDs: map[3824-32F5:/dev/nvme0n1p1 b9b2226a-85c2-4c74-b722-f215f13a6315:/dev/dm-1 d8b584b1-e960-41cb-8a22-2da06af38fbd:/dev/nvme1n1p1 d9b22159-0016-4744-a1bf-a82f40c8c896:/dev/dm-4 ee8a66b1-b0d5-4410-ba24-8bfcb970f1e8:/dev/nvme0n1p2 f2d49b8a-db2e-475a-884b-6aea2746aa34:/dev/dm-2]
I0821 11:36:54.552260   19489 fs.go:134] Filesystem partitions: map[/dev/mapper/home-home:{mountpoint:/home major:0 minor:33 fsType:btrfs blockSize:0} /dev/mapper/system-root:{mountpoint:/var/lib/docker major:0 minor:25 fsType:btrfs blockSize:0} /dev/shm:{mountpoint:/dev/shm major:0 minor:23 fsType:tmpfs blockSize:0} /run:{mountpoint:/run major:0 minor:21 fsType:tmpfs blockSize:0} /run/user/1000:{mountpoint:/run/user/1000 major:0 minor:37 fsType:tmpfs blockSize:0}]
I0821 11:36:54.556421   19489 manager.go:210] Machine: {Timestamp:2023-08-21 11:36:54.555851791 +0200 CEST m=+0.027355965 CPUVendorID:AuthenticAMD NumCores:16 NumPhysicalCores:8 NumSockets:1 CpuFrequency:5572265 MemoryCapacity:66491809792 SwapCapacity:103079211008 MemoryByType:map[] NVMInfo:{MemoryModeCapacity:0 AppDirectModeCapacity:0 AvgPowerBudget:0} HugePages:[{PageSize:1048576 NumPages:0} {PageSize:2048 NumPages:0}] MachineID:b56e1da4cc6ea0e773bafb5c6429870e SystemUUID:b56e1da4cc6ea0e773bafb5c6429870e BootID:692aa8a4-ab15-4685-b618-5f486c62bacf Filesystems:[{Device:/run DeviceMajor:0 DeviceMinor:21 Capacity:33245904896 Type:vfs Inodes:8116676 HasInodes:true} {Device:/dev/shm DeviceMajor:0 DeviceMinor:23 Capacity:33245904896 Type:vfs Inodes:8116676 HasInodes:true} {Device:/dev/mapper/system-root DeviceMajor:0 DeviceMinor:26 Capacity:395933908992 Type:vfs Inodes:0 HasInodes:true} {Device:/dev/mapper/home-home DeviceMajor:0 DeviceMinor:33 Capacity:2000376823808 Type:vfs Inodes:0 HasInodes:true} {Device:/run/user/1000 DeviceMajor:0 DeviceMinor:37 Capacity:6649180160 Type:vfs Inodes:1623335 HasInodes:true}] DiskMap:map[252:0:{Name:dm-0 Major:252 Minor:0 Size:499016269824 Scheduler:none} 252:1:{Name:dm-1 Major:252 Minor:1 Size:103079215104 Scheduler:none} 252:2:{Name:dm-2 Major:252 Minor:2 Size:395933908992 Scheduler:none} 252:3:{Name:dm-3 Major:252 Minor:3 Size:2000381018112 Scheduler:none} 252:4:{Name:dm-4 Major:252 Minor:4 Size:2000376823808 Scheduler:none} 259:0:{Name:nvme0n1 Major:259 Minor:0 Size:500107862016 Scheduler:none} 259:1:{Name:nvme1n1 Major:259 Minor:1 Size:2000398934016 Scheduler:none} 8:0:{Name:sda Major:8 Minor:0 Size:0 Scheduler:mq-deadline} 8:16:{Name:sdb Major:8 Minor:16 Size:0 Scheduler:mq-deadline} 8:32:{Name:sdc Major:8 Minor:32 Size:0 Scheduler:mq-deadline} 8:48:{Name:sdd Major:8 Minor:48 Size:0 Scheduler:mq-deadline}] NetworkDevices:[{Name:br-852a75b59c01 MacAddress:02:42:ff:b5:18:77 Speed:-1 Mtu:1500} {Name:dummy0 MacAddress:d2:d0:67:cc:37:de Speed:0 Mtu:1500} {Name:eno1 MacAddress:c8:7f:54:05:3b:7f Speed:-1 Mtu:1500} {Name:virbr0 MacAddress:52:54:00:4c:e8:0b Speed:-1 Mtu:1500} {Name:wlp13s0 MacAddress:10:6f:d9:db:d3:9f Speed:0 Mtu:1500}] Topology:[{Id:0 Memory:66491809792 HugePages:[{PageSize:1048576 NumPages:0} {PageSize:2048 NumPages:0}] Cores:[{Id:0 Threads:[0 8] Caches:[{Id:0 Size:32768 Type:Data Level:1} {Id:0 Size:32768 Type:Instruction Level:1} {Id:0 Size:1048576 Type:Unified Level:2}] UncoreCaches:[] SocketID:0} {Id:1 Threads:[1 9] Caches:[{Id:1 Size:32768 Type:Data Level:1} {Id:1 Size:32768 Type:Instruction Level:1} {Id:1 Size:1048576 Type:Unified Level:2}] UncoreCaches:[] SocketID:0} {Id:2 Threads:[10 2] Caches:[{Id:2 Size:32768 Type:Data Level:1} {Id:2 Size:32768 Type:Instruction Level:1} {Id:2 Size:1048576 Type:Unified Level:2}] UncoreCaches:[] SocketID:0} {Id:3 Threads:[11 3] Caches:[{Id:3 Size:32768 Type:Data Level:1} {Id:3 Size:32768 Type:Instruction Level:1} {Id:3 Size:1048576 Type:Unified Level:2}] UncoreCaches:[] SocketID:0} {Id:4 Threads:[12 4] Caches:[{Id:4 Size:32768 Type:Data Level:1} {Id:4 Size:32768 Type:Instruction Level:1} {Id:4 Size:1048576 Type:Unified Level:2}] UncoreCaches:[] SocketID:0} {Id:5 Threads:[13 5] Caches:[{Id:5 Size:32768 Type:Data Level:1} {Id:5 Size:32768 Type:Instruction Level:1} {Id:5 Size:1048576 Type:Unified Level:2}] UncoreCaches:[] SocketID:0} {Id:6 Threads:[14 6] Caches:[{Id:6 Size:32768 Type:Data Level:1} {Id:6 Size:32768 Type:Instruction Level:1} {Id:6 Size:1048576 Type:Unified Level:2}] UncoreCaches:[] SocketID:0} {Id:7 Threads:[15 7] Caches:[{Id:7 Size:32768 Type:Data Level:1} {Id:7 Size:32768 Type:Instruction Level:1} {Id:7 Size:1048576 Type:Unified Level:2}] UncoreCaches:[] SocketID:0}] Caches:[{Id:0 Size:33554432 Type:Unified Level:3}] Distances:[10]}] CloudProvider:Unknown InstanceType:Unknown InstanceID:None}
I0821 11:36:54.556486   19489 manager_no_libpfm.go:29] cAdvisor is build without cgo and/or libpfm support. Perf event counters are not available.
I0821 11:36:54.556537   19489 manager.go:226] Version: {KernelVersion:6.4.11-gentoo-x86_64 ContainerOsVersion:Gentoo Linux DockerVersion: DockerAPIVersion: CadvisorVersion:v0.47.2 CadvisorRevision:6730be15}
I0821 11:36:54.562243   19489 factory.go:378] Registering Docker factory
I0821 11:36:54.562255   19489 factory.go:55] Registering systemd factory
I0821 11:36:54.562569   19489 factory.go:103] Registering Raw factory
I0821 11:36:54.562576   19489 manager.go:1186] Started watching for new ooms in manager
I0821 11:36:54.562734   19489 manager.go:299] Starting recovery of all containers
I0821 11:36:54.566759   19489 manager.go:304] Recovery completed
I0821 11:36:54.570970   19489 cadvisor.go:173] Starting cAdvisor version: v0.47.2-6730be15 on port 8080
^CI0821 11:37:04.288171   19489 manager.go:1176] Exiting thread watching subcontainers
I0821 11:37:04.288200   19489 manager.go:401] Exiting global housekeeping thread
I0821 11:37:04.288216   19489 collector_no_libpfm.go:33] cAdvisor is build without cgo and/or libpfm support. Nothing to be finalized
I0821 11:37:04.288229   19489 cadvisor.go:210] Exiting given signal: interrupt

@frebib
Copy link
Contributor Author

frebib commented Aug 22, 2023

So it seems this might be related to Go 1.21. I've seen reports that building cadvisor works with 1.20, but not 1.21 (without this patch)

@frebib frebib changed the title Set verbosity after flag definition Go 1.21 compatibility: Set verbosity after flag definition Aug 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants