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

fix(manager): declare status port in tikv pods #6020

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

omar-mohamed-khallaf
Copy link

What problem does this PR solve?

When scraping pods with victoriametrics, it automatically drops targets (pods in this case) that don't have the port to scrape declared in its spec.ports.

What is changed and how does it work?

Declared the status port in the pod spec.

Code changes

  • Has Go code change
  • Has CI related scripts change

Tests

  • Unit test
  • E2E test
  • Manual test
  • No code

Side effects

  • Breaking backward compatibility
  • Other side effects

Related changes

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

Release Notes

NONE

When scraping pods with victoriametrics, it automatically drops targets
(pods) that don't have the port to scrape declared in its spec.ports.
This patch adds the status port.

Signed-off-by: Omar Mohamed <[email protected]>
Copy link
Contributor

ti-chi-bot bot commented Jan 2, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign charleszheng44 for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found 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 bot requested a review from shonge January 2, 2025 22:13
@sre-bot
Copy link
Contributor

sre-bot commented Jan 2, 2025

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

ti-chi-bot bot commented Jan 2, 2025

Welcome @omar-mohamed-khallaf! It looks like this is your first PR to pingcap/tidb-operator 🎉

@ti-chi-bot ti-chi-bot bot added the size/XS label Jan 2, 2025
Copy link
Member

@csuzhangxc csuzhangxc left a comment

Choose a reason for hiding this comment

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

if we update the Pod spec directly, all TiKV pods will be re-created after the TiDB Operator is upgraded, this may be not acceptable for some users.

@omar-mohamed-khallaf
Copy link
Author

They will be recreated one by one, right?
What should I do?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants