Bug 1954634 - apirequestcounts does not honor max users
Summary: apirequestcounts does not honor max users
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: OpenShift Container Platform
Classification: Red Hat
Component: kube-apiserver
Version: 4.8
Hardware: Unspecified
OS: Unspecified
high
high
Target Milestone: ---
: 4.8.0
Assignee: David Eads
QA Contact: Xingxing Xia
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2021-04-28 13:47 UTC by David Eads
Modified: 2021-07-27 23:04 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2021-07-27 23:04:32 UTC
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Github openshift kubernetes pull 689 0 None open bug 1954634: UPSTREAM: <carry>: honor max users 2021-04-28 13:47:27 UTC
Red Hat Product Errata RHSA-2021:2438 0 None None None 2021-07-27 23:04:43 UTC

Description David Eads 2021-04-28 13:47:07 UTC
setting the field on spec has no effect.

Comment 2 Xingxing Xia 2021-04-30 10:03:36 UTC
Tested 4.8.0-0.nightly-2021-04-29-222100:
$ oc edit apirequestcount configmaps.v1
We can see numberOfUsersToReport is set to 10 (the default). Change it to 3, save, the operation can succeed. Change it to -3, abc, 3.1, the operation can fail, oc edit session can prompt validation message correctly.

However:
$ for api in $apis; do USERS="`oc get apirequestcount $api -o yaml | grep username | sort | uniq`" ; CT=`echo "$USERS" | wc -l`; echo "$CT: $api"; echo "$USERS"; done > users
$ grep '^[1-9]' users | sort -n
...       
32: pods.v1                                            
38: services.v1 
47: secrets.v1 
48: tokenreviews.v1.authentication.k8s.io 
53: configmaps.v1 
 
$ vi users 
53: configmaps.v1 
        username: system:admin 
        username: system:apiserver 
        username: system:kube-controller-manager 
        username: system:kube-scheduler 
        username: system:node:xxia0430-48-vn8gc-master-0.c.openshift-qe.internal 
        username: system:node:xxia0430-48-vn8gc-master-1.c.openshift-qe.internal 
        username: system:node:xxia0430-48-vn8gc-master-2.c.openshift-qe.internal 
        username: system:node:xxia0430-48-vn8gc-worker-a-fh2z6.c.openshift-qe.internal 
        username: system:node:xxia0430-48-vn8gc-worker-b-r6zxj.c.openshift-qe.internal 
        username: system:node:xxia0430-48-vn8gc-worker-c-m8gdd.c.openshift-qe.internal 
        username: system:serviceaccount:openshift-apiserver:openshift-apiserver-sa 
        username: system:serviceaccount:openshift-apiserver-operator:openshift-apiserver-operator
        ...

That means, even if the numberOfUsersToReport is set to 10, the user numbers recorded in many apirequestcount objects are higher than 10, as shown in above sort.

Comment 3 Xingxing Xia 2021-04-30 10:13:01 UTC
In addition, if I understand the numberOfUsersToReport field incorrectly, as a user I would instinctively run below to understand it:
$ oc explain apirequestcount.spec.numberOfUsersToReport
...
DESCRIPTION:
     numberOfUsersToReport is the number of users to include in the report. If
     unspecified or zero, the default is ten. This is default is subject to
     change.

If the field is designed to for normal user number only, the DESCRIPTION should be fixed to clearly point it out.

I see byUser mentioned .spec.numberOfUsersToReport, where it seems to mean, numberOfUsersToReport consierders only normal user number instead of system user number, and only for each node instead of all nodes? If so, `oc explain apirequestcount.spec.numberOfUsersToReport` should be fixed to clearly point these out to customer users, rather than let them to go though all fields of apirequestcount like below:
$ oc explain apirequestcount.status.currentHour.byNode.byUser
...
DESCRIPTION:
     byUser contains request details by top .spec.numberOfUsersToReport users.
     Note that because in the case of an apiserver, restart the list of top
     users is determined on a best-effort basis, the list might be imprecise. In
     addition, some system users may be explicitly included in the list
...

Comment 4 Xingxing Xia 2021-04-30 10:35:39 UTC
Even if numberOfUsersToReport considers only normal user number for each node, after I set numberOfUsersToReport: 1 for apirequestcount/configmaps.v1, and create configmaps with 10 normal users, the reported user number for "nodeName: 10.0.0.5" is > 1.
spec:
  numberOfUsersToReport: 1
...
        username: testuser-9
      - byVerb:
        - requestCount: 1
          verb: create
        - requestCount: 1
          verb: get
        requestCount: 2
        userAgent: ""
        username: testuser-8
      - byVerb:
        - requestCount: 1
          verb: get
        requestCount: 1
        userAgent: ""
        username: testuser-1
      - byVerb:
        - requestCount: 1
          verb: create
        requestCount: 1
        userAgent: ""
        username: testuser-6
      - byVerb:
        - requestCount: 1
          verb: get
        requestCount: 1
        userAgent: ""
        username: testuser-7
      nodeName: 10.0.0.5
...

Comment 5 Xingxing Xia 2021-04-30 10:38:09 UTC
So, in short:
1. DESCRIPTION for `oc explain apirequestcount.spec.numberOfUsersToReport` should be updated to more clearly.
2. As comment 4, numberOfUsersToReport sets 1 but more than one users are reported for one node, this is bug.

Comment 6 Xingxing Xia 2021-04-30 11:38:34 UTC
> Tested 4.8.0-0.nightly-2021-04-29-222100:
Ah, sorry, found it is earlier than the PR merge time. Will test again in newer payload. But would like to insist DESCRIPTION for `oc explain apirequestcount.spec.numberOfUsersToReport` should be updated to more clearly.

Comment 7 Xingxing Xia 2021-04-30 12:43:19 UTC
Verified in new payload 4.8.0-0.nightly-2021-04-30-102231:
oc patch apirequestcount configmaps.v1 -p '{"spec":{"numberOfUsersToReport":3}}' --type=merge
Then create configmaps via ten normal users, then check apirequestcount/configmaps.v1 YAML, the number of reported normal users for each node is not larger than 3.

Comment 10 errata-xmlrpc 2021-07-27 23:04:32 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory (Moderate: OpenShift Container Platform 4.8.2 bug fix and security update), and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHSA-2021:2438


Note You need to log in before you can comment on or make changes to this bug.