Add feature gating to REST Compression #46966

ilackarms · 2017-06-05T13:44:34Z

What this PR does / why we need it: Adds feature gating to opt out of REST API compression

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #46963

Special notes for your reviewer: This PR is a fix / addendum to #45666

Release note:

k8s-ci-robot · 2017-06-05T13:44:42Z

Hi @ilackarms. Thanks for your PR.

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

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

liggitt · 2017-06-05T14:47:53Z

pkg/features/kube_features.go

+	// alpha: v1.7
+	//
+	// Enables compression of REST responses (GET and LIST only)
+	EnableRESTCompression utilfeature.Feature = "EnableRESTCompression"


don't prefix with "Enable", maybe something like APIResponseCompression

liggitt · 2017-06-05T14:48:27Z

pkg/features/kube_features.go

@@ -138,6 +144,7 @@ var defaultKubernetesFeatureGates = map[utilfeature.Feature]utilfeature.FeatureS
 	RotateKubeletClientCertificate:              {Default: false, PreRelease: utilfeature.Alpha},
 	PersistentLocalVolumes:                      {Default: false, PreRelease: utilfeature.Alpha},
 	LocalStorageCapacityIsolation:               {Default: false, PreRelease: utilfeature.Alpha},
+	EnableRESTCompression:                       {Default: true, PreRelease: utilfeature.Alpha},


I'd consider it GA, it's enabled by default, it can just be disabled if needed

liggitt · 2017-06-05T14:49:40Z

required if #45666 is going into 1.7

smarterclayton · 2017-06-05T15:00:29Z

I'm ok with it being on by default but disable able in 1.7. Needs to get into the release list.

…

On Mon, Jun 5, 2017 at 10:49 AM, Jordan Liggitt ***@***.***> wrote: required if #45666 <#45666> is going into 1.7 — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub <#46966 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABG_p0GP84121usyxJyhmgd3xgmRpjlJks5sBBWSgaJpZM4NwDxl> .

deads2k · 2017-06-06T14:22:05Z

staging/src/k8s.io/apiserver/pkg/endpoints/installer.go

@@ -588,7 +590,9 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
 				handler = restfulGetResource(getter, exporter, reqScope)
 			}
 			handler = metrics.InstrumentRouteFunc(action.Verb, resource, subresource, handler)
