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

Use TiDB v2.1.8 by default & remove pushgateway #435

Merged
merged 17 commits into from
May 7, 2019
Merged

Use TiDB v2.1.8 by default & remove pushgateway #435

merged 17 commits into from
May 7, 2019

Conversation

tennix
Copy link
Member

@tennix tennix commented Apr 29, 2019

What problem does this PR solve?

This PR removes pushgateway sidecar to embrace newer version (>=v2.1.3) of TiKV. And also upgrade default TiDB to v2.1.8.

What is changed and how it works?

  • TiKV pod pushgateway sidecar is removed
  • Default TiDB version upgraded to v2.1.8
  • Remove unused privileged tidb

Check List

Tests

  • Manual test (add detailed scripts or steps below)
  1. Build new tidb-operator and install into DinD via helm install charts/tidb-operator --name=tidb-operator --namespace=tidb-admin
  2. Install tidb-cluster via helm install charts/tidb-cluster --name=tidb-cluster --namespace=tidb

Code changes

  • Has Helm charts change
  • Has Go code change
  • Has documents change

Side effects

  • Breaking backward compatibility: old version of TiDB cluster (< v2.1.3) monitor would fail to load TiKV dashboard.

Related changes

  • Need to cherry-pick to the release branch
  • Need to update the documentation

@tennix tennix requested review from gregwebs, weekface, onlymellb and qiffang and removed request for gregwebs and weekface April 29, 2019 07:20
@gregwebs
Copy link
Contributor

Is "Remove unused privileged tidb" related to this change? Can this be done in a separate PR? I think privileged tidb for password reset is still a useful concept.

weekface
weekface reviewed Apr 30, 2019
View reviewed changes
charts/tidb-cluster/templates/config/_grafana-prestart.tpl Outdated Show resolved Hide resolved
charts/tidb-cluster/templates/config/_grafana-prestart.tpl Outdated Show resolved Hide resolved
charts/tidb-cluster/templates/config/_tikv-config.tpl Outdated Show resolved Hide resolved
charts/tidb-cluster/templates/config/_tikv-config.tpl Outdated Show resolved Hide resolved
charts/tidb-cluster/values.yaml Outdated Show resolved Hide resolved
@weekface
Copy link
Contributor

/run-e2e-tests

@weekface
Copy link
Contributor

/run-e2e-tests

@gregwebs gregwebs mentioned this pull request May 3, 2019
Closed
@weekface
Copy link
Contributor

weekface commented May 6, 2019

/run-e2e-tests

1 similar comment
@tennix
Copy link
Member Author

tennix commented May 6, 2019

/run-e2e-tests

weekface
weekface previously approved these changes May 6, 2019
View reviewed changes
@tennix
Copy link
Member Author

tennix commented May 6, 2019

@onlymellb I've addressed your comment, PTAL again.

@tennix
Copy link
Member Author

tennix commented May 6, 2019

/run-e2e-tests

@tennix tennix requested a review from aylei May 6, 2019 11:46
weekface
weekface previously approved these changes May 6, 2019
View reviewed changes
@tennix tennix changed the title use TiDB v2.1.8 by default & remove pushgateway Use TiDB v2.1.8 by default & remove pushgateway May 6, 2019
aylei
aylei approved these changes May 6, 2019
View reviewed changes
Copy link
Contributor

@aylei aylei left a comment

Choose a reason for hiding this comment

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

LGTM

onlymellb
onlymellb approved these changes May 7, 2019
View reviewed changes
Copy link
Contributor

@onlymellb onlymellb left a comment

Choose a reason for hiding this comment

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

LGTM

@tennix
Copy link
Member Author

tennix commented May 7, 2019

/run-e2e-tests

@tennix tennix merged commit 8bcb333 into pingcap:master May 7, 2019
@tennix tennix deleted the remove-pushgateway branch May 7, 2019 04:10
@tennix tennix mentioned this pull request May 7, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gregwebs gregwebs gregwebs left review comments

@aylei aylei aylei approved these changes

@onlymellb onlymellb onlymellb approved these changes

@weekface weekface weekface left review comments

@qiffang qiffang Awaiting requested review from qiffang

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@tennix @gregwebs @weekface @onlymellb @aylei @qiffang