When debugging, it helps to keep output from different connections
separate. This can be done with contextual logging and using different loggers
for each connection.
Cancellation is handled separately for requests. Therefore the new APIs only
add support for passing a logger instance.
The main purpose is to replace context.TODO with a context provided by the
caller. A secondary purpose is to enable contextual logging.
Modifying the existing interfaces and APIs would have a big impact on the
ecosystem. This is a no-go. Instead, the following approach was taken:
- All interfaces get duplicated in a *WithContext variant where the methods
also have a *WithContext suffix and the ctx parameter. All methods are
treated this way except for obvious local get methods (like RESTClient)
because it cannot be ruled out entirely that some implementation may
need a context.
- Implementations of these interfaces implement both method variants
which is possible because the method names are different.
The old methods are implemented as thin wrappers around the updated
code which is now the body of the new methods or shared helpers.
In some cases there is additional overhead (type checks, potentially
additional allocations) when using the old methods.
- To*WithContext helpers bridge from the old to the new interfaces. They
try a type cast first. Because the in-tree implementations implement
both, they can be used directly. For other implementations wrappers
are used.
- All old APIs and interfaces are marked as deprecated. There is no
intent to ever remove them, but consumers should be made aware
that there are now better alternatives. Implementations also
get marked this way even if nothing ever calls them directly
because it shows which code, at least theoretically, could get
removed.
- Existing unit tests do not get updated to the new APIs. This gives
us unit test coverage of the old and new API because the old
APIs call the new ones.
- In-tree consumers will be updated in follow-up PRs. This is likely
to be a longer process. Because of the deprecation comment,
`hack/golangci-lint.sh -n` can be used to find code which needs
to be updated.
I wasn't entirely sure whether this should return a value or a pointer to
satisfy the interface. Both works, so I benchmarked it elsewhere (REST
mapper). Mem allocs are the same (one alloc/call), but returning a value is 10%
slower when calling one method.
What I then benchmarked is whether pointer vs value receiver in the wrapper
makes a difference. Converting from value receiver (what I had before) to
pointer receiver reduced call overhead by 6%. That's because with a value
receiver, Go has to auto-generate a variant with pointer receiver and calls the
value receiver through that.
That can be seen in a debugger (call stack) and when setting breakpoints:
(dlv) b restMapperWrapper.KindForWithContext
Command failed: Location "restMapperWrapper.KindForWithContext" ambiguous: k8s.io/apimachinery/pkg/api/meta.restMapperWrapper.KindForWithContext, k8s.io/apimachinery/pkg/api/meta.(*restMapperWrapper).KindForWithContext…
Conventional wisdom is to define types with value receiver because those can be
called also on unmutable instances, making them more flexible.
But for types which will only ever be used via a pointer, I think pointer
receiver is better for the reasons above (small performance difference, easier
to debug).
Fixes issue #134023 where alpha API warnings were being logged
when binary version (1.34.1) and emulation version (1.34) differed
only in patch version.
The issue was in api_enablement.go where the version comparison
was using EqualTo() which compares all version components including
patch versions. The fix changes the comparison to only check
major.minor versions using version.MajorMinor().
Changes:
- Modified version comparison logic in ApplyTo() method to only
compare major.minor versions, not patch versions
- Added comprehensive test cases to verify the fix works correctly
- Tests confirm that warnings are still logged for different
major/minor versions but not for different patch versions
This prevents spurious warnings when emulation version is set to
major.minor (e.g., 1.34) and binary version includes patch (e.g., 1.34.1).
Update the procMount test expectations to match the intentional PSA
policy relaxation introduced in commit e8bd3f629d.
As of Kubernetes 1.35+, Pod Security Admission Baseline policy
allows UnmaskedProcMount for pods with user namespaces (hostUsers:
false). This was an intentional change to support nested container
use cases while maintaining security through user namespace isolation.
The test "will fail to unmask proc mounts if not privileged" was
written before this relaxation and expected Baseline level to reject
UnmaskedProcMount. Since Baseline now allows it (for user namespace
pods), the test needs to use Restricted level instead, which
unconditionally blocks UnmaskedProcMount regardless of user namespace
settings.
Changes:
- Change PSA level from Baseline to Restricted
- Update test name to clarify it's testing Restricted level behavior
- Update framework name from "proc-mount-baseline-test" to
"proc-mount-restricted-test"
Fixes the ci-crio-userns-e2e-serial test failure that started occurring
when runtimes began reporting user namespace support.
Signed-off-by: Sascha Grunert <sgrunert@redhat.com>
* First version of batching w/out signatures.
* First version of pod signatures.
* Integrate batching with signatures.
* Fix merge conflicts.
* Fixes from self-review.
* Test fixes.
* Fix a bug that limited batches to size 2
Also add some new high-level logging and
simplify the pod affinity signature.
* Re-enable batching on perf tests for now.
* fwk.NewStatus(fwk.Success)
* Review feedback.
* Review feedback.
* Comment fix.
* Two plugin specific unit tests.:
* Add cycle state to the sign call, apply to topo spread.
Also add unit tests for several plugi signature
calls.
* Review feedback.
* Switch to distinct stats for hint and store calls.
* Switch signature from string to []byte
* Revert cyclestate in signs. Update node affinity.
Node affinity now sorts all of the various
nested arrays in the structure. CycleState no
longer in signature; revert to signing fewer
cases for pod spread.
* hack/update-vendor.sh
* Disable signatures when extenders are configured.
* Update pkg/scheduler/framework/runtime/batch.go
Co-authored-by: Maciej Skoczeń <87243939+macsko@users.noreply.github.com>
* Update staging/src/k8s.io/kube-scheduler/framework/interface.go
Co-authored-by: Maciej Skoczeń <87243939+macsko@users.noreply.github.com>
* Review feedback.
* Disable node resource signatures when extended DRA enabled.
* Review feedback.
* Update pkg/scheduler/framework/plugins/imagelocality/image_locality.go
Co-authored-by: Maciej Skoczeń <87243939+macsko@users.noreply.github.com>
* Update pkg/scheduler/framework/interface.go
Co-authored-by: Maciej Skoczeń <87243939+macsko@users.noreply.github.com>
* Update pkg/scheduler/framework/plugins/nodedeclaredfeatures/nodedeclaredfeatures.go
Co-authored-by: Maciej Skoczeń <87243939+macsko@users.noreply.github.com>
* Update pkg/scheduler/framework/runtime/batch.go
Co-authored-by: Maciej Skoczeń <87243939+macsko@users.noreply.github.com>
* Review feedback.
* Fixes for review suggestions.
* Add integration tests.
* Linter fixes, test fix.
* Whitespace fix.
* Remove broken test.
* Unschedulable test.
* Remove go.mod changes.
---------
Co-authored-by: Maciej Skoczeń <87243939+macsko@users.noreply.github.com>
Remove the e2e test since we switched to beta (enabled by default)
instead of GA. We re-add the test in 1.36.
Signed-off-by: Sascha Grunert <sgrunert@redhat.com>
fix [sig-node] Container Runtime blackbox test when running a container with a new image [Serial] should be able to pull from private registry with secret [NodeConformance]
