Merge pull request kubernetes#112064 from aojea/dryrun_cleanup

Dryrun cleanup
1511563371 · Aug 27, 2022 · 1959fe1 · 1959fe1
2 parents 41df816 + a1bfb76
commit 1959fe1
Show file tree

Hide file tree

Showing 7 changed files with 2 additions and 96 deletions.
diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go
@@ -1124,7 +1124,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS
 
 	genericfeatures.CustomResourceValidationExpressions: {Default: true, PreRelease: featuregate.Beta},
 
-	genericfeatures.DryRun: {Default: true, PreRelease: featuregate.GA},
+	genericfeatures.DryRun: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.28
 
 	genericfeatures.OpenAPIEnums: {Default: true, PreRelease: featuregate.Beta},
 

diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go
@@ -4001,16 +4001,6 @@ func runRequest(t testing.TB, path, verb string, data []byte, contentType string
 	return response
 }
 
-// encodeOrFatal is used by TestDryRun to parse an object and stop right
-// away if it fails.
-func encodeOrFatal(t *testing.T, obj runtime.Object) []byte {
-	data, err := runtime.Encode(testCodec, obj)
-	if err != nil {
-		t.Fatalf("unexpected error: %v", err)
-	}
-	return data
-}
-
 type SimpleRESTStorageWithDeleteCollection struct {
 	SimpleRESTStorage
 }
