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

Test output of installing rbac policies is too noisy #70448

Closed
gnufied opened this issue Oct 30, 2018 · 3 comments · Fixed by #70475
Closed

Test output of installing rbac policies is too noisy #70448

gnufied opened this issue Oct 30, 2018 · 3 comments · Fixed by #70475
Assignees
Labels
sig/storage Categorizes an issue or PR as relevant to SIG Storage.

Comments

@gnufied
Copy link
Member

gnufied commented Oct 30, 2018

Looks like after #69868 got merged, our test output has become too noisy and we are printing entire globs of RBAC policies.

Oct 30 14:34:41.307: INFO: patching original content of *v1.ServiceAccount:
{
  "kind": "ServiceAccount",
  "apiVersion": "v1",
  "metadata": {
    "name": "csi-driver-registrar",
    "namespace": "default",
    "creationTimestamp": null
  }
}
Oct 30 14:34:41.307: INFO: patching original content of *v1.ClusterRole:
{
  "kind": "ClusterRole",
  "apiVersion": "rbac.authorization.k8s.io/v1",
  "metadata": {
    "name": "driver-registrar-runner",
    "creationTimestamp": null
  },

I do not think, this is necessary.

/sig storage

cc @pohly

@k8s-ci-robot k8s-ci-robot added the sig/storage Categorizes an issue or PR as relevant to SIG Storage. label Oct 30, 2018
@pohly
Copy link
Contributor

pohly commented Oct 30, 2018

It was useful while we started with loading .yaml and patching it and it might be useful again for some other developer who starts to do the same. But I agree, for continuous usage in CI it's less useful.

How about we remove the"patching original content" message (because it mostly represents what's in the .yaml file, except when it uses deprecated aliases) and keep the "creating ..." message? Should that be:

  • multiple lines (as right now)
  • a single line with a JSON dump of the item
  • just a single line with the name of the item?

I'd prefer to see the content, but would be fine with having that in a single line because while less readable, the information is at least there when needed.

@pohly
Copy link
Contributor

pohly commented Oct 30, 2018

/assign

@davidz627
Copy link
Contributor

davidz627 commented Oct 30, 2018

+1 we seem to be logging all the deployment yamls including statefulsets etc. It is hard to see whats going on in the tests at all and probably adds non-trivial execution time as well over the span of hundreds of tests

I would recommend not dumping the items at all in the standard case and only logging actions i.e. creating csi-gce-pd-controller and errors i.e. failed creating csi-gce-pd-controller: {json_dump_here}.

/cc @davidz627

pohly added a commit to pohly/kubernetes that referenced this issue Oct 31, 2018
The detailed dumps of original and patched item content was useful
while developing the feature, but is less relevant now and too
verbose. It might be relevant again, so it's left in the code as
comments.

What gets logged now is just a single-line "creating" resp. "deleting"
message with the type of the item and its unique name.

This also enhances up some other aspects of the original logging:
- the namespace is included for item types that are namespaced
- the "deleting" message no longer gets replicated in each factory
  method

Fixes: kubernetes#70448
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sig/storage Categorizes an issue or PR as relevant to SIG Storage.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants