From b0c7207279712e799cd3a2a90b374e7f65eb5544 Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Tue, 27 Jan 2026 17:21:02 +0100 Subject: [PATCH] Revert "apimachinery: contextual logging in network util code" Kubernetes-commit: 9d65b9be20e5ee0a4ef34f0ba071d35987da4ab0 --- discovery/cached/disk/cached_discovery.go | 139 +---- .../cached/disk/cached_discovery_test.go | 8 +- discovery/cached/disk/round_tripper.go | 2 +- discovery/cached/memory/memcache.go | 162 +----- discovery/cached/memory/memcache_test.go | 81 +-- discovery/discovery_client.go | 502 +----------------- discovery/fake/discovery.go | 74 +-- features/envvar.go | 15 +- features/features.go | 6 +- openapi/cached/client.go | 42 +- openapi/cached/groupversion.go | 17 +- openapi/client.go | 54 +- openapi/groupversion.go | 44 +- plugin/pkg/client/auth/azure/azure_stub.go | 1 - plugin/pkg/client/auth/exec/exec.go | 3 +- plugin/pkg/client/auth/exec/metrics.go | 6 +- plugin/pkg/client/auth/gcp/gcp_stub.go | 1 - plugin/pkg/client/auth/oidc/oidc.go | 6 +- restmapper/category_expansion.go | 100 +--- restmapper/discovery.go | 220 ++------ restmapper/shortcut.go | 99 +--- restmapper/shortcut_test.go | 9 +- tools/cache/listwatch.go | 6 +- tools/clientcmd/client_config.go | 2 - tools/clientcmd/config.go | 1 - tools/clientcmd/loader.go | 2 - tools/clientcmd/loader_test.go | 3 - tools/clientcmd/merged_client_builder.go | 2 - tools/portforward/fallback_dialer.go | 1 - tools/portforward/fallback_dialer_test.go | 2 +- tools/portforward/portforward.go | 52 +- tools/portforward/portforward_test.go | 1 - tools/portforward/tunneling_connection.go | 42 +- .../portforward/tunneling_connection_test.go | 9 +- tools/portforward/tunneling_dialer.go | 23 +- tools/remotecommand/errorstream.go | 5 +- tools/remotecommand/fallback.go | 2 +- tools/remotecommand/remotecommand.go | 3 +- tools/remotecommand/spdy.go | 5 +- tools/remotecommand/spdy_test.go | 4 +- tools/remotecommand/v1.go | 11 +- tools/remotecommand/v2.go | 35 +- tools/remotecommand/v2_test.go | 4 +- tools/remotecommand/v3.go | 21 +- tools/remotecommand/v4.go | 17 +- tools/remotecommand/v5.go | 6 +- tools/remotecommand/websocket.go | 55 +- tools/remotecommand/websocket_test.go | 11 +- 48 files changed, 302 insertions(+), 1614 deletions(-) diff --git a/discovery/cached/disk/cached_discovery.go b/discovery/cached/disk/cached_discovery.go index 53f3f72c1..158810ee5 100644 --- a/discovery/cached/disk/cached_discovery.go +++ b/discovery/cached/disk/cached_discovery.go @@ -17,7 +17,6 @@ limitations under the License. package disk import ( - "context" "errors" "io" "net/http" @@ -43,7 +42,7 @@ import ( // CachedDiscoveryClient implements the functions that discovery server-supported API groups, // versions and resources. type CachedDiscoveryClient struct { - delegate discovery.DiscoveryInterfaceWithContext + delegate discovery.DiscoveryInterface // cacheDirectory is the directory where discovery docs are held. It must be unique per host:port combination to work well. cacheDirectory string @@ -62,97 +61,72 @@ type CachedDiscoveryClient struct { fresh bool // caching openapi v3 client which wraps the delegate's client - openapiClient *cachedopenapi.Client + openapiClient openapi.Client } -var ( - _ discovery.CachedDiscoveryInterface = &CachedDiscoveryClient{} - _ discovery.CachedDiscoveryInterfaceWithContext = &CachedDiscoveryClient{} -) +var _ discovery.CachedDiscoveryInterface = &CachedDiscoveryClient{} // ServerResourcesForGroupVersion returns the supported resources for a group and version. -// -// Deprecated: use ServerResourcesForGroupVersionWithContext instead. func (d *CachedDiscoveryClient) ServerResourcesForGroupVersion(groupVersion string) (*metav1.APIResourceList, error) { - return d.ServerResourcesForGroupVersionWithContext(context.Background(), groupVersion) -} - -// ServerResourcesForGroupVersionWithContext returns the supported resources for a group and version. -func (d *CachedDiscoveryClient) ServerResourcesForGroupVersionWithContext(ctx context.Context, groupVersion string) (*metav1.APIResourceList, error) { filename := filepath.Join(d.cacheDirectory, groupVersion, "serverresources.json") cachedBytes, err := d.getCachedFile(filename) // don't fail on errors, we either don't have a file or won't be able to run the cached check. Either way we can fallback. if err == nil { cachedResources := &metav1.APIResourceList{} if err := runtime.DecodeInto(scheme.Codecs.UniversalDecoder(), cachedBytes, cachedResources); err == nil { - klog.FromContext(ctx).V(10).Info("Returning cached discovery info", "fileName", filename) + klog.V(10).Infof("returning cached discovery info from %v", filename) return cachedResources, nil } } - liveResources, err := d.delegate.ServerResourcesForGroupVersionWithContext(ctx, groupVersion) + liveResources, err := d.delegate.ServerResourcesForGroupVersion(groupVersion) if err != nil { - klog.FromContext(ctx).V(3).Info("Skipped caching discovery info due to error", "err", err) + klog.V(3).Infof("skipped caching discovery info due to %v", err) return liveResources, err } if liveResources == nil || len(liveResources.APIResources) == 0 { - klog.FromContext(ctx).V(3).Info("skipped caching discovery info, no resources found") + klog.V(3).Infof("skipped caching discovery info, no resources found") return liveResources, err } if err := d.writeCachedFile(filename, liveResources); err != nil { - klog.FromContext(ctx).V(1).Info("Failed to write cache", "fileName", filename, "err", err) + klog.V(1).Infof("failed to write cache to %v due to %v", filename, err) } return liveResources, nil } // ServerGroupsAndResources returns the supported groups and resources for all groups and versions. -// -// Deprecated: use ServerGroupsAndResourcesWithContext instead. func (d *CachedDiscoveryClient) ServerGroupsAndResources() ([]*metav1.APIGroup, []*metav1.APIResourceList, error) { - return d.ServerGroupsAndResourcesWithContext(context.Background()) -} - -// ServerGroupsAndResourcesWithContext returns the supported groups and resources for all groups and versions. -func (d *CachedDiscoveryClient) ServerGroupsAndResourcesWithContext(ctx context.Context) ([]*metav1.APIGroup, []*metav1.APIResourceList, error) { - return discovery.ServerGroupsAndResourcesWithContext(ctx, d) + return discovery.ServerGroupsAndResources(d) } // ServerGroups returns the supported groups, with information like supported versions and the // preferred version. -// -// Deprecated: use ServerGroupsWithContext instead. func (d *CachedDiscoveryClient) ServerGroups() (*metav1.APIGroupList, error) { - return d.ServerGroupsWithContext(context.Background()) -} - -// ServerGroupsWithContext returns the supported groups, with information like supported versions and the -// preferred version. -func (d *CachedDiscoveryClient) ServerGroupsWithContext(ctx context.Context) (*metav1.APIGroupList, error) { filename := filepath.Join(d.cacheDirectory, "servergroups.json") cachedBytes, err := d.getCachedFile(filename) // don't fail on errors, we either don't have a file or won't be able to run the cached check. Either way we can fallback. if err == nil { cachedGroups := &metav1.APIGroupList{} if err := runtime.DecodeInto(scheme.Codecs.UniversalDecoder(), cachedBytes, cachedGroups); err == nil { - klog.FromContext(ctx).V(10).Info("Returning cached discovery info", "fileName", filename) + klog.V(10).Infof("returning cached discovery info from %v", filename) return cachedGroups, nil } } - liveGroups, err := d.delegate.ServerGroupsWithContext(ctx) + liveGroups, err := d.delegate.ServerGroups() if err != nil { - klog.FromContext(ctx).V(3).Info("Skipped caching discovery info due to error", "err", err) + klog.V(3).Infof("skipped caching discovery info due to %v", err) return liveGroups, err } if liveGroups == nil || len(liveGroups.Groups) == 0 { - klog.FromContext(ctx).V(3).Info("Skipped caching discovery info, no groups found") + klog.V(3).Infof("skipped caching discovery info, no groups found") return liveGroups, err } if err := d.writeCachedFile(filename, liveGroups); err != nil { - klog.FromContext(ctx).V(1).Info("Failed to write cache", "fileName", filename, "err", err) + klog.V(1).Infof("failed to write cache to %v due to %v", filename, err) } return liveGroups, nil @@ -245,67 +219,28 @@ func (d *CachedDiscoveryClient) RESTClient() restclient.Interface { // ServerPreferredResources returns the supported resources with the version preferred by the // server. -// -// Deprecated: use ServerPreferredResourcesWithContext instead. func (d *CachedDiscoveryClient) ServerPreferredResources() ([]*metav1.APIResourceList, error) { - return d.ServerPreferredResourcesWithContext(context.Background()) -} - -// ServerPreferredResourcesWithContext returns the supported resources with the version preferred by the -// server. -func (d *CachedDiscoveryClient) ServerPreferredResourcesWithContext(ctx context.Context) ([]*metav1.APIResourceList, error) { - return discovery.ServerPreferredResourcesWithContext(ctx, d) + return discovery.ServerPreferredResources(d) } // ServerPreferredNamespacedResources returns the supported namespaced resources with the // version preferred by the server. -// -// Deprecated: use ServerPreferredNamespacedResourcesWithContext instead. func (d *CachedDiscoveryClient) ServerPreferredNamespacedResources() ([]*metav1.APIResourceList, error) { - return d.ServerPreferredNamespacedResourcesWithContext(context.Background()) -} - -// ServerPreferredNamespacedResourcesWithContext returns the supported namespaced resources with the -// version preferred by the server. -func (d *CachedDiscoveryClient) ServerPreferredNamespacedResourcesWithContext(ctx context.Context) ([]*metav1.APIResourceList, error) { - return discovery.ServerPreferredNamespacedResourcesWithContext(ctx, d) + return discovery.ServerPreferredNamespacedResources(d) } // ServerVersion retrieves and parses the server's version (git version). -// -// Deprecated: use ServerVersionWithContext instead. func (d *CachedDiscoveryClient) ServerVersion() (*version.Info, error) { - return d.ServerVersionWithContext(context.Background()) -} - -// ServerVersionWithContext retrieves and parses the server's version (git version). -func (d *CachedDiscoveryClient) ServerVersionWithContext(ctx context.Context) (*version.Info, error) { - return d.delegate.ServerVersionWithContext(ctx) + return d.delegate.ServerVersion() } // OpenAPISchema retrieves and parses the swagger API schema the server supports. -// -// Deprecated: use OpenAPISchemaWithContext instead. func (d *CachedDiscoveryClient) OpenAPISchema() (*openapi_v2.Document, error) { - return d.OpenAPISchemaWithContext(context.Background()) -} - -// OpenAPISchemaWithContext retrieves and parses the swagger API schema the server supports. -func (d *CachedDiscoveryClient) OpenAPISchemaWithContext(ctx context.Context) (*openapi_v2.Document, error) { - return d.delegate.OpenAPISchemaWithContext(ctx) + return d.delegate.OpenAPISchema() } // OpenAPIV3 retrieves and parses the OpenAPIV3 specs exposed by the server func (d *CachedDiscoveryClient) OpenAPIV3() openapi.Client { - return d.openAPIV3(context.Background()) -} - -// OpenAPIV3WithContext retrieves and parses the OpenAPIV3 specs exposed by the server -func (d *CachedDiscoveryClient) OpenAPIV3WithContext(ctx context.Context) openapi.ClientWithContext { - return d.openAPIV3(ctx) -} - -func (d *CachedDiscoveryClient) openAPIV3(ctx context.Context) *cachedopenapi.Client { // Must take lock since Invalidate call may modify openapiClient d.mutex.Lock() defer d.mutex.Unlock() @@ -313,7 +248,7 @@ func (d *CachedDiscoveryClient) openAPIV3(ctx context.Context) *cachedopenapi.Cl if d.openapiClient == nil { // Delegate is discovery client created with special HTTP client which // respects E-Tag cache responses to serve cache from disk. - d.openapiClient = cachedopenapi.NewClientWithContext(d.delegate.OpenAPIV3WithContext(ctx)) + d.openapiClient = cachedopenapi.NewClient(d.delegate.OpenAPIV3()) } return d.openapiClient @@ -321,15 +256,7 @@ func (d *CachedDiscoveryClient) openAPIV3(ctx context.Context) *cachedopenapi.Cl // Fresh is supposed to tell the caller whether or not to retry if the cache // fails to find something (false = retry, true = no need to retry). -// -// Deprecated: use FreshWithContext instead. func (d *CachedDiscoveryClient) Fresh() bool { - return d.FreshWithContext(context.Background()) -} - -// FreshWithContext is supposed to tell the caller whether or not to retry if the cache -// fails to find something (false = retry, true = no need to retry). -func (d *CachedDiscoveryClient) FreshWithContext(ctx context.Context) bool { d.mutex.Lock() defer d.mutex.Unlock() @@ -337,14 +264,7 @@ func (d *CachedDiscoveryClient) FreshWithContext(ctx context.Context) bool { } // Invalidate enforces that no cached data is used in the future that is older than the current time. -// -// Deprecated: use InvalidateWithContext instead. func (d *CachedDiscoveryClient) Invalidate() { - d.InvalidateWithContext(context.Background()) -} - -// InvalidateWithContext enforces that no cached data is used in the future that is older than the current time. -func (d *CachedDiscoveryClient) InvalidateWithContext(ctx context.Context) { d.mutex.Lock() defer d.mutex.Unlock() @@ -352,15 +272,8 @@ func (d *CachedDiscoveryClient) InvalidateWithContext(ctx context.Context) { d.fresh = true d.invalidated = true d.openapiClient = nil - ad, ok := d.delegate.(discovery.CachedDiscoveryInterfaceWithContext) - if !ok { - if ad2, ok2 := d.delegate.(discovery.CachedDiscoveryInterface); ok2 { - ad = discovery.ToCachedDiscoveryInterfaceWithContext(ad2) - ok = true - } - } - if ok { - ad.InvalidateWithContext(ctx) + if ad, ok := d.delegate.(discovery.CachedDiscoveryInterface); ok { + ad.Invalidate() } } @@ -370,12 +283,6 @@ func (d *CachedDiscoveryClient) WithLegacy() discovery.DiscoveryInterface { return d } -// WithLegacyWithContext returns current cached discovery client; -// current client does not support legacy-only discovery. -func (d *CachedDiscoveryClient) WithLegacyWithContext(ctx context.Context) discovery.DiscoveryInterfaceWithContext { - return d -} - // NewCachedDiscoveryClientForConfig creates a new DiscoveryClient for the given config, and wraps // the created client in a CachedDiscoveryClient. The provided configuration is updated with a // custom transport that understands cache responses. @@ -403,11 +310,11 @@ func NewCachedDiscoveryClientForConfig(config *restclient.Config, discoveryCache // The delegate caches the discovery groups and resources (memcache). "ServerGroups", // which usually only returns (and caches) the groups, can now store the resources as // well if the server supports the newer aggregated discovery format. - return newCachedDiscoveryClient(memory.NewMemCacheClientWithContext(discoveryClient), discoveryCacheDir, ttl), nil + return newCachedDiscoveryClient(memory.NewMemCacheClient(discoveryClient), discoveryCacheDir, ttl), nil } // NewCachedDiscoveryClient creates a new DiscoveryClient. cacheDirectory is the directory where discovery docs are held. It must be unique per host:port combination to work well. -func newCachedDiscoveryClient(delegate discovery.DiscoveryInterfaceWithContext, cacheDirectory string, ttl time.Duration) *CachedDiscoveryClient { +func newCachedDiscoveryClient(delegate discovery.DiscoveryInterface, cacheDirectory string, ttl time.Duration) *CachedDiscoveryClient { return &CachedDiscoveryClient{ delegate: delegate, cacheDirectory: cacheDirectory, diff --git a/discovery/cached/disk/cached_discovery_test.go b/discovery/cached/disk/cached_discovery_test.go index e3b78b909..94cbfc8b7 100644 --- a/discovery/cached/disk/cached_discovery_test.go +++ b/discovery/cached/disk/cached_discovery_test.go @@ -52,7 +52,7 @@ func TestCachedDiscoveryClient_Fresh(t *testing.T) { defer os.RemoveAll(d) c := fakeDiscoveryClient{} - cdc := newCachedDiscoveryClient(discovery.ToDiscoveryInterfaceWithContext(&c), d, 60*time.Second) + cdc := newCachedDiscoveryClient(&c, d, 60*time.Second) assert.True(cdc.Fresh(), "should be fresh after creation") cdc.ServerGroups() @@ -71,7 +71,7 @@ func TestCachedDiscoveryClient_Fresh(t *testing.T) { assert.True(cdc.Fresh(), "should be fresh after another resources call") assert.Equal(1, c.resourceCalls) - cdc = newCachedDiscoveryClient(discovery.ToDiscoveryInterfaceWithContext(&c), d, 60*time.Second) + cdc = newCachedDiscoveryClient(&c, d, 60*time.Second) cdc.ServerGroups() assert.False(cdc.Fresh(), "should NOT be fresh after recreation with existing groups cache") assert.Equal(1, c.groupCalls) @@ -96,7 +96,7 @@ func TestNewCachedDiscoveryClient_TTL(t *testing.T) { defer os.RemoveAll(d) c := fakeDiscoveryClient{} - cdc := newCachedDiscoveryClient(discovery.ToDiscoveryInterfaceWithContext(&c), d, 1*time.Nanosecond) + cdc := newCachedDiscoveryClient(&c, d, 1*time.Nanosecond) cdc.ServerGroups() assert.Equal(1, c.groupCalls) @@ -115,7 +115,7 @@ func TestNewCachedDiscoveryClient_PathPerm(t *testing.T) { defer os.RemoveAll(d) c := fakeDiscoveryClient{} - cdc := newCachedDiscoveryClient(discovery.ToDiscoveryInterfaceWithContext(&c), d, 1*time.Nanosecond) + cdc := newCachedDiscoveryClient(&c, d, 1*time.Nanosecond) cdc.ServerGroups() err = filepath.Walk(d, func(path string, info os.FileInfo, err error) error { diff --git a/discovery/cached/disk/round_tripper.go b/discovery/cached/disk/round_tripper.go index db26773b7..b978a2bed 100644 --- a/discovery/cached/disk/round_tripper.go +++ b/discovery/cached/disk/round_tripper.go @@ -60,7 +60,7 @@ func (rt *cacheRoundTripper) CancelRequest(req *http.Request) { if cr, ok := rt.rt.Transport.(canceler); ok { cr.CancelRequest(req) } else { - klog.FromContext(req.Context()).Error(nil, "CancelRequest not implemented by round-tripper", "type", fmt.Sprintf("%T", rt.rt.Transport)) + klog.Errorf("CancelRequest not implemented by %T", rt.rt.Transport) } } diff --git a/discovery/cached/memory/memcache.go b/discovery/cached/memory/memcache.go index 6f5da2c66..3829b3cc0 100644 --- a/discovery/cached/memory/memcache.go +++ b/discovery/cached/memory/memcache.go @@ -17,7 +17,6 @@ limitations under the License. package memory import ( - "context" "errors" "fmt" "sync" @@ -48,13 +47,13 @@ type cacheEntry struct { // TODO: Switch to a watch interface. Right now it will poll after each // Invalidate() call. type memCacheClient struct { - delegate discovery.DiscoveryInterfaceWithContext + delegate discovery.DiscoveryInterface lock sync.RWMutex groupToServerResources map[string]*cacheEntry groupList *metav1.APIGroupList cacheValid bool - openapiClient *cachedopenapi.Client + openapiClient openapi.Client receivedAggregatedDiscovery bool } @@ -73,7 +72,6 @@ func (e *emptyResponseError) Error() string { } var _ discovery.CachedDiscoveryInterface = &memCacheClient{} -var _ discovery.CachedDiscoveryInterfaceWithContext = &memCacheClient{} // isTransientConnectionError checks whether given error is "Connection refused" or // "Connection reset" error which usually means that apiserver is temporarily @@ -100,15 +98,10 @@ func isTransientError(err error) bool { // ServerResourcesForGroupVersion returns the supported resources for a group and version. func (d *memCacheClient) ServerResourcesForGroupVersion(groupVersion string) (*metav1.APIResourceList, error) { - return d.ServerResourcesForGroupVersionWithContext(context.Background(), groupVersion) -} - -// ServerResourcesForGroupVersion returns the supported resources for a group and version. -func (d *memCacheClient) ServerResourcesForGroupVersionWithContext(ctx context.Context, groupVersion string) (*metav1.APIResourceList, error) { d.lock.Lock() defer d.lock.Unlock() if !d.cacheValid { - if err := d.refreshLocked(ctx); err != nil { + if err := d.refreshLocked(); err != nil { return nil, err } } @@ -118,14 +111,14 @@ func (d *memCacheClient) ServerResourcesForGroupVersionWithContext(ctx context.C } if cachedVal.err != nil && isTransientError(cachedVal.err) { - r, err := d.serverResourcesForGroupVersion(ctx, groupVersion) + r, err := d.serverResourcesForGroupVersion(groupVersion) if err != nil { // Don't log "empty response" as an error; it is a common response for metrics. if _, emptyErr := err.(*emptyResponseError); emptyErr { // Log at same verbosity as disk cache. - klog.FromContext(ctx).V(3).Info(err.Error()) + klog.V(3).Infof("%v", err) } else { - utilruntime.HandleErrorWithContext(ctx, err, "Couldn't get resource list", "gv", groupVersion) + utilruntime.HandleError(fmt.Errorf("couldn't get resource list for %v: %v", groupVersion, err)) } } cachedVal = &cacheEntry{r, err} @@ -136,35 +129,19 @@ func (d *memCacheClient) ServerResourcesForGroupVersionWithContext(ctx context.C } // ServerGroupsAndResources returns the groups and supported resources for all groups and versions. -// -// Deprecated: use ServerGroupsAndResourcesWithContext instead. func (d *memCacheClient) ServerGroupsAndResources() ([]*metav1.APIGroup, []*metav1.APIResourceList, error) { - return d.ServerGroupsAndResourcesWithContext(context.Background()) -} - -// ServerGroupsAndResources returns the groups and supported resources for all groups and versions. -func (d *memCacheClient) ServerGroupsAndResourcesWithContext(ctx context.Context) ([]*metav1.APIGroup, []*metav1.APIResourceList, error) { - return discovery.ServerGroupsAndResourcesWithContext(ctx, d) + return discovery.ServerGroupsAndResources(d) } // GroupsAndMaybeResources returns the list of APIGroups, and possibly the map of group/version // to resources. The returned groups will never be nil, but the resources map can be nil // if there are no cached resources. -// -// Deprecated: use GroupsAndMaybeResourcesWithContext instead. func (d *memCacheClient) GroupsAndMaybeResources() (*metav1.APIGroupList, map[schema.GroupVersion]*metav1.APIResourceList, map[schema.GroupVersion]error, error) { - return d.GroupsAndMaybeResourcesWithContext(context.Background()) -} - -// GroupsAndMaybeResourcesWithContext returns the list of APIGroups, and possibly the map of group/version -// to resources. The returned groups will never be nil, but the resources map can be nil -// if there are no cached resources. -func (d *memCacheClient) GroupsAndMaybeResourcesWithContext(ctx context.Context) (*metav1.APIGroupList, map[schema.GroupVersion]*metav1.APIResourceList, map[schema.GroupVersion]error, error) { d.lock.Lock() defer d.lock.Unlock() if !d.cacheValid { - if err := d.refreshLocked(ctx); err != nil { + if err := d.refreshLocked(); err != nil { return nil, nil, nil, err } } @@ -189,13 +166,8 @@ func (d *memCacheClient) GroupsAndMaybeResourcesWithContext(ctx context.Context) return d.groupList, resourcesMap, failedGVs, nil } -// Deprecated: use ServerGroupsWithContext instead. func (d *memCacheClient) ServerGroups() (*metav1.APIGroupList, error) { - return d.ServerGroupsWithContext(context.Background()) -} - -func (d *memCacheClient) ServerGroupsWithContext(ctx context.Context) (*metav1.APIGroupList, error) { - groups, _, _, err := d.GroupsAndMaybeResourcesWithContext(ctx) + groups, _, _, err := d.GroupsAndMaybeResources() if err != nil { return nil, err } @@ -206,69 +178,35 @@ func (d *memCacheClient) RESTClient() restclient.Interface { return d.delegate.RESTClient() } -// Deprecated: use ServerPreferredResourcesWithContext instead. func (d *memCacheClient) ServerPreferredResources() ([]*metav1.APIResourceList, error) { - return d.ServerPreferredResourcesWithContext(context.Background()) + return discovery.ServerPreferredResources(d) } -func (d *memCacheClient) ServerPreferredResourcesWithContext(ctx context.Context) ([]*metav1.APIResourceList, error) { - return discovery.ServerPreferredResourcesWithContext(ctx, d) -} - -// Deprecated: use ServerPreferredNamespacedResourcesWithContext instead. func (d *memCacheClient) ServerPreferredNamespacedResources() ([]*metav1.APIResourceList, error) { - return d.ServerPreferredNamespacedResourcesWithContext(context.Background()) + return discovery.ServerPreferredNamespacedResources(d) } -func (d *memCacheClient) ServerPreferredNamespacedResourcesWithContext(ctx context.Context) ([]*metav1.APIResourceList, error) { - return discovery.ServerPreferredNamespacedResourcesWithContext(ctx, d) -} - -// Deprecated: use ServerVersionWithContext instead. func (d *memCacheClient) ServerVersion() (*version.Info, error) { - return d.ServerVersionWithContext(context.Background()) + return d.delegate.ServerVersion() } -func (d *memCacheClient) ServerVersionWithContext(ctx context.Context) (*version.Info, error) { - return d.delegate.ServerVersionWithContext(ctx) -} - -// Deprecated: use OpenAPISchemaWithContext instead. func (d *memCacheClient) OpenAPISchema() (*openapi_v2.Document, error) { - return d.OpenAPISchemaWithContext(context.Background()) + return d.delegate.OpenAPISchema() } -func (d *memCacheClient) OpenAPISchemaWithContext(ctx context.Context) (*openapi_v2.Document, error) { - return d.delegate.OpenAPISchemaWithContext(ctx) -} - -// Deprecated: use OpenAPIV3WithContext instead. func (d *memCacheClient) OpenAPIV3() openapi.Client { - return d.openAPIV3(context.Background()) -} - -func (d *memCacheClient) OpenAPIV3WithContext(ctx context.Context) openapi.ClientWithContext { - return d.openAPIV3(ctx) -} - -func (d *memCacheClient) openAPIV3(ctx context.Context) *cachedopenapi.Client { // Must take lock since Invalidate call may modify openapiClient d.lock.Lock() defer d.lock.Unlock() if d.openapiClient == nil { - d.openapiClient = cachedopenapi.NewClientWithContext(d.delegate.OpenAPIV3WithContext(ctx)) + d.openapiClient = cachedopenapi.NewClient(d.delegate.OpenAPIV3()) } return d.openapiClient } -// Deprecated: use FreshWithContext instead. func (d *memCacheClient) Fresh() bool { - return d.FreshWithContext(context.Background()) -} - -func (d *memCacheClient) FreshWithContext(ctx context.Context) bool { d.lock.RLock() defer d.lock.RUnlock() // Return whether the cache is populated at all. It is still possible that @@ -279,15 +217,7 @@ func (d *memCacheClient) FreshWithContext(ctx context.Context) bool { // Invalidate enforces that no cached data that is older than the current time // is used. -// -// Deprecated: use InvalidateWithContext instead. func (d *memCacheClient) Invalidate() { - d.InvalidateWithContext(context.Background()) -} - -// InvalidateWithContext enforces that no cached data that is older than the current time -// is used. -func (d *memCacheClient) InvalidateWithContext(ctx context.Context) { d.lock.Lock() defer d.lock.Unlock() d.cacheValid = false @@ -295,39 +225,24 @@ func (d *memCacheClient) InvalidateWithContext(ctx context.Context) { d.groupList = nil d.openapiClient = nil d.receivedAggregatedDiscovery = false - ad, ok := d.delegate.(discovery.CachedDiscoveryInterfaceWithContext) - if !ok { - ad2, ok2 := d.delegate.(discovery.CachedDiscoveryInterface) - if ok2 { - ad = discovery.ToCachedDiscoveryInterfaceWithContext(ad2) - ok = true - } - } - if ok { - ad.InvalidateWithContext(ctx) + if ad, ok := d.delegate.(discovery.CachedDiscoveryInterface); ok { + ad.Invalidate() } } // refreshLocked refreshes the state of cache. The caller must hold d.lock for // writing. -func (d *memCacheClient) refreshLocked(ctx context.Context) error { +func (d *memCacheClient) refreshLocked() error { // TODO: Could this multiplicative set of calls be replaced by a single call // to ServerResources? If it's possible for more than one resulting // APIResourceList to have the same GroupVersion, the lists would need merged. var gl *metav1.APIGroupList var err error - ad, ok := d.delegate.(discovery.AggregatedDiscoveryInterfaceWithContext) - if !ok { - if ad2, ok2 := d.delegate.(discovery.AggregatedDiscoveryInterface); ok2 { - ad = discovery.ToAggregatedDiscoveryInterfaceWithContext(ad2) - ok = true - } - } - if ok { + if ad, ok := d.delegate.(discovery.AggregatedDiscoveryInterface); ok { var resources map[schema.GroupVersion]*metav1.APIResourceList var failedGVs map[schema.GroupVersion]error - gl, resources, failedGVs, err = ad.GroupsAndMaybeResourcesWithContext(ctx) + gl, resources, failedGVs, err = ad.GroupsAndMaybeResources() if resources != nil && err == nil { // Cache the resources. d.groupToServerResources = map[string]*cacheEntry{} @@ -344,10 +259,10 @@ func (d *memCacheClient) refreshLocked(ctx context.Context) error { return nil } } else { - gl, err = d.delegate.ServerGroupsWithContext(ctx) + gl, err = d.delegate.ServerGroups() } if err != nil || len(gl.Groups) == 0 { - utilruntime.HandleErrorWithContext(ctx, err, "Couldn't get current server API group list") + utilruntime.HandleError(fmt.Errorf("couldn't get current server API group list: %v", err)) return err } @@ -360,16 +275,16 @@ func (d *memCacheClient) refreshLocked(ctx context.Context) error { wg.Add(1) go func() { defer wg.Done() - defer utilruntime.HandleCrashWithContext(ctx) + defer utilruntime.HandleCrash() - r, err := d.serverResourcesForGroupVersion(ctx, gv) + r, err := d.serverResourcesForGroupVersion(gv) if err != nil { // Don't log "empty response" as an error; it is a common response for metrics. if _, emptyErr := err.(*emptyResponseError); emptyErr { // Log at same verbosity as disk cache. - klog.FromContext(ctx).V(3).Info(err.Error()) + klog.V(3).Infof("%v", err) } else { - utilruntime.HandleErrorWithContext(ctx, err, "Couldn't get resource list", "groupVersion", gv) + utilruntime.HandleError(fmt.Errorf("couldn't get resource list for %v: %v", gv, err)) } } @@ -386,8 +301,8 @@ func (d *memCacheClient) refreshLocked(ctx context.Context) error { return nil } -func (d *memCacheClient) serverResourcesForGroupVersion(ctx context.Context, groupVersion string) (*metav1.APIResourceList, error) { - r, err := d.delegate.ServerResourcesForGroupVersionWithContext(ctx, groupVersion) +func (d *memCacheClient) serverResourcesForGroupVersion(groupVersion string) (*metav1.APIResourceList, error) { + r, err := d.delegate.ServerResourcesForGroupVersion(groupVersion) if err != nil { return r, err } @@ -399,39 +314,16 @@ func (d *memCacheClient) serverResourcesForGroupVersion(ctx context.Context, gro // WithLegacy returns current memory-cached discovery client; // current client does not support legacy-only discovery. -// -// Deprecated: use WithLegacyWithContext instead. func (d *memCacheClient) WithLegacy() discovery.DiscoveryInterface { return d } -// WithLegacyWithContext returns current memory-cached discovery client; -// current client does not support legacy-only discovery. -func (d *memCacheClient) WithLegacyWithContext(ctx context.Context) discovery.DiscoveryInterfaceWithContext { - return d -} - // NewMemCacheClient creates a new CachedDiscoveryInterface which caches // discovery information in memory and will stay up-to-date if Invalidate is // called with regularity. // // NOTE: The client will NOT resort to live lookups on cache misses. -// -// Deprecated: use NewMemCacheClientWithContext instead. func NewMemCacheClient(delegate discovery.DiscoveryInterface) discovery.CachedDiscoveryInterface { - return newMemCacheClient(discovery.ToDiscoveryInterfaceWithContext(delegate)) -} - -// NewMemCacheClientWithContext creates a new CachedDiscoveryInterfaceWithContext which caches -// discovery information in memory and will stay up-to-date if Invalidate is -// called with regularity. -// -// NOTE: The client will NOT resort to live lookups on cache misses. -func NewMemCacheClientWithContext(delegate discovery.DiscoveryInterfaceWithContext) discovery.CachedDiscoveryInterfaceWithContext { - return newMemCacheClient(delegate) -} - -func newMemCacheClient(delegate discovery.DiscoveryInterfaceWithContext) *memCacheClient { return &memCacheClient{ delegate: delegate, groupToServerResources: map[string]*cacheEntry{}, diff --git a/discovery/cached/memory/memcache_test.go b/discovery/cached/memory/memcache_test.go index cd5885f9c..1412a961b 100644 --- a/discovery/cached/memory/memcache_test.go +++ b/discovery/cached/memory/memcache_test.go @@ -35,10 +35,10 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/client-go/discovery" + "k8s.io/client-go/discovery/fake" "k8s.io/client-go/openapi" "k8s.io/client-go/rest" testutil "k8s.io/client-go/util/testing" - "k8s.io/klog/v2/ktesting" ) type resourceMapEntry struct { @@ -47,10 +47,7 @@ type resourceMapEntry struct { } type fakeDiscovery struct { - // Intentionally limited to discovery.DiscoveryInterface because that is all that this - // code knows about and partly overrides. *fake.FakeDiscovery would inherit - // e.g. ServerGroupsWithContext which then would have to be overridden. - discovery.DiscoveryInterface + *fake.FakeDiscovery lock sync.Mutex groupList *metav1.APIGroupList @@ -438,8 +435,6 @@ func TestOpenAPIMemCache(t *testing.T) { continue } - // This is the original OpenAPI client. It is not affected - // by client.Invalidate() below. pathsAgain, err := openapiClient.Paths() if !assert.NoError(t, err) { continue @@ -450,8 +445,7 @@ func TestOpenAPIMemCache(t *testing.T) { continue } - // The map itself might be different, but the content should not be. - assert.Equal(t, reflect.ValueOf(paths[k]).Pointer(), reflect.ValueOf(pathsAgain[k]).Pointer()) + assert.Equal(t, reflect.ValueOf(paths).Pointer(), reflect.ValueOf(pathsAgain).Pointer()) assert.Equal(t, reflect.ValueOf(original).Pointer(), reflect.ValueOf(schemaAgain).Pointer()) // Invalidate and try again. This time pointers should not be equal @@ -467,75 +461,6 @@ func TestOpenAPIMemCache(t *testing.T) { continue } - assert.NotEqual(t, reflect.ValueOf(paths[k]).Pointer(), reflect.ValueOf(pathsAgain[k]).Pointer()) - assert.NotEqual(t, reflect.ValueOf(original).Pointer(), reflect.ValueOf(schemaAgain).Pointer()) - assert.Equal(t, original, schemaAgain) - } - }) - } -} - -// Tests that schema instances returned by openapi are cached and returned after -// successive calls. -func TestOpenAPIMemCacheWithContext(t *testing.T) { - _, ctx := ktesting.NewTestContext(t) - fakeServer, err := testutil.NewFakeOpenAPIV3Server("../../testdata") - require.NoError(t, err) - defer fakeServer.HttpServer.Close() - - require.NotEmpty(t, fakeServer.ServedDocuments) - - client := NewMemCacheClientWithContext( - discovery.NewDiscoveryClientForConfigOrDie( - &rest.Config{Host: fakeServer.HttpServer.URL}, - ), - ) - openapiClient := client.OpenAPIV3WithContext(ctx) - - paths, err := openapiClient.PathsWithContext(ctx) - require.NoError(t, err) - - contentTypes := []string{ - runtime.ContentTypeJSON, openapi.ContentTypeOpenAPIV3PB, - } - - for _, contentType := range contentTypes { - t.Run(contentType, func(t *testing.T) { - for k, v := range paths { - original, err := v.SchemaWithContext(ctx, contentType) - if !assert.NoError(t, err) { - continue - } - - // This is the original OpenAPI client. It is not affected - // by client.Invalidate() below. - pathsAgain, err := openapiClient.PathsWithContext(ctx) - if !assert.NoError(t, err) { - continue - } - - schemaAgain, err := pathsAgain[k].SchemaWithContext(ctx, contentType) - if !assert.NoError(t, err) { - continue - } - - // When using the *WithContext API, the map itself is cached. - assert.Equal(t, reflect.ValueOf(paths).Pointer(), reflect.ValueOf(pathsAgain).Pointer()) - assert.Equal(t, reflect.ValueOf(original).Pointer(), reflect.ValueOf(schemaAgain).Pointer()) - - // Invalidate and try again. This time pointers should not be equal - client.InvalidateWithContext(ctx) - - pathsAgain, err = client.OpenAPIV3WithContext(ctx).PathsWithContext(ctx) - if !assert.NoError(t, err) { - continue - } - - schemaAgain, err = pathsAgain[k].SchemaWithContext(ctx, contentType) - if !assert.NoError(t, err) { - continue - } - assert.NotEqual(t, reflect.ValueOf(paths).Pointer(), reflect.ValueOf(pathsAgain).Pointer()) assert.NotEqual(t, reflect.ValueOf(original).Pointer(), reflect.ValueOf(schemaAgain).Pointer()) assert.Equal(t, original, schemaAgain) diff --git a/discovery/discovery_client.go b/discovery/discovery_client.go index fb232eee7..aae94f9b8 100644 --- a/discovery/discovery_client.go +++ b/discovery/discovery_client.go @@ -76,8 +76,6 @@ var v2GVK = schema.GroupVersionKind{Group: "apidiscovery.k8s.io", Version: "v2", // DiscoveryInterface holds the methods that discover server-supported API groups, // versions and resources. -// -// Deprecated: use DiscoveryInterfaceWithContext instead. type DiscoveryInterface interface { RESTClient() restclient.Interface ServerGroupsInterface @@ -88,109 +86,22 @@ type DiscoveryInterface interface { // Returns copy of current discovery client that will only // receive the legacy discovery format, or pointer to current // discovery client if it does not support legacy-only discovery. - // - // Deprecated: use DiscoveryInterfaceWithContext.WithLegacyWithContext instead. WithLegacy() DiscoveryInterface } -// DiscoveryInterfaceWithContext holds the methods that discover server-supported API groups, -// versions and resources. -type DiscoveryInterfaceWithContext interface { - RESTClient() restclient.Interface - ServerGroupsInterfaceWithContext - ServerResourcesInterfaceWithContext - ServerVersionInterfaceWithContext - OpenAPISchemaInterfaceWithContext - OpenAPIV3SchemaInterfaceWithContext - // Returns copy of current discovery client that will only - // receive the legacy discovery format, or pointer to current - // discovery client if it does not support legacy-only discovery. - WithLegacyWithContext(ctx context.Context) DiscoveryInterfaceWithContext -} - -func ToDiscoveryInterfaceWithContext(i DiscoveryInterface) DiscoveryInterfaceWithContext { - if i == nil { - return nil - } - if i, ok := i.(DiscoveryInterfaceWithContext); ok { - return i - } - return &discoveryInterfaceWrapper{ - ServerGroupsInterfaceWithContext: ToServerGroupsInterfaceWithContext(i), - ServerResourcesInterfaceWithContext: ToServerResourcesInterfaceWithContext(i), - ServerVersionInterfaceWithContext: ToServerVersionInterfaceWithContext(i), - OpenAPISchemaInterfaceWithContext: ToOpenAPISchemaInterfaceWithContext(i), - OpenAPIV3SchemaInterfaceWithContext: OpenAPIV3ToSchemaInterfaceWithContext(i), - delegate: i, - } -} - -type discoveryInterfaceWrapper struct { - ServerGroupsInterfaceWithContext - ServerResourcesInterfaceWithContext - ServerVersionInterfaceWithContext - OpenAPISchemaInterfaceWithContext - OpenAPIV3SchemaInterfaceWithContext - delegate DiscoveryInterface -} - -func (i *discoveryInterfaceWrapper) RESTClient() restclient.Interface { - return i.delegate.RESTClient() -} - -func (i *discoveryInterfaceWrapper) WithLegacyWithContext(ctx context.Context) DiscoveryInterfaceWithContext { - return ToDiscoveryInterfaceWithContext(i.delegate.WithLegacy()) -} - // AggregatedDiscoveryInterface extends DiscoveryInterface to include a method to possibly // return discovery resources along with the discovery groups, which is what the newer // aggregated discovery format does (APIGroupDiscoveryList). -// -// Deprecated: use AggregatedDiscoveryInterfaceWithContext instead. type AggregatedDiscoveryInterface interface { DiscoveryInterface - // Deprecated: use AggregatedDiscoveryInterfaceWithContext.GroupsAndMaybeResourcesWithContext instead. GroupsAndMaybeResources() (*metav1.APIGroupList, map[schema.GroupVersion]*metav1.APIResourceList, map[schema.GroupVersion]error, error) } -// AggregatedDiscoveryInterfaceWithContext extends DiscoveryInterface to include a method to possibly -// return discovery resources along with the discovery groups, which is what the newer -// aggregated discovery format does (APIGroupDiscoveryList). -type AggregatedDiscoveryInterfaceWithContext interface { - DiscoveryInterfaceWithContext - - GroupsAndMaybeResourcesWithContext(ctx context.Context) (*metav1.APIGroupList, map[schema.GroupVersion]*metav1.APIResourceList, map[schema.GroupVersion]error, error) -} - -func ToAggregatedDiscoveryInterfaceWithContext(i AggregatedDiscoveryInterface) AggregatedDiscoveryInterfaceWithContext { - if i == nil { - return nil - } - if i, ok := i.(AggregatedDiscoveryInterfaceWithContext); ok { - return i - } - return &aggregatedDiscoveryInterfaceWrapper{ - DiscoveryInterfaceWithContext: ToDiscoveryInterfaceWithContext(i), - delegate: i, - } -} - -type aggregatedDiscoveryInterfaceWrapper struct { - DiscoveryInterfaceWithContext - delegate AggregatedDiscoveryInterface -} - -func (i *aggregatedDiscoveryInterfaceWrapper) GroupsAndMaybeResourcesWithContext(ctx context.Context) (*metav1.APIGroupList, map[schema.GroupVersion]*metav1.APIResourceList, map[schema.GroupVersion]error, error) { - return i.delegate.GroupsAndMaybeResources() -} - // CachedDiscoveryInterface is a DiscoveryInterface with cache invalidation and freshness. // Note that If the ServerResourcesForGroupVersion method returns a cache miss // error, the user needs to explicitly call Invalidate to clear the cache, // otherwise the same cache miss error will be returned next time. -// -// Deprecated: use CachedDiscoveryInterfaceWithContext instead. type CachedDiscoveryInterface interface { DiscoveryInterface // Fresh is supposed to tell the caller whether or not to retry if the cache @@ -198,312 +109,58 @@ type CachedDiscoveryInterface interface { // // TODO: this needs to be revisited, this interface can't be locked properly // and doesn't make a lot of sense. - // - // Deprecated: use CachedDiscoveryInterfaceWithContext.FreshWithContext instead. Fresh() bool // Invalidate enforces that no cached data that is older than the current time // is used. - // - // Deprecated: use CachedDiscoveryInterfaceWithContext.InvalidateWithContext instead. Invalidate() } -// CachedDiscoveryInterfaceWithContext is a DiscoveryInterfaceWithContext with cache invalidation and freshness. -// Note that If the ServerResourcesForGroupVersion method returns a cache miss -// error, the user needs to explicitly call Invalidate to clear the cache, -// otherwise the same cache miss error will be returned next time. -type CachedDiscoveryInterfaceWithContext interface { - DiscoveryInterfaceWithContext - // Fresh is supposed to tell the caller whether or not to retry if the cache - // fails to find something (false = retry, true = no need to retry). - // - // TODO: this needs to be revisited, this interface can't be locked properly - // and doesn't make a lot of sense. - FreshWithContext(ctx context.Context) bool - // Invalidate enforces that no cached data that is older than the current time - // is used. - InvalidateWithContext(ctx context.Context) -} - -func ToCachedDiscoveryInterfaceWithContext(i CachedDiscoveryInterface) CachedDiscoveryInterfaceWithContext { - if i == nil { - return nil - } - if i, ok := i.(CachedDiscoveryInterfaceWithContext); ok { - return i - } - return &cachedDiscoveryInterfaceWrapper{ - DiscoveryInterfaceWithContext: ToDiscoveryInterfaceWithContext(i), - delegate: i, - } -} - -type cachedDiscoveryInterfaceWrapper struct { - DiscoveryInterfaceWithContext - delegate CachedDiscoveryInterface -} - -func (i *cachedDiscoveryInterfaceWrapper) FreshWithContext(ctx context.Context) bool { - return i.delegate.Fresh() -} - -func (i *cachedDiscoveryInterfaceWrapper) InvalidateWithContext(ctx context.Context) { - i.delegate.Invalidate() -} - // ServerGroupsInterface has methods for obtaining supported groups on the API server -// -// Deprecated: use ServerGroupsInterfaceWithContext instead. type ServerGroupsInterface interface { // ServerGroups returns the supported groups, with information like supported versions and the // preferred version. - // - // Deprecated: use ServerGroupsInterfaceWithContext.ServerGroups instead. ServerGroups() (*metav1.APIGroupList, error) } -// ServerGroupsInterfaceWithContext has methods for obtaining supported groups on the API server -type ServerGroupsInterfaceWithContext interface { - // ServerGroups returns the supported groups, with information like supported versions and the - // preferred version. - ServerGroupsWithContext(ctx context.Context) (*metav1.APIGroupList, error) -} - -func ToServerGroupsInterfaceWithContext(i ServerGroupsInterface) ServerGroupsInterfaceWithContext { - if i == nil { - return nil - } - if i, ok := i.(ServerGroupsInterfaceWithContext); ok { - return i - } - return &serverGroupsInterfaceWrapper{ - delegate: i, - } -} - -type serverGroupsInterfaceWrapper struct { - delegate ServerGroupsInterface -} - -func (i *serverGroupsInterfaceWrapper) ServerGroupsWithContext(ctx context.Context) (*metav1.APIGroupList, error) { - return i.delegate.ServerGroups() -} - // ServerResourcesInterface has methods for obtaining supported resources on the API server -// -// Deprecated: use ServerGroupsAndResourcesWithContext instead. type ServerResourcesInterface interface { // ServerResourcesForGroupVersion returns the supported resources for a group and version. - // - // Deprecated: use ServerGroupsAndResourcesWithContext.ServerResourcesForGroupVersionWithContext instead. ServerResourcesForGroupVersion(groupVersion string) (*metav1.APIResourceList, error) // ServerGroupsAndResources returns the supported groups and resources for all groups and versions. // // The returned group and resource lists might be non-nil with partial results even in the // case of non-nil error. - // - // Deprecated: use ServerGroupsAndResourcesWithContext.ServerGroupsAndResourcesWithContext instead. ServerGroupsAndResources() ([]*metav1.APIGroup, []*metav1.APIResourceList, error) // ServerPreferredResources returns the supported resources with the version preferred by the // server. // // The returned group and resource lists might be non-nil with partial results even in the // case of non-nil error. - // - // Deprecated: use ServerResourcesInterfaceWithContext.ServerPreferredResourcesWithContext instead. ServerPreferredResources() ([]*metav1.APIResourceList, error) // ServerPreferredNamespacedResources returns the supported namespaced resources with the // version preferred by the server. // // The returned resource list might be non-nil with partial results even in the case of // non-nil error. - // - // Deprecated: use ServerResourcesInterfaceWithContext.ServerPreferredNamespacedResourcesWithContext instead. ServerPreferredNamespacedResources() ([]*metav1.APIResourceList, error) } -// ServerResourcesInterfaceWithContext has methods for obtaining supported resources on the API server -type ServerResourcesInterfaceWithContext interface { - // ServerResourcesForGroupVersionWithContext returns the supported resources for a group and version. - ServerResourcesForGroupVersionWithContext(ctx context.Context, groupVersion string) (*metav1.APIResourceList, error) - // ServerGroupsAndResourcesWithContext returns the supported groups and resources for all groups and versions. - // - // The returned group and resource lists might be non-nil with partial results even in the - // case of non-nil error. - ServerGroupsAndResourcesWithContext(ctx context.Context) ([]*metav1.APIGroup, []*metav1.APIResourceList, error) - // ServerPreferredResourcesWithContext returns the supported resources with the version preferred by the - // server. - // - // The returned group and resource lists might be non-nil with partial results even in the - // case of non-nil error. - ServerPreferredResourcesWithContext(ctx context.Context) ([]*metav1.APIResourceList, error) - // ServerPreferredNamespacedResourcesWithContext returns the supported namespaced resources with the - // version preferred by the server. - // - // The returned resource list might be non-nil with partial results even in the case of - // non-nil error. - ServerPreferredNamespacedResourcesWithContext(ctx context.Context) ([]*metav1.APIResourceList, error) -} - -func ToServerResourcesInterfaceWithContext(i ServerResourcesInterface) ServerResourcesInterfaceWithContext { - if i == nil { - return nil - } - if i, ok := i.(ServerResourcesInterfaceWithContext); ok { - return i - } - return &serverResourcesInterfaceWrapper{ - delegate: i, - } -} - -type serverResourcesInterfaceWrapper struct { - delegate ServerResourcesInterface -} - -func (i *serverResourcesInterfaceWrapper) ServerResourcesForGroupVersionWithContext(ctx context.Context, groupVersion string) (*metav1.APIResourceList, error) { - return i.delegate.ServerResourcesForGroupVersion(groupVersion) -} - -func (i *serverResourcesInterfaceWrapper) ServerGroupsAndResourcesWithContext(ctx context.Context) ([]*metav1.APIGroup, []*metav1.APIResourceList, error) { - return i.delegate.ServerGroupsAndResources() -} - -func (i *serverResourcesInterfaceWrapper) ServerPreferredResourcesWithContext(ctx context.Context) ([]*metav1.APIResourceList, error) { - return i.delegate.ServerPreferredResources() -} - -func (i *serverResourcesInterfaceWrapper) ServerPreferredNamespacedResourcesWithContext(ctx context.Context) ([]*metav1.APIResourceList, error) { - return i.delegate.ServerPreferredNamespacedResources() -} - // ServerVersionInterface has a method for retrieving the server's version. -// -// Deprecated: use ServerVersionInterfaceWithContext instead. type ServerVersionInterface interface { // ServerVersion retrieves and parses the server's version (git version). - // - // Deprecated: use ServerVersionInterfaceWithContext.ServerVersionWithContext instead. ServerVersion() (*version.Info, error) } -// ServerVersionInterface has a method for retrieving the server's version. -type ServerVersionInterfaceWithContext interface { - // ServerVersion retrieves and parses the server's version (git version). - ServerVersionWithContext(ctx context.Context) (*version.Info, error) -} - -func ToServerVersionInterfaceWithContext(i ServerVersionInterface) ServerVersionInterfaceWithContext { - if i == nil { - return nil - } - if i, ok := i.(ServerVersionInterfaceWithContext); ok { - return i - } - return &serverVersionInterfaceWrapper{ - delegate: i, - } -} - -type serverVersionInterfaceWrapper struct { - delegate ServerVersionInterface -} - -func (i *serverVersionInterfaceWrapper) ServerVersionWithContext(ctx context.Context) (*version.Info, error) { - return i.delegate.ServerVersion() -} - // OpenAPISchemaInterface has a method to retrieve the open API schema. -// -// Deprecated: use OpenAPISchemaInterfaceWithContext instead. type OpenAPISchemaInterface interface { // OpenAPISchema retrieves and parses the swagger API schema the server supports. - // - // Deprecated: use OpenAPISchemaInterfaceWithContext.OpenAPISchemaWithContext instead. OpenAPISchema() (*openapi_v2.Document, error) } -// OpenAPISchemaInterfaceWithContext has a method to retrieve the open API schema. -type OpenAPISchemaInterfaceWithContext interface { - // OpenAPISchemaWithContext retrieves and parses the swagger API schema the server supports. - OpenAPISchemaWithContext(ctx context.Context) (*openapi_v2.Document, error) -} - -func ToOpenAPISchemaInterfaceWithContext(i OpenAPISchemaInterface) OpenAPISchemaInterfaceWithContext { - if i == nil { - return nil - } - if i, ok := i.(OpenAPISchemaInterfaceWithContext); ok { - return i - } - return &openAPISchemaInterfaceWrapper{ - delegate: i, - } -} - -type openAPISchemaInterfaceWrapper struct { - delegate OpenAPISchemaInterface -} - -func (i *openAPISchemaInterfaceWrapper) OpenAPISchemaWithContext(ctx context.Context) (*openapi_v2.Document, error) { - return i.delegate.OpenAPISchema() -} - -// Deprecated: use OpenAPIV3SchemaInterfaceWithContext instead. type OpenAPIV3SchemaInterface interface { - // Deprecated: use OpenAPIV3SchemaInterfaceWithContext.OpenAPIV3WithContext instead. OpenAPIV3() openapi.Client } -type OpenAPIV3SchemaInterfaceWithContext interface { - OpenAPIV3WithContext(ctx context.Context) openapi.ClientWithContext -} - -func OpenAPIV3ToSchemaInterfaceWithContext(i OpenAPIV3SchemaInterface) OpenAPIV3SchemaInterfaceWithContext { - if i == nil { - return nil - } - if i, ok := i.(OpenAPIV3SchemaInterfaceWithContext); ok { - return i - } - return &openAPIV3SchemaInterfaceWrapper{ - delegate: i, - } -} - -type openAPIV3SchemaInterfaceWrapper struct { - delegate OpenAPIV3SchemaInterface -} - -func (i *openAPIV3SchemaInterfaceWrapper) OpenAPIV3WithContext(ctx context.Context) openapi.ClientWithContext { - return ToOpenAPIClientWithContext(i.delegate.OpenAPIV3()) -} - -func ToOpenAPIClientWithContext(i openapi.Client) openapi.ClientWithContext { - if i == nil { - return nil - } - if i, ok := i.(openapi.ClientWithContext); ok { - return i - } - return &openAPIClientWrapper{ - delegate: i, - } -} - -type openAPIClientWrapper struct { - delegate openapi.Client -} - -func (i *openAPIClientWrapper) PathsWithContext(ctx context.Context) (map[string]openapi.GroupVersionWithContext, error) { - resultWithoutContext, err := i.delegate.Paths() - result := make(map[string]openapi.GroupVersionWithContext, len(resultWithoutContext)) - for key, entry := range resultWithoutContext { - result[key] = openapi.ToGroupVersionWithContext(entry) - } - return result, err -} - // DiscoveryClient implements the functions that discover server-supported API groups, // versions and resources. type DiscoveryClient struct { @@ -545,36 +202,19 @@ func apiVersionsToAPIGroup(apiVersions *metav1.APIVersions) (apiGroup metav1.API // as aggregated discovery format or legacy format. For safety, resources will only // be returned if both endpoints returned resources. Returned "failedGVs" can be // empty, but will only be nil in the case an error is returned. -// -// Deprecated: use GroupsAndMaybeResourcesWithContext instead. func (d *DiscoveryClient) GroupsAndMaybeResources() ( - *metav1.APIGroupList, - map[schema.GroupVersion]*metav1.APIResourceList, - map[schema.GroupVersion]error, - error) { - return d.GroupsAndMaybeResourcesWithContext(context.Background()) -} - -// GroupsAndMaybeResources returns the discovery groups, and (if new aggregated -// discovery format) the resources keyed by group/version. Merges discovery groups -// and resources from /api and /apis (either aggregated or not). Legacy groups -// must be ordered first. The server will either return both endpoints (/api, /apis) -// as aggregated discovery format or legacy format. For safety, resources will only -// be returned if both endpoints returned resources. Returned "failedGVs" can be -// empty, but will only be nil in the case an error is returned. -func (d *DiscoveryClient) GroupsAndMaybeResourcesWithContext(ctx context.Context) ( *metav1.APIGroupList, map[schema.GroupVersion]*metav1.APIResourceList, map[schema.GroupVersion]error, error) { // Legacy group ordered first (there is only one -- core/v1 group). Returned groups must // be non-nil, but it could be empty. Returned resources, apiResources map could be nil. - groups, resources, failedGVs, err := d.downloadLegacy(ctx) + groups, resources, failedGVs, err := d.downloadLegacy() if err != nil { return nil, nil, nil, err } // Discovery groups and (possibly) resources downloaded from /apis. - apiGroups, apiResources, failedApisGVs, aerr := d.downloadAPIs(ctx) + apiGroups, apiResources, failedApisGVs, aerr := d.downloadAPIs() if aerr != nil { return nil, nil, nil, aerr } @@ -602,7 +242,7 @@ func (d *DiscoveryClient) GroupsAndMaybeResourcesWithContext(ctx context.Context // possible for the resource map to be nil if the server returned // the unaggregated discovery. Returned "failedGVs" can be empty, but // will only be nil in the case of a returned error. -func (d *DiscoveryClient) downloadLegacy(ctx context.Context) ( +func (d *DiscoveryClient) downloadLegacy() ( *metav1.APIGroupList, map[schema.GroupVersion]*metav1.APIResourceList, map[schema.GroupVersion]error, @@ -612,7 +252,7 @@ func (d *DiscoveryClient) downloadLegacy(ctx context.Context) ( body, err := d.restClient.Get(). AbsPath("/api"). SetHeader("Accept", accept). - Do(ctx). + Do(context.TODO()). ContentType(&responseContentType). Raw() apiGroupList := &metav1.APIGroupList{} @@ -665,7 +305,7 @@ func (d *DiscoveryClient) downloadLegacy(ctx context.Context) ( // discovery resources. The returned groups will always exist, but the // resources map may be nil. Returned "failedGVs" can be empty, but will // only be nil in the case of a returned error. -func (d *DiscoveryClient) downloadAPIs(ctx context.Context) ( +func (d *DiscoveryClient) downloadAPIs() ( *metav1.APIGroupList, map[schema.GroupVersion]*metav1.APIResourceList, map[schema.GroupVersion]error, @@ -675,7 +315,7 @@ func (d *DiscoveryClient) downloadAPIs(ctx context.Context) ( body, err := d.restClient.Get(). AbsPath("/apis"). SetHeader("Accept", accept). - Do(ctx). + Do(context.TODO()). ContentType(&responseContentType). Raw() if err != nil { @@ -748,16 +388,8 @@ func ContentTypeIsGVK(contentType string, gvk schema.GroupVersionKind) (bool, er // ServerGroups returns the supported groups, with information like supported versions and the // preferred version. -// -// Deprecated: use ServerGroupsWithContext instead. func (d *DiscoveryClient) ServerGroups() (*metav1.APIGroupList, error) { - return d.ServerGroupsWithContext(context.Background()) -} - -// ServerGroupsWithContext returns the supported groups, with information like supported versions and the -// preferred version. -func (d *DiscoveryClient) ServerGroupsWithContext(ctx context.Context) (*metav1.APIGroupList, error) { - groups, _, _, err := d.GroupsAndMaybeResourcesWithContext(ctx) + groups, _, _, err := d.GroupsAndMaybeResources() if err != nil { return nil, err } @@ -765,13 +397,7 @@ func (d *DiscoveryClient) ServerGroupsWithContext(ctx context.Context) (*metav1. } // ServerResourcesForGroupVersion returns the supported resources for a group and version. -// Deprecated: use ServerResourcesForGroupVersionWithContext instead. func (d *DiscoveryClient) ServerResourcesForGroupVersion(groupVersion string) (resources *metav1.APIResourceList, err error) { - return d.ServerResourcesForGroupVersionWithContext(context.Background(), groupVersion) -} - -// ServerResourcesForGroupVersionWithContext returns the supported resources for a group and version. -func (d *DiscoveryClient) ServerResourcesForGroupVersionWithContext(ctx context.Context, groupVersion string) (resources *metav1.APIResourceList, err error) { url := url.URL{} if len(groupVersion) == 0 { return nil, fmt.Errorf("groupVersion shouldn't be empty") @@ -784,7 +410,7 @@ func (d *DiscoveryClient) ServerResourcesForGroupVersionWithContext(ctx context. resources = &metav1.APIResourceList{ GroupVersion: groupVersion, } - err = d.restClient.Get().AbsPath(url.String()).Do(ctx).Into(resources) + err = d.restClient.Get().AbsPath(url.String()).Do(context.TODO()).Into(resources) if err != nil { // Tolerate core/v1 not found response by returning empty resource list; // this probably should not happen. But we should verify all callers are @@ -799,13 +425,8 @@ func (d *DiscoveryClient) ServerResourcesForGroupVersionWithContext(ctx context. // ServerGroupsAndResources returns the supported resources for all groups and versions. func (d *DiscoveryClient) ServerGroupsAndResources() ([]*metav1.APIGroup, []*metav1.APIResourceList, error) { - return d.ServerGroupsAndResourcesWithContext(context.Background()) -} - -// ServerGroupsAndResourcesWithContext returns the supported resources for all groups and versions. -func (d *DiscoveryClient) ServerGroupsAndResourcesWithContext(ctx context.Context) ([]*metav1.APIGroup, []*metav1.APIResourceList, error) { return withRetries(defaultRetries, func() ([]*metav1.APIGroup, []*metav1.APIResourceList, error) { - return ServerGroupsAndResourcesWithContext(ctx, d) + return ServerGroupsAndResources(d) }) } @@ -848,12 +469,7 @@ func GroupDiscoveryFailedErrorGroups(err error) (map[schema.GroupVersion]error, return nil, false } -// Deprecated: use ServerGroupsAndResourcesWithContext instead. func ServerGroupsAndResources(d DiscoveryInterface) ([]*metav1.APIGroup, []*metav1.APIResourceList, error) { - return ServerGroupsAndResourcesWithContext(context.Background(), ToDiscoveryInterfaceWithContext(d)) -} - -func ServerGroupsAndResourcesWithContext(ctx context.Context, d DiscoveryInterfaceWithContext) ([]*metav1.APIGroup, []*metav1.APIResourceList, error) { var sgs *metav1.APIGroupList var resources []*metav1.APIResourceList var failedGVs map[schema.GroupVersion]error @@ -861,21 +477,14 @@ func ServerGroupsAndResourcesWithContext(ctx context.Context, d DiscoveryInterfa // If the passed discovery object implements the wider AggregatedDiscoveryInterface, // then attempt to retrieve aggregated discovery with both groups and the resources. - ad, ok := d.(AggregatedDiscoveryInterfaceWithContext) - if !ok { - if ad2, ok2 := d.(AggregatedDiscoveryInterface); ok2 { - ad = ToAggregatedDiscoveryInterfaceWithContext(ad2) - ok = true - } - } - if ok { + if ad, ok := d.(AggregatedDiscoveryInterface); ok { var resourcesByGV map[schema.GroupVersion]*metav1.APIResourceList - sgs, resourcesByGV, failedGVs, err = ad.GroupsAndMaybeResourcesWithContext(ctx) + sgs, resourcesByGV, failedGVs, err = ad.GroupsAndMaybeResources() for _, resourceList := range resourcesByGV { resources = append(resources, resourceList) } } else { - sgs, err = d.ServerGroupsWithContext(ctx) + sgs, err = d.ServerGroups() } if sgs == nil { @@ -896,7 +505,7 @@ func ServerGroupsAndResourcesWithContext(ctx context.Context, d DiscoveryInterfa return resultGroups, resources, ferr } - groupVersionResources, failedGroups := fetchGroupVersionResources(ctx, d, sgs) + groupVersionResources, failedGroups := fetchGroupVersionResources(d, sgs) // order results by group/version discovery order result := []*metav1.APIResourceList{} @@ -917,14 +526,7 @@ func ServerGroupsAndResourcesWithContext(ctx context.Context, d DiscoveryInterfa } // ServerPreferredResources uses the provided discovery interface to look up preferred resources -// -// Deprecated: use ServerPreferredResourcesWithContext instead. func ServerPreferredResources(d DiscoveryInterface) ([]*metav1.APIResourceList, error) { - return ServerPreferredResourcesWithContext(context.Background(), ToDiscoveryInterfaceWithContext(d)) -} - -// ServerPreferredResourcesWithContext uses the provided discovery interface to look up preferred resources -func ServerPreferredResourcesWithContext(ctx context.Context, d DiscoveryInterfaceWithContext) ([]*metav1.APIResourceList, error) { var serverGroupList *metav1.APIGroupList var failedGroups map[schema.GroupVersion]error var groupVersionResources map[schema.GroupVersion]*metav1.APIResourceList @@ -933,24 +535,18 @@ func ServerPreferredResourcesWithContext(ctx context.Context, d DiscoveryInterfa // If the passed discovery object implements the wider AggregatedDiscoveryInterface, // then it is attempt to retrieve both the groups and the resources. "failedGroups" // are Group/Versions returned as stale in AggregatedDiscovery format. - ad, ok := d.(AggregatedDiscoveryInterfaceWithContext) - if !ok { - if ad2, ok2 := d.(AggregatedDiscoveryInterface); ok2 { - ad = ToAggregatedDiscoveryInterfaceWithContext(ad2) - ok = true - } - } + ad, ok := d.(AggregatedDiscoveryInterface) if ok { - serverGroupList, groupVersionResources, failedGroups, err = ad.GroupsAndMaybeResourcesWithContext(ctx) + serverGroupList, groupVersionResources, failedGroups, err = ad.GroupsAndMaybeResources() } else { - serverGroupList, err = d.ServerGroupsWithContext(ctx) + serverGroupList, err = d.ServerGroups() } if err != nil { return nil, err } // Non-aggregated discovery must fetch resources from Groups. if groupVersionResources == nil { - groupVersionResources, failedGroups = fetchGroupVersionResources(ctx, d, serverGroupList) + groupVersionResources, failedGroups = fetchGroupVersionResources(d, serverGroupList) } result := []*metav1.APIResourceList{} @@ -1006,7 +602,7 @@ func ServerPreferredResourcesWithContext(ctx context.Context, d DiscoveryInterfa } // fetchServerResourcesForGroupVersions uses the discovery client to fetch the resources for the specified groups in parallel. -func fetchGroupVersionResources(ctx context.Context, d DiscoveryInterfaceWithContext, apiGroups *metav1.APIGroupList) (map[schema.GroupVersion]*metav1.APIResourceList, map[schema.GroupVersion]error) { +func fetchGroupVersionResources(d DiscoveryInterface, apiGroups *metav1.APIGroupList) (map[schema.GroupVersion]*metav1.APIResourceList, map[schema.GroupVersion]error) { groupVersionResources := make(map[schema.GroupVersion]*metav1.APIResourceList) failedGroups := make(map[schema.GroupVersion]error) @@ -1018,9 +614,9 @@ func fetchGroupVersionResources(ctx context.Context, d DiscoveryInterfaceWithCon wg.Add(1) go func() { defer wg.Done() - defer utilruntime.HandleCrashWithContext(ctx) + defer utilruntime.HandleCrash() - apiResourceList, err := d.ServerResourcesForGroupVersionWithContext(ctx, groupVersion.String()) + apiResourceList, err := d.ServerResourcesForGroupVersion(groupVersion.String()) // lock to record results resultLock.Lock() @@ -1044,8 +640,6 @@ func fetchGroupVersionResources(ctx context.Context, d DiscoveryInterfaceWithCon // ServerPreferredResources returns the supported resources with the version preferred by the // server. -// -// Deprecated: use ServerPreferredResourcesWithContext instead. func (d *DiscoveryClient) ServerPreferredResources() ([]*metav1.APIResourceList, error) { _, rs, err := withRetries(defaultRetries, func() ([]*metav1.APIGroup, []*metav1.APIResourceList, error) { rs, err := ServerPreferredResources(d) @@ -1054,40 +648,15 @@ func (d *DiscoveryClient) ServerPreferredResources() ([]*metav1.APIResourceList, return rs, err } -// ServerPreferredResourcesWithContext returns the supported resources with the version preferred by the -// server. -func (d *DiscoveryClient) ServerPreferredResourcesWithContext(ctx context.Context) ([]*metav1.APIResourceList, error) { - _, rs, err := withRetries(defaultRetries, func() ([]*metav1.APIGroup, []*metav1.APIResourceList, error) { - rs, err := ServerPreferredResourcesWithContext(ctx, d) - return nil, rs, err - }) - return rs, err -} - // ServerPreferredNamespacedResources returns the supported namespaced resources with the // version preferred by the server. -// -// Deprecated: use ServerPreferredNamespacedResourcesWithContext instead. func (d *DiscoveryClient) ServerPreferredNamespacedResources() ([]*metav1.APIResourceList, error) { return ServerPreferredNamespacedResources(d) } -// ServerPreferredNamespacedResources returns the supported namespaced resources with the -// version preferred by the server. -func (d *DiscoveryClient) ServerPreferredNamespacedResourcesWithContext(ctx context.Context) ([]*metav1.APIResourceList, error) { - return ServerPreferredNamespacedResourcesWithContext(ctx, d) -} - // ServerPreferredNamespacedResources uses the provided discovery interface to look up preferred namespaced resources -// -// Deprecated: use ServerPreferredNamespacedResourcesWithContext instead. func ServerPreferredNamespacedResources(d DiscoveryInterface) ([]*metav1.APIResourceList, error) { - return ServerPreferredNamespacedResourcesWithContext(context.Background(), ToDiscoveryInterfaceWithContext(d)) -} - -// ServerPreferredNamespacedResourcesWithContext uses the provided discovery interface to look up preferred namespaced resources -func ServerPreferredNamespacedResourcesWithContext(ctx context.Context, d DiscoveryInterfaceWithContext) ([]*metav1.APIResourceList, error) { - all, err := ServerPreferredResourcesWithContext(ctx, d) + all, err := ServerPreferredResources(d) return FilteredBy(ResourcePredicateFunc(func(groupVersion string, r *metav1.APIResource) bool { return r.Namespaced }), all), err @@ -1095,12 +664,7 @@ func ServerPreferredNamespacedResourcesWithContext(ctx context.Context, d Discov // ServerVersion retrieves and parses the server's version (git version). func (d *DiscoveryClient) ServerVersion() (*version.Info, error) { - return d.ServerVersionWithContext(context.Background()) -} - -// ServerVersionWithContext retrieves and parses the server's version (git version). -func (d *DiscoveryClient) ServerVersionWithContext(ctx context.Context) (*version.Info, error) { - body, err := d.restClient.Get().AbsPath("/version").Do(ctx).Raw() + body, err := d.restClient.Get().AbsPath("/version").Do(context.TODO()).Raw() if err != nil { return nil, err } @@ -1114,12 +678,7 @@ func (d *DiscoveryClient) ServerVersionWithContext(ctx context.Context) (*versio // OpenAPISchema fetches the open api v2 schema using a rest client and parses the proto. func (d *DiscoveryClient) OpenAPISchema() (*openapi_v2.Document, error) { - return d.OpenAPISchemaWithContext(context.Background()) -} - -// OpenAPISchema fetches the open api v2 schema using a rest client and parses the proto. -func (d *DiscoveryClient) OpenAPISchemaWithContext(ctx context.Context) (*openapi_v2.Document, error) { - data, err := d.restClient.Get().AbsPath("/openapi/v2").SetHeader("Accept", openAPIV2mimePb).Do(ctx).Raw() + data, err := d.restClient.Get().AbsPath("/openapi/v2").SetHeader("Accept", openAPIV2mimePb).Do(context.TODO()).Raw() if err != nil { return nil, err } @@ -1131,33 +690,18 @@ func (d *DiscoveryClient) OpenAPISchemaWithContext(ctx context.Context) (*openap return document, nil } -// Deprecated: used OpenAVPIV3WithContext instead. func (d *DiscoveryClient) OpenAPIV3() openapi.Client { return openapi.NewClient(d.restClient) } -func (d *DiscoveryClient) OpenAPIV3WithContext(ctx context.Context) openapi.ClientWithContext { - return openapi.NewClientWithContext(d.restClient) -} - // WithLegacy returns copy of current discovery client that will only // receive the legacy discovery format. -// -// Deprecated: use WithLegacyWithContext instead. func (d *DiscoveryClient) WithLegacy() DiscoveryInterface { client := *d client.UseLegacyDiscovery = true return &client } -// WithLegacyWithContext returns copy of current discovery client that will only -// receive the legacy discovery format. -func (d *DiscoveryClient) WithLegacyWithContext(ctx context.Context) DiscoveryInterfaceWithContext { - client := *d - client.UseLegacyDiscovery = true - return &client -} - // withRetries retries the given recovery function in case the groups supported by the server change after ServerGroup() returns. func withRetries(maxRetries int, f func() ([]*metav1.APIGroup, []*metav1.APIResourceList, error)) ([]*metav1.APIGroup, []*metav1.APIResourceList, error) { var result []*metav1.APIResourceList diff --git a/discovery/fake/discovery.go b/discovery/fake/discovery.go index 99af16c88..e5d9e7f80 100644 --- a/discovery/fake/discovery.go +++ b/discovery/fake/discovery.go @@ -17,7 +17,6 @@ limitations under the License. package fake import ( - "context" "fmt" "net/http" @@ -34,30 +33,16 @@ import ( "k8s.io/client-go/testing" ) -// FakeDiscovery implements discovery.DiscoveryInterface and discovery.DiscoveryInterfaceWithContext. -// It sometimes calls testing.Fake.Invoke with an action, +// FakeDiscovery implements discovery.DiscoveryInterface and sometimes calls testing.Fake.Invoke with an action, // but doesn't respect the return value if any. There is a way to fake static values like ServerVersion by using the Faked... fields on the struct. type FakeDiscovery struct { *testing.Fake FakedServerVersion *version.Info } -var ( - _ discovery.DiscoveryInterface = &FakeDiscovery{} - _ discovery.DiscoveryInterfaceWithContext = &FakeDiscovery{} -) - // ServerResourcesForGroupVersion returns the supported resources for a group // and version. -// -// Deprecated: use ServerResourcesForGroupVersionWithContext instead. func (c *FakeDiscovery) ServerResourcesForGroupVersion(groupVersion string) (*metav1.APIResourceList, error) { - return c.ServerResourcesForGroupVersionWithContext(context.Background(), groupVersion) -} - -// ServerResourcesForGroupVersionWithContext returns the supported resources for a group -// and version. -func (c *FakeDiscovery) ServerResourcesForGroupVersionWithContext(ctx context.Context, groupVersion string) (*metav1.APIResourceList, error) { action := testing.ActionImpl{ Verb: "get", Resource: schema.GroupVersionResource{Resource: "resource"}, @@ -80,15 +65,8 @@ func (c *FakeDiscovery) ServerResourcesForGroupVersionWithContext(ctx context.Co } // ServerGroupsAndResources returns the supported groups and resources for all groups and versions. -// -// Deprecated: use ServerGroupsAndResourcesWithContext instead. func (c *FakeDiscovery) ServerGroupsAndResources() ([]*metav1.APIGroup, []*metav1.APIResourceList, error) { - return c.ServerGroupsAndResourcesWithContext(context.Background()) -} - -// ServerGroupsAndResourcesWithContext returns the supported groups and resources for all groups and versions. -func (c *FakeDiscovery) ServerGroupsAndResourcesWithContext(ctx context.Context) ([]*metav1.APIGroup, []*metav1.APIResourceList, error) { - sgs, err := c.ServerGroupsWithContext(ctx) + sgs, err := c.ServerGroups() if err != nil { return nil, nil, err } @@ -109,43 +87,19 @@ func (c *FakeDiscovery) ServerGroupsAndResourcesWithContext(ctx context.Context) // ServerPreferredResources returns the supported resources with the version // preferred by the server. -// -// Deprecated: use ServerPreferredResourcesWithContext instead. func (c *FakeDiscovery) ServerPreferredResources() ([]*metav1.APIResourceList, error) { - return c.ServerPreferredResourcesWithContext(context.Background()) -} - -// ServerPreferredResourcesWithContext returns the supported resources with the version -// preferred by the server. -func (c *FakeDiscovery) ServerPreferredResourcesWithContext(ctx context.Context) ([]*metav1.APIResourceList, error) { return nil, nil } // ServerPreferredNamespacedResources returns the supported namespaced resources // with the version preferred by the server. -// -// Deprecated: use ServerPreferredNamespacedResourcesWithContext instead. func (c *FakeDiscovery) ServerPreferredNamespacedResources() ([]*metav1.APIResourceList, error) { - return c.ServerPreferredNamespacedResourcesWithContext(context.Background()) -} - -// ServerPreferredNamespacedResourcesWithContext returns the supported namespaced resources -// with the version preferred by the server. -func (c *FakeDiscovery) ServerPreferredNamespacedResourcesWithContext(ctx context.Context) ([]*metav1.APIResourceList, error) { return nil, nil } // ServerGroups returns the supported groups, with information like supported // versions and the preferred version. -// -// Deprecated: use ServerGroupsWithContext instead. func (c *FakeDiscovery) ServerGroups() (*metav1.APIGroupList, error) { - return c.ServerGroupsWithContext(context.Background()) -} - -// ServerGroupsWithContext returns the supported groups, with information like supported -// versions and the preferred version. -func (c *FakeDiscovery) ServerGroupsWithContext(ctx context.Context) (*metav1.APIGroupList, error) { action := testing.ActionImpl{ Verb: "get", Resource: schema.GroupVersionResource{Resource: "group"}, @@ -189,14 +143,7 @@ func (c *FakeDiscovery) ServerGroupsWithContext(ctx context.Context) (*metav1.AP } // ServerVersion retrieves and parses the server's version. -// -// Deprecated: use ServerVersionWithContext instead. func (c *FakeDiscovery) ServerVersion() (*version.Info, error) { - return c.ServerVersionWithContext(context.Background()) -} - -// ServerVersionWithContext retrieves and parses the server's version. -func (c *FakeDiscovery) ServerVersionWithContext(ctx context.Context) (*version.Info, error) { action := testing.ActionImpl{} action.Verb = "get" action.Resource = schema.GroupVersionResource{Resource: "version"} @@ -214,37 +161,20 @@ func (c *FakeDiscovery) ServerVersionWithContext(ctx context.Context) (*version. } // OpenAPISchema retrieves and parses the swagger API schema the server supports. -// -// Deprecated: use OpenAPISchemaWithContext instead. func (c *FakeDiscovery) OpenAPISchema() (*openapi_v2.Document, error) { - return c.OpenAPISchemaWithContext(context.Background()) -} - -// OpenAPISchemaWithContext retrieves and parses the swagger API schema the server supports. -func (c *FakeDiscovery) OpenAPISchemaWithContext(ctx context.Context) (*openapi_v2.Document, error) { return &openapi_v2.Document{}, nil } -// Deprecated: use OpenAPIV3WithContext instead. func (c *FakeDiscovery) OpenAPIV3() openapi.Client { panic("unimplemented") } -func (c *FakeDiscovery) OpenAPIV3WithContext(ctx context.Context) openapi.ClientWithContext { - panic("unimplemented") -} - // RESTClient returns a RESTClient that is used to communicate with API server // by this client implementation. func (c *FakeDiscovery) RESTClient() restclient.Interface { return nil } -// Deprecated: use WithLegacyWithContext instead. func (c *FakeDiscovery) WithLegacy() discovery.DiscoveryInterface { panic("unimplemented") } - -func (c *FakeDiscovery) WithLegacyWithContext(ctx context.Context) discovery.DiscoveryInterfaceWithContext { - panic("unimplemented") -} diff --git a/features/envvar.go b/features/envvar.go index 7ac21b383..8c3f887dc 100644 --- a/features/envvar.go +++ b/features/envvar.go @@ -141,13 +141,6 @@ func (f *envVarFeatureGates) Set(featureName Feature, featureValue bool) error { // read from the corresponding environmental variable. func (f *envVarFeatureGates) getEnabledMapFromEnvVar() map[Feature]bool { f.readEnvVarsOnce.Do(func() { - // This code does not really support contextual logging. Making it do so has huge - // implications for several call chains because the Enabled call then needs - // a `*WithLogger` variant. This does not matter in Kubernetes itself because - // all Kubernetes components replace the feature gate implementation used - // by client-go, but it might matter elsewhere. - logger := klog.Background() - featureGatesState := map[Feature]bool{} for feature, featureSpec := range f.known { featureState, featureStateSet := os.LookupEnv(fmt.Sprintf("KUBE_FEATURE_%s", feature)) @@ -157,10 +150,10 @@ func (f *envVarFeatureGates) getEnabledMapFromEnvVar() map[Feature]bool { boolVal, boolErr := strconv.ParseBool(featureState) switch { case boolErr != nil: - utilruntime.HandleErrorWithLogger(logger, boolErr, "Could not set feature gate", "feature", feature, "desiredState", featureState) + utilruntime.HandleError(fmt.Errorf("cannot set feature gate %q to %q, due to %v", feature, featureState, boolErr)) case featureSpec.LockToDefault: if boolVal != featureSpec.Default { - utilruntime.HandleErrorWithLogger(logger, nil, "Could not set feature gate, feature is locked", "feature", feature, "desiredState", featureState, "lockedState", featureSpec.Default) + utilruntime.HandleError(fmt.Errorf("cannot set feature gate %q to %q, feature is locked to %v", feature, featureState, featureSpec.Default)) break } featureGatesState[feature] = featureSpec.Default @@ -173,10 +166,10 @@ func (f *envVarFeatureGates) getEnabledMapFromEnvVar() map[Feature]bool { for feature, featureSpec := range f.known { if featureState, ok := featureGatesState[feature]; ok { - logger.V(1).Info("Feature gate updated state", "feature", feature, "enabled", featureState) + klog.V(1).InfoS("Feature gate updated state", "feature", feature, "enabled", featureState) continue } - logger.V(1).Info("Feature gate default state", "feature", feature, "enabled", featureSpec.Default) + klog.V(1).InfoS("Feature gate default state", "feature", feature, "enabled", featureSpec.Default) } }) return f.enabledViaEnvVar.Load().(map[Feature]bool) diff --git a/features/features.go b/features/features.go index f21511dca..cabb7468d 100644 --- a/features/features.go +++ b/features/features.go @@ -17,7 +17,7 @@ limitations under the License. package features import ( - "context" + "errors" "sync/atomic" utilruntime "k8s.io/apimachinery/pkg/util/runtime" @@ -127,9 +127,7 @@ func AddVersionedFeaturesToExistingFeatureGates(registry VersionedRegistry) erro // clientgofeaturegate.ReplaceFeatureGates(utilfeature.DefaultMutableFeatureGate) func ReplaceFeatureGates(newFeatureGates Gates) { if replaceFeatureGatesWithWarningIndicator(newFeatureGates) { - // TODO (?): A new API would be needed where callers pass in a context or logger. - // Probably not worth it. - utilruntime.HandleErrorWithContext(context.TODO(), nil, "The default feature gates implementation has already been used and now it's being overwritten. This might lead to unexpected behaviour. Check your initialization order.") + utilruntime.HandleError(errors.New("the default feature gates implementation has already been used and now it's being overwritten. This might lead to unexpected behaviour. Check your initialization order")) } } diff --git a/openapi/cached/client.go b/openapi/cached/client.go index 793014ac9..17f63ed26 100644 --- a/openapi/cached/client.go +++ b/openapi/cached/client.go @@ -17,62 +17,34 @@ limitations under the License. package cached import ( - "context" "sync" "k8s.io/client-go/openapi" ) -type Client struct { - delegate openapi.ClientWithContext +type client struct { + delegate openapi.Client once sync.Once - result map[string]openapi.GroupVersionWithContext + result map[string]openapi.GroupVersion err error } -var ( - _ openapi.Client = &Client{} - _ openapi.ClientWithContext = &Client{} -) - -// Deprecated: use NewClientWithContext instead. func NewClient(other openapi.Client) openapi.Client { - return newClient(openapi.ToClientWithContext(other)) -} - -func NewClientWithContext(other openapi.ClientWithContext) *Client { - return newClient(other) -} - -func newClient(other openapi.ClientWithContext) *Client { - return &Client{ + return &client{ delegate: other, } } -// Deprecated: use PathsWithContext instead. -func (c *Client) Paths() (map[string]openapi.GroupVersion, error) { - // For efficiency reasons in the *WithContext case this is a map to - // openapi.GroupVersionWithContext. But we know that all entries - // also implement openapi.GroupVersion. - resultWithContext, err := c.PathsWithContext(context.Background()) - result := make(map[string]openapi.GroupVersion, len(resultWithContext)) - for key, entry := range resultWithContext { - result[key] = entry.(openapi.GroupVersion) - } - return result, err -} - -func (c *Client) PathsWithContext(ctx context.Context) (map[string]openapi.GroupVersionWithContext, error) { +func (c *client) Paths() (map[string]openapi.GroupVersion, error) { c.once.Do(func() { - uncached, err := c.delegate.PathsWithContext(ctx) + uncached, err := c.delegate.Paths() if err != nil { c.err = err return } - result := make(map[string]openapi.GroupVersionWithContext, len(uncached)) + result := make(map[string]openapi.GroupVersion, len(uncached)) for k, v := range uncached { result[k] = newGroupVersion(v) } diff --git a/openapi/cached/groupversion.go b/openapi/cached/groupversion.go index 04e408a26..73730c51b 100644 --- a/openapi/cached/groupversion.go +++ b/openapi/cached/groupversion.go @@ -17,41 +17,30 @@ limitations under the License. package cached import ( - "context" "sync" "k8s.io/client-go/openapi" ) type groupversion struct { - delegate openapi.GroupVersionWithContext + delegate openapi.GroupVersion lock sync.Mutex docs map[string]docInfo } -var ( - _ openapi.GroupVersion = &groupversion{} - _ openapi.GroupVersionWithContext = &groupversion{} -) - type docInfo struct { data []byte err error } -func newGroupVersion(delegate openapi.GroupVersionWithContext) *groupversion { +func newGroupVersion(delegate openapi.GroupVersion) *groupversion { return &groupversion{ delegate: delegate, } } -// Deprecated: use SchemaWithContext instead. func (g *groupversion) Schema(contentType string) ([]byte, error) { - return g.SchemaWithContext(context.Background(), contentType) -} - -func (g *groupversion) SchemaWithContext(ctx context.Context, contentType string) ([]byte, error) { g.lock.Lock() defer g.lock.Unlock() @@ -61,7 +50,7 @@ func (g *groupversion) SchemaWithContext(ctx context.Context, contentType string g.docs = make(map[string]docInfo) } - cachedInfo.data, cachedInfo.err = g.delegate.SchemaWithContext(ctx, contentType) + cachedInfo.data, cachedInfo.err = g.delegate.Schema(contentType) g.docs[contentType] = cachedInfo } diff --git a/openapi/client.go b/openapi/client.go index c8dccb675..d32895a2f 100644 --- a/openapi/client.go +++ b/openapi/client.go @@ -25,75 +25,25 @@ import ( "k8s.io/kube-openapi/pkg/handler3" ) -// Deprecated: use ClientWithContext instead. type Client interface { Paths() (map[string]GroupVersion, error) } -type ClientWithContext interface { - PathsWithContext(ctx context.Context) (map[string]GroupVersionWithContext, error) -} - -func ToClientWithContext(c Client) ClientWithContext { - if c == nil { - return nil - } - if c, ok := c.(ClientWithContext); ok { - return c - } - return &clientWrapper{ - delegate: c, - } -} - -type clientWrapper struct { - delegate Client -} - -func (c *clientWrapper) PathsWithContext(ctx context.Context) (map[string]GroupVersionWithContext, error) { - resultWithoutContext, err := c.delegate.Paths() - result := make(map[string]GroupVersionWithContext, len(resultWithoutContext)) - for key, entry := range resultWithoutContext { - result[key] = ToGroupVersionWithContext(entry) - } - return result, err -} - type client struct { // URL includes the `hash` query param to take advantage of cache busting restClient rest.Interface } -// Deprecated: use NewClientWithContext instead. func NewClient(restClient rest.Interface) Client { - return newClient(restClient) -} - -func NewClientWithContext(restClient rest.Interface) ClientWithContext { - return newClient(restClient) -} - -func newClient(restClient rest.Interface) *client { return &client{ restClient: restClient, } } -// Deprecated: use PathsWithContext instead. func (c *client) Paths() (map[string]GroupVersion, error) { - resultWithContext, err := c.PathsWithContext(context.Background()) - result := make(map[string]GroupVersion, len(resultWithContext)) - for key, entry := range resultWithContext { - // We know that this is a *groupversion which implements GroupVersion. - result[key] = entry.(GroupVersion) - } - return result, err -} - -func (c *client) PathsWithContext(ctx context.Context) (map[string]GroupVersionWithContext, error) { data, err := c.restClient.Get(). AbsPath("/openapi/v3"). - Do(ctx). + Do(context.TODO()). Raw() if err != nil { @@ -109,7 +59,7 @@ func (c *client) PathsWithContext(ctx context.Context) (map[string]GroupVersionW // Calculate the client-side prefix for a "root" request rootPrefix := strings.TrimSuffix(c.restClient.Get().AbsPath("/").URL().Path, "/") // Create GroupVersions for each element of the result - result := make(map[string]GroupVersionWithContext, len(discoMap.Paths)) + result := map[string]GroupVersion{} for k, v := range discoMap.Paths { // Trim off the prefix that will always be added in client-side v.ServerRelativeURL = strings.TrimPrefix(v.ServerRelativeURL, rootPrefix) diff --git a/openapi/groupversion.go b/openapi/groupversion.go index 093af6d8f..40d91b9a5 100644 --- a/openapi/groupversion.go +++ b/openapi/groupversion.go @@ -25,7 +25,6 @@ import ( const ContentTypeOpenAPIV3PB = "application/com.github.proto-openapi.spec.v3@v1.0+protobuf" -// Deprecated: use GroupVersionWithContext instead. type GroupVersion interface { Schema(contentType string) ([]byte, error) @@ -36,61 +35,22 @@ type GroupVersion interface { ServerRelativeURL() string } -type GroupVersionWithContext interface { - SchemaWithContext(ctx context.Context, contentType string) ([]byte, error) - - // ServerRelativeURL. Returns the path and parameters used to fetch the schema. - // You should use the Schema method to fetch it, but this value can be used - // to key the current version of the schema in a cache since it contains a - // hash string which changes upon schema update. - ServerRelativeURL() string -} - -func ToGroupVersionWithContext(gv GroupVersion) GroupVersionWithContext { - if gv == nil { - return nil - } - if gv, ok := gv.(GroupVersionWithContext); ok { - return gv - } - return &groupVersionWrapper{ - GroupVersion: gv, - } -} - -type groupVersionWrapper struct { - GroupVersion -} - -func (gv *groupVersionWrapper) SchemaWithContext(ctx context.Context, contentType string) ([]byte, error) { - return gv.Schema(contentType) -} - type groupversion struct { client *client item handler3.OpenAPIV3DiscoveryGroupVersion useClientPrefix bool } -var ( - _ GroupVersion = &groupversion{} - _ GroupVersionWithContext = &groupversion{} -) - func newGroupVersion(client *client, item handler3.OpenAPIV3DiscoveryGroupVersion, useClientPrefix bool) *groupversion { return &groupversion{client: client, item: item, useClientPrefix: useClientPrefix} } func (g *groupversion) Schema(contentType string) ([]byte, error) { - return g.SchemaWithContext(context.Background(), contentType) -} - -func (g *groupversion) SchemaWithContext(ctx context.Context, contentType string) ([]byte, error) { if !g.useClientPrefix { return g.client.restClient.Get(). RequestURI(g.item.ServerRelativeURL). SetHeader("Accept", contentType). - Do(ctx). + Do(context.TODO()). Raw() } @@ -112,7 +72,7 @@ func (g *groupversion) SchemaWithContext(ctx context.Context, contentType string } } - return path.Do(ctx).Raw() + return path.Do(context.TODO()).Raw() } // URL used for fetching the schema. The URL includes a hash and can be used diff --git a/plugin/pkg/client/auth/azure/azure_stub.go b/plugin/pkg/client/auth/azure/azure_stub.go index d82b6766c..22d3c6b3f 100644 --- a/plugin/pkg/client/auth/azure/azure_stub.go +++ b/plugin/pkg/client/auth/azure/azure_stub.go @@ -25,7 +25,6 @@ import ( func init() { if err := rest.RegisterAuthProviderPlugin("azure", newAzureAuthProvider); err != nil { - //nolint:logcheck // Should not happen. klog.Fatalf("Failed to register azure auth plugin: %v", err) } } diff --git a/plugin/pkg/client/auth/exec/exec.go b/plugin/pkg/client/auth/exec/exec.go index f21ca89bb..1af2afdb9 100644 --- a/plugin/pkg/client/auth/exec/exec.go +++ b/plugin/pkg/client/auth/exec/exec.go @@ -363,7 +363,6 @@ func (r *roundTripper) RoundTrip(req *http.Request) (*http.Response, error) { return r.base.RoundTrip(req) } - ctx := req.Context() creds, err := r.a.getCreds() if err != nil { return nil, fmt.Errorf("getting credentials: %v", err) @@ -378,7 +377,7 @@ func (r *roundTripper) RoundTrip(req *http.Request) (*http.Response, error) { } if res.StatusCode == http.StatusUnauthorized { if err := r.a.maybeRefreshCreds(creds); err != nil { - klog.FromContext(ctx).Error(err, "Refreshing credentials failed") + klog.Errorf("refreshing credentials: %v", err) } } return res, nil diff --git a/plugin/pkg/client/auth/exec/metrics.go b/plugin/pkg/client/auth/exec/metrics.go index 3cdeeced6..fb300ae12 100644 --- a/plugin/pkg/client/auth/exec/metrics.go +++ b/plugin/pkg/client/auth/exec/metrics.go @@ -112,11 +112,7 @@ func incrementCallsMetric(err error) { metrics.ExecPluginCalls.Increment(failureExitCode, pluginNotFoundError) default: // We don't know about this error type. - // TODO (?): this code gets called through - // https://github.com/kubernetes/kubernetes/blob/8a5cf7b66f4bf8c76c4a78e249ee7c21f7d7403e/staging/src/k8s.io/client-go/transport/config.go#L152-L154 - // which would have to be changed to accept a context. - // Probably not worth it? - klog.TODO().V(2).Info("unexpected exec plugin return error type", "type", reflect.TypeOf(err).String(), "err", err) + klog.V(2).InfoS("unexpected exec plugin return error type", "type", reflect.TypeOf(err).String(), "err", err) metrics.ExecPluginCalls.Increment(failureExitCode, clientInternalError) } } diff --git a/plugin/pkg/client/auth/gcp/gcp_stub.go b/plugin/pkg/client/auth/gcp/gcp_stub.go index 2aa21f003..99585f939 100644 --- a/plugin/pkg/client/auth/gcp/gcp_stub.go +++ b/plugin/pkg/client/auth/gcp/gcp_stub.go @@ -25,7 +25,6 @@ import ( func init() { if err := rest.RegisterAuthProviderPlugin("gcp", newGCPAuthProvider); err != nil { - //nolint:logcheck // Should not happen. klog.Fatalf("Failed to register gcp auth plugin: %v", err) } } diff --git a/plugin/pkg/client/auth/oidc/oidc.go b/plugin/pkg/client/auth/oidc/oidc.go index 70f15c3a3..70e8b57b1 100644 --- a/plugin/pkg/client/auth/oidc/oidc.go +++ b/plugin/pkg/client/auth/oidc/oidc.go @@ -49,7 +49,6 @@ const ( func init() { if err := restclient.RegisterAuthProviderPlugin("oidc", newOIDCAuthProvider); err != nil { - //nolint:logcheck // Should not happen. klog.Fatalf("Failed to register oidc auth plugin: %v", err) } } @@ -126,9 +125,8 @@ func newOIDCAuthProvider(clusterAddress string, cfg map[string]string, persister } if len(cfg[cfgExtraScopes]) > 0 { - // TODO (?): the auth provider factory API would have to be changed - // to support contextual logging here. Not worth it? - klog.TODO().V(2).Info("Ignoring deprecated extra scopes, refresh request don't send scopes", "ignoredConfigField", cfgExtraScopes) + klog.V(2).Infof("%s auth provider field depricated, refresh request don't send scopes", + cfgExtraScopes) } var certAuthData []byte diff --git a/restmapper/category_expansion.go b/restmapper/category_expansion.go index f27f87546..484e4c839 100644 --- a/restmapper/category_expansion.go +++ b/restmapper/category_expansion.go @@ -17,96 +17,38 @@ limitations under the License. package restmapper import ( - "context" - "iter" - "slices" - "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/client-go/discovery" ) // CategoryExpander maps category strings to GroupResources. // Categories are classification or 'tag' of a group of resources. -// -// Deprecated: use CategoryExpanderWithContext instead. type CategoryExpander interface { Expand(category string) ([]schema.GroupResource, bool) } -// CategoryExpanderWithContext maps category strings to GroupResources. -// Categories are classification or 'tag' of a group of resources. -type CategoryExpanderWithContext interface { - ExpandWithContext(ctx context.Context, category string) ([]schema.GroupResource, bool) -} - -func ToCategoryExpanderWithContext(c CategoryExpander) CategoryExpanderWithContext { - if c == nil { - return nil - } - if c == nil { - return nil - } - if c, ok := c.(CategoryExpanderWithContext); ok { - return c - } - return &categoryExpanderWrapper{ - delegate: c, - } -} - -type categoryExpanderWrapper struct { - delegate CategoryExpander -} - -func (c *categoryExpanderWrapper) ExpandWithContext(ctx context.Context, category string) ([]schema.GroupResource, bool) { - return c.delegate.Expand(category) -} - -// SimpleCategoryExpander implements CategoryExpander and CategoryExpanderWithContext interface +// SimpleCategoryExpander implements CategoryExpander interface // using a static mapping of categories to GroupResource mapping. type SimpleCategoryExpander struct { Expansions map[string][]schema.GroupResource } -var ( - _ CategoryExpander = SimpleCategoryExpander{} - _ CategoryExpanderWithContext = SimpleCategoryExpander{} -) - // Expand fulfills CategoryExpander func (e SimpleCategoryExpander) Expand(category string) ([]schema.GroupResource, bool) { ret, ok := e.Expansions[category] return ret, ok } -// Expand fulfills CategoryExpanderWithContext -func (e SimpleCategoryExpander) ExpandWithContext(ctx context.Context, category string) ([]schema.GroupResource, bool) { - return e.Expand(category) -} - // discoveryCategoryExpander struct lets a REST Client wrapper (discoveryClient) to retrieve list of APIResourceList, // and then convert to fallbackExpander type discoveryCategoryExpander struct { - discoveryClient discovery.DiscoveryInterfaceWithContext + discoveryClient discovery.DiscoveryInterface } // NewDiscoveryCategoryExpander returns a category expander that makes use of the "categories" fields from // the API, found through the discovery client. In case of any error or no category found (which likely // means we're at a cluster prior to categories support, fallback to the expander provided. -// -// Deprecated: use NewDiscoveryCategoryExpanderWithContext instead. func NewDiscoveryCategoryExpander(client discovery.DiscoveryInterface) CategoryExpander { - return newDiscoveryCategoryExpander(discovery.ToDiscoveryInterfaceWithContext(client)) -} - -// NewDiscoveryCategoryExpanderWithContext returns a category expander that makes use of the "categories" fields from -// the API, found through the discovery client. In case of any error or no category found (which likely -// means we're at a cluster prior to categories support, fallback to the expander provided. -func NewDiscoveryCategoryExpanderWithContext(client discovery.DiscoveryInterfaceWithContext) CategoryExpanderWithContext { - return newDiscoveryCategoryExpander(client) -} - -func newDiscoveryCategoryExpander(client discovery.DiscoveryInterfaceWithContext) discoveryCategoryExpander { if client == nil { panic("Please provide discovery client to shortcut expander") } @@ -114,16 +56,9 @@ func newDiscoveryCategoryExpander(client discovery.DiscoveryInterfaceWithContext } // Expand fulfills CategoryExpander -// -// Deprecated: use ExpandWithContext instead. func (e discoveryCategoryExpander) Expand(category string) ([]schema.GroupResource, bool) { - return e.ExpandWithContext(context.Background(), category) -} - -// ExpandWithContext fulfills CategoryExpanderWithContext -func (e discoveryCategoryExpander) ExpandWithContext(ctx context.Context, category string) ([]schema.GroupResource, bool) { // Get all supported resources for groups and versions from server, if no resource found, fallback anyway. - _, apiResourceLists, _ := e.discoveryClient.ServerGroupsAndResourcesWithContext(ctx) + _, apiResourceLists, _ := e.discoveryClient.ServerGroupsAndResources() if len(apiResourceLists) == 0 { return nil, false } @@ -154,41 +89,16 @@ func (e discoveryCategoryExpander) ExpandWithContext(ctx context.Context, catego // UnionCategoryExpander implements CategoryExpander interface. // It maps given category string to union of expansions returned by all the CategoryExpanders in the list. -// -// Deprecated: use UnionCategoryWithContext instead. type UnionCategoryExpander []CategoryExpander -var _ CategoryExpander = UnionCategoryExpander{} - // Expand fulfills CategoryExpander func (u UnionCategoryExpander) Expand(category string) ([]schema.GroupResource, bool) { - return unionExpand(context.Background(), func(yield func(i int, expander CategoryExpanderWithContext) bool) { - for i, expander := range u { - if !yield(i, ToCategoryExpanderWithContext(expander)) { - break - } - } - }, category) -} - -// UnionCategoryExpanderWithContext implements CategoryExpanderWithContext interface. -// It maps given category string to union of expansions returned by all the CategoryExpanders in the list. -type UnionCategoryExpanderWithContext []CategoryExpanderWithContext - -var _ CategoryExpanderWithContext = UnionCategoryExpanderWithContext{} - -// ExpandWithContext fulfills CategoryExpanderWithContext -func (u UnionCategoryExpanderWithContext) ExpandWithContext(ctx context.Context, category string) ([]schema.GroupResource, bool) { - return unionExpand(ctx, slices.All(u), category) -} - -func unionExpand(ctx context.Context, expanders iter.Seq2[int, CategoryExpanderWithContext], category string) ([]schema.GroupResource, bool) { ret := []schema.GroupResource{} ok := false // Expand the category for each CategoryExpander in the list and merge/combine the results. - for _, expansion := range expanders { - curr, currOk := expansion.ExpandWithContext(ctx, category) + for _, expansion := range u { + curr, currOk := expansion.Expand(category) for _, currGR := range curr { found := false diff --git a/restmapper/discovery.go b/restmapper/discovery.go index a5d52d6d7..3505178b6 100644 --- a/restmapper/discovery.go +++ b/restmapper/discovery.go @@ -17,7 +17,6 @@ limitations under the License. package restmapper import ( - "context" "fmt" "strings" "sync" @@ -41,38 +40,8 @@ type APIGroupResources struct { // NewDiscoveryRESTMapper returns a PriorityRESTMapper based on the discovered // groups and resources passed in. -// -// Deprecated: use NewDiscoveryRESTMapperWithContext instead. func NewDiscoveryRESTMapper(groupResources []*APIGroupResources) meta.RESTMapper { - mappers, resourcePriority, kindPriority := newDiscoveryRESTMapper(groupResources) - multiMapper := make(meta.MultiRESTMapper, len(mappers)) - for i, m := range mappers { - multiMapper[i] = m - } - return &meta.PriorityRESTMapper{ - Delegate: multiMapper, - ResourcePriority: resourcePriority, - KindPriority: kindPriority, - } -} - -// NewDiscoveryRESTMapperWithContext returns a PriorityRESTMapper based on the discovered -// groups and resources passed in. -func NewDiscoveryRESTMapperWithContext(groupResources []*APIGroupResources) meta.RESTMapperWithContext { - mappers, resourcePriority, kindPriority := newDiscoveryRESTMapper(groupResources) - multiMapper := make(meta.MultiRESTMapperWithContext, len(mappers)) - for i, m := range mappers { - multiMapper[i] = m - } - return &meta.PriorityRESTMapperWithContext{ - Delegate: multiMapper, - ResourcePriority: resourcePriority, - KindPriority: kindPriority, - } -} - -func newDiscoveryRESTMapper(groupResources []*APIGroupResources) ([]*meta.DefaultRESTMapper, []schema.GroupVersionResource, []schema.GroupVersionKind) { - var unionMapper []*meta.DefaultRESTMapper + unionMapper := meta.MultiRESTMapper{} var groupPriority []string // /v1 is special. It should always come first @@ -166,21 +135,17 @@ func newDiscoveryRESTMapper(groupResources []*APIGroupResources) ([]*meta.Defaul }) } - return unionMapper, resourcePriority, kindPriority + return meta.PriorityRESTMapper{ + Delegate: unionMapper, + ResourcePriority: resourcePriority, + KindPriority: kindPriority, + } } // GetAPIGroupResources uses the provided discovery client to gather // discovery information and populate a slice of APIGroupResources. -// -// Deprecated: use GetAPIGroupResourcesWithContext instead. func GetAPIGroupResources(cl discovery.DiscoveryInterface) ([]*APIGroupResources, error) { - return GetAPIGroupResourcesWithContext(context.Background(), discovery.ToDiscoveryInterfaceWithContext(cl)) -} - -// GetAPIGroupResourcesWithContext uses the provided discovery client to gather -// discovery information and populate a slice of APIGroupResources. -func GetAPIGroupResourcesWithContext(ctx context.Context, cl discovery.DiscoveryInterfaceWithContext) ([]*APIGroupResources, error) { - gs, rs, err := cl.ServerGroupsAndResourcesWithContext(ctx) + gs, rs, err := cl.ServerGroupsAndResources() if rs == nil || gs == nil { return nil, err // TODO track the errors and update callers to handle partial errors. @@ -208,42 +173,25 @@ func GetAPIGroupResourcesWithContext(ctx context.Context, cl discovery.Discovery return result, nil } -// DeferredDiscoveryRESTMapper is a RESTMapper and RESTMapperContext that will defer +// DeferredDiscoveryRESTMapper is a RESTMapper that will defer // initialization of the RESTMapper until the first mapping is // requested. type DeferredDiscoveryRESTMapper struct { initMu sync.Mutex - delegate meta.RESTMapperWithContext - cl discovery.CachedDiscoveryInterfaceWithContext + delegate meta.RESTMapper + cl discovery.CachedDiscoveryInterface } -var ( - _ meta.ResettableRESTMapper = &DeferredDiscoveryRESTMapper{} - _ meta.ResettableRESTMapperWithContext = &DeferredDiscoveryRESTMapper{} - _ fmt.Stringer = &DeferredDiscoveryRESTMapper{} -) - // NewDeferredDiscoveryRESTMapper returns a // DeferredDiscoveryRESTMapper that will lazily query the provided // client for discovery information to do REST mappings. -// -// Deprecated: use NewDeferredDiscoveryRESTMapperWithContext instead. NewDeferredDiscoveryRESTMapper will try to convert cl to discovery.CachedDiscoveryInterfaceWithContext and use a wrapper if that is not possible, but NewDeferredDiscoveryRESTMapperWithContext ensures that no such conversion is necessary. func NewDeferredDiscoveryRESTMapper(cl discovery.CachedDiscoveryInterface) *DeferredDiscoveryRESTMapper { - return &DeferredDiscoveryRESTMapper{ - cl: discovery.ToCachedDiscoveryInterfaceWithContext(cl), - } -} - -// NewDeferredDiscoveryRESTMapperWithContext returns a -// DeferredDiscoveryRESTMapper that will lazily query the provided -// client for discovery information to do REST mappings. -func NewDeferredDiscoveryRESTMapperWithContext(cl discovery.CachedDiscoveryInterfaceWithContext) *DeferredDiscoveryRESTMapper { return &DeferredDiscoveryRESTMapper{ cl: cl, } } -func (d *DeferredDiscoveryRESTMapper) getDelegate(ctx context.Context) (meta.RESTMapperWithContext, error) { +func (d *DeferredDiscoveryRESTMapper) getDelegate() (meta.RESTMapper, error) { d.initMu.Lock() defer d.initMu.Unlock() @@ -251,144 +199,98 @@ func (d *DeferredDiscoveryRESTMapper) getDelegate(ctx context.Context) (meta.RES return d.delegate, nil } - groupResources, err := GetAPIGroupResourcesWithContext(ctx, d.cl) + groupResources, err := GetAPIGroupResources(d.cl) if err != nil { return nil, err } - d.delegate = NewDiscoveryRESTMapperWithContext(groupResources) + d.delegate = NewDiscoveryRESTMapper(groupResources) return d.delegate, nil } // Reset resets the internally cached Discovery information and will // cause the next mapping request to re-discover. func (d *DeferredDiscoveryRESTMapper) Reset() { - d.ResetWithContext(context.Background()) -} - -// ResetWithContext resets the internally cached Discovery information and will -// cause the next mapping request to re-discover. -func (d *DeferredDiscoveryRESTMapper) ResetWithContext(ctx context.Context) { - klog.FromContext(ctx).V(5).Info("Invalidating discovery information") + klog.V(5).Info("Invalidating discovery information") d.initMu.Lock() defer d.initMu.Unlock() - d.cl.InvalidateWithContext(ctx) + d.cl.Invalidate() d.delegate = nil } // KindFor takes a partial resource and returns back the single match. // It returns an error if there are multiple matches. -// -// Deprecated: use KindForWithContext instead. func (d *DeferredDiscoveryRESTMapper) KindFor(resource schema.GroupVersionResource) (gvk schema.GroupVersionKind, err error) { - return d.KindForWithContext(context.Background(), resource) -} - -// KindForWithContext takes a partial resource and returns back the single match. -// It returns an error if there are multiple matches. -func (d *DeferredDiscoveryRESTMapper) KindForWithContext(ctx context.Context, resource schema.GroupVersionResource) (gvk schema.GroupVersionKind, err error) { - del, err := d.getDelegate(ctx) + del, err := d.getDelegate() if err != nil { return schema.GroupVersionKind{}, err } - gvk, err = del.KindForWithContext(ctx, resource) - if err != nil && !d.cl.FreshWithContext(ctx) { - d.ResetWithContext(ctx) - gvk, err = d.KindForWithContext(ctx, resource) + gvk, err = del.KindFor(resource) + if err != nil && !d.cl.Fresh() { + d.Reset() + gvk, err = d.KindFor(resource) } return } // KindsFor takes a partial resource and returns back the list of // potential kinds in priority order. -// -// Deprecated: use KindsForWithContext instead. func (d *DeferredDiscoveryRESTMapper) KindsFor(resource schema.GroupVersionResource) (gvks []schema.GroupVersionKind, err error) { - return d.KindsForWithContext(context.Background(), resource) -} - -// KindsForWithContext takes a partial resource and returns back the list of -// potential kinds in priority order. -func (d *DeferredDiscoveryRESTMapper) KindsForWithContext(ctx context.Context, resource schema.GroupVersionResource) (gvks []schema.GroupVersionKind, err error) { - del, err := d.getDelegate(ctx) + del, err := d.getDelegate() if err != nil { return nil, err } - gvks, err = del.KindsForWithContext(ctx, resource) - if len(gvks) == 0 && !d.cl.FreshWithContext(ctx) { - d.ResetWithContext(ctx) - gvks, err = d.KindsForWithContext(ctx, resource) + gvks, err = del.KindsFor(resource) + if len(gvks) == 0 && !d.cl.Fresh() { + d.Reset() + gvks, err = d.KindsFor(resource) } return } // ResourceFor takes a partial resource and returns back the single // match. It returns an error if there are multiple matches. -// -// Deprecated: use ResourceForWithContext instead. func (d *DeferredDiscoveryRESTMapper) ResourceFor(input schema.GroupVersionResource) (gvr schema.GroupVersionResource, err error) { - return d.ResourceForWithContext(context.Background(), input) -} - -// ResourceForWithContext takes a partial resource and returns back the single -// match. It returns an error if there are multiple matches. -func (d *DeferredDiscoveryRESTMapper) ResourceForWithContext(ctx context.Context, input schema.GroupVersionResource) (gvr schema.GroupVersionResource, err error) { - del, err := d.getDelegate(ctx) + del, err := d.getDelegate() if err != nil { return schema.GroupVersionResource{}, err } - gvr, err = del.ResourceForWithContext(ctx, input) - if err != nil && !d.cl.FreshWithContext(ctx) { - d.ResetWithContext(ctx) - gvr, err = d.ResourceForWithContext(ctx, input) + gvr, err = del.ResourceFor(input) + if err != nil && !d.cl.Fresh() { + d.Reset() + gvr, err = d.ResourceFor(input) } return } // ResourcesFor takes a partial resource and returns back the list of // potential resource in priority order. -// -// Deprecated: use ResourcesForWithContext instead. func (d *DeferredDiscoveryRESTMapper) ResourcesFor(input schema.GroupVersionResource) (gvrs []schema.GroupVersionResource, err error) { - return d.ResourcesForWithContext(context.Background(), input) -} - -// ResourcesForWithContext takes a partial resource and returns back the list of -// potential resource in priority order. -func (d *DeferredDiscoveryRESTMapper) ResourcesForWithContext(ctx context.Context, input schema.GroupVersionResource) (gvrs []schema.GroupVersionResource, err error) { - del, err := d.getDelegate(ctx) + del, err := d.getDelegate() if err != nil { return nil, err } - gvrs, err = del.ResourcesForWithContext(ctx, input) - if len(gvrs) == 0 && !d.cl.FreshWithContext(ctx) { - d.ResetWithContext(ctx) - gvrs, err = d.ResourcesForWithContext(ctx, input) + gvrs, err = del.ResourcesFor(input) + if len(gvrs) == 0 && !d.cl.Fresh() { + d.Reset() + gvrs, err = d.ResourcesFor(input) } return } // RESTMapping identifies a preferred resource mapping for the // provided group kind. -// -// Deprecated: use RESTMappingWithContext instead. func (d *DeferredDiscoveryRESTMapper) RESTMapping(gk schema.GroupKind, versions ...string) (m *meta.RESTMapping, err error) { - return d.RESTMappingWithContext(context.Background(), gk, versions...) -} - -// RESTMappingWithContext identifies a preferred resource mapping for the -// provided group kind. -func (d *DeferredDiscoveryRESTMapper) RESTMappingWithContext(ctx context.Context, gk schema.GroupKind, versions ...string) (m *meta.RESTMapping, err error) { - del, err := d.getDelegate(ctx) + del, err := d.getDelegate() if err != nil { return nil, err } - m, err = del.RESTMappingWithContext(ctx, gk, versions...) - if err != nil && !d.cl.FreshWithContext(ctx) { - d.ResetWithContext(ctx) - m, err = d.RESTMappingWithContext(ctx, gk, versions...) + m, err = del.RESTMapping(gk, versions...) + if err != nil && !d.cl.Fresh() { + d.Reset() + m, err = d.RESTMapping(gk, versions...) } return } @@ -396,55 +298,41 @@ func (d *DeferredDiscoveryRESTMapper) RESTMappingWithContext(ctx context.Context // RESTMappings returns the RESTMappings for the provided group kind // in a rough internal preferred order. If no kind is found, it will // return a NoResourceMatchError. -// -// Deprecated: use RESTMappingsWithContext instead. func (d *DeferredDiscoveryRESTMapper) RESTMappings(gk schema.GroupKind, versions ...string) (ms []*meta.RESTMapping, err error) { - return d.RESTMappingsWithContext(context.Background(), gk, versions...) -} - -// RESTMappingsWithContext returns the RESTMappings for the provided group kind -// in a rough internal preferred order. If no kind is found, it will -// return a NoResourceMatchError. -func (d *DeferredDiscoveryRESTMapper) RESTMappingsWithContext(ctx context.Context, gk schema.GroupKind, versions ...string) (ms []*meta.RESTMapping, err error) { - del, err := d.getDelegate(ctx) + del, err := d.getDelegate() if err != nil { return nil, err } - ms, err = del.RESTMappingsWithContext(ctx, gk, versions...) - if len(ms) == 0 && !d.cl.FreshWithContext(ctx) { - d.ResetWithContext(ctx) - ms, err = d.RESTMappingsWithContext(ctx, gk, versions...) + ms, err = del.RESTMappings(gk, versions...) + if len(ms) == 0 && !d.cl.Fresh() { + d.Reset() + ms, err = d.RESTMappings(gk, versions...) } return } // ResourceSingularizer converts a resource name from plural to // singular (e.g., from pods to pod). -// -// Deprecated: use ResourceSingularizerWithContext instead. func (d *DeferredDiscoveryRESTMapper) ResourceSingularizer(resource string) (singular string, err error) { - return d.ResourceSingularizerWithContext(context.Background(), resource) -} - -// ResourceSingularizerWithContext converts a resource name from plural to -// singular (e.g., from pods to pod). -func (d *DeferredDiscoveryRESTMapper) ResourceSingularizerWithContext(ctx context.Context, resource string) (singular string, err error) { - del, err := d.getDelegate(ctx) + del, err := d.getDelegate() if err != nil { return resource, err } - singular, err = del.ResourceSingularizerWithContext(ctx, resource) - if err != nil && !d.cl.FreshWithContext(ctx) { - d.ResetWithContext(ctx) - singular, err = d.ResourceSingularizerWithContext(ctx, resource) + singular, err = del.ResourceSingularizer(resource) + if err != nil && !d.cl.Fresh() { + d.Reset() + singular, err = d.ResourceSingularizer(resource) } return } func (d *DeferredDiscoveryRESTMapper) String() string { - del, err := d.getDelegate(context.Background() /* hopefully we already have a delegate and don't need the context */) + del, err := d.getDelegate() if err != nil { return fmt.Sprintf("DeferredDiscoveryRESTMapper{%v}", err) } return fmt.Sprintf("DeferredDiscoveryRESTMapper{\n\t%v\n}", del) } + +// Make sure it satisfies the interface +var _ meta.ResettableRESTMapper = &DeferredDiscoveryRESTMapper{} diff --git a/restmapper/shortcut.go b/restmapper/shortcut.go index b58afb5d8..0afc8689d 100644 --- a/restmapper/shortcut.go +++ b/restmapper/shortcut.go @@ -17,7 +17,6 @@ limitations under the License. package restmapper import ( - "context" "fmt" "strings" @@ -31,39 +30,22 @@ import ( // shortcutExpander is a RESTMapper that can be used for Kubernetes resources. It expands the resource first, then invokes the wrapped type shortcutExpander struct { - RESTMapper meta.RESTMapperWithContext + RESTMapper meta.RESTMapper - discoveryClient discovery.DiscoveryInterfaceWithContext + discoveryClient discovery.DiscoveryInterface warningHandler func(string) } var _ meta.ResettableRESTMapper = shortcutExpander{} -var _ meta.ResettableRESTMapperWithContext = shortcutExpander{} // NewShortcutExpander wraps a restmapper in a layer that expands shortcuts found via discovery -// -// Deprecated: use NewShortcutExpanderWithContext instead. func NewShortcutExpander(delegate meta.RESTMapper, client discovery.DiscoveryInterface, warningHandler func(string)) meta.RESTMapper { - return newShortcutExpander(meta.ToRESTMapperWithContext(delegate), discovery.ToDiscoveryInterfaceWithContext(client), warningHandler) -} - -// NewShortcutExpanderWithContext wraps a restmapper in a layer that expands shortcuts found via discovery -func NewShortcutExpanderWithContext(delegate meta.RESTMapperWithContext, client discovery.DiscoveryInterfaceWithContext, warningHandler func(string)) meta.RESTMapperWithContext { - return newShortcutExpander(delegate, client, warningHandler) -} - -func newShortcutExpander(delegate meta.RESTMapperWithContext, client discovery.DiscoveryInterfaceWithContext, warningHandler func(string)) shortcutExpander { return shortcutExpander{RESTMapper: delegate, discoveryClient: client, warningHandler: warningHandler} } // KindFor fulfills meta.RESTMapper func (e shortcutExpander) KindFor(resource schema.GroupVersionResource) (schema.GroupVersionKind, error) { - return e.KindForWithContext(context.Background(), resource) -} - -// KindForWithContext fulfills meta.RESTMapperWithContext -func (e shortcutExpander) KindForWithContext(ctx context.Context, resource schema.GroupVersionResource) (schema.GroupVersionKind, error) { // expandResourceShortcut works with current API resources as read from discovery cache. // In case of new CRDs this means we potentially don't have current state of discovery. // In the current wiring in k8s.io/cli-runtime/pkg/genericclioptions/config_flags.go#toRESTMapper, @@ -71,101 +53,59 @@ func (e shortcutExpander) KindForWithContext(ctx context.Context, resource schem // cache and fetch all data from a cluster (see k8s.io/client-go/restmapper/discovery.go#KindFor). // Thus another call to expandResourceShortcut, after a NoMatchError should successfully // read Kind to the user or an error. - gvk, err := e.RESTMapper.KindForWithContext(ctx, e.expandResourceShortcut(ctx, resource)) + gvk, err := e.RESTMapper.KindFor(e.expandResourceShortcut(resource)) if meta.IsNoMatchError(err) { - return e.RESTMapper.KindForWithContext(ctx, e.expandResourceShortcut(ctx, resource)) + return e.RESTMapper.KindFor(e.expandResourceShortcut(resource)) } return gvk, err } // KindsFor fulfills meta.RESTMapper -// -// Deprecated: use KindsForWithContext instead. func (e shortcutExpander) KindsFor(resource schema.GroupVersionResource) ([]schema.GroupVersionKind, error) { - return e.RESTMapper.KindsForWithContext(context.Background(), resource) -} - -// KindsForWithContext fulfills meta.RESTMapperWithContext -func (e shortcutExpander) KindsForWithContext(ctx context.Context, resource schema.GroupVersionResource) ([]schema.GroupVersionKind, error) { - return e.RESTMapper.KindsForWithContext(ctx, e.expandResourceShortcut(ctx, resource)) + return e.RESTMapper.KindsFor(e.expandResourceShortcut(resource)) } // ResourcesFor fulfills meta.RESTMapper -// -// Deprecated: use ResourcesForWithContext instead. func (e shortcutExpander) ResourcesFor(resource schema.GroupVersionResource) ([]schema.GroupVersionResource, error) { - return e.ResourcesForWithContext(context.Background(), resource) -} - -// ResourcesForWithContext fulfills meta.RESTMapperWithContext -func (e shortcutExpander) ResourcesForWithContext(ctx context.Context, resource schema.GroupVersionResource) ([]schema.GroupVersionResource, error) { - return e.RESTMapper.ResourcesForWithContext(ctx, e.expandResourceShortcut(ctx, resource)) + return e.RESTMapper.ResourcesFor(e.expandResourceShortcut(resource)) } // ResourceFor fulfills meta.RESTMapper -// -// Deprecated: use ResourceForWithContext instead. func (e shortcutExpander) ResourceFor(resource schema.GroupVersionResource) (schema.GroupVersionResource, error) { - return e.ResourceForWithContext(context.Background(), resource) -} - -// ResourceForWithContext fulfills meta.RESTMapperWithContext -func (e shortcutExpander) ResourceForWithContext(ctx context.Context, resource schema.GroupVersionResource) (schema.GroupVersionResource, error) { - return e.RESTMapper.ResourceForWithContext(ctx, e.expandResourceShortcut(ctx, resource)) + return e.RESTMapper.ResourceFor(e.expandResourceShortcut(resource)) } // ResourceSingularizer fulfills meta.RESTMapper -// -// Deprecated: use ResourceSingularizerWithContext instead. func (e shortcutExpander) ResourceSingularizer(resource string) (string, error) { - return e.ResourceSingularizerWithContext(context.Background(), resource) -} - -// ResourceSingularizerWithContext fulfills meta.RESTMapperWithContext -func (e shortcutExpander) ResourceSingularizerWithContext(ctx context.Context, resource string) (string, error) { - return e.RESTMapper.ResourceSingularizerWithContext(ctx, e.expandResourceShortcut(ctx, schema.GroupVersionResource{Resource: resource}).Resource) + return e.RESTMapper.ResourceSingularizer(e.expandResourceShortcut(schema.GroupVersionResource{Resource: resource}).Resource) } // RESTMapping fulfills meta.RESTMapper -// -// Deprecated: use RESTMappingWithContext instead. func (e shortcutExpander) RESTMapping(gk schema.GroupKind, versions ...string) (*meta.RESTMapping, error) { - return e.RESTMappingWithContext(context.Background(), gk, versions...) -} - -// RESTMappingWithContext fulfills meta.RESTMapperWithContext -func (e shortcutExpander) RESTMappingWithContext(ctx context.Context, gk schema.GroupKind, versions ...string) (*meta.RESTMapping, error) { - return e.RESTMapper.RESTMappingWithContext(ctx, gk, versions...) + return e.RESTMapper.RESTMapping(gk, versions...) } // RESTMappings fulfills meta.RESTMapper -// -// Deprecated: use KindForWithContext instead. func (e shortcutExpander) RESTMappings(gk schema.GroupKind, versions ...string) ([]*meta.RESTMapping, error) { - return e.RESTMappingsWithContext(context.Background(), gk, versions...) -} - -// RESTMappingsWithContext fulfills meta.RESTMapperWithContext -func (e shortcutExpander) RESTMappingsWithContext(ctx context.Context, gk schema.GroupKind, versions ...string) ([]*meta.RESTMapping, error) { - return e.RESTMapper.RESTMappingsWithContext(ctx, gk, versions...) + return e.RESTMapper.RESTMappings(gk, versions...) } // getShortcutMappings returns a set of tuples which holds short names for resources. // First the list of potential resources will be taken from the API server. // Next we will append the hardcoded list of resources - to be backward compatible with old servers. // NOTE that the list is ordered by group priority. -func (e shortcutExpander) getShortcutMappings(ctx context.Context) ([]*metav1.APIResourceList, []resourceShortcuts, error) { +func (e shortcutExpander) getShortcutMappings() ([]*metav1.APIResourceList, []resourceShortcuts, error) { res := []resourceShortcuts{} // get server resources // This can return an error *and* the results it was able to find. We don't need to fail on the error. - _, apiResList, err := e.discoveryClient.ServerGroupsAndResourcesWithContext(ctx) + _, apiResList, err := e.discoveryClient.ServerGroupsAndResources() if err != nil { - klog.FromContext(ctx).V(1).Info("Error loading discovery information", "err", err) + klog.V(1).Infof("Error loading discovery information: %v", err) } for _, apiResources := range apiResList { gv, err := schema.ParseGroupVersion(apiResources.GroupVersion) if err != nil { - klog.FromContext(ctx).V(1).Info("Unable to parse group/version", "groupVersion", apiResources.GroupVersion, "err", err.Error()) + klog.V(1).Infof("Unable to parse groupversion = %s due to = %s", apiResources.GroupVersion, err.Error()) continue } for _, apiRes := range apiResources.APIResources { @@ -186,9 +126,9 @@ func (e shortcutExpander) getShortcutMappings(ctx context.Context) ([]*metav1.AP // (something that a pkg/api/meta.RESTMapper can understand), if it is // indeed a shortcut. If no match has been found, we will match on group prefixing. // Lastly we will return resource unmodified. -func (e shortcutExpander) expandResourceShortcut(ctx context.Context, resource schema.GroupVersionResource) schema.GroupVersionResource { +func (e shortcutExpander) expandResourceShortcut(resource schema.GroupVersionResource) schema.GroupVersionResource { // get the shortcut mappings and return on first match. - if allResources, shortcutResources, err := e.getShortcutMappings(ctx); err == nil { + if allResources, shortcutResources, err := e.getShortcutMappings(); err == nil { // avoid expanding if there's an exact match to a full resource name for _, apiResources := range allResources { gv, err := schema.ParseGroupVersion(apiResources.GroupVersion) @@ -259,13 +199,8 @@ func (e shortcutExpander) expandResourceShortcut(ctx context.Context, resource s return resource } -// Deprecated: use ResetWithContext instead. func (e shortcutExpander) Reset() { - e.ResetWithContext(context.Background()) -} - -func (e shortcutExpander) ResetWithContext(ctx context.Context) { - meta.MaybeResetRESTMapperWithContext(ctx, e.RESTMapper) + meta.MaybeResetRESTMapper(e.RESTMapper) } // ResourceShortcuts represents a structure that holds the information how to diff --git a/restmapper/shortcut_test.go b/restmapper/shortcut_test.go index 0a47fad06..e1f87ae26 100644 --- a/restmapper/shortcut_test.go +++ b/restmapper/shortcut_test.go @@ -31,12 +31,9 @@ import ( "k8s.io/client-go/openapi" restclient "k8s.io/client-go/rest" "k8s.io/client-go/rest/fake" - "k8s.io/klog/v2/ktesting" ) func TestReplaceAliases(t *testing.T) { - _, ctx := ktesting.NewTestContext(t) - tests := []struct { name string arg string @@ -138,7 +135,7 @@ func TestReplaceAliases(t *testing.T) { } mapper := NewShortcutExpander(&fakeRESTMapper{}, ds, nil).(shortcutExpander) - actual := mapper.expandResourceShortcut(ctx, schema.GroupVersionResource{Resource: test.arg}) + actual := mapper.expandResourceShortcut(schema.GroupVersionResource{Resource: test.arg}) if actual != test.expected { t.Errorf("%s: unexpected argument: expected %s, got %s", test.name, test.expected, actual) } @@ -263,8 +260,6 @@ func TestKindForWithNewCRDs(t *testing.T) { } func TestWarnAmbigious(t *testing.T) { - _, ctx := ktesting.NewTestContext(t) - tests := []struct { name string arg string @@ -444,7 +439,7 @@ func TestWarnAmbigious(t *testing.T) { actualWarnings = append(actualWarnings, a) }).(shortcutExpander) - actual := mapper.expandResourceShortcut(ctx, schema.GroupVersionResource{Resource: test.arg}) + actual := mapper.expandResourceShortcut(schema.GroupVersionResource{Resource: test.arg}) if actual != test.expected { t.Errorf("%s: unexpected argument: expected %s, got %s", test.name, test.expected, actual) } diff --git a/tools/cache/listwatch.go b/tools/cache/listwatch.go index 48f3e7a9e..2c4065f01 100644 --- a/tools/cache/listwatch.go +++ b/tools/cache/listwatch.go @@ -90,7 +90,7 @@ func ToWatcherWithContext(w Watcher) WatcherWithContext { if w, ok := w.(WatcherWithContext); ok { return w } - return &watcherWrapper{ + return watcherWrapper{ parent: w, } } @@ -99,7 +99,7 @@ type watcherWrapper struct { parent Watcher } -func (l *watcherWrapper) WatchWithContext(ctx context.Context, options metav1.ListOptions) (watch.Interface, error) { +func (l watcherWrapper) WatchWithContext(ctx context.Context, options metav1.ListOptions) (watch.Interface, error) { return l.parent.Watch(options) } @@ -121,7 +121,7 @@ func ToListerWatcherWithContext(lw ListerWatcher) ListerWatcherWithContext { if lw, ok := lw.(ListerWatcherWithContext); ok { return lw } - return &listerWatcherWrapper{ + return listerWatcherWrapper{ ListerWithContext: ToListerWithContext(lw), WatcherWithContext: ToWatcherWithContext(lw), } diff --git a/tools/clientcmd/client_config.go b/tools/clientcmd/client_config.go index efac2830b..ed35891e5 100644 --- a/tools/clientcmd/client_config.go +++ b/tools/clientcmd/client_config.go @@ -679,13 +679,11 @@ func (config *inClusterClientConfig) Possible() bool { // to the default config. func BuildConfigFromFlags(masterUrl, kubeconfigPath string) (*restclient.Config, error) { if kubeconfigPath == "" && masterUrl == "" { - //nolint:logcheck // A helper function like this should not log. But this is probably part of the the established client-go API and not worth changing. klog.Warning("Neither --kubeconfig nor --master was specified. Using the inClusterConfig. This might not work.") kubeconfig, err := restclient.InClusterConfig() if err == nil { return kubeconfig, nil } - //nolint:logcheck // A helper function like this should not log. But this is probably part of the the established client-go API and not worth changing. klog.Warning("error creating inClusterConfig, falling back to default config: ", err) } return NewNonInteractiveDeferredLoadingClientConfig( diff --git a/tools/clientcmd/config.go b/tools/clientcmd/config.go index 828ac974d..2cd213ccb 100644 --- a/tools/clientcmd/config.go +++ b/tools/clientcmd/config.go @@ -492,7 +492,6 @@ func getConfigFromFile(filename string) (*clientcmdapi.Config, error) { func GetConfigFromFileOrDie(filename string) *clientcmdapi.Config { config, err := getConfigFromFile(filename) if err != nil { - //nolint:logcheck // A helper function like this should not log. But this is probably part of the the established client-go API and not worth changing. klog.FatalDepth(1, err) } diff --git a/tools/clientcmd/loader.go b/tools/clientcmd/loader.go index d1d0a8295..b127e2e08 100644 --- a/tools/clientcmd/loader.go +++ b/tools/clientcmd/loader.go @@ -137,7 +137,6 @@ type WarningHandler func(error) func (handler WarningHandler) Warn(err error) { if handler == nil { - //nolint:logcheck // This is the fallback when logging is not initialized. With nothing provided, using the global logger is the only option. klog.V(1).Info(err) } else { handler(err) @@ -403,7 +402,6 @@ func LoadFromFile(filename string) (*clientcmdapi.Config, error) { if err != nil { return nil, err } - //nolint:logcheck // A helper function like this should not log. But this is probably part of the the established client-go API and not worth changing. klog.V(6).Infoln("Config loaded from file: ", filename) // set LocationOfOrigin on every Cluster, User, and Context diff --git a/tools/clientcmd/loader_test.go b/tools/clientcmd/loader_test.go index 22006aa6c..e9d78bbe8 100644 --- a/tools/clientcmd/loader_test.go +++ b/tools/clientcmd/loader_test.go @@ -122,7 +122,6 @@ func TestNonExistentCommandLineFile(t *testing.T) { } } -//nolint:logcheck // Tests klog APIs. func TestToleratingMissingFiles(t *testing.T) { envVarValue := "bogus" loadingRules := ClientConfigLoadingRules{ @@ -147,7 +146,6 @@ func TestToleratingMissingFiles(t *testing.T) { } } -//nolint:logcheck // Tests klog APIs. func TestWarningMissingFiles(t *testing.T) { envVarValue := "bogus" t.Setenv(RecommendedConfigPathEnvVar, envVarValue) @@ -173,7 +171,6 @@ func TestWarningMissingFiles(t *testing.T) { } } -//nolint:logcheck // Tests klog APIs. func TestNoWarningMissingFiles(t *testing.T) { envVarValue := "bogus" t.Setenv(RecommendedConfigPathEnvVar, envVarValue) diff --git a/tools/clientcmd/merged_client_builder.go b/tools/clientcmd/merged_client_builder.go index db7bf0a82..0fc2fd0a0 100644 --- a/tools/clientcmd/merged_client_builder.go +++ b/tools/clientcmd/merged_client_builder.go @@ -118,7 +118,6 @@ func (config *DeferredLoadingClientConfig) ClientConfig() (*restclient.Config, e // check for in-cluster configuration and use it if config.icc.Possible() { - //nolint:logcheck // A helper function like this should not log. But this is probably part of the the established client-go API and not worth changing. klog.V(4).Infof("Using in-cluster configuration") return config.icc.ClientConfig() } @@ -161,7 +160,6 @@ func (config *DeferredLoadingClientConfig) Namespace() (string, bool, error) { } } - //nolint:logcheck // A helper function like this should not log. But this is probably part of the the established client-go API and not worth changing. klog.V(4).Infof("Using in-cluster namespace") // allow the namespace from the service account token directory to be used. diff --git a/tools/portforward/fallback_dialer.go b/tools/portforward/fallback_dialer.go index e5e0f0866..7fcc2492b 100644 --- a/tools/portforward/fallback_dialer.go +++ b/tools/portforward/fallback_dialer.go @@ -50,7 +50,6 @@ func NewFallbackDialer(primary, secondary httpstream.Dialer, shouldFallback func func (f *FallbackDialer) Dial(protocols ...string) (httpstream.Connection, string, error) { conn, version, err := f.primary.Dial(protocols...) if err != nil && f.shouldFallback(err) { - //nolint:logcheck // This code is only used by kubectl where contextual logging is not that useful. klog.V(4).Infof("fallback to secondary dialer from primary dialer err: %v", err) return f.secondary.Dial(protocols...) } diff --git a/tools/portforward/fallback_dialer_test.go b/tools/portforward/fallback_dialer_test.go index 3e3b27204..70583958e 100644 --- a/tools/portforward/fallback_dialer_test.go +++ b/tools/portforward/fallback_dialer_test.go @@ -40,7 +40,7 @@ func TestFallbackDialer(t *testing.T) { assert.Equal(t, primaryProtocol, negotiated, "primary negotiated protocol returned") require.NoError(t, err, "error from primary dialer should be nil") // If primary dialer error is upgrade error, then fallback returning secondary dial response. - primary = &fakeDialer{dialed: false, negotiatedProtocol: primaryProtocol, err: &httpstream.UpgradeFailureError{Cause: fmt.Errorf("fake error")}} + primary = &fakeDialer{dialed: false, negotiatedProtocol: primaryProtocol, err: &httpstream.UpgradeFailureError{}} secondary = &fakeDialer{dialed: false, negotiatedProtocol: secondaryProtocol} fallbackDialer = NewFallbackDialer(primary, secondary, httpstream.IsUpgradeFailure) _, negotiated, err = fallbackDialer.Dial(protocols...) diff --git a/tools/portforward/portforward.go b/tools/portforward/portforward.go index c1e89a92b..126c14e8f 100644 --- a/tools/portforward/portforward.go +++ b/tools/portforward/portforward.go @@ -17,7 +17,6 @@ limitations under the License. package portforward import ( - "context" "errors" "fmt" "io" @@ -31,8 +30,6 @@ import ( v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/util/httpstream" "k8s.io/apimachinery/pkg/util/runtime" - "k8s.io/apimachinery/pkg/util/wait" - "k8s.io/klog/v2" netutils "k8s.io/utils/net" ) @@ -55,7 +52,6 @@ type PortForwarder struct { ports []ForwardedPort stopChan <-chan struct{} - logger klog.Logger dialer httpstream.Dialer streamConn httpstream.Connection listeners []io.Closer @@ -169,14 +165,7 @@ func New(dialer httpstream.Dialer, ports []string, stopChan <-chan struct{}, rea } // NewOnAddresses creates a new PortForwarder with custom listen addresses. -// -//logcheck:context // NewOnAddressesWithContext should be used instead of NewOnAddresses in code which supports contextual logging. func NewOnAddresses(dialer httpstream.Dialer, addresses []string, ports []string, stopChan <-chan struct{}, readyChan chan struct{}, out, errOut io.Writer) (*PortForwarder, error) { - return NewOnAddressesWithContext(wait.ContextForChannel(stopChan), dialer, addresses, ports, readyChan, out, errOut) -} - -// NewOnAddressesWithContext creates a new PortForwarder with custom listen addresses. -func NewOnAddressesWithContext(ctx context.Context, dialer httpstream.Dialer, addresses []string, ports []string, readyChan chan struct{}, out, errOut io.Writer) (*PortForwarder, error) { if len(addresses) == 0 { return nil, errors.New("you must specify at least 1 address") } @@ -192,11 +181,10 @@ func NewOnAddressesWithContext(ctx context.Context, dialer httpstream.Dialer, ad return nil, err } return &PortForwarder{ - logger: klog.FromContext(ctx), dialer: dialer, addresses: parsedAddresses, ports: parsedPorts, - stopChan: ctx.Done(), + stopChan: stopChan, Ready: readyChan, out: out, errOut: errOut, @@ -331,7 +319,7 @@ func (pf *PortForwarder) waitForConnection(listener net.Listener, port Forwarded if err != nil { // TODO consider using something like https://github.com/hydrogen18/stoppableListener? if !strings.Contains(strings.ToLower(err.Error()), networkClosedError) { - runtime.HandleErrorWithLogger(pf.logger, err, "Error accepting connection", "localPort", port.Local) + runtime.HandleError(fmt.Errorf("error accepting connection on port %d: %v", port.Local, err)) } return } @@ -366,23 +354,21 @@ func (pf *PortForwarder) handleConnection(conn net.Conn, port ForwardedPort) { headers.Set(v1.PortForwardRequestIDHeader, strconv.Itoa(requestID)) errorStream, err := pf.streamConn.CreateStream(headers) if err != nil { - runtime.HandleErrorWithLogger(pf.logger, err, "Error creating error stream", "localPort", port.Local, "remotePort", port.Remote) + runtime.HandleError(fmt.Errorf("error creating error stream for port %d -> %d: %v", port.Local, port.Remote, err)) return } // we're not writing to this stream errorStream.Close() defer pf.streamConn.RemoveStreams(errorStream) - type readAllResult struct { - message []byte - err error - } - errorChan := make(chan readAllResult) + errorChan := make(chan error) go func() { message, err := io.ReadAll(errorStream) - errorChan <- readAllResult{ - message: message, - err: err, + switch { + case err != nil: + errorChan <- fmt.Errorf("error reading from error stream for port %d -> %d: %v", port.Local, port.Remote, err) + case len(message) > 0: + errorChan <- fmt.Errorf("an error occurred forwarding %d -> %d: %v", port.Local, port.Remote, string(message)) } close(errorChan) }() @@ -391,7 +377,7 @@ func (pf *PortForwarder) handleConnection(conn net.Conn, port ForwardedPort) { headers.Set(v1.StreamType, v1.StreamTypeData) dataStream, err := pf.streamConn.CreateStream(headers) if err != nil { - runtime.HandleErrorWithLogger(pf.logger, err, "Error creating forwarding stream", "localPort", port.Local, "remotePort", port.Remote) + runtime.HandleError(fmt.Errorf("error creating forwarding stream for port %d -> %d: %v", port.Local, port.Remote, err)) return } defer pf.streamConn.RemoveStreams(dataStream) @@ -402,7 +388,7 @@ func (pf *PortForwarder) handleConnection(conn net.Conn, port ForwardedPort) { go func() { // Copy from the remote side to the local port. if _, err := io.Copy(conn, dataStream); err != nil && !strings.Contains(strings.ToLower(err.Error()), networkClosedError) { - runtime.HandleErrorWithLogger(pf.logger, err, "Error copying from remote stream to local connection", "localPort", port.Local, "remotePort", port.Remote) + runtime.HandleError(fmt.Errorf("error copying from remote stream to local connection: %v", err)) } // inform the select below that the remote copy is done @@ -415,7 +401,7 @@ func (pf *PortForwarder) handleConnection(conn net.Conn, port ForwardedPort) { // Copy from the local port to the remote side. if _, err := io.Copy(dataStream, conn); err != nil && !strings.Contains(strings.ToLower(err.Error()), networkClosedError) { - runtime.HandleErrorWithLogger(pf.logger, err, "Error copying from local connection to remote stream", "localPort", port.Local, "remotePort", port.Remote) + runtime.HandleError(fmt.Errorf("error copying from local connection to remote stream: %v", err)) // break out of the select below without waiting for the other copy to finish close(localError) } @@ -432,14 +418,10 @@ func (pf *PortForwarder) handleConnection(conn net.Conn, port ForwardedPort) { // the blocking data will affect errorStream and cause <-errorChan to block indefinitely. _ = dataStream.Reset() - // always expect something on errorChan (it may be empty) - errResult := <-errorChan - switch { - case errResult.err != nil: - runtime.HandleErrorWithLogger(pf.logger, errResult.err, "Error reading from error stream", "localPort", port.Local, "remotePort", port.Remote) - pf.streamConn.Close() - case len(errResult.message) > 0: - runtime.HandleErrorWithLogger(pf.logger, errors.New(string(errResult.message)), "An error occurred forwarding", "localPort", port.Local, "remotePort", port.Remote) + // always expect something on errorChan (it may be nil) + err = <-errorChan + if err != nil { + runtime.HandleError(err) pf.streamConn.Close() } } @@ -449,7 +431,7 @@ func (pf *PortForwarder) Close() { // stop all listeners for _, l := range pf.listeners { if err := l.Close(); err != nil { - runtime.HandleErrorWithLogger(pf.logger, err, "Error closing listener") + runtime.HandleError(fmt.Errorf("error closing listener: %v", err)) } } } diff --git a/tools/portforward/portforward_test.go b/tools/portforward/portforward_test.go index 96db9dff1..075a22e62 100644 --- a/tools/portforward/portforward_test.go +++ b/tools/portforward/portforward_test.go @@ -299,7 +299,6 @@ func TestParsePortsAndNew(t *testing.T) { var pf *PortForwarder if len(test.addresses) > 0 { - //nolint:logcheck // Testing the original function. pf, err = NewOnAddresses(dialer, test.addresses, test.input, expectedStopChan, readyChan, os.Stdout, os.Stderr) } else { pf, err = New(dialer, test.input, expectedStopChan, readyChan, os.Stdout, os.Stderr) diff --git a/tools/portforward/tunneling_connection.go b/tools/portforward/tunneling_connection.go index b90306919..a9c9b18fd 100644 --- a/tools/portforward/tunneling_connection.go +++ b/tools/portforward/tunneling_connection.go @@ -34,7 +34,7 @@ var _ net.Conn = &TunnelingConnection{} // TunnelingConnection implements the "httpstream.Connection" interface, wrapping // a websocket connection that tunnels SPDY. type TunnelingConnection struct { - logger klog.Logger + name string conn *gwebsocket.Conn inProgressMessage io.Reader closeOnce sync.Once @@ -42,46 +42,29 @@ type TunnelingConnection struct { // NewTunnelingConnection wraps the passed gorilla/websockets connection // with the TunnelingConnection struct (implementing net.Conn). -// The name is added to all log entries with [klog.LoggerWithName]. -// -//logcheck:context // NewTunnelingConnectionWithLogger should be used instead of NewTunnelingConnection in code which supports contextual logging. func NewTunnelingConnection(name string, conn *gwebsocket.Conn) *TunnelingConnection { - logger := klog.LoggerWithName(klog.Background(), name) - return NewTunnelingConnectionWithLogger(logger, conn) -} - -// NewTunnelingConnectionWithLogger is a variant of NewTunnelingConnection where -// the caller is in control of logging. For example, [klog.LoggerWithName] can be used -// to add a common name for all log entries to identify the connection. -func NewTunnelingConnectionWithLogger(logger klog.Logger, conn *gwebsocket.Conn) *TunnelingConnection { return &TunnelingConnection{ - logger: logger, - conn: conn, + name: name, + conn: conn, } } // Read implements "io.Reader" interface, reading from the stored connection // into the passed buffer "p". Returns the number of bytes read and an error. // Can keep track of the "inProgress" messsage from the tunneled connection. -func (c *TunnelingConnection) Read(p []byte) (len int, err error) { - c.logger.V(7).Info("Tunneling connection read...") - defer func() { - if loggerV := c.logger.V(8); loggerV.Enabled() { - loggerV.Info("Tunneling connection read...complete", "length", len, "data", p[:len], "err", err) - } else { - c.logger.V(7).Info("Tunneling connection read...complete") - } - }() +func (c *TunnelingConnection) Read(p []byte) (int, error) { + klog.V(7).Infof("%s: tunneling connection read...", c.name) + defer klog.V(7).Infof("%s: tunneling connection read...complete", c.name) for { if c.inProgressMessage == nil { - c.logger.V(8).Info("Tunneling connection read before NextReader()...") + klog.V(8).Infof("%s: tunneling connection read before NextReader()...", c.name) messageType, nextReader, err := c.conn.NextReader() if err != nil { closeError := &gwebsocket.CloseError{} if errors.As(err, &closeError) && closeError.Code == gwebsocket.CloseNormalClosure { return 0, io.EOF } - c.logger.V(4).Info("Tunneling connection NextReader() failed", "err", err) + klog.V(4).Infof("%s:tunneling connection NextReader() error: %v", c.name, err) return 0, err } if messageType != gwebsocket.BinaryMessage { @@ -89,11 +72,12 @@ func (c *TunnelingConnection) Read(p []byte) (len int, err error) { } c.inProgressMessage = nextReader } - c.logger.V(8).Info("Tunneling connection read in progress...") + klog.V(8).Infof("%s: tunneling connection read in progress message...", c.name) i, err := c.inProgressMessage.Read(p) if i == 0 && err == io.EOF { c.inProgressMessage = nil } else { + klog.V(8).Infof("%s: read %d bytes, error=%v, bytes=% X", c.name, i, err, p[:i]) return i, err } } @@ -103,8 +87,8 @@ func (c *TunnelingConnection) Read(p []byte) (len int, err error) { // byte array "p" into the stored tunneled connection. Returns the number // of bytes written and an error. func (c *TunnelingConnection) Write(p []byte) (n int, err error) { - c.logger.V(7).Info("Tunneling connection write", "length", len(p), "data", p) - defer c.logger.V(7).Info("Tunneling connection write...complete") + klog.V(7).Infof("%s: write: %d bytes, bytes=% X", c.name, len(p), p) + defer klog.V(7).Infof("%s: tunneling connection write...complete", c.name) w, err := c.conn.NextWriter(gwebsocket.BinaryMessage) if err != nil { return 0, err @@ -127,7 +111,7 @@ func (c *TunnelingConnection) Write(p []byte) (n int, err error) { func (c *TunnelingConnection) Close() error { var err error c.closeOnce.Do(func() { - c.logger.V(7).Info("Tunneling connection Close()...") + klog.V(7).Infof("%s: tunneling connection Close()...", c.name) // Signal other endpoint that websocket connection is closing; ignore error. normalCloseMsg := gwebsocket.FormatCloseMessage(gwebsocket.CloseNormalClosure, "") writeControlErr := c.conn.WriteControl(gwebsocket.CloseMessage, normalCloseMsg, time.Now().Add(time.Second)) diff --git a/tools/portforward/tunneling_connection_test.go b/tools/portforward/tunneling_connection_test.go index 0852404c6..afbdb6b0c 100644 --- a/tools/portforward/tunneling_connection_test.go +++ b/tools/portforward/tunneling_connection_test.go @@ -36,13 +36,8 @@ import ( "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/rest" "k8s.io/client-go/transport/websocket" - "k8s.io/klog/v2" ) -func init() { - klog.InitFlags(nil) -} - func TestTunnelingConnection_ReadWriteClose(t *testing.T) { // Stream channel that will receive streams created on upstream SPDY server. streamChan := make(chan httpstream.Stream) @@ -65,7 +60,7 @@ func TestTunnelingConnection_ReadWriteClose(t *testing.T) { t.Errorf("Not acceptable agreement Subprotocol: %v", conn.Subprotocol()) return } - tunnelingConn := NewTunnelingConnectionWithLogger(klog.LoggerWithName(klog.Background(), "server"), conn) + tunnelingConn := NewTunnelingConnection("server", conn) spdyConn, err := spdy.NewServerConnection(tunnelingConn, justQueueStream(streamChan)) if err != nil { t.Errorf("unexpected error %v", err) @@ -78,7 +73,6 @@ func TestTunnelingConnection_ReadWriteClose(t *testing.T) { // Dial the client tunneling connection to the tunneling server. url, err := url.Parse(tunnelingServer.URL) require.NoError(t, err) - //nolint:logcheck // Intentionally uses the old API. dialer, err := NewSPDYOverWebsocketDialer(url, &rest.Config{Host: url.Host}) require.NoError(t, err) spdyClient, protocol, err := dialer.Dial(constants.PortForwardV1Name) @@ -211,7 +205,6 @@ func dialForTunnelingConnection(url *url.URL) (*TunnelingConnection, error) { if err != nil { return nil, err } - //nolint:logcheck // Intentionally uses the old API. return NewTunnelingConnection("client", conn), nil } diff --git a/tools/portforward/tunneling_dialer.go b/tools/portforward/tunneling_dialer.go index ce3f9c503..2bef5ecd7 100644 --- a/tools/portforward/tunneling_dialer.go +++ b/tools/portforward/tunneling_dialer.go @@ -17,7 +17,6 @@ limitations under the License. package portforward import ( - "context" "fmt" "net/http" "net/url" @@ -36,7 +35,6 @@ const PingPeriod = 10 * time.Second // tunnelingDialer implements "httpstream.Dial" interface type tunnelingDialer struct { - logger klog.Logger url *url.URL transport http.RoundTripper holder websocket.ConnectionHolder @@ -45,22 +43,12 @@ type tunnelingDialer struct { // NewTunnelingDialer creates and returns the tunnelingDialer structure which implemements the "httpstream.Dialer" // interface. The dialer can upgrade a websocket request, creating a websocket connection. This function // returns an error if one occurs. -// -//logcheck:context // NewSPDYOverWebsocketDialerWithLogger should be used instead of NewSPDYOverWebsocketDialer in code which supports contextual logging. func NewSPDYOverWebsocketDialer(url *url.URL, config *restclient.Config) (httpstream.Dialer, error) { - return NewSPDYOverWebsocketDialerWithLogger(klog.Background(), url, config) -} - -// NewTunnelingDialer creates and returns the tunnelingDialer structure which implemements the "httpstream.Dialer" -// interface. The dialer can upgrade a websocket request, creating a websocket connection. This function -// returns an error if one occurs. -func NewSPDYOverWebsocketDialerWithLogger(logger klog.Logger, url *url.URL, config *restclient.Config) (httpstream.Dialer, error) { transport, holder, err := websocket.RoundTripperFor(config) if err != nil { return nil, err } return &tunnelingDialer{ - logger: logger, url: url, transport: transport, holder: holder, @@ -71,10 +59,9 @@ func NewSPDYOverWebsocketDialerWithLogger(logger klog.Logger, url *url.URL, conf // containing a WebSockets connection (which implements "net.Conn"). Also // returns the protocol negotiated, or an error. func (d *tunnelingDialer) Dial(protocols ...string) (httpstream.Connection, string, error) { - // There is no passed context, so use the background context when creating request for now. - ctx := klog.NewContext(context.Background(), d.logger) + // There is no passed context, so skip the context when creating request for now. // Websockets requires "GET" method: RFC 6455 Sec. 4.1 (page 17). - req, err := http.NewRequestWithContext(ctx, "GET", d.url.String(), nil) + req, err := http.NewRequest("GET", d.url.String(), nil) if err != nil { return nil, "", err } @@ -85,7 +72,7 @@ func (d *tunnelingDialer) Dial(protocols ...string) (httpstream.Connection, stri tunnelingProtocol := constants.WebsocketsSPDYTunnelingPrefix + protocol tunnelingProtocols = append(tunnelingProtocols, tunnelingProtocol) } - d.logger.V(4).Info("Before WebSocket Upgrade Connection...") + klog.V(4).Infoln("Before WebSocket Upgrade Connection...") conn, err := websocket.Negotiate(d.transport, d.holder, req, tunnelingProtocols...) if err != nil { return nil, "", err @@ -95,10 +82,10 @@ func (d *tunnelingDialer) Dial(protocols ...string) (httpstream.Connection, stri } protocol := conn.Subprotocol() protocol = strings.TrimPrefix(protocol, constants.WebsocketsSPDYTunnelingPrefix) - d.logger.V(4).Info("Negotiation complete", "protocol", protocol) + klog.V(4).Infof("negotiated protocol: %s", protocol) // Wrap the websocket connection which implements "net.Conn". - tConn := NewTunnelingConnectionWithLogger(klog.LoggerWithName(d.logger, "client"), conn) + tConn := NewTunnelingConnection("client", conn) // Create SPDY connection injecting the previously created tunneling connection. spdyConn, err := spdy.NewClientConnectionWithPings(tConn, PingPeriod) diff --git a/tools/remotecommand/errorstream.go b/tools/remotecommand/errorstream.go index 23dd50778..90bb39b4a 100644 --- a/tools/remotecommand/errorstream.go +++ b/tools/remotecommand/errorstream.go @@ -21,7 +21,6 @@ import ( "io" "k8s.io/apimachinery/pkg/util/runtime" - "k8s.io/klog/v2" ) // errorStreamDecoder interprets the data on the error channel and creates a go error object from it. @@ -33,11 +32,11 @@ type errorStreamDecoder interface { // decodes it with the given errorStreamDecoder, sends the decoded error (or nil if the remote // command exited successfully) to the returned error channel, and closes it. // This function returns immediately. -func watchErrorStream(logger klog.Logger, errorStream io.Reader, d errorStreamDecoder) chan error { +func watchErrorStream(errorStream io.Reader, d errorStreamDecoder) chan error { errorChan := make(chan error) go func() { - defer runtime.HandleCrashWithLogger(logger) + defer runtime.HandleCrash() message, err := io.ReadAll(errorStream) switch { diff --git a/tools/remotecommand/fallback.go b/tools/remotecommand/fallback.go index bcd5fd313..503323080 100644 --- a/tools/remotecommand/fallback.go +++ b/tools/remotecommand/fallback.go @@ -53,7 +53,7 @@ func (f *FallbackExecutor) Stream(options StreamOptions) error { func (f *FallbackExecutor) StreamWithContext(ctx context.Context, options StreamOptions) error { err := f.primary.StreamWithContext(ctx, options) if err != nil && f.shouldFallback(err) { - klog.FromContext(ctx).V(4).Info("RemoteCommand fallback", "err", err) + klog.V(4).Infof("RemoteCommand fallback: %v", err) return f.secondary.StreamWithContext(ctx, options) } return err diff --git a/tools/remotecommand/remotecommand.go b/tools/remotecommand/remotecommand.go index 8663b88a8..4cff05cd8 100644 --- a/tools/remotecommand/remotecommand.go +++ b/tools/remotecommand/remotecommand.go @@ -22,7 +22,6 @@ import ( "net/http" "k8s.io/apimachinery/pkg/util/httpstream" - "k8s.io/klog/v2" ) // StreamOptions holds information pertaining to the current streaming session: @@ -55,5 +54,5 @@ type streamCreator interface { } type streamProtocolHandler interface { - stream(logger klog.Logger, conn streamCreator, ready chan<- struct{}) error + stream(conn streamCreator, ready chan<- struct{}) error } diff --git a/tools/remotecommand/spdy.go b/tools/remotecommand/spdy.go index 2f36e925d..34825771a 100644 --- a/tools/remotecommand/spdy.go +++ b/tools/remotecommand/spdy.go @@ -121,7 +121,6 @@ func (e *spdyStreamExecutor) newConnectionAndStream(ctx context.Context, options var streamer streamProtocolHandler - logger := klog.FromContext(ctx) switch protocol { case remotecommand.StreamProtocolV5Name: streamer = newStreamProtocolV5(options) @@ -132,7 +131,7 @@ func (e *spdyStreamExecutor) newConnectionAndStream(ctx context.Context, options case remotecommand.StreamProtocolV2Name: streamer = newStreamProtocolV2(options) case "": - logger.V(4).Info("The server did not negotiate a streaming protocol version, falling back", "protocol", remotecommand.StreamProtocolV1Name) + klog.V(4).Infof("The server did not negotiate a streaming protocol version. Falling back to %s", remotecommand.StreamProtocolV1Name) fallthrough case remotecommand.StreamProtocolV1Name: streamer = newStreamProtocolV1(options) @@ -162,7 +161,7 @@ func (e *spdyStreamExecutor) StreamWithContext(ctx context.Context, options Stre // The SPDY executor does not need to synchronize stream creation, so we pass a nil // ready channel. The underlying spdystream library handles stream multiplexing // without a race condition. - errorChan <- streamer.stream(klog.FromContext(ctx), conn, nil) + errorChan <- streamer.stream(conn, nil) }() select { diff --git a/tools/remotecommand/spdy_test.go b/tools/remotecommand/spdy_test.go index 35cd09a9f..9948832a5 100644 --- a/tools/remotecommand/spdy_test.go +++ b/tools/remotecommand/spdy_test.go @@ -38,7 +38,6 @@ import ( remotecommandconsts "k8s.io/apimachinery/pkg/util/remotecommand" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/rest" - "k8s.io/klog/v2/ktesting" ) type AttachFunc func(in io.Reader, out, err io.WriteCloser, tty bool, resize <-chan TerminalSize) error @@ -329,7 +328,6 @@ func (w *writeDetector) Write(p []byte) (n int, err error) { // and expects the deferred close of the connection leads to the exit of the goroutine on cancellation. // This test verifies that works. func TestStreamExitsAfterConnectionIsClosed(t *testing.T) { - logger, _ := ktesting.NewTestContext(t) writeDetector := newWriterDetector(&fakeEmptyDataPty{}) options := StreamOptions{ Stdin: &fakeEmptyDataPty{}, @@ -354,7 +352,7 @@ func TestStreamExitsAfterConnectionIsClosed(t *testing.T) { errorChan := make(chan error) go func() { - errorChan <- streamer.stream(logger, conn, nil) + errorChan <- streamer.stream(conn, nil) }() // Wait until stream goroutine starts. diff --git a/tools/remotecommand/v1.go b/tools/remotecommand/v1.go index 60891fbe5..293d809dd 100644 --- a/tools/remotecommand/v1.go +++ b/tools/remotecommand/v1.go @@ -21,7 +21,7 @@ import ( "io" "net/http" - v1 "k8s.io/api/core/v1" + "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/util/httpstream" "k8s.io/klog/v2" ) @@ -31,7 +31,6 @@ import ( // non-interactive stdin data has ended. See https://issues.k8s.io/13394 and // https://issues.k8s.io/13395 for more details. type streamProtocolV1 struct { - logger klog.Logger StreamOptions errorStream httpstream.Stream @@ -48,15 +47,15 @@ func newStreamProtocolV1(options StreamOptions) streamProtocolHandler { } } -func (p *streamProtocolV1) stream(logger klog.Logger, conn streamCreator, ready chan<- struct{}) error { +func (p *streamProtocolV1) stream(conn streamCreator, ready chan<- struct{}) error { doneChan := make(chan struct{}, 2) errorChan := make(chan error) cp := func(s string, dst io.Writer, src io.Reader) { - p.logger.V(6).Info("Copying", "data", s) - defer p.logger.V(6).Info("Done copying", "data", s) + klog.V(6).Infof("Copying %s", s) + defer klog.V(6).Infof("Done copying %s", s) if _, err := io.Copy(dst, src); err != nil && err != io.EOF { - p.logger.Error(err, "Error copying", "data", s) + klog.Errorf("Error copying %s: %v", s, err) } if s == v1.StreamTypeStdout || s == v1.StreamTypeStderr { doneChan <- struct{}{} diff --git a/tools/remotecommand/v2.go b/tools/remotecommand/v2.go index 75286a12f..a81538a09 100644 --- a/tools/remotecommand/v2.go +++ b/tools/remotecommand/v2.go @@ -22,9 +22,8 @@ import ( "net/http" "sync" - v1 "k8s.io/api/core/v1" + "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/util/runtime" - "k8s.io/klog/v2" ) // streamProtocolV2 implements version 2 of the streaming protocol for attach @@ -88,13 +87,13 @@ func (p *streamProtocolV2) createStreams(conn streamCreator) error { return nil } -func (p *streamProtocolV2) copyStdin(logger klog.Logger) { +func (p *streamProtocolV2) copyStdin() { if p.Stdin != nil { var once sync.Once // copy from client's stdin to container's stdin go func() { - defer runtime.HandleCrashWithLogger(logger) + defer runtime.HandleCrash() // if p.stdin is noninteractive, p.g. `echo abc | kubectl exec -i -- cat`, make sure // we close remoteStdin as soon as the copy from p.stdin to remoteStdin finishes. Otherwise @@ -102,7 +101,7 @@ func (p *streamProtocolV2) copyStdin(logger klog.Logger) { defer once.Do(func() { p.remoteStdin.Close() }) if _, err := io.Copy(p.remoteStdin, readerWrapper{p.Stdin}); err != nil { - runtime.HandleErrorWithLogger(logger, err, "Copying stdin failed") + runtime.HandleError(err) } }() @@ -121,26 +120,26 @@ func (p *streamProtocolV2) copyStdin(logger klog.Logger) { // When that happens, we must Close() on our side of remoteStdin, to // allow the copy in hijack to complete, and hijack to return. go func() { - defer runtime.HandleCrashWithLogger(logger) + defer runtime.HandleCrash() defer once.Do(func() { p.remoteStdin.Close() }) // this "copy" doesn't actually read anything - it's just here to wait for // the server to close remoteStdin. if _, err := io.Copy(io.Discard, p.remoteStdin); err != nil { - runtime.HandleErrorWithLogger(logger, err, "Waiting for server to close stdin failed") + runtime.HandleError(err) } }() } } -func (p *streamProtocolV2) copyStdout(logger klog.Logger, wg *sync.WaitGroup) { +func (p *streamProtocolV2) copyStdout(wg *sync.WaitGroup) { if p.Stdout == nil { return } wg.Add(1) go func() { - defer runtime.HandleCrashWithLogger(logger) + defer runtime.HandleCrash() defer wg.Done() // make sure, packet in queue can be consumed. // block in queue may lead to deadlock in conn.server @@ -148,29 +147,29 @@ func (p *streamProtocolV2) copyStdout(logger klog.Logger, wg *sync.WaitGroup) { defer io.Copy(io.Discard, p.remoteStdout) if _, err := io.Copy(p.Stdout, p.remoteStdout); err != nil { - runtime.HandleErrorWithLogger(logger, err, "Copying stdout failed") + runtime.HandleError(err) } }() } -func (p *streamProtocolV2) copyStderr(logger klog.Logger, wg *sync.WaitGroup) { +func (p *streamProtocolV2) copyStderr(wg *sync.WaitGroup) { if p.Stderr == nil || p.Tty { return } wg.Add(1) go func() { - defer runtime.HandleCrashWithLogger(logger) + defer runtime.HandleCrash() defer wg.Done() defer io.Copy(io.Discard, p.remoteStderr) if _, err := io.Copy(p.Stderr, p.remoteStderr); err != nil { - runtime.HandleErrorWithLogger(logger, err, "Copying stderr failed") + runtime.HandleError(err) } }() } -func (p *streamProtocolV2) stream(logger klog.Logger, conn streamCreator, ready chan<- struct{}) error { +func (p *streamProtocolV2) stream(conn streamCreator, ready chan<- struct{}) error { if err := p.createStreams(conn); err != nil { return err } @@ -182,13 +181,13 @@ func (p *streamProtocolV2) stream(logger klog.Logger, conn streamCreator, ready // now that all the streams have been created, proceed with reading & copying - errorChan := watchErrorStream(logger, p.errorStream, &errorDecoderV2{}) + errorChan := watchErrorStream(p.errorStream, &errorDecoderV2{}) - p.copyStdin(logger) + p.copyStdin() var wg sync.WaitGroup - p.copyStdout(logger, &wg) - p.copyStderr(logger, &wg) + p.copyStdout(&wg) + p.copyStderr(&wg) // we're waiting for stdout/stderr to finish copying wg.Wait() diff --git a/tools/remotecommand/v2_test.go b/tools/remotecommand/v2_test.go index e22dd685e..412cee8d2 100644 --- a/tools/remotecommand/v2_test.go +++ b/tools/remotecommand/v2_test.go @@ -28,7 +28,6 @@ import ( v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/util/httpstream" "k8s.io/apimachinery/pkg/util/wait" - "k8s.io/klog/v2/ktesting" ) type fakeReader struct { @@ -180,7 +179,6 @@ func TestV2CreateStreams(t *testing.T) { } func TestV2ErrorStreamReading(t *testing.T) { - logger, _ := ktesting.NewTestContext(t) tests := []struct { name string stream io.Reader @@ -219,7 +217,7 @@ func TestV2ErrorStreamReading(t *testing.T) { h := newStreamProtocolV2(StreamOptions{}).(*streamProtocolV2) h.errorStream = test.stream - ch := watchErrorStream(logger, h.errorStream, &errorDecoderV2{}) + ch := watchErrorStream(h.errorStream, &errorDecoderV2{}) if ch == nil { t.Fatalf("%s: unexpected nil channel", test.name) } diff --git a/tools/remotecommand/v3.go b/tools/remotecommand/v3.go index b1e533a8a..ece4cfafe 100644 --- a/tools/remotecommand/v3.go +++ b/tools/remotecommand/v3.go @@ -22,9 +22,8 @@ import ( "net/http" "sync" - v1 "k8s.io/api/core/v1" + "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/util/runtime" - "k8s.io/klog/v2" ) // streamProtocolV3 implements version 3 of the streaming protocol for attach @@ -63,12 +62,12 @@ func (p *streamProtocolV3) createStreams(conn streamCreator) error { return nil } -func (p *streamProtocolV3) handleResizes(logger klog.Logger) { +func (p *streamProtocolV3) handleResizes() { if p.resizeStream == nil || p.TerminalSizeQueue == nil { return } go func() { - defer runtime.HandleCrashWithLogger(logger) + defer runtime.HandleCrash() encoder := json.NewEncoder(p.resizeStream) for { @@ -77,13 +76,13 @@ func (p *streamProtocolV3) handleResizes(logger klog.Logger) { return } if err := encoder.Encode(&size); err != nil { - runtime.HandleErrorWithLogger(logger, err, "Encoding terminal size failed") + runtime.HandleError(err) } } }() } -func (p *streamProtocolV3) stream(logger klog.Logger, conn streamCreator, ready chan<- struct{}) error { +func (p *streamProtocolV3) stream(conn streamCreator, ready chan<- struct{}) error { if err := p.createStreams(conn); err != nil { return err } @@ -95,15 +94,15 @@ func (p *streamProtocolV3) stream(logger klog.Logger, conn streamCreator, ready // now that all the streams have been created, proceed with reading & copying - errorChan := watchErrorStream(logger, p.errorStream, &errorDecoderV3{}) + errorChan := watchErrorStream(p.errorStream, &errorDecoderV3{}) - p.handleResizes(logger) + p.handleResizes() - p.copyStdin(logger) + p.copyStdin() var wg sync.WaitGroup - p.copyStdout(logger, &wg) - p.copyStderr(logger, &wg) + p.copyStdout(&wg) + p.copyStderr(&wg) // we're waiting for stdout/stderr to finish copying wg.Wait() diff --git a/tools/remotecommand/v4.go b/tools/remotecommand/v4.go index 47018ba5f..ecedad071 100644 --- a/tools/remotecommand/v4.go +++ b/tools/remotecommand/v4.go @@ -26,7 +26,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/remotecommand" "k8s.io/client-go/util/exec" - "k8s.io/klog/v2" ) // streamProtocolV4 implements version 4 of the streaming protocol for attach @@ -48,11 +47,11 @@ func (p *streamProtocolV4) createStreams(conn streamCreator) error { return p.streamProtocolV3.createStreams(conn) } -func (p *streamProtocolV4) handleResizes(logger klog.Logger) { - p.streamProtocolV3.handleResizes(logger) +func (p *streamProtocolV4) handleResizes() { + p.streamProtocolV3.handleResizes() } -func (p *streamProtocolV4) stream(logger klog.Logger, conn streamCreator, ready chan<- struct{}) error { +func (p *streamProtocolV4) stream(conn streamCreator, ready chan<- struct{}) error { if err := p.createStreams(conn); err != nil { return err } @@ -64,15 +63,15 @@ func (p *streamProtocolV4) stream(logger klog.Logger, conn streamCreator, ready // now that all the streams have been created, proceed with reading & copying - errorChan := watchErrorStream(logger, p.errorStream, &errorDecoderV4{}) + errorChan := watchErrorStream(p.errorStream, &errorDecoderV4{}) - p.handleResizes(logger) + p.handleResizes() - p.copyStdin(logger) + p.copyStdin() var wg sync.WaitGroup - p.copyStdout(logger, &wg) - p.copyStderr(logger, &wg) + p.copyStdout(&wg) + p.copyStderr(&wg) // we're waiting for stdout/stderr to finish copying wg.Wait() diff --git a/tools/remotecommand/v5.go b/tools/remotecommand/v5.go index ca79a8828..edfd3ccb2 100644 --- a/tools/remotecommand/v5.go +++ b/tools/remotecommand/v5.go @@ -16,8 +16,6 @@ limitations under the License. package remotecommand -import "k8s.io/klog/v2" - // streamProtocolV5 add support for V5 of the remote command subprotocol. // For the streamProtocolHandler, this version is the same as V4. type streamProtocolV5 struct { @@ -32,6 +30,6 @@ func newStreamProtocolV5(options StreamOptions) streamProtocolHandler { } } -func (p *streamProtocolV5) stream(logger klog.Logger, conn streamCreator, ready chan<- struct{}) error { - return p.streamProtocolV4.stream(logger, conn, ready) +func (p *streamProtocolV5) stream(conn streamCreator, ready chan<- struct{}) error { + return p.streamProtocolV4.stream(conn, ready) } diff --git a/tools/remotecommand/websocket.go b/tools/remotecommand/websocket.go index ce03c1834..e0433198f 100644 --- a/tools/remotecommand/websocket.go +++ b/tools/remotecommand/websocket.go @@ -130,8 +130,7 @@ func (e *wsStreamExecutor) StreamWithContext(ctx context.Context, options Stream } defer conn.Close() e.negotiated = conn.Subprotocol() - logger := klog.FromContext(ctx) - logger.V(4).Info("Subprotocol negotiated", "protocol", e.negotiated) + klog.V(4).Infof("The subprotocol is %s", e.negotiated) var streamer streamProtocolHandler switch e.negotiated { @@ -144,7 +143,7 @@ func (e *wsStreamExecutor) StreamWithContext(ctx context.Context, options Stream case remotecommand.StreamProtocolV2Name: streamer = newStreamProtocolV2(options) case "": - logger.V(4).Info("The server did not negotiate a streaming protocol version, falling back", "protocol", remotecommand.StreamProtocolV1Name) + klog.V(4).Infof("The server did not negotiate a streaming protocol version. Falling back to %s", remotecommand.StreamProtocolV1Name) fallthrough case remotecommand.StreamProtocolV1Name: streamer = newStreamProtocolV1(options) @@ -160,7 +159,7 @@ func (e *wsStreamExecutor) StreamWithContext(ctx context.Context, options Stream }() readyChan := make(chan struct{}) - creator := newWSStreamCreator(logger, conn) + creator := newWSStreamCreator(conn) go func() { select { // Wait until all streams have been created before starting the readDemuxLoop. @@ -178,7 +177,7 @@ func (e *wsStreamExecutor) StreamWithContext(ctx context.Context, options Stream e.heartbeatDeadline, ) }() - errorChan <- streamer.stream(logger, creator, readyChan) + errorChan <- streamer.stream(creator, readyChan) }() select { @@ -192,8 +191,7 @@ func (e *wsStreamExecutor) StreamWithContext(ctx context.Context, options Stream } type wsStreamCreator struct { - logger klog.Logger - conn *gwebsocket.Conn + conn *gwebsocket.Conn // Protects writing to websocket connection; reading is lock-free connWriteLock sync.Mutex // map of stream id to stream; multiple streams read/write the connection @@ -204,9 +202,8 @@ type wsStreamCreator struct { setStreamErr error } -func newWSStreamCreator(logger klog.Logger, conn *gwebsocket.Conn) *wsStreamCreator { +func newWSStreamCreator(conn *gwebsocket.Conn) *wsStreamCreator { return &wsStreamCreator{ - logger: logger, conn: conn, streams: map[byte]*stream{}, } @@ -241,7 +238,6 @@ func (c *wsStreamCreator) CreateStream(headers http.Header) (httpstream.Stream, } reader, writer := io.Pipe() s := &stream{ - logger: klog.LoggerWithValues(c.logger, "id", id), headers: headers, readPipe: reader, writePipe: writer, @@ -264,11 +260,11 @@ func (c *wsStreamCreator) CreateStream(headers http.Header) (httpstream.Stream, // connection reader at a time (a read mutex would provide no benefit). func (c *wsStreamCreator) readDemuxLoop(bufferSize int, period time.Duration, deadline time.Duration) { // Initialize and start the ping/pong heartbeat. - h := newHeartbeat(c.logger, c.conn, period, deadline) + h := newHeartbeat(c.conn, period, deadline) // Set initial timeout for websocket connection reading. - c.logger.V(5).Info("Websocket read starts", "deadline", deadline) + klog.V(5).Infof("Websocket initial read deadline: %s", deadline) if err := c.conn.SetReadDeadline(time.Now().Add(deadline)); err != nil { - c.logger.Error(err, "Websocket initial setting read deadline failed") + klog.Errorf("Websocket initial setting read deadline failed %v", err) return } go h.start() @@ -312,7 +308,7 @@ func (c *wsStreamCreator) readDemuxLoop(bufferSize int, period time.Duration, de streamID := readBuffer[0] s := c.getStream(streamID) if s == nil { - c.logger.Error(nil, "Unknown stream, discarding message", "id", streamID) + klog.Errorf("Unknown stream id %d, discarding message", streamID) continue } for { @@ -355,7 +351,6 @@ func (c *wsStreamCreator) closeAllStreamReaders(err error) { } type stream struct { - logger klog.Logger headers http.Header readPipe *io.PipeReader writePipe *io.PipeWriter @@ -374,8 +369,8 @@ func (s *stream) Read(p []byte) (n int, err error) { // Write writes directly to the underlying WebSocket connection. func (s *stream) Write(p []byte) (n int, err error) { - s.logger.V(8).Info("Write() on stream") - defer s.logger.V(8).Info("Write() done on stream") + klog.V(8).Infof("Write() on stream %d", s.id) + defer klog.V(8).Infof("Write() done on stream %d", s.id) s.connWriteLock.Lock() defer s.connWriteLock.Unlock() if s.conn == nil { @@ -383,7 +378,7 @@ func (s *stream) Write(p []byte) (n int, err error) { } err = s.conn.SetWriteDeadline(time.Now().Add(writeDeadline)) if err != nil { - s.logger.V(4).Info("Websocket setting write deadline failed", "err", err) + klog.V(4).Infof("Websocket setting write deadline failed %v", err) return 0, err } // Message writer buffers the message data, so we don't need to do that ourselves. @@ -412,8 +407,8 @@ func (s *stream) Write(p []byte) (n int, err error) { // Close half-closes the stream, indicating this side is finished with the stream. func (s *stream) Close() error { - s.logger.V(6).Info("Close() on stream") - defer s.logger.V(6).Info("Close() done on stream") + klog.V(6).Infof("Close() on stream %d", s.id) + defer klog.V(6).Infof("Close() done on stream %d", s.id) s.connWriteLock.Lock() defer s.connWriteLock.Unlock() if s.conn == nil { @@ -426,8 +421,8 @@ func (s *stream) Close() error { } func (s *stream) Reset() error { - s.logger.V(4).Info("Reset() on stream") - defer s.logger.V(4).Info("Reset() done on stream") + klog.V(4).Infof("Reset() on stream %d", s.id) + defer klog.V(4).Infof("Reset() done on stream %d", s.id) s.Close() return s.writePipe.Close() } @@ -447,8 +442,7 @@ func (s *stream) Identifier() uint32 { // inside the "readDemuxLoop" will return an i/o error prompting a connection close // and cleanup. type heartbeat struct { - logger klog.Logger - conn *gwebsocket.Conn + conn *gwebsocket.Conn // period defines how often a "ping" heartbeat message is sent to the other endpoint period time.Duration // closing the "closer" channel will clean up the heartbeat timers @@ -462,9 +456,8 @@ type heartbeat struct { // newHeartbeat creates heartbeat structure encapsulating fields necessary to // run the websocket connection ping/pong mechanism and sets up handlers on // the websocket connection. -func newHeartbeat(logger klog.Logger, conn *gwebsocket.Conn, period time.Duration, deadline time.Duration) *heartbeat { +func newHeartbeat(conn *gwebsocket.Conn, period time.Duration, deadline time.Duration) *heartbeat { h := &heartbeat{ - logger: logger, conn: conn, period: period, closer: make(chan struct{}), @@ -474,10 +467,10 @@ func newHeartbeat(logger klog.Logger, conn *gwebsocket.Conn, period time.Duratio // be empty. h.conn.SetPongHandler(func(msg string) error { // Push the read deadline into the future. - logger.V(6).Info("Pong message received -- resetting read deadline", "message", msg) + klog.V(6).Infof("Pong message received (%s)--resetting read deadline", msg) err := h.conn.SetReadDeadline(time.Now().Add(deadline)) if err != nil { - logger.Error(err, "Websocket setting read deadline failed") + klog.Errorf("Websocket setting read deadline failed %v", err) return err } if len(msg) > 0 { @@ -509,16 +502,16 @@ func (h *heartbeat) start() { for { select { case <-h.closer: - h.logger.V(5).Info("Closed channel -- returning") + klog.V(5).Infof("closed channel--returning") return case <-t.C: // "WriteControl" does not need to be protected by a mutex. According to // gorilla/websockets library docs: "The Close and WriteControl methods can // be called concurrently with all other methods." if err := h.conn.WriteControl(gwebsocket.PingMessage, h.message, time.Now().Add(pingReadDeadline)); err == nil { - h.logger.V(6).Info("Websocket Ping succeeeded") + klog.V(6).Infof("Websocket Ping succeeeded") } else { - h.logger.Error(err, "Websocket Ping failed") + klog.Errorf("Websocket Ping failed: %v", err) if errors.Is(err, gwebsocket.ErrCloseSent) { // we continue because c.conn.CloseChan will manage closing the connection already continue diff --git a/tools/remotecommand/websocket_test.go b/tools/remotecommand/websocket_test.go index ef23d8cb5..9f064d935 100644 --- a/tools/remotecommand/websocket_test.go +++ b/tools/remotecommand/websocket_test.go @@ -48,7 +48,6 @@ import ( "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/rest" clientcmdapi "k8s.io/client-go/tools/clientcmd/api" - "k8s.io/klog/v2/ktesting" ) // TestWebSocketClient_LoopbackStdinToStdout returns random data sent on the STDIN channel @@ -1050,7 +1049,6 @@ func TestWebSocketClient_ExecutorErrors(t *testing.T) { } func TestWebSocketClient_HeartbeatSucceeds(t *testing.T) { - logger, _ := ktesting.NewTestContext(t) var upgrader = gwebsocket.Upgrader{ CheckOrigin: func(r *http.Request) bool { return true // Accepting all requests @@ -1083,7 +1081,7 @@ func TestWebSocketClient_HeartbeatSucceeds(t *testing.T) { var expectedMsg = "test heartbeat message" var period = 100 * time.Millisecond var deadline = 200 * time.Millisecond - heartbeat := newHeartbeat(logger, client, period, deadline) + heartbeat := newHeartbeat(client, period, deadline) heartbeat.setMessage(expectedMsg) // Add a channel to the handler to retrieve the "pong" message. pongMsgCh := make(chan string) @@ -1123,8 +1121,7 @@ func TestWebSocketClient_HeartbeatSucceeds(t *testing.T) { } func TestLateStreamCreation(t *testing.T) { - logger, _ := ktesting.NewTestContext(t) - c := newWSStreamCreator(logger, nil) + c := newWSStreamCreator(nil) c.closeAllStreamReaders(nil) if err := c.setStream(0, nil); err == nil { t.Fatal("expected error adding stream after closeAllStreamReaders") @@ -1132,10 +1129,8 @@ func TestLateStreamCreation(t *testing.T) { } func TestWebSocketClient_StreamsAndExpectedErrors(t *testing.T) { - logger, _ := ktesting.NewTestContext(t) - // Validate Stream functions. - c := newWSStreamCreator(logger, nil) + c := newWSStreamCreator(nil) headers := http.Header{} headers.Set(v1.StreamType, v1.StreamTypeStdin) s, err := c.CreateStream(headers)