Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FEATURE BRANCH] Plumbs up request-management system #81224

Conversation

@yue9944882
Copy link
Member

commented Aug 9, 2019

What type of PR is this?

Uncomment only one /kind <> line, hit enter to put that in a new line, and remove leading whitespaces from that line:

/kind api-change
/kind bug
/kind cleanup
/kind design
/kind documentation
/kind failing-test
/kind feature
/kind flake

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:


@MikeSpreitzer

This comment has been minimized.

Copy link
Member

commented Aug 10, 2019

@MikeSpreitzer
Copy link
Member

left a comment

Thanks for starting this.

In addition to the in-line comments, let me make an overall comment about Namespace. In any request there are up to two namespaces, and they can be different. One possible namespace is the namespace of the user that is making the request; there is such a namespace only if the user is a serviceaccount user. Another possible namespace is the namespace of the resource that the request is about accessing. It is possible for either namespace to be empty/omitted while the other is not. For example, there can be a request from a normal user to access a namespaced resource. For another example, there can be a request from a serviceAccount user to access a non-namespaced resource. Finally, a request can have two different non-empty namespaces, for example when a controller (running as a serviceAccount, necessarily in one particular namespace) accesses a resource in another namespace; for example, the scheduler accesses pods in all namespaces.

Looking to RBAC for inspiration can be confusing in this regard. In RBAC it is never necessary to explicitly mention both namespaces. In the cases where the two namespaces are non-empty and different, RBAC can express permission only by being oblivious to the namespace of the object being accessed (i.e., by using a ClusterRole).

