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.

Sign up for GitHub

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

Jump to bottom

chore(dashboard): update TiDB Dashboard to v6.5.11-9d2b0ce6 [release-6.5] #8601

Merged
merged 3 commits into from
Sep 10, 2024
Merged

chore(dashboard): update TiDB Dashboard to v6.5.11-9d2b0ce6 [release-6.5] #8601

merged 3 commits into from
Sep 10, 2024

Conversation

baurine
Copy link
Contributor

@baurine baurine commented Sep 9, 2024 •
edited by rleungx
Loading

What problem does this PR solve?

Issue Number: ref #8611.

Update TiDB Dashboard to v6.5.11-9d2b0ce6.

Release note

None

@ti-chi-bot ti-chi-bot bot added do-not-merge/cherry-pick-not-approved release-note-none Denotes a PR that doesn't merit a release note. dco-signoff: yes Indicates the PR's author has signed the dco. needs-ok-to-test Indicates a PR created by contributors and need ORG member send '/ok-to-test' to start testing. labels Sep 9, 2024
@ti-chi-bot ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Sep 9, 2024

Hi @baurine. Thanks for your PR.

I'm waiting for a tikv member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@ti-chi-bot ti-chi-bot bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Sep 9, 2024
@mornyx
add config item
e365041 
Signed-off-by: mornyx <[email protected]>
@ti-chi-bot ti-chi-bot bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Sep 9, 2024
@HuSharp
Copy link
Member

HuSharp commented Sep 9, 2024

/ok-to-test

@ti-chi-bot ti-chi-bot bot added ok-to-test Indicates a PR is ready to be tested. and removed needs-ok-to-test Indicates a PR created by contributors and need ORG member send '/ok-to-test' to start testing. labels Sep 9, 2024
@mornyx
Copy link
Contributor

mornyx commented Sep 9, 2024

This is a 6.5 cherry-pick of #8581, PTAL, thx!
/cc @lhy1024 @rleungx @niubell

@ti-chi-bot ti-chi-bot bot requested review from lhy1024 and rleungx September 9, 2024 08:35
@ti-chi-bot ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Sep 9, 2024

@mornyx: GitHub didn't allow me to request PR reviews from the following users: niubell.

Note that only tikv members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

This is a 6.5 cherry-pick of #8581, PTAL, thx!
/cc @lhy1024 @rleungx @niubell

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

rleungx
rleungx reviewed Sep 9, 2024
View reviewed changes
pkg/dashboard/adapter/config.go Outdated
@@ -35,6 +35,7 @@ func GenDashboardConfig(srv *server.Server) (*config.Config, error) {
dashboardCfg.PublicPathPrefix = cfg.Dashboard.PublicPathPrefix
dashboardCfg.EnableTelemetry = cfg.Dashboard.EnableTelemetry
dashboardCfg.EnableExperimental = cfg.Dashboard.EnableExperimental
dashboardCfg.DisableCustomPromAddr = cfg.Dashboard.DisableCustomPromAddr
Copy link
Member

Choose a reason for hiding this comment

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

Will the config be lost when we upgrade?

Copy link
Contributor

Choose a reason for hiding this comment

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

Will the config be lost when we upgrade?

This configuration already exists in the master and release-6.5 branches of tidb-dashboard and will not be lost during a normal upgrade.

Copy link
Member

Choose a reason for hiding this comment

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

What if we set it to true in 6.5.11, then the cluster is upgraded to 7.1.x which doesn't contain this PR?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, I get what you mean. This change will be cherry-picked to all LTS branches later. (At least for now only 6.5.x customers explicitly need to configure this.)

Copy link
Contributor

Choose a reason for hiding this comment

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

If an upgrade occurs during this period, this configuration will indeed lose its effect.

Copy link
Member

Choose a reason for hiding this comment

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

After losing it, if we upgrade again to a higher version with this config, the config value will be changed to false.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is acceptable and I will doc this behavior. It is expected that this configuration will not work after upgrading to a version that does not support this feature.

@ti-chi-bot ti-chi-bot bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Sep 9, 2024
@ti-chi-bot ti-chi-bot bot added needs-1-more-lgtm Indicates a PR needs 1 more LGTM. approved cherry-pick-approved Cherry pick PR approved by release team. and removed do-not-merge/cherry-pick-not-approved labels Sep 10, 2024
@ti-chi-bot ti-chi-bot bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Sep 10, 2024
@ti-chi-bot ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Sep 10, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: HuSharp, rleungx

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

@ti-chi-bot ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Sep 10, 2024

[LGTM Timeline notifier]

Timeline:

  • 2024-09-10 03:21:59.190964083 +0000 UTC m=+326588.931388017: ☑️ agreed by rleungx.
  • 2024-09-10 04:07:46.234557071 +0000 UTC m=+329335.974981011: ☑️ agreed by HuSharp.

@ti-chi-bot ti-chi-bot bot merged commit 86ddba1 into tikv:release-6.5 Sep 10, 2024
18 checks passed
@mornyx mornyx deleted the update-tidb-dashboard/release-6.5-v6.5.11-9d2b0ce6-1725850168 branch September 10, 2024 04:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mornyx mornyx mornyx left review comments

@HuSharp HuSharp HuSharp approved these changes

@rleungx rleungx rleungx approved these changes

@lhy1024 lhy1024 Awaiting requested review from lhy1024

Assignees
No one assigned
Labels
approved cherry-pick-approved Cherry pick PR approved by release team. dco-signoff: yes Indicates the PR's author has signed the dco. lgtm ok-to-test Indicates a PR is ready to be tested. release-note-none Denotes a PR that doesn't merit a release note. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@baurine @HuSharp @mornyx @rleungx