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

Automated cherry pick of #85350: Enable mutation detection #85801

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 8 additions & 0 deletions hack/make-rules/test-cmd.sh
Expand Up @@ -21,6 +21,14 @@ set -o errexit
set -o nounset
set -o pipefail

# start the cache mutation detector by default so that cache mutators will be found
KUBE_CACHE_MUTATION_DETECTOR="${KUBE_CACHE_MUTATION_DETECTOR:-true}"
export KUBE_CACHE_MUTATION_DETECTOR

# panic the server on watch decode errors since they are considered coder mistakes
KUBE_PANIC_WATCH_DECODE_ERROR="${KUBE_PANIC_WATCH_DECODE_ERROR:-true}"
export KUBE_PANIC_WATCH_DECODE_ERROR

KUBE_ROOT=$(dirname "${BASH_SOURCE[0]}")/../..
source "${KUBE_ROOT}/hack/lib/init.sh"
source "${KUBE_ROOT}/hack/lib/test.sh"
Expand Down
8 changes: 8 additions & 0 deletions hack/make-rules/test-e2e-node.sh
Expand Up @@ -17,6 +17,14 @@
KUBE_ROOT=$(dirname "${BASH_SOURCE[0]}")/../..
source "${KUBE_ROOT}/hack/lib/init.sh"

# start the cache mutation detector by default so that cache mutators will be found
KUBE_CACHE_MUTATION_DETECTOR="${KUBE_CACHE_MUTATION_DETECTOR:-true}"
export KUBE_CACHE_MUTATION_DETECTOR

# panic the server on watch decode errors since they are considered coder mistakes
KUBE_PANIC_WATCH_DECODE_ERROR="${KUBE_PANIC_WATCH_DECODE_ERROR:-true}"
export KUBE_PANIC_WATCH_DECODE_ERROR

focus=${FOCUS:-""}
skip=${SKIP-"\[Flaky\]|\[Slow\]|\[Serial\]"}
# The number of tests that can run in parallel depends on what tests
Expand Down
8 changes: 8 additions & 0 deletions hack/make-rules/test-integration.sh
Expand Up @@ -21,6 +21,14 @@ set -o pipefail
KUBE_ROOT=$(dirname "${BASH_SOURCE[0]}")/../..
source "${KUBE_ROOT}/hack/lib/init.sh"

# start the cache mutation detector by default so that cache mutators will be found
KUBE_CACHE_MUTATION_DETECTOR="${KUBE_CACHE_MUTATION_DETECTOR:-true}"
export KUBE_CACHE_MUTATION_DETECTOR

# panic the server on watch decode errors since they are considered coder mistakes
KUBE_PANIC_WATCH_DECODE_ERROR="${KUBE_PANIC_WATCH_DECODE_ERROR:-true}"
export KUBE_PANIC_WATCH_DECODE_ERROR

# Give integration tests longer to run by default.
KUBE_TIMEOUT=${KUBE_TIMEOUT:--timeout=600s}
KUBE_INTEGRATION_TEST_MAX_CONCURRENCY=${KUBE_INTEGRATION_TEST_MAX_CONCURRENCY:-"-1"}
Expand Down
20 changes: 19 additions & 1 deletion staging/src/k8s.io/client-go/tools/cache/mutation_detector.go
Expand Up @@ -48,7 +48,7 @@ func NewCacheMutationDetector(name string) MutationDetector {
return dummyMutationDetector{}
}
klog.Warningln("Mutation detector is enabled, this will result in memory leakage.")
return &defaultCacheMutationDetector{name: name, period: 1 * time.Second}
return &defaultCacheMutationDetector{name: name, period: 1 * time.Second, retainDuration: 2 * time.Minute}
}

type dummyMutationDetector struct{}
Expand All @@ -68,6 +68,10 @@ type defaultCacheMutationDetector struct {
lock sync.Mutex
cachedObjs []cacheObj

retainDuration time.Duration
lastRotated time.Time
retainedCachedObjs []cacheObj

// failureFunc is injectable for unit testing. If you don't have it, the process will panic.
// This panic is intentional, since turning on this detection indicates you want a strong
// failure signal. This failure is effectively a p0 bug and you can't trust process results
Expand All @@ -84,6 +88,14 @@ type cacheObj struct {
func (d *defaultCacheMutationDetector) Run(stopCh <-chan struct{}) {
// we DON'T want protection from panics. If we're running this code, we want to die
for {
if d.lastRotated.IsZero() {
d.lastRotated = time.Now()
} else if time.Now().Sub(d.lastRotated) > d.retainDuration {
d.retainedCachedObjs = d.cachedObjs
d.cachedObjs = nil
d.lastRotated = time.Now()
}

d.CompareObjects()

select {
Expand Down Expand Up @@ -120,6 +132,12 @@ func (d *defaultCacheMutationDetector) CompareObjects() {
altered = true
}
}
for i, obj := range d.retainedCachedObjs {
if !reflect.DeepEqual(obj.cached, obj.copied) {
fmt.Printf("CACHE %s[%d] ALTERED!\n%v\n", d.name, i, diff.ObjectGoPrintSideBySide(obj.cached, obj.copied))
altered = true
}
}

if altered {
msg := fmt.Sprintf("cache %s modified", d.name)
Expand Down