return ip < jp
}
// if the matching-precedence equals, the flow-schema w/ longer/stricter rules is prior
if ir, jr := len(s[i].Spec.Rules), len(s[j].Spec.Rules); ir != jr {

This comment has been minimized.

Copy link
@MikeSpreitzer

MikeSpreitzer Aug 10, 2019

Member

The KEP does not call for this, but also does not forbid this.
The logic here examines only the number of rules, but the comment suggests a more expansive comparison.
I am fine with this as an initial implementation.

This comment has been minimized.

Copy link
@yue9944882

yue9944882 Aug 12, 2019

Author Member

i updated to alphabetical comparison to make it cleaner, how do you think about this?

This comment has been minimized.

Copy link
@MikeSpreitzer

MikeSpreitzer Aug 13, 2019

Member

I am happy with the way it was (apart from the inaccuracy of the comment), I am happy with the way it is.

// TODO: implement
return 0
hash := sha256.Sum256([]byte(fsName + fDistinguisher))
return big.NewInt(0).SetBytes(hash[:8]).Uint64()

This comment has been minimized.

Copy link
@MikeSpreitzer

MikeSpreitzer Aug 10, 2019

Member

I think this may be more expensive than we want. I expect hash/crc64 will be cheaper and adequate for our needs.

This comment has been minimized.

Copy link
@yue9944882

yue9944882 Aug 12, 2019

Author Member

updated to crc64

func matchPolicyRuleNonResourceURL(policyRuleRequestURLs []string, requestPath string) bool {
matched := false
foreach(policyRuleRequestURLs, func(path string) {
matched = matched || (path == requestPath || path == rmtypesv1alpha1.NonResourceAll)

This comment has been minimized.

Copy link
@MikeSpreitzer

MikeSpreitzer Aug 10, 2019

Member

Our types file calls out a more general matching condition: *s are allowed, but only as the full, final step in the path.
Full implementation can come in a follow-up PR.

return matched
}

func matchPolicyRuleResource(policyRuleRequestResources []string, requestResource string) bool {

This comment has been minimized.

Copy link
@MikeSpreitzer

MikeSpreitzer Aug 10, 2019

Member

I see essentially the same function defined multiple times, differing only in how the wildcard value is referenced. How about one common function that takes the wildcard as a parameter?

This comment has been minimized.

Copy link
@yue9944882

yue9944882 Aug 12, 2019

Author Member

refactored. please take another look

This comment has been minimized.

Copy link
@MikeSpreitzer

MikeSpreitzer Aug 13, 2019

Member

I was thinking something more along the lines of

func stringInList(x string, list []string, wildcard string) bool {
    if len(list) == 1 && list[0] == wildcard {
        return true
    }
    for _, y := range list {
        if x == y {
            return true
        }
    }
    return false
}

With that in place, instead of writing matchPolicyRuleResource(policyRule.Resources, digest.RequestInfo.Resource) we can write stringInList(digest.RequestInfo.Resource, policyRule.Resources, ResourceAll); and instead of matchPolicyRuleAPIGroup(policyRule.APIGroups, digest.RequestInfo.APIGroup) we can write stringInList(digest.RequestInfo.APIGroup, policyRule.APIGroups, APIGroupAll); and so on.

This comment has been minimized.

Copy link
@MikeSpreitzer

MikeSpreitzer Aug 13, 2019

Member

If we want to goove on abstractions note that there are a lot more interesting things going on here than just iteration. I see key-less map/reduce: each element of a list of strings is mapped to a boolean, and the list of booleans is reduced with "OR" (which has an early-out).

This comment has been minimized.

Copy link
@yue9944882

yue9944882 Aug 13, 2019

Author Member

refactored w/ your code snippet

This comment has been minimized.

Copy link
@MikeSpreitzer

MikeSpreitzer Aug 13, 2019

Member

I also suggest that we do not need four functions that just call containsString; we can replace calls on them with calls on containsString.

@yue9944882 yue9944882 force-pushed the yue9944882:feature/pick-flow-schema branch 2 times, most recently from 6f58921 to c254555 Aug 12, 2019

@yue9944882

This comment has been minimized.

Copy link
Member Author

commented Aug 12, 2019

Yes, in the RBAC world, the service-account-kind subject not only checks the namespace from the user-identity, but also it validates the requesting namespace to operate on. we don't need that in the flow-control system, the flow-schema should be applied to any requests from that service-account. it's supposed to be much simpler than RBAC's matching logic.

@yue9944882

This comment has been minimized.

Copy link
Member Author

commented Aug 12, 2019

/kind feature

@yue9944882 yue9944882 force-pushed the yue9944882:feature/pick-flow-schema branch from c254555 to a4789ab Aug 12, 2019

@k8s-ci-robot k8s-ci-robot added size/XL and removed size/L labels Aug 12, 2019

@yue9944882 yue9944882 force-pushed the yue9944882:feature/pick-flow-schema branch from 2e08b72 to a5891f4 Aug 12, 2019

@yue9944882 yue9944882 force-pushed the yue9944882:feature/pick-flow-schema branch 4 times, most recently from 365ee62 to c50bdea Aug 15, 2019

hashValue := hashFlowID(matchingflowSchemaName, flowDistinguisher)

// 3. executing
ps := rmState.priorityLevelStates[matchingpriorityLevelName]
if ps.config.Exempt {
klog.V(7).Infof("Serving %v without delay", requestDigest)

This comment has been minimized.

Copy link
@MikeSpreitzer

MikeSpreitzer Aug 15, 2019

Member

This produces log entries like the following.

I0815 21:01:28.555153    5092 reqmgmt.go:241] Serving {0xc0082711e0 0xc003cc2640} without delay

--- which are pretty uninformative. The more verbose log statements like

			klog.V(6).Infof("Serving RequestInfo=%#+v, user.Info=%#+v after queuing\n", requestInfo, user)

produce more informative log statements, like

I0815 21:01:28.555169    5092 reqmgmt.go:70] Serving RequestInfo=&request.RequestInfo{IsResourceRequest:true, Path:"/apis/flowcontrol.apiserver.k8s.io/v1alpha1/prioritylevelconfigurations", Verb:"list", APIPrefix:"apis", APIGroup:"flowcontrol.apiserver.k8s.io", APIVersion:"v1alpha1", Namespace:"", Resource:"prioritylevelconfigurations", Subresource:"", Name:"", Parts:[]string{"prioritylevelconfigurations"}}, user.Info=&user.DefaultInfo{Name:"system:apiserver", UID:"0c86221d-db07-40dd-b150-9fa17227bbef", Groups:[]string{"system:masters"}, Extra:map[string][]string(nil)} after queuing
@MikeSpreitzer

This comment has been minimized.

Copy link
Member

commented Aug 15, 2019

FYI, here is the server-side logging for a request from kubectl:

I0815 21:01:33.181181    5092 reqmgmt.go:70] Serving RequestInfo=&request.RequestInfo{IsResourceRequest:false, Path:"/api", Verb:"get", APIPrefix:"", APIGroup:"", APIVersion:"", Namespace:"", Resource:"", Subresource:"", Name:"", Parts:[]string(nil)}, user.Info=&user.DefaultInfo{Name:"system:admin", UID:"", Groups:[]string{"system:masters", "system:authenticated"}, Extra:map[string][]string(nil)} after queuing

I0815 21:01:33.181264    5092 handler.go:153] kube-aggregator: GET "/api" satisfied by nonGoRestful

I0815 21:01:33.181282    5092 pathrecorder.go:240] kube-aggregator: "/api" satisfied by exact match

