Skip to content

Implement scale subresource on Pod owners#6864

Open
SuperQ wants to merge 1 commit into
pingcap:mainfrom
SuperQ:superq/tikv_scale_subresource
Open

Implement scale subresource on Pod owners#6864
SuperQ wants to merge 1 commit into
pingcap:mainfrom
SuperQ:superq/tikv_scale_subresource

Conversation

@SuperQ
Copy link
Copy Markdown
Contributor

@SuperQ SuperQ commented Apr 27, 2026

In order for PodDisruptionBudgets to work on various groups of Pods we need to implement the Scale subresource at the direct owner of the Pod.

Otherwise the PDB will return errors like:

status:
  conditions:
    - lastTransitionTime: "2026-04..."
      message: tikvs.core.pingcap.com does not implement the scale subresource
      reason: SyncFailed
      status: "False"
      type: DisruptionAllowed

Followup to #6213

In order for PodDisruptionBudgets to work on various groups of Pods
we need to implement the Scale subresource at the direct owner of
the Pod.

Otherwise the PDB will return errors like:

```yaml
status:
  conditions:
    - lastTransitionTime: "2026-04..."
      message: tikvs.core.pingcap.com does not implement the scale subresource
      reason: SyncFailed
      status: "False"
      type: DisruptionAllowed
```

Followup to pingcap#6213

Signed-off-by: SuperQ <superq@gmail.com>
@ti-chi-bot
Copy link
Copy Markdown
Contributor

ti-chi-bot Bot commented Apr 27, 2026

[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 csuzhangxc for approval. For more information see the Code Review Process.
Please ensure that each of them provides their approval before proceeding.

The full list of commands accepted by this bot can be found here.

Details 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
Copy link
Copy Markdown
Contributor

ti-chi-bot Bot commented Apr 27, 2026

Hi @SuperQ. Thanks for your PR.

I'm waiting for a pingcap 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.

Details

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-sigs/prow repository.

@github-actions github-actions Bot added the v2 for operator v2 label Apr 27, 2026
@ti-chi-bot ti-chi-bot Bot added the size/M label Apr 27, 2026
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 37.44%. Comparing base (7b536e6) to head (38939e1).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6864   +/-   ##
=======================================
  Coverage   37.44%   37.44%           
=======================================
  Files         392      392           
  Lines       22434    22434           
=======================================
  Hits         8400     8400           
  Misses      14034    14034           
Flag Coverage Δ
unittest 37.44% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@liubog2008
Copy link
Copy Markdown
Member

It seems not reasonable to add /scale for instance crds. Could you comment your PDB spec?

@SuperQ
Copy link
Copy Markdown
Contributor Author

SuperQ commented Apr 28, 2026

Sure, the PDB is pretty simple / standard.

apiVersion: policy/v1
kind: PodDisruptionBudget
metadata:
  name: test-tikv
  namespace: test
spec:
  maxUnavailable: 1
  selector:
    matchLabels:
      app.kubernetes.io/component: tikv
      app.kubernetes.io/instance: test

My understanding is that the PodDisruptionBudget acts on the Pod resource and looks for the Pod's owner.

So, this example:

apiVersion: v1
kind: Pod
metadata:
  name: test-tikv-326vs
  namespace: test
  labels:
    app.kubernetes.io/component: tikv
    app.kubernetes.io/instance: test
    ...
  ownerReferences:
  - apiVersion: core.pingcap.com/v1alpha1
    blockOwnerDeletion: true
    controller: true
    kind: TiKV
    name: test-tikv
    uid: ...

So when Kubernetes tries to gather all the Pods matching the label spec it looks at the owner references and finds that TiKV is the owner, no mention of TiKVGroup.

So the way things are now, the PDB can't work.

@liubog2008
Copy link
Copy Markdown
Member

https://kubernetes.io/docs/tasks/run-application/configure-pdb/#arbitrary-controllers-and-selectors

It seems that maxUnavailable is not supported for current implementation. I'll dig more about this issue.

Anyway, this PR cannot fix the issue.

@SuperQ
Copy link
Copy Markdown
Contributor Author

SuperQ commented May 9, 2026

Oof, that's an unfortunate restriction.

So the only real option is that the TiDB Operator will need to create pods via an indirect method, for example each Pod needs to have an individual StatefulSet.

@SuperQ
Copy link
Copy Markdown
Contributor Author

SuperQ commented May 9, 2026

If you would like, I can file a tracking issue detailing the need here.

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