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

Configure logging #1140

Merged
merged 1 commit into from
Jan 11, 2022
Merged

Configure logging #1140

merged 1 commit into from
Jan 11, 2022

Conversation

adfost
Copy link
Contributor

@adfost adfost commented Oct 22, 2021

Add Tenant Logs view and edit modal
Screen Shot 2021-10-28 at 1 25 56 PM

swagger-operator.yml Outdated Show resolved Hide resolved
portal-ui/build/index.html Outdated Show resolved Hide resolved
@adfost adfost force-pushed the configure_logging branch 2 times, most recently from 75cb18f to 17b4352 Compare October 26, 2021 20:10
@adfost adfost self-assigned this Oct 28, 2021
@dvaldivia
Copy link
Collaborator

dvaldivia commented Oct 28, 2021

After editing the tenant for the first time, the label editors start showing empty lines
Screen Shot 2021-10-28 at 10 38 19 AM

Screen Shot 2021-10-28 at 10 39 01 AM

Screen Shot 2021-10-28 at 10 39 14 AM

@dvaldivia
Copy link
Collaborator

The tenant CRD became corrupt with empty data:

log:
    annotations:
      "": ""
    audit:
      diskCapacityGB: 5
    db:
      annotations:
        "": ""
      image: library/postgres:13
      initimage: busybox:1.33.1
      labels:
        "": ""
      nodeSelector:
        "": ""

Copy link
Collaborator

@dvaldivia dvaldivia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rebase and address bug

@dvaldivia
Copy link
Collaborator

verify the layout of the grid to match the spacing on this tab
Screen Shot 2021-10-28 at 10 44 47 AM
Screen Shot 2021-10-28 at 10 44 43 AM

@dvaldivia
Copy link
Collaborator

If the tenant doesn't have a .spec.log field set, I cannot edit. Please add a capability to enable/disable logs

  • if enabling: set the logging defaults we set when creating a tenant
  • if disabling: set .spec.log to null

@adfost adfost force-pushed the configure_logging branch 4 times, most recently from f841f84 to 9929bd4 Compare October 29, 2021 00:59
@dvaldivia
Copy link
Collaborator

When editing and adding only labels, containers images are being injected even if not specified.

New State after Editing (left side) vs Previous State before Editing (right side)
Screen Shot 2021-10-29 at 10 23 59 AM

operatorapi/operator_tenants.go Outdated Show resolved Hide resolved
@adfost adfost force-pushed the configure_logging branch 4 times, most recently from 6715bf6 to 32d433a Compare November 4, 2021 20:00
@jinapurapu
Copy link
Contributor

GET is being called twice when loading Logs tab
Screenshot from 2021-11-04 14-51-59

@adfost adfost force-pushed the configure_logging branch 2 times, most recently from d7f0e23 to 9bc7fe7 Compare November 9, 2021 03:25
dvaldivia
dvaldivia previously approved these changes Nov 12, 2021
@adfost adfost force-pushed the configure_logging branch 2 times, most recently from 0f1d115 to acfb498 Compare November 15, 2021 21:09
@adfost adfost force-pushed the configure_logging branch 2 times, most recently from 7a391f4 to 6ee586c Compare November 29, 2021 17:39
@dvaldivia
Copy link
Collaborator

it doesn't let me create a three character service account called uno which kubernetes does let me create, thus it's a valid sa name

Screen Shot 2021-11-30 at 11 14 17 AM

Screen Shot 2021-11-30 at 11 13 26 AM

</td>
<td>{logInfo?.serviceAccountName}</td>
</tr>
<h4>Labels</h4>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warning: validateDOMNesting(...): <h4> cannot appear as a child of <tbody>.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all these html elements must be inside a column, it's invalid html if they are inside the body of the table

@dvaldivia
Copy link
Collaborator

dvaldivia commented Nov 30, 2021

Please fix all these browser warnings
Screen Shot 2021-11-30 at 11 19 40 AM
Screen Shot 2021-11-30 at 11 19 49 AM
Screen Shot 2021-11-30 at 11 19 46 AM

Copy link
Collaborator

@bexsoft bexsoft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice if delete button gets disabled when only one option is set
Screen Shot 2021-11-30 at 13 28 43

Also found another issue, when you type fast in these fields, DOM gets frozen for a couple of seconds and then all the text appears suddenly

Screen.Recording.2021-11-30.at.13.27.08-hevcmp4.mp4

@Alevsk
Copy link
Contributor

Alevsk commented Nov 30, 2021

@adfost please check why the request is triggering twice when someone click on the logs menu

Screen Shot 2021-11-30 at 14 05 33

I have seen a similar behavior on other sections of the application, we are doing exactly the same request twice

Alevsk
Alevsk previously requested changes Nov 30, 2021
operatorapi/operator_tenants.go Show resolved Hide resolved
@Alevsk
Copy link
Contributor

Alevsk commented Nov 30, 2021

When logsearchApi and Postgres are enabled but user didn't defined an image explicitly we are displaying a blank value in the UI

Screen Shot 2021-11-30 at 14 24 27

Screen Shot 2021-11-30 at 14 24 51

We should display display the default

@adfost adfost force-pushed the configure_logging branch 2 times, most recently from 0f0580a to 233601b Compare December 1, 2021 20:12
@adfost adfost force-pushed the configure_logging branch 2 times, most recently from 6719adb to 58ada4f Compare December 15, 2021 18:05
@adfost adfost force-pushed the configure_logging branch 2 times, most recently from 8cf4b68 to 8d576e7 Compare December 31, 2021 01:41
@dvaldivia
Copy link
Collaborator

my tenant doesn't specify a log search image, if I edit and save, the default image is stored in the tenant CR, it shouldn't so that this version is updated with the operator

  log:
    audit:
      diskCapacityGB: 5
    db:
      resources: {}
      volumeClaimTemplate:
        metadata:
          creationTimestamp: null
          name: videos-log
        spec:
          accessModes:
          - ReadWriteOnce
          resources:
            requests:
              storage: "5368709120"
          storageClassName: standard
        status: {}
    resources: {}

@adfost adfost force-pushed the configure_logging branch 3 times, most recently from e3f8060 to 9d21ddf Compare January 4, 2022 16:16
@bexsoft bexsoft requested a review from Alevsk January 11, 2022 20:29
@dvaldivia dvaldivia dismissed Alevsk’s stale review January 11, 2022 23:18

no need to display

@dvaldivia dvaldivia merged commit 0400e0c into minio:master Jan 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants