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

[Integration branch]To merge all Node PRs #2881

Merged

Conversation

ChengYanJin
Copy link
Contributor

@ChengYanJin ChengYanJin commented Oct 20, 2020

Component: ui, nodes

Context:
The integration branch with all the Node Page PRs to be merged.

Summary:
The new commits that do not cover in the open PRs

  • Rename from /newNodes to /nodes
  • Auto select the first node in the list when hit URL /nodes
  • Add Advanced Metrics button in Metrics. (Need to figure out how to redirect to the dashboard specific for this node)
  • Retrieve unode-name from Prometheus in order to be redirect to the right Node Detailed dashboard in Grafana

Acceptance criteria:
Overview Tab:
image
image
Metrics Tab:
image
Volume Tab:
image
Pod Tab:
image
Detail Tab:
image


Closes: #2877
Closes: #2775
Closes: #2791
Closes: #2777
Closes: #2802

ChengYanJin and others added 30 commits October 14, 2020 17:21
- Update the dark mode with final version
- Update the status color of light mode with proposal, but keep the primary/text/border as the previous version,
  since it doesn't work well.
In order to the childrens to retrieve alerts

Refs: #2775
The circle next to the Node Name should depend on the health

Refs: #2775
Fix the bug of URL when click on the tab

Refs: #2775
- Update the dark mode with final version
- Update the status color of light mode with proposal, but keep the primary/text/border as the previous version,
  since it doesn't work well.
Out of the scope of this PR:
Since we update the color-palette, there is some sightly changes in order to align with the design

Refs: #2791
We used to copy-paste the contents of the ui/public/brand/theme.json
file to this ConfigMap manifest.
To make sure we never forget about it nor make any mistake when copying,
we switch to templating this manifest from the contents of the original
file.
- When switch bwtween the nodes, keep the same tab, and same timespan if metric tab is selected
- When switch bwtween the nodes, if there no tab selected, by default `health` tab is selected

Refs: #2776
Fix the bug of progressbar in core-ui

Refs: #2777
Todo: Need to work on the path before change the URL back to /volumes

Refs: #2777
Copy link
Contributor

@thomasdanan thomasdanan left a comment

Choose a reason for hiding this comment

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

Excellent !!!

I noticed few things:

  • the advanced metrics link (in metrics tab) was not bringing me to the right node instance
  • If I click on Create Node and Cancel, the node selection from where I was coming from is lost.
  • still see the little bug about capacity usage bar
  • I realize that it is not obvious that one node is having one Volume which is in Critical state. We may want this to appear in the tab header (more or less like we do for alerts: displaying number of Volumes and maybe number of volumes in critical health) => probably another improvement ticket
  • If I search a node with some free text in the search bar, then if I select a node and start navigating through the tabs, the search filter is not applied anymore (and is removed from url)
  • ideally, the up/down arrow key should let the user switching from one node to another

@ChengYanJin ChengYanJin force-pushed the feature/integration-branch-to-merge-all-node-prs branch from b42e062 to efcd7cc Compare October 20, 2020 13:46
@bert-e
Copy link
Contributor

bert-e commented Oct 20, 2020

History mismatch

Merge commit #0435bddbf1ba7373563bb55fba5079fef6b3ba61 on the integration branch
w/2.7/feature/integration-branch-to-merge-all-node-prs is merging a branch which is neither the current
branch feature/integration-branch-to-merge-all-node-prs nor the development branch
development/2.7.

It is likely due to a rebase of the branch feature/integration-branch-to-merge-all-node-prs and the
merge is not possible until all related w/* branches are deleted or updated.

Please use the reset command to have me reinitialize these branches.

@ChengYanJin ChengYanJin force-pushed the feature/integration-branch-to-merge-all-node-prs branch from 8741ff4 to 04bbf54 Compare October 21, 2020 07:32
@ChengYanJin
Copy link
Contributor Author

Hi @thomas,
Thanks for the early feedback!

I fixed the bugs/issues you raised in the comments.
But not these two bullet points. I will create tickets to track them.

  • I realize that it is not obvious that one node is having one Volume which is in Critical state. We may want this to appear in the tab header (more or less like we do for alerts: displaying number of Volumes and maybe number of volumes in critical health) => probably another improvement ticket

  • ideally, the up/down arrow key should let the user switching from one node to another

thomasdanan
thomasdanan previously approved these changes Oct 21, 2020
@ChengYanJin ChengYanJin added the topic:ui UI-related issues label Oct 21, 2020
@ChengYanJin ChengYanJin force-pushed the feature/integration-branch-to-merge-all-node-prs branch 3 times, most recently from eff6a50 to 72e7a29 Compare October 22, 2020 05:57
@ChengYanJin
Copy link
Contributor Author

/reset

@bert-e
Copy link
Contributor

bert-e commented Oct 23, 2020

Reset complete

I have successfully deleted this pull request's integration branches.

@bert-e
Copy link
Contributor

bert-e commented Oct 23, 2020

Integration data created

I have created the integration data for the additional destination branches.

The following branches will NOT be impacted:

  • development/1.0
  • development/1.1
  • development/1.2
  • development/1.3
  • development/2.0
  • development/2.1
  • development/2.2
  • development/2.3
  • development/2.4
  • development/2.5

You can set option create_pull_requests if you need me to create
integration pull requests in addition to integration branches, with:

@bert-e create_pull_requests

@bert-e
Copy link
Contributor

bert-e commented Oct 23, 2020

Waiting for approval

The following approvals are needed before I can proceed with the merge:

  • the author

  • one peer

Peer approvals must include at least 1 approval from the following list:

@bert-e
Copy link
Contributor

bert-e commented Oct 23, 2020

Waiting for approval

The following approvals are needed before I can proceed with the merge:

  • the author

  • one peer

Peer approvals must include at least 1 approval from the following list:

@ChengYanJin
Copy link
Contributor Author

/approve

@bert-e
Copy link
Contributor

bert-e commented Oct 23, 2020

In the queue

The changeset has received all authorizations and has been added to the
relevant queue(s). The queue(s) will be merged in the target development
branch(es) as soon as builds have passed.

The changeset will be merged in:

  • ✔️ development/2.6

  • ✔️ development/2.7

The following branches will NOT be impacted:

  • development/1.0
  • development/1.1
  • development/1.2
  • development/1.3
  • development/2.0
  • development/2.1
  • development/2.2
  • development/2.3
  • development/2.4
  • development/2.5

There is no action required on your side. You will be notified here once
the changeset has been merged. In the unlikely event that the changeset
fails permanently on the queue, a member of the admin team will
contact you to help resolve the matter.

IMPORTANT

Please do not attempt to modify this pull request.

  • Any commit you add on the source branch will trigger a new cycle after the
    current queue is merged.
  • Any commit you add on one of the integration branches will be lost.

If you need this pull request to be removed from the queue, please contact a
member of the admin team now.

The following options are set: approve

@bert-e
Copy link
Contributor

bert-e commented Oct 23, 2020

I have successfully merged the changeset of this pull request
into targetted development branches:

  • ✔️ development/2.6

  • ✔️ development/2.7

The following branches have NOT changed:

  • development/1.0
  • development/1.1
  • development/1.2
  • development/1.3
  • development/2.0
  • development/2.1
  • development/2.2
  • development/2.3
  • development/2.4
  • development/2.5

Please check the status of the associated issue None.

Goodbye chengyanjin.

@bert-e bert-e merged commit 6494107 into development/2.6 Oct 23, 2020
@bert-e bert-e deleted the feature/integration-branch-to-merge-all-node-prs branch October 23, 2020 14:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic:ui UI-related issues
Projects
None yet
4 participants