I0815 21:01:33.181299    5092 handler.go:143] kube-apiserver: GET "/api" satisfied by gorestful with webservice /api

I0815 21:01:33.181819    5092 httplog.go:90] GET /api?timeout=32s: (995.748µs) 200 [kubectl/v1.16.0 (linux/amd64) kubernetes/6fd9c25 127.0.0.1:45220]

For comparison, here is (I think) a request done over loopback:

I0815 21:01:31.915389    5092 reqmgmt.go:70] Serving RequestInfo=&request.RequestInfo{IsResourceRequest:true, Path:"/api/v1/namespaces/default/endpoints", Verb:"create", APIPrefix:"api", APIGroup:"", APIVersion:"v1", Namespace:"default", Resource:"endpoints", Subresource:"", Name:"", Parts:[]string{"endpoints"}}, user.Info=&user.DefaultInfo{Name:"system:apiserver", UID:"0c86221d-db07-40dd-b150-9fa17227bbef", Groups:[]string{"system:masters"}, Extra:map[string][]string(nil)} after queuing

Interestingly, the groups do not include "system:authenticated" NOR "system:unauthenticated".

@MikeSpreitzer

This comment has been minimized.

Copy link
Member

commented Aug 15, 2019

FYI, kubectl get ns produced the following in the apiserver log:

E0815 21:01:40.033048    5092 maxinflight.go:50] no User found in context
I0815 21:01:40.033092    5092 httplog.go:90] GET /api/v1/namespaces?limit=500: (172.332µs) 500
goroutine 10066 [running]:
k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server/httplog.(*respLogger).recordStatus(0xc0006063f0, 0x1f4)
        /home/mspreitz/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server/httplog/httplog.go:217 +0xc8
k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server/httplog.(*respLogger).WriteHeader(0xc0006063f0, 0x1f4)
        /home/mspreitz/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server/httplog/httplog.go:196 +0x35
net/http.Error(0x8910e80, 0xc0006063f0, 0xc0042459c0, 0x35, 0x1f4)
        /usr/local/go/src/net/http/server.go:2007 +0x213
k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server/filters.handleError(0x8910e80, 0xc0006063f0, 0xc007785200, 0x888d0a0, 0xc008127ba0)
        /home/mspreitz/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server/filters/maxinflight.go:49 +0xca
k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server/filters.WithRequestManagement.func1(0x8910e80, 0xc0006063f0, 0xc007785200)
        /home/mspreitz/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server/filters/reqmgmt.go:56 +0x7a3
net/http.HandlerFunc.ServeHTTP(0xc003faf0b0, 0x8910e80, 0xc0006063f0, 0xc007785200)
        /usr/local/go/src/net/http/server.go:1995 +0x44
k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server/filters.WithWaitGroup.func1(0x8910e80, 0xc0006063f0, 0xc007785200)
        /home/mspreitz/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server/filters/waitgroup.go:47 +0xf3
net/http.HandlerFunc.ServeHTTP(0xc003faf0e0, 0x8910e80, 0xc0006063f0, 0xc007785200)
        /usr/local/go/src/net/http/server.go:1995 +0x44
k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/endpoints/filters.WithRequestInfo.func1(0x8910e80, 0xc0006063f0, 0xc007785100)
        /home/mspreitz/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/endpoints/filters/requestinfo.go:39 +0x2b8
net/http.HandlerFunc.ServeHTTP(0xc003faf170, 0x8910e80, 0xc0006063f0, 0xc007785100)
        /usr/local/go/src/net/http/server.go:1995 +0x44
k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server/httplog.WithLogging.func1(0x8914940, 0xc008820000, 0xc007785000)
        /home/mspreitz/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server/httplog/httplog.go:89 +0x29c
net/http.HandlerFunc.ServeHTTP(0xc003fe2380, 0x8914940, 0xc008820000, 0xc007785000)
        /usr/local/go/src/net/http/server.go:1995 +0x44
k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server/filters.withPanicRecovery.func1(0x8914940, 0xc008820000, 0xc007785000)
        /home/mspreitz/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server/filters/wrap.go:44 +0x105
net/http.HandlerFunc.ServeHTTP(0xc003fe23a0, 0x8914940, 0xc008820000, 0xc007785000)
        /usr/local/go/src/net/http/server.go:1995 +0x44
net/http.serverHandler.ServeHTTP(0xc00161dc70, 0x8914940, 0xc008820000, 0xc007785000)
        /usr/local/go/src/net/http/server.go:2774 +0xa8
net/http.(*conn).serve(0xc009ac3860, 0x8925600, 0xc007235880)
        /usr/local/go/src/net/http/server.go:1878 +0x851
created by net/http.(*Server).Serve
        /usr/local/go/src/net/http/server.go:2884 +0x2f4

logging error output: "Internal Server Error: \"/api/v1/namespaces?limit=500\"\n"
 [kubectl/v0.0.0 (linux/amd64) kubernetes/$Format 127.0.0.1:43766]
@MikeSpreitzer

This comment has been minimized.

Copy link
Member

commented Aug 15, 2019

FYI, here is a log entry from a kube-apiserver handling a SubjectAccessReveiw request from an aggregated apiserver:

I0815 23:39:05.743947    2541 reqmgmt.go:74] Serving RequestInfo=&request.RequestInfo{IsResourceRequest:true, Path:"/apis/authorization.k8s.io/v1beta1/subjectaccessreviews", Verb:"create", APIPrefix:"apis", APIGroup:"authorization.k8s.io", APIVersion:"v1beta1", Namespace:"", Resource:"subjectaccessreviews", Subresource:"", Name:"", Parts:[]string{"subjectaccessreviews"}}, user.Info=&user.DefaultInfo{Name:"system:serviceaccount:example-com:network-apiserver", UID:"31c58204-c442-43b9-91e0-7886c5ba2847", Groups:[]string{"system:serviceaccounts", "system:serviceaccounts:example-com", "system:authenticated"}, Extra:map[string][]string(nil)} after queuing
@MikeSpreitzer

This comment has been minimized.

Copy link
Member

commented Aug 16, 2019

I am OK with fixing problems in later PRs. I would like to get this or #81485 merged, it does have a lot of changes that we want to build on.

flow-control system
compute flow-distinguisher

config controller

plumbs default-confifuration into post-start-hook

addressing reviews

apiserver flow-control

@yue9944882 yue9944882 force-pushed the yue9944882:feature/pick-flow-schema branch from c50bdea to 421e7c0 Aug 16, 2019

@yue9944882

This comment has been minimized.

Copy link
Member Author

commented Aug 16, 2019

/test pull-kubernetes-integration

@MikeSpreitzer

This comment has been minimized.

Copy link
Member

commented Aug 16, 2019

This is strange: the CI robot thinks https://prow.k8s.io/view/gcs/kubernetes-jenkins/pr-logs/pull/81224/pull-kubernetes-bazel-test/1162202537730772992 is a failure but looking there it claims success!

@yue9944882

This comment has been minimized.

Copy link
Member Author

commented Aug 16, 2019

/test pull-kubernetes-bazel-test

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

commented Aug 16, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: yue9944882

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

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@yue9944882

This comment has been minimized.

Copy link
Member Author

commented Aug 16, 2019

/test pull-kubernetes-e2e-gce-device-plugin-gpu

@adohe

This comment has been minimized.

Copy link
Member

commented Aug 16, 2019

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm label Aug 16, 2019

@k8s-ci-robot k8s-ci-robot merged commit 6089157 into kubernetes:feature-rate-limiting Aug 16, 2019

22 of 23 checks passed

pull-kubernetes-e2e-gce-device-plugin-gpu Job triggered.
Details
cla/linuxfoundation yue9944882 authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-conformance-image-test Skipped.
pull-kubernetes-cross Skipped.
pull-kubernetes-dependencies Job succeeded.
Details
pull-kubernetes-e2e-gce Job succeeded.
Details
pull-kubernetes-e2e-gce-100-performance Skipped.
pull-kubernetes-e2e-gce-csi-serial Skipped.
pull-kubernetes-e2e-gce-iscsi Skipped.
pull-kubernetes-e2e-gce-iscsi-serial Skipped.
pull-kubernetes-e2e-gce-storage-slow Skipped.
pull-kubernetes-godeps Skipped.
pull-kubernetes-integration Job succeeded.
Details
pull-kubernetes-kubemark-e2e-gce-big Job succeeded.
Details
pull-kubernetes-local-e2e Skipped.
pull-kubernetes-node-e2e Job succeeded.
Details
pull-kubernetes-node-e2e-containerd Skipped.
pull-kubernetes-typecheck Job succeeded.
Details
pull-kubernetes-verify Job succeeded.
Details
pull-publishing-bot-validate Skipped.
tide In merge pool.
Details
danglingCondition.LastTransitionTime = metav1.Now()
}

rmtypesv1a1.SetFlowSchemaCondition(fs, *danglingCondition)

This comment has been minimized.

Copy link
@MikeSpreitzer

MikeSpreitzer Aug 16, 2019

Member

It's worth attempting this only if it is a change, and emitting a log message on success as well as failure.

(Same for the priority level status update.)

@MikeSpreitzer
Copy link
Member

left a comment

I am not sure I understand the behavior that this revision of the code is intended to produce. What I see is that admins can do anything they want with config objects, the only thing that is preserved is the priority levels for which Spec.Exempt is set in the requestManagementState (and updates to the corresponding API objects have no effect on the requestManagementState), and thus requests can fail to match any FlowSchema in the requestManagementState and consequently get rejected. I also see that for aggregated apiservers, the initial configuration in the requestManagementState is empty, meaning all requests will be rejected until the config controller has digested enough config objects.

I think it is bad that edits to some objects are ignored.

I think it is bad that some requests can be rejected due to lack of config objects.

// `globalDefault` specifies whether this priority level should be considered as the default priority for requests
// that do not have any priority level. Only one PriorityClass should be marked as `globalDefault`.
// +optional
GlobalDefault bool

This comment has been minimized.

Copy link
@MikeSpreitzer

MikeSpreitzer Aug 17, 2019

Member

I think the GlobalDefault field has utility for scenarios in which the admins replace the set of config objects.

@@ -204,12 +204,6 @@ func ValidatePriorityLevelConfiguration(pl *flowcontrol.PriorityLevelConfigurati

func ValidatePriorityLevelConfigurationSpec(spec *flowcontrol.PriorityLevelConfigurationSpec, name string, fldPath *field.Path) field.ErrorList {
var allErrs field.ErrorList
if name != flowcontrol.PriorityLevelConfigurationNameSystemTop && spec.Exempt {

This comment has been minimized.

Copy link
@MikeSpreitzer

MikeSpreitzer Aug 17, 2019

Member

It looks like this PR lets the admins do whatever they way (make any edit, or delete) the PriorityLevelConfiguration object named "exempt" but the configuration controller is intended to ignore whatever the admins do to that object and behave as if no changes are made. I think that is a confusing behavior.

DefaultFlowSchemaCatchAll = fs(
"catch-all",
"default",
1500, rmv1a1.FlowDistinguisherMethodByUserType,

This comment has been minimized.

Copy link
@MikeSpreitzer

MikeSpreitzer Aug 17, 2019

Member

This should have a higher matching precedence. It should certainly be higher than that of the other default objects, and perhaps the highest allowed by its datatype or a decimal round number nearer that.

The KEP says that the flow distinguisher method for workload-low (its name for the catch-all) is by namespace rather than by user.

Rule: nonResourceRule([]string{rmv1a1.VerbAll}, []string{rmv1a1.NonResourceAll}),
},
),
fs("workload-high", "workload-high", 7500,

This comment has been minimized.

Copy link
@MikeSpreitzer

MikeSpreitzer Aug 17, 2019

Member

It seems odd to have two flow schemas that match everything. The KEP calls for just one, using the object namespace to make the flow distinguisher.

@@ -95,7 +88,7 @@ type StatusREST struct {

// New creates a new Job object.

This comment has been minimized.

Copy link
@MikeSpreitzer

MikeSpreitzer Aug 17, 2019

Member

Wrong type name in this comment. Same problem for flowschema storage.


fsLister rmlistersv1alpha1.FlowSchemaLister
readyFunc func() bool

This comment has been minimized.

Copy link
@MikeSpreitzer

MikeSpreitzer Aug 17, 2019

Member

This function is only used to delay when the config controller launches its queue worker. It does nothing about preserving objects after that point in time.

}
} else {
if state.config.Exempt {
continue

This comment has been minimized.

Copy link
@MikeSpreitzer

MikeSpreitzer Aug 17, 2019

Member

If it is intended that updates to the exempt API object have effects then the only distinct treatment needed for an exempt priority level in this loop is to avoid adding its concurrency shares to shareSum.

}

fsSeq := make(rmtypesv1a1.FlowSchemaSequence, 0, len(newFSs))
for i, fs := range newFSs {

This comment has been minimized.

Copy link
@MikeSpreitzer

MikeSpreitzer Aug 17, 2019

Member

I see nothing here that preserves FlowSchema objects in the requestManagementState.

} else if plState.emptyHandler != nil && plState.emptyHandler.IsEmpty() {
// undesired, empty, and never going to get another request
klog.V(3).Infof("Priority level %s removed from implementation", plName)
} else if !plState.config.Exempt {

This comment has been minimized.

Copy link
@MikeSpreitzer

MikeSpreitzer Aug 17, 2019

Member

I thought the plan here was to avoid both the exempt and the default PriorityLevelConfiguration.

preservingPriorityLevels,
preservingFlowSchemas,
)
}

This comment has been minimized.

Copy link
@MikeSpreitzer

MikeSpreitzer Aug 17, 2019

Member

else the filter will reject all requests until the config controller has digested enough config objects. That's not very good behavior.

@@ -529,6 +530,19 @@ func BuildAuthorizer(s *options.ServerRunOptions, versionedInformers clientgoinf
return authorizationConfig.New()
}

// BuildAuthenticator constructs the authenticator

This comment has been minimized.

Copy link
@yliaog

yliaog Aug 19, 2019

Contributor

the comment is off, please fix

@@ -0,0 +1,69 @@
/*
Copyright 2016 The Kubernetes Authors.

This comment has been minimized.

Copy link
@yliaog

yliaog Aug 19, 2019

Contributor

s/2016/2019/

existingCondition.LastTransitionTime = newCondition.LastTransitionTime
}

existingCondition.Reason = newCondition.Reason

This comment has been minimized.

Copy link
@yliaog

yliaog Aug 19, 2019

Contributor

if the Status is not changed, does Reason or Message need to be changed?

kubernetes.NewForConfigOrDie(config.ClientConfig).FlowcontrolV1alpha1(),
fairqueuing.NewNoRestraintFactory( /* TODO: switch to real implementation */ ),
config.MaxRequestsInFlight+config.MaxMutatingRequestsInFlight,
config.RequestTimeout/4)

This comment has been minimized.

Copy link
@yliaog

yliaog Aug 19, 2019

Contributor

why is it config.RequestTimeout/4? where does 4 come from?

}

func (reqMgmt *requestManagementSystem) triggerReload() {
klog.V(7).Info("triggered request-management system reloading")

This comment has been minimized.

Copy link
@yliaog

yliaog Aug 19, 2019

Contributor

how is the log level decided? sometimes it is 6, and here it is 7

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.