From bfa9b4e50c6d5d7cf2ead571aa051ed9e27e69cb Mon Sep 17 00:00:00 2001 From: yue9944882 <291271447@qq.com> Date: Thu, 29 Aug 2019 11:36:32 +0800 Subject: [PATCH] review: addressing comments from yliaog --- pkg/apis/flowcontrol/types.go | 2 +- .../flowcontrol/flowschema/storage/storage.go | 11 ++++++----- pkg/registry/flowcontrol/flowschema/strategy.go | 2 +- .../storage/storage.go | 11 ++++++----- .../prioritylevelconfiguration/strategy.go | 6 +++--- .../src/k8s.io/api/flowcontrol/v1alpha1/types.go | 2 +- .../flowcontrol/bootstrap/default-objects.go | 16 +++++++++++----- 7 files changed, 29 insertions(+), 21 deletions(-) diff --git a/pkg/apis/flowcontrol/types.go b/pkg/apis/flowcontrol/types.go index 2d053d04215..9c2a743a758 100644 --- a/pkg/apis/flowcontrol/types.go +++ b/pkg/apis/flowcontrol/types.go @@ -102,7 +102,7 @@ type FlowSchemaList struct { // FlowSchemaSpec describes how the flow-schema's specification looks like. type FlowSchemaSpec struct { // `priorityLevelConfiguration` should reference a PriorityLevelConfiguration in the cluster. If the reference cannot - // be resolved, the flow-schema will ignored and marked as invalid in its status. + // be resolved, the flow-schema will be ignored and marked as invalid in its status. // Required. PriorityLevelConfiguration PriorityLevelConfigurationReference // `matchingPrecedence` is used to choose among the FlowSchemas that match a given request. The chosen diff --git a/pkg/registry/flowcontrol/flowschema/storage/storage.go b/pkg/registry/flowcontrol/flowschema/storage/storage.go index 0afd478e68c..c93f3631d27 100644 --- a/pkg/registry/flowcontrol/flowschema/storage/storage.go +++ b/pkg/registry/flowcontrol/flowschema/storage/storage.go @@ -18,6 +18,7 @@ package storage import ( "context" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apiserver/pkg/registry/generic" @@ -30,7 +31,7 @@ import ( "k8s.io/kubernetes/pkg/registry/flowcontrol/flowschema" ) -// Container includes dummy storage for RC pods and experimental storage for Scale. +// FlowSchemaStorage implements storage for flow schema. type FlowSchemaStorage struct { FlowSchema *REST Status *StatusREST @@ -45,12 +46,12 @@ func NewStorage(optsGetter generic.RESTOptionsGetter) FlowSchemaStorage { } } -// REST implements a RESTStorage for jobs against etcd +// REST implements a RESTStorage for flow schema against etcd type REST struct { *genericregistry.Store } -// NewREST returns a RESTStorage object that will work against Jobs. +// NewREST returns a RESTStorage object that will work against flow schemas. func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST) { store := &genericregistry.Store{ NewFunc: func() runtime.Object { return &flowcontrol.FlowSchema{} }, @@ -81,12 +82,12 @@ func (r *REST) ShortNames() []string { return []string{"fs"} } -// StatusREST implements the REST endpoint for changing the status of a resourcequota. +// StatusREST implements the REST endpoint for changing the status of a flow schema. type StatusREST struct { store *genericregistry.Store } -// New creates a new Job object. +// New creates a new flow schema object. func (r *StatusREST) New() runtime.Object { return &flowcontrol.FlowSchema{} } diff --git a/pkg/registry/flowcontrol/flowschema/strategy.go b/pkg/registry/flowcontrol/flowschema/strategy.go index d75f517d8cb..8d6826191b5 100644 --- a/pkg/registry/flowcontrol/flowschema/strategy.go +++ b/pkg/registry/flowcontrol/flowschema/strategy.go @@ -34,7 +34,7 @@ type flowSchemaStrategy struct { names.NameGenerator } -// Strategy is the default logic that applies when creating and updating Replication Controller objects. +// Strategy is the default logic that applies when creating and updating flow schema objects. var Strategy = flowSchemaStrategy{legacyscheme.Scheme, names.SimpleNameGenerator} // NamespaceScoped returns false because all PriorityClasses are global. diff --git a/pkg/registry/flowcontrol/prioritylevelconfiguration/storage/storage.go b/pkg/registry/flowcontrol/prioritylevelconfiguration/storage/storage.go index 2d42dcc8e05..c7f124eeb08 100644 --- a/pkg/registry/flowcontrol/prioritylevelconfiguration/storage/storage.go +++ b/pkg/registry/flowcontrol/prioritylevelconfiguration/storage/storage.go @@ -18,6 +18,7 @@ package storage import ( "context" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apiserver/pkg/registry/generic" @@ -30,7 +31,7 @@ import ( "k8s.io/kubernetes/pkg/registry/flowcontrol/prioritylevelconfiguration" ) -// Container includes dummy storage for RC pods and experimental storage for Scale. +// PriorityLevelConfigurationStorage implements storage for priority level configuration. type PriorityLevelConfigurationStorage struct { PriorityLevelConfiguration *REST Status *StatusREST @@ -45,12 +46,12 @@ func NewStorage(optsGetter generic.RESTOptionsGetter) PriorityLevelConfiguration } } -// REST implements a RESTStorage for jobs against etcd +// REST implements a RESTStorage for priority level configuration against etcd type REST struct { *genericregistry.Store } -// NewREST returns a RESTStorage object that will work against Jobs. +// NewREST returns a RESTStorage object that will work against priority level configuration. func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST) { store := &genericregistry.Store{ NewFunc: func() runtime.Object { return &flowcontrol.PriorityLevelConfiguration{} }, @@ -81,12 +82,12 @@ func (r *REST) ShortNames() []string { return []string{"pl"} } -// StatusREST implements the REST endpoint for changing the status of a resourcequota. +// StatusREST implements the REST endpoint for changing the status of a priority level configuration. type StatusREST struct { store *genericregistry.Store } -// New creates a new Job object. +// New creates a new priority level configuration object. func (r *StatusREST) New() runtime.Object { return &flowcontrol.PriorityLevelConfiguration{} } diff --git a/pkg/registry/flowcontrol/prioritylevelconfiguration/strategy.go b/pkg/registry/flowcontrol/prioritylevelconfiguration/strategy.go index 48506cf330d..e6490c0ccd1 100644 --- a/pkg/registry/flowcontrol/prioritylevelconfiguration/strategy.go +++ b/pkg/registry/flowcontrol/prioritylevelconfiguration/strategy.go @@ -28,13 +28,13 @@ import ( "k8s.io/kubernetes/pkg/apis/flowcontrol/validation" ) -// flowSchemaStrategy implements verification logic for FlowSchema. +// priorityLevelConfigurationStrategy implements verification logic for priority level configurations. type priorityLevelConfigurationStrategy struct { runtime.ObjectTyper names.NameGenerator } -// Strategy is the default logic that applies when creating and updating Replication Controller objects. +// Strategy is the default logic that applies when creating and updating priority level configuration objects. var Strategy = priorityLevelConfigurationStrategy{legacyscheme.Scheme, names.SimpleNameGenerator} // NamespaceScoped returns false because all PriorityClasses are global. @@ -61,7 +61,7 @@ func (priorityLevelConfigurationStrategy) PrepareForUpdate(ctx context.Context, newPriorityLevelConfiguration.Status = oldPriorityLevelConfiguration.Status } -// Validate validates a new flow-schema. +// Validate validates a new priority-level. func (priorityLevelConfigurationStrategy) Validate(ctx context.Context, obj runtime.Object) field.ErrorList { return validation.ValidatePriorityLevelConfiguration(obj.(*flowcontrol.PriorityLevelConfiguration)) } diff --git a/staging/src/k8s.io/api/flowcontrol/v1alpha1/types.go b/staging/src/k8s.io/api/flowcontrol/v1alpha1/types.go index 56f18176523..4838a0836b9 100644 --- a/staging/src/k8s.io/api/flowcontrol/v1alpha1/types.go +++ b/staging/src/k8s.io/api/flowcontrol/v1alpha1/types.go @@ -94,7 +94,7 @@ type FlowSchemaList struct { // FlowSchemaSpec describes how the flow-schema's specification looks like. type FlowSchemaSpec struct { // `priorityLevelConfiguration` should reference a PriorityLevelConfiguration in the cluster. If the reference cannot - // be resolved, the flow-schema will ignored and marked as invalid in its status. + // be resolved, the flow-schema will be ignored and marked as invalid in its status. // Required. PriorityLevelConfiguration PriorityLevelConfigurationReference `json:"priorityLevelConfiguration,omitempty" protobuf:"bytes,1,opt,name=priorityLevelConfiguration"` // `matchingPrecedence` is used to choose among the FlowSchemas that match a given request. The chosen diff --git a/staging/src/k8s.io/apiserver/pkg/util/flowcontrol/bootstrap/default-objects.go b/staging/src/k8s.io/apiserver/pkg/util/flowcontrol/bootstrap/default-objects.go index 601d9a275a8..d8cdae58a4f 100644 --- a/staging/src/k8s.io/apiserver/pkg/util/flowcontrol/bootstrap/default-objects.go +++ b/staging/src/k8s.io/apiserver/pkg/util/flowcontrol/bootstrap/default-objects.go @@ -160,19 +160,23 @@ func PredefinedFlowSchemas() []*rmv1a1.FlowSchema { rmv1a1.FlowDistinguisherMethodByUserType, rmv1a1.PolicyRuleWithSubjects{ Subjects: groups(user.NodesGroup), - Rule: ruleRscAll}, + Rule: ruleRscAll, + }, rmv1a1.PolicyRuleWithSubjects{ Subjects: kubeSystemServiceAccount(rmv1a1.NameAll), - Rule: resourceRule(verbAll, []string{apiGroupCore}, []string{"endpoints", "configmaps", "leases"})}, + Rule: resourceRule(verbAll, []string{apiGroupCore}, []string{"endpoints", "configmaps", "leases"}), + }, rmv1a1.PolicyRuleWithSubjects{ Subjects: kubeSystemServiceAccount(rmv1a1.NameAll), - Rule: ruleNonRscAll}, + Rule: ruleNonRscAll, + }, ), fs("system-leader-election", "system", 2500, rmv1a1.FlowDistinguisherMethodByUserType, rmv1a1.PolicyRuleWithSubjects{ Subjects: users(user.KubeControllerManager, user.KubeScheduler), - Rule: resourceRule(verbAll, []string{"coordination.k8s.io"}, []string{"leases"})}, + Rule: resourceRule(verbAll, []string{"coordination.k8s.io"}, []string{"leases"}), + }, ), fs("kube-controller-manager", "workload-high", 3500, rmv1a1.FlowDistinguisherMethodByNamespaceType, @@ -205,7 +209,9 @@ func PredefinedFlowSchemas() []*rmv1a1.FlowSchema { Rule: ruleRscAll}, rmv1a1.PolicyRuleWithSubjects{ Subjects: groups("system:serviceaccounts"), - Rule: ruleNonRscAll}), + Rule: ruleNonRscAll, + }, + ), DefaultFlowSchemaDefault, } }