-			handler = genericfilters.RestfulWithCompression(handler, a.group.Context)
+			if utilfeature.DefaultFeatureGate.Enabled(features.APIResponseCompression) {


Not here. Please plumb as an arg to this method and plumb it back out. You can set the feature flag as a value on the apiserver/pkg/server/config

deads2k · 2017-06-06T15:13:09Z

pkg/features/kube_features.go

+	// alpha: v1.7
+	//
+	// Enables compression of REST responses (GET and LIST only)
+	APIResponseCompression utilfeature.Feature = "APIResponseCompression"


This is actually staging/src/k8s.io/apiserver/pkg/features/kube_features.go. It seems unusual, but the gate is actually generic since your changes were generic.

deads2k · 2017-06-06T15:14:12Z

staging/src/k8s.io/apiserver/pkg/server/config.go

@@ -160,6 +161,10 @@ type Config struct {
 	// Predicate which is true for paths of long-running http requests
 	LongRunningFunc apirequest.LongRunningRequestCheck

+	// APIResponseCompression indicates whether API Responses should support compression
+	// if the client requests it via Accept-Encoding
+	APIResponseCompression bool


Rename to EnableAPIResponseCompression

deads2k · 2017-06-06T15:14:36Z

staging/src/k8s.io/apiserver/pkg/server/config.go

@@ -61,6 +61,7 @@ import (
 	"k8s.io/client-go/informers"
 	restclient "k8s.io/client-go/rest"
 	certutil "k8s.io/client-go/util/cert"
+	featuregates "k8s.io/kubernetes/pkg/features"


this should cause you to fail a verify script.

It's why you need a generic feature flag.

marun · 2017-06-20T22:05:18Z

/retest

marun · 2017-06-21T18:15:25Z

@liggitt @deads2k: Please review.

liggitt · 2017-06-23T15:20:00Z

staging/src/k8s.io/apiserver/pkg/features/kube_features.go

@@ -53,4 +59,5 @@ func init() {
 var defaultKubernetesFeatureGates = map[utilfeature.Feature]utilfeature.FeatureSpec{
 	StreamingProxyRedirects: {Default: true, PreRelease: utilfeature.Beta},
 	AdvancedAuditing:        {Default: false, PreRelease: utilfeature.Alpha},
+	APIResponseCompression:  {Default: false, PreRelease: utilfeature.GA},


until we're sure we want this defaulting on, and as this is the first release it's included, I'd mark this Alpha...

liggitt · 2017-06-23T15:20:48Z

gated and marked alpha, this LGTM. @deads2k?

this feature is gated; disabled by default

deads2k · 2017-06-23T15:30:58Z

/lgtm

k8s-github-robot · 2017-06-23T15:33:36Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deads2k, ilackarms

Associated issue: 46963

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

~~staging/src/k8s.io/apiserver/OWNERS~~ [deads2k]

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

k8s-github-robot · 2017-06-23T22:48:39Z

Automatic merge from submit-queue (batch tested with PRs 47883, 47179, 46966, 47982, 47945)

pacoxu · 2025-01-22T06:56:46Z

Wrong milestone here.
/milestone clear

pacoxu · 2025-01-22T06:56:59Z

/milestone v1.8

k8s-ci-robot · 2025-01-22T06:57:02Z

@pacoxu: The provided milestone is not valid for this repository. Milestones in this repository: [next-candidate, v1.26, v1.27, v1.28, v1.29, v1.30, v1.31, v1.32, v1.33, v1.34]

Use /milestone clear to clear the milestone.

In response to this:

/milestone v1.8

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jun 5, 2017

k8s-github-robot assigned caesarxuchao and smarterclayton Jun 5, 2017

k8s-github-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. release-note-label-needed labels Jun 5, 2017

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jun 5, 2017

liggitt reviewed Jun 5, 2017

View reviewed changes

liggitt added this to the v1.7 milestone Jun 5, 2017

liggitt added release-note-none Denotes a PR that doesn't merit a release note. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. and removed release-note-label-needed labels Jun 5, 2017

ilackarms force-pushed the compression-gating branch 2 times, most recently from ab026df to ef49439 Compare June 6, 2017 12:44

deads2k reviewed Jun 6, 2017

View reviewed changes

deads2k self-assigned this Jun 6, 2017

k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 6, 2017

ilackarms force-pushed the compression-gating branch from ef49439 to 7122dec Compare June 6, 2017 14:59

k8s-github-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jun 6, 2017

ilackarms force-pushed the compression-gating branch from 7122dec to b3503b2 Compare June 6, 2017 15:11

k8s-github-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jun 6, 2017

deads2k reviewed Jun 6, 2017

View reviewed changes

ilackarms force-pushed the compression-gating branch from 47b2152 to 40a473b Compare June 20, 2017 03:24

caesarxuchao added the cherrypick-candidate label Jun 22, 2017

liggitt reviewed Jun 23, 2017

View reviewed changes

add compression to GET and LIST api requests

c305f72

this feature is gated; disabled by default

ilackarms force-pushed the compression-gating branch from 40a473b to c305f72 Compare June 23, 2017 15:25

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 23, 2017

k8s-github-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 23, 2017

dchen1107 removed the cherrypick-candidate label Jun 23, 2017

k8s-github-robot merged commit 80af10c into kubernetes:master Jun 23, 2017

deads2k mentioned this pull request Jun 26, 2017

OpenAPI downloads protobuf rather than Json #46803

Merged

ilackarms mentioned this pull request Jul 4, 2017

Disable Requesting Compression by default on restclient #48477

Closed

ilackarms mentioned this pull request Aug 4, 2017

add gzip compression to GET and LIST requests #45666

Merged

ilackarms mentioned this pull request Aug 29, 2017

default 'true' for APIResponseCompression feature flag #51508

Closed

cben mentioned this pull request Dec 27, 2017

Allow compression of json response body using Accept-Encoding header #110349

Closed

sftim mentioned this pull request Nov 25, 2021

Document API response compression kubernetes/website#30639

Closed

pacoxu added a commit to pacoxu/kubernetes that referenced this pull request Jan 22, 2025

APIResponseCompression was added as alpha in v1.7 kubernetes#46966

25253fe

pacoxu mentioned this pull request Jan 22, 2025

promote APIResponseCompression to GA #129748

Open

pacoxu added a commit to pacoxu/kubernetes that referenced this pull request Jan 22, 2025

APIResponseCompression was added as alpha in v1.7 kubernetes#46966

3a51d57

pacoxu added a commit to pacoxu/kubernetes that referenced this pull request Jan 22, 2025

APIResponseCompression was added as alpha in v1.7 kubernetes#46966

5441be3

k8s-ci-robot removed this from the v1.7 milestone Jan 22, 2025

pacoxu mentioned this pull request Jan 22, 2025

API gzip compression support kubernetes/enhancements#2338

Open

13 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add feature gating to REST Compression #46966

Add feature gating to REST Compression #46966

ilackarms commented Jun 5, 2017

k8s-ci-robot commented Jun 5, 2017

liggitt Jun 5, 2017

liggitt Jun 5, 2017

liggitt commented Jun 5, 2017

smarterclayton commented Jun 5, 2017 via email

deads2k Jun 6, 2017

deads2k Jun 6, 2017

deads2k Jun 6, 2017

deads2k Jun 6, 2017

deads2k Jun 6, 2017

marun commented Jun 20, 2017

marun commented Jun 21, 2017

liggitt Jun 23, 2017

liggitt commented Jun 23, 2017

deads2k commented Jun 23, 2017

k8s-github-robot commented Jun 23, 2017

k8s-github-robot commented Jun 23, 2017

pacoxu commented Jan 22, 2025

pacoxu commented Jan 22, 2025

k8s-ci-robot commented Jan 22, 2025

Add feature gating to REST Compression #46966

Add feature gating to REST Compression #46966

Conversation

ilackarms commented Jun 5, 2017

k8s-ci-robot commented Jun 5, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liggitt commented Jun 5, 2017

smarterclayton commented Jun 5, 2017 via email

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marun commented Jun 20, 2017

marun commented Jun 21, 2017

Choose a reason for hiding this comment

liggitt commented Jun 23, 2017

deads2k commented Jun 23, 2017

k8s-github-robot commented Jun 23, 2017

k8s-github-robot commented Jun 23, 2017

pacoxu commented Jan 22, 2025

pacoxu commented Jan 22, 2025

k8s-ci-robot commented Jan 22, 2025