@@ -4272,57 +4262,6 @@ other: bar`)
 	}
 }
 
-func TestDryRunDisabled(t *testing.T) {
-	defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.DryRun, false)()
-
-	tests := []struct {
-		path        string
-		verb        string
-		data        []byte
-		contentType string
-	}{
-		{path: "/namespaces/default/simples", verb: "POST", data: encodeOrFatal(t, &genericapitesting.Simple{Other: "bar"})},
-		{path: "/namespaces/default/simples/id", verb: "PUT", data: encodeOrFatal(t, &genericapitesting.Simple{ObjectMeta: metav1.ObjectMeta{Name: "id"}, Other: "bar"})},
-		{path: "/namespaces/default/simples/id", verb: "PATCH", data: []byte(`{"labels":{"foo":"bar"}}`), contentType: "application/merge-patch+json; charset=UTF-8"},
-		{path: "/namespaces/default/simples/id", verb: "DELETE"},
-		{path: "/namespaces/default/simples", verb: "DELETE"},
-		{path: "/namespaces/default/simples/id/subsimple", verb: "DELETE"},
-	}
-
-	server := httptest.NewServer(handle(map[string]rest.Storage{
-		"simples": &SimpleRESTStorageWithDeleteCollection{
-			SimpleRESTStorage{
-				item: genericapitesting.Simple{
-					ObjectMeta: metav1.ObjectMeta{
-						Name:      "id",
-						Namespace: "",
-						UID:       "uid",
-					},
-					Other: "bar",
-				},
-			},
-		},
-		"simples/subsimple": &SimpleXGSubresourceRESTStorage{
-			item: genericapitesting.SimpleXGSubresource{
-				SubresourceInfo: "foo",
-			},
-			itemGVK: testGroup2Version.WithKind("SimpleXGSubresource"),
-		},
-	}))
-	defer server.Close()
-	for _, test := range tests {
-		baseURL := server.URL + "/" + prefix + "/" + testGroupVersion.Group + "/" + testGroupVersion.Version
-		response := runRequest(t, baseURL+test.path, test.verb, test.data, test.contentType)
-		if response.StatusCode == http.StatusBadRequest {
-			t.Fatalf("unexpected BadRequest: %#v", response)
-		}
-		response = runRequest(t, baseURL+test.path+"?dryRun", test.verb, test.data, test.contentType)
-		if response.StatusCode != http.StatusBadRequest {
-			t.Fatalf("unexpected non BadRequest: %#v", response)
-		}
-	}
-}
-
 type SimpleXGSubresourceRESTStorage struct {
 	item    genericapitesting.SimpleXGSubresource
 	itemGVK schema.GroupVersionKind

diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/create.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/create.go
@@ -39,10 +39,8 @@ import (
 	"k8s.io/apiserver/pkg/endpoints/handlers/finisher"
 	"k8s.io/apiserver/pkg/endpoints/handlers/negotiation"
 	"k8s.io/apiserver/pkg/endpoints/request"
-	"k8s.io/apiserver/pkg/features"
 	"k8s.io/apiserver/pkg/registry/rest"
 	"k8s.io/apiserver/pkg/util/dryrun"
-	utilfeature "k8s.io/apiserver/pkg/util/feature"
 	"k8s.io/klog/v2"
 	utiltrace "k8s.io/utils/trace"
 )
@@ -55,11 +53,6 @@ func createHandler(r rest.NamedCreater, scope *RequestScope, admit admission.Int
 		trace := utiltrace.New("Create", traceFields(req)...)
 		defer trace.LogIfLong(500 * time.Millisecond)
 
-		if isDryRun(req.URL) && !utilfeature.DefaultFeatureGate.Enabled(features.DryRun) {
-			scope.err(errors.NewBadRequest("the dryRun feature is disabled"), w, req)
-			return
-		}
-
 		namespace, name, err := scope.Namer.Name(req)
 		if err != nil {
 			if includeName {

diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/delete.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/delete.go
@@ -35,10 +35,8 @@ import (
 	"k8s.io/apiserver/pkg/endpoints/handlers/finisher"
 	"k8s.io/apiserver/pkg/endpoints/handlers/negotiation"
 	"k8s.io/apiserver/pkg/endpoints/request"
-	"k8s.io/apiserver/pkg/features"
 	"k8s.io/apiserver/pkg/registry/rest"
 	"k8s.io/apiserver/pkg/util/dryrun"
-	utilfeature "k8s.io/apiserver/pkg/util/feature"
 	utiltrace "k8s.io/utils/trace"
 )
 
@@ -50,11 +48,6 @@ func DeleteResource(r rest.GracefulDeleter, allowsOptions bool, scope *RequestSc
 		trace := utiltrace.New("Delete", traceFields(req)...)
 		defer trace.LogIfLong(500 * time.Millisecond)
 
-		if isDryRun(req.URL) && !utilfeature.DefaultFeatureGate.Enabled(features.DryRun) {
-			scope.err(errors.NewBadRequest("the dryRun feature is disabled"), w, req)
-			return
-		}
-
 		namespace, name, err := scope.Namer.Name(req)
 		if err != nil {
 			scope.err(err, w, req)
@@ -172,11 +165,6 @@ func DeleteCollection(r rest.CollectionDeleter, checkBody bool, scope *RequestSc
 		trace := utiltrace.New("Delete", traceFields(req)...)
 		defer trace.LogIfLong(500 * time.Millisecond)
 
-		if isDryRun(req.URL) && !utilfeature.DefaultFeatureGate.Enabled(features.DryRun) {
-			scope.err(errors.NewBadRequest("the dryRun feature is disabled"), w, req)
-			return
-		}
-
 		namespace, err := scope.Namer.Namespace(req)
 		if err != nil {
 			scope.err(err, w, req)

diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go
@@ -47,10 +47,8 @@ import (
 	"k8s.io/apiserver/pkg/endpoints/handlers/finisher"
 	"k8s.io/apiserver/pkg/endpoints/handlers/negotiation"
 	"k8s.io/apiserver/pkg/endpoints/request"
-	"k8s.io/apiserver/pkg/features"
 	"k8s.io/apiserver/pkg/registry/rest"
 	"k8s.io/apiserver/pkg/util/dryrun"
-	utilfeature "k8s.io/apiserver/pkg/util/feature"
 	utiltrace "k8s.io/utils/trace"
 )
 
@@ -66,11 +64,6 @@ func PatchResource(r rest.Patcher, scope *RequestScope, admit admission.Interfac
 		trace := utiltrace.New("Patch", traceFields(req)...)
 		defer trace.LogIfLong(500 * time.Millisecond)
 
-		if isDryRun(req.URL) && !utilfeature.DefaultFeatureGate.Enabled(features.DryRun) {
-			scope.err(errors.NewBadRequest("the dryRun feature is disabled"), w, req)
-			return
-		}
-
 		// Do this first, otherwise name extraction can fail for unrecognized content types
 		// TODO: handle this in negotiation
 		contentType := req.Header.Get("Content-Type")

diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/update.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/update.go
@@ -37,10 +37,8 @@ import (
 	"k8s.io/apiserver/pkg/endpoints/handlers/finisher"
 	"k8s.io/apiserver/pkg/endpoints/handlers/negotiation"
 	"k8s.io/apiserver/pkg/endpoints/request"
-	"k8s.io/apiserver/pkg/features"
 	"k8s.io/apiserver/pkg/registry/rest"
 	"k8s.io/apiserver/pkg/util/dryrun"
-	utilfeature "k8s.io/apiserver/pkg/util/feature"
 	"k8s.io/klog/v2"
 	utiltrace "k8s.io/utils/trace"
 )
@@ -52,11 +50,6 @@ func UpdateResource(r rest.Updater, scope *RequestScope, admit admission.Interfa
 		trace := utiltrace.New("Update", traceFields(req)...)
 		defer trace.LogIfLong(500 * time.Millisecond)
 
-		if isDryRun(req.URL) && !utilfeature.DefaultFeatureGate.Enabled(features.DryRun) {
-			scope.err(errors.NewBadRequest("the dryRun feature is disabled"), w, req)
-			return
-		}
-
 		namespace, name, err := scope.Namer.Name(req)
 		if err != nil {
 			scope.err(err, w, req)

diff --git a/staging/src/k8s.io/apiserver/pkg/features/kube_features.go b/staging/src/k8s.io/apiserver/pkg/features/kube_features.go
@@ -208,7 +208,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS
 
 	CustomResourceValidationExpressions: {Default: true, PreRelease: featuregate.Beta},
 
-	DryRun: {Default: true, PreRelease: featuregate.GA},
+	DryRun: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.28
 
 	EfficientWatchResumption: {Default: true, PreRelease: featuregate.GA, LockToDefault: true},