deploy count statsd key, cuts, test repairs, comment register - #759
Merged
Merged
Conversation
SendDeployCount sent the key template itself, so every core bumped one core.%s.deploy.count counter and the per-host statsd series got nothing since the resource plugin rewrite.
Applies the /code modern Go and naming rules: per-iteration loop variables make the Copy wrapper closure redundant, cmp.Or picks the RunAndWait timeout, fmt.Appendf builds the error frame, a range over an empty Files slice needs no length guard, the RunAndWait options local is lower camel, and BatchCreateAndDecr drops a named result it never uses.
…nly test TestETCD asserted the watch from a goroutine it never joined, and TestDeployOptions checked that t was non-nil through the shadowed assert. The watch now starts at the create's revision and asserts on the test goroutine. TestGrant only exercised the mock and is removed. Also: t.Context over context.Background, AddNode mocks take its two arguments, assert.ErrorIs, no ResetTimer before b.Loop, no cleanup on a per-test store, unexported etcd test helpers, bytes.Buffer instead of zap's (zap drops to indirect), and two test-name typos.
Net -53 production lines, no behavior change: - LinuxFile.Clone has had no production caller since 678d1f3 - DeployOptions.Lambda, VirtualizationCreateOptions.Lambda and DeployOptions.Debug were write-only once the docker and virt engines went (#671, #672); the pb fields stay for wire compatibility - newEngine looks the factory up once, so the second map read and its unreachable branch are gone - cobalt getNodeResourceInfo returns the plugin call directly - withoutNetSysctls uses maps.Clone and maps.DeleteFunc, additionalGids a nil slice, the cpumem share default cmp.Or, bestSplit slices.Repeat - doGetDeployStrategy returns strategy.Deploy directly - the store and cpumem key lists use utils.Map A/B on the key build (100 ids, 3 rounds, arms interleaved, order swapped): 201 allocs and 8199 B in both arms, time within noise.
check_calico.py audits the calico setup of the virt engine removed in #672. meta_transfer_as_rename2workload.py is the 2020 container to workload rename. meta_transfer_resource_plugin.py is the 2022 resource plugin migration and writes resource_args/engine_args, which core no longer reads.
str.replace removed every occurrence of the root, so a key that carried the root again further in was rewritten wrongly.
heldLock and mockLocks replace 34 copies of the lock mock block (net -99 test lines). The lock tests that pin an order, a Once or a failing Lock keep their own mocks.
Applies the /code comment register. A comment that leads with a code identifier or a tool's own name keeps that case.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Whole-repo read after v0.1.6: one bug fix, a verified cut-list, test repairs and the comment register. Eight commits, each self-contained.
Bug
The deploy count reached statsd under the key template itself.
SendDeployCountsentcore.%s.deploy.countwithout the hostname, so every core bumped one shared counter and the per-host series got nothing since #491. Prometheus was not affected (the hostname is a label there). Regression test: a UDP listener reads the packet; it readcore.%s.deploy.count:3|cbefore the fix andcore.host1.deploy.count:3|cafter.Cuts (net -53 production lines, no behavior change)
LinuxFile.CloneDeployOptions.Lambda,VirtualizationCreateOptions.Lambda,DeployOptions.DebugnewEnginesecond map read and its!okbranchgetNodeResourceInfoclosurewithoutNetSysctls,additionalGids, cpumem share default,bestSplitinitmaps.Clone+maps.DeleteFunc, a nil slice,cmp.Or,slices.RepeatdoGetDeployStrategytailstrategy.Deploydirectly; the only plan that returns a map with an error returns an empty one, and both callers discard itutils.MapThe key lists sit on the status read path, so they got an A/B (100 ids, 3 rounds, arms interleaved, order swapped): 201 allocs and 8199 B in both arms, time within noise.
Three scripts are removed:
check_calico.py(audits the virt engine's calico setup),meta_transfer_as_rename2workload.py(2020 rename) andmeta_transfer_resource_plugin.py(2022 migration; it writesresource_args/engine_args, which core no longer reads).redis_as_broker.pystays and now strips only the leading root from a key.Tests
TestETCDasserted its watch from a goroutine it never joined. The watch now starts at the create's revision and asserts on the test goroutine.TestDeployOptionschecked thattwas non-nil through the shadowedassert. It now checksGetProcessing.TestGrantonly exercised the mock and is removed.heldLockandmockLocksreplace 34 copies of the calcium lock mock block (net -99 test lines). The lock tests that pin an order, aOnceor a failingLockkeep their own mocks.t.Context()overcontext.Background(),assert.ErrorIs, two-argumentAddNodemocks, noResetTimerbeforeb.Loop, unexported etcd test helpers,bytes.Bufferinstead of zap's (zap drops to indirect).Style
Copywrapper closure redundant,cmp.Orpicks theRunAndWaittimeout,fmt.Appendf, lower camel for theRunAndWaitoptions local.Evidence
GOWORK=off:make lint0 issues on linux and darwin,make fmt-check,asl ./...on both,go test -race -count=1 ./...all pass.go test -race -count=3 ./cluster/calciumpasses with the shared lock helpers. Comment count: +0 in every commit.Release note: agent #142 needs a core release that carries #757, so core ships before agent.