Skip to content

Commit

Permalink
Remove global variable dependency from runtimeclass admission
Browse files Browse the repository at this point in the history
  • Loading branch information
liggitt committed Feb 27, 2020
1 parent 13beb9b commit 57ea7a1
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 20 deletions.
5 changes: 1 addition & 4 deletions plugin/pkg/admission/runtimeclass/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ go_library(
"//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library",
"//staging/src/k8s.io/apiserver/pkg/admission:go_default_library",
"//staging/src/k8s.io/apiserver/pkg/admission/initializer:go_default_library",
"//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library",
"//staging/src/k8s.io/client-go/informers:go_default_library",
"//staging/src/k8s.io/client-go/listers/node/v1beta1:go_default_library",
"//staging/src/k8s.io/component-base/featuregate:go_default_library",
],
)

Expand All @@ -28,16 +28,13 @@ go_test(
embed = [":go_default_library"],
deps = [
"//pkg/apis/core:go_default_library",
"//pkg/features:go_default_library",
"//staging/src/k8s.io/api/core/v1:go_default_library",
"//staging/src/k8s.io/api/node/v1beta1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/api/resource:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library",
"//staging/src/k8s.io/apiserver/pkg/admission:go_default_library",
"//staging/src/k8s.io/apiserver/pkg/authentication/user:go_default_library",
"//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library",
"//staging/src/k8s.io/component-base/featuregate/testing:go_default_library",
"//vendor/github.com/stretchr/testify/assert:go_default_library",
],
)
Expand Down
28 changes: 21 additions & 7 deletions plugin/pkg/admission/runtimeclass/admission.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@ import (
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apiserver/pkg/admission"
genericadmissioninitailizer "k8s.io/apiserver/pkg/admission/initializer"
utilfeature "k8s.io/apiserver/pkg/util/feature"
"k8s.io/client-go/informers"
nodev1beta1listers "k8s.io/client-go/listers/node/v1beta1"
"k8s.io/component-base/featuregate"
api "k8s.io/kubernetes/pkg/apis/core"
node "k8s.io/kubernetes/pkg/apis/node"
nodev1beta1 "k8s.io/kubernetes/pkg/apis/node/v1beta1"
Expand All @@ -58,16 +58,27 @@ func Register(plugins *admission.Plugins) {
type RuntimeClass struct {
*admission.Handler
runtimeClassLister nodev1beta1listers.RuntimeClassLister

inspectedFeatures bool
runtimeClassEnabled bool
podOverheadEnabled bool
}

var _ admission.MutationInterface = &RuntimeClass{}
var _ admission.ValidationInterface = &RuntimeClass{}

var _ genericadmissioninitailizer.WantsExternalKubeInformerFactory = &RuntimeClass{}

// InspectFeatureGates allows setting bools without taking a dep on a global variable
func (r *RuntimeClass) InspectFeatureGates(featureGates featuregate.FeatureGate) {
r.runtimeClassEnabled = featureGates.Enabled(features.RuntimeClass)
r.podOverheadEnabled = featureGates.Enabled(features.PodOverhead)
r.inspectedFeatures = true
}

// SetExternalKubeInformerFactory implements the WantsExternalKubeInformerFactory interface.
func (r *RuntimeClass) SetExternalKubeInformerFactory(f informers.SharedInformerFactory) {
if !utilfeature.DefaultFeatureGate.Enabled(features.RuntimeClass) {
if !r.runtimeClassEnabled {
return
}
runtimeClassInformer := f.Node().V1beta1().RuntimeClasses()
Expand All @@ -77,7 +88,10 @@ func (r *RuntimeClass) SetExternalKubeInformerFactory(f informers.SharedInformer

// ValidateInitialization implements the WantsExternalKubeInformerFactory interface.
func (r *RuntimeClass) ValidateInitialization() error {
if !utilfeature.DefaultFeatureGate.Enabled(features.RuntimeClass) {
if !r.inspectedFeatures {
return fmt.Errorf("InspectFeatureGates was not called")
}
if !r.runtimeClassEnabled {
return nil
}
if r.runtimeClassLister == nil {
Expand All @@ -88,7 +102,7 @@ func (r *RuntimeClass) ValidateInitialization() error {

// Admit makes an admission decision based on the request attributes
func (r *RuntimeClass) Admit(ctx context.Context, attributes admission.Attributes, o admission.ObjectInterfaces) error {
if !utilfeature.DefaultFeatureGate.Enabled(features.RuntimeClass) {
if !r.runtimeClassEnabled {
return nil
}

Expand All @@ -101,7 +115,7 @@ func (r *RuntimeClass) Admit(ctx context.Context, attributes admission.Attribute
if err != nil {
return err
}
if utilfeature.DefaultFeatureGate.Enabled(features.PodOverhead) {
if r.podOverheadEnabled {
if err := setOverhead(attributes, pod, runtimeClass); err != nil {
return err
}
Expand All @@ -116,7 +130,7 @@ func (r *RuntimeClass) Admit(ctx context.Context, attributes admission.Attribute

// Validate makes sure that pod adhere's to RuntimeClass's definition
func (r *RuntimeClass) Validate(ctx context.Context, attributes admission.Attributes, o admission.ObjectInterfaces) error {
if !utilfeature.DefaultFeatureGate.Enabled(features.RuntimeClass) {
if !r.runtimeClassEnabled {
return nil
}

Expand All @@ -129,7 +143,7 @@ func (r *RuntimeClass) Validate(ctx context.Context, attributes admission.Attrib
if err != nil {
return err
}
if utilfeature.DefaultFeatureGate.Enabled(features.PodOverhead) {
if r.podOverheadEnabled {
if err := validateOverhead(attributes, pod, runtimeClass); err != nil {
return err
}
Expand Down
7 changes: 2 additions & 5 deletions plugin/pkg/admission/runtimeclass/admission_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,7 @@ import (
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apiserver/pkg/admission"
"k8s.io/apiserver/pkg/authentication/user"
utilfeature "k8s.io/apiserver/pkg/util/feature"
featuregatetesting "k8s.io/component-base/featuregate/testing"
"k8s.io/kubernetes/pkg/apis/core"
"k8s.io/kubernetes/pkg/features"

"github.com/stretchr/testify/assert"
)
Expand Down Expand Up @@ -319,8 +316,6 @@ func NewObjectInterfacesForTest() admission.ObjectInterfaces {
}

func TestValidate(t *testing.T) {
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.PodOverhead, true)()

tests := []struct {
name string
runtimeClass *v1beta1.RuntimeClass
Expand Down Expand Up @@ -368,6 +363,8 @@ func TestValidate(t *testing.T) {
},
}
rt := NewRuntimeClass()
rt.runtimeClassEnabled = true
rt.podOverheadEnabled = true
o := NewObjectInterfacesForTest()
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,11 @@ func New(
// Initialize checks the initialization interfaces implemented by a plugin
// and provide the appropriate initialization data
func (i pluginInitializer) Initialize(plugin admission.Interface) {
// First tell the plugin about enabled features, so it can decide whether to start informers or not
if wants, ok := plugin.(WantsFeatures); ok {
wants.InspectFeatureGates(i.featureGates)
}

if wants, ok := plugin.(WantsExternalKubeClientSet); ok {
wants.SetExternalKubeClientSet(i.externalClient)
}
Expand All @@ -62,10 +67,6 @@ func (i pluginInitializer) Initialize(plugin admission.Interface) {
if wants, ok := plugin.(WantsAuthorizer); ok {
wants.SetAuthorizer(i.authorizer)
}

if wants, ok := plugin.(WantsFeatures); ok {
wants.InspectFeatureGates(i.featureGates)
}
}

var _ admission.PluginInitializer = pluginInitializer{}

0 comments on commit 57ea7a1

Please sign in to comment.