close
Skip to content

fix(dataprotection): shorten backup policy derived names#10274

Merged
leon-ape merged 1 commit into
mainfrom
clara/dp-backup-policy-name-shortening
May 22, 2026
Merged

fix(dataprotection): shorten backup policy derived names#10274
leon-ape merged 1 commit into
mainfrom
clara/dp-backup-policy-name-shortening

Conversation

@weicao
Copy link
Copy Markdown
Contributor

@weicao weicao commented May 20, 2026

Summary

  • Shorten generated BackupPolicy and BackupSchedule names with constant.ShortenKubeName.
  • Preserve existing short-name output for normal cluster/component names.
  • Add focused coverage for overlength backup policy/schedule names and collision avoidance on different long inputs.

Motivation

OceanBase Restore long-name validation for #10266 reached patch-image scoped PASS on the 70-character live path, but the strongest 74-character attempt exposed an earlier Kubernetes name-length blocker before the Restore path could run:

  • generated BackupPolicy name: <restore>-<component>-backup-policy can reach 65 chars;
  • generated BackupSchedule name: <restore>-<component>-backup-schedule can reach 67 chars.

These names are DataProtection-derived resources, not OceanBase-specific resources, so the name hardening should live in the controller helper that generates them.

Validation

  • go generate ./pkg/testutil/k8s/mocks
  • KUBEBUILDER_ASSETS="$(/Users/wei/ApeCloud/kubeblocks/bin/setup-envtest-release-0.21 use 1.26.1 -p path)" go test ./controllers/dataprotection -run 'TestAPIs/BackupPolicyDriver' -count=1 -v PASS, 130/130 specs
  • go test -c ./controllers/dataprotection PASS
  • KUBEBUILDER_ASSETS="$(/Users/wei/ApeCloud/kubeblocks/bin/setup-envtest-release-0.21 use 1.26.1 -p path)" go test ./pkg/dataprotection/restore -count=1 PASS
  • git diff --check PASS

Boundary

This PR only hardens generated BackupPolicy/BackupSchedule names. It does not change the #10266 Restore prepareData/postReady name-shortening path and does not claim release-ready validation. The affected addon team still needs a patch-image sideload run for the 74-character long-name lane.

@weicao weicao requested review from a team, ldming and wangyelei as code owners May 20, 2026 14:02
@github-actions github-actions Bot added the size/M Denotes a PR that changes 30-99 lines. label May 20, 2026
@apecloud-bot
Copy link
Copy Markdown
Collaborator

Auto Cherry-pick Instructions

Usage:
  - /nopick: Not auto cherry-pick when PR merged.
  - /pick: release-x.x [release-x.x]: Auto cherry-pick to the specified branch when PR merged.

Example:
  - /nopick
  - /pick release-1.1

@codecov
Copy link
Copy Markdown

codecov Bot commented May 20, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 52.92%. Comparing base (fca08ae) to head (17661f7).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #10274      +/-   ##
==========================================
+ Coverage   52.73%   52.92%   +0.19%     
==========================================
  Files         534      534              
  Lines       61231    61250      +19     
==========================================
+ Hits        32288    32415     +127     
+ Misses      25683    25589      -94     
+ Partials     3260     3246      -14     
Flag Coverage Δ
unittests 52.92% <100.00%> (+0.19%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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

@weicao
Copy link
Copy Markdown
Contributor Author

weicao commented May 20, 2026

Runtime validation update for the naming fix scope:

  • PR head under test: 17661f7251a9c71cc7c867eb834b320773cab3e6.
  • Patch image used by the OceanBase validation lane: docker.io/apecloud/kubeblocks-dataprotection:clara-dp-bps-name-shortening-17661f7.
  • Patch image identity: linux/amd64 manifest sha256:5833cf487072ccdd1df7ad40e53dcda5112341b59568628a497289afc32dc619, image config sha256:0bda8b0b5f8d98ef7c059cba11cea722391940851db97a63b264493495dc20c1, imported on all 3 test nodes with rc=0.
  • In the 74-char strongest-legal long-name lane, the Restore-derived BackupPolicy raw name length was 65 and was shortened to a 63-char name: ob-pitr-p5n2-longname-n1-dp1-restorexxxxx-oceanbase-ba-b9e9e775.
  • The Restore-derived BackupSchedule raw name length was 67 and was shortened to a 63-char name: ob-pitr-p5n2-longname-n1-dp1-restorexxxxx-oceanbase-ba-977298ba.
  • prepareData Restore completed in 1m33s; no Kubernetes API name-too-long rejection was observed for BackupPolicy, BackupSchedule, Job volume, or volumeMount names.
  • The observed names match the implementation contract: constant.ShortenKubeName(raw, 63) emits <54-char prefix>-<8-char fnv32a hash> for these inputs. The observed hashes are b9e9e775 and 977298ba, matching the raw BackupPolicy and BackupSchedule names.

Boundary: this is patch-image scoped validation of the #10274 naming behavior. The full PITR readback/cleanup lane later hit an environment blocker in the vcluster API NodePort path, so this comment does not claim release-ready or full OceanBase PITR acceptance.

@weicao
Copy link
Copy Markdown
Contributor Author

weicao commented May 20, 2026

Final runtime validation update for the #10274 naming scope:

  • PR head under test: 17661f7251a9c71cc7c867eb834b320773cab3e6.
  • Patch image: docker.io/apecloud/kubeblocks-dataprotection:clara-dp-bps-name-shortening-17661f7.
  • Patch image identity: linux/amd64 manifest sha256:5833cf487072ccdd1df7ad40e53dcda5112341b59568628a497289afc32dc619, image config sha256:0bda8b0b5f8d98ef7c059cba11cea722391940851db97a63b264493495dc20c1, imported on all 3 test nodes with rc=0.
  • 74-char strongest-legal OceanBase PITR lane completed the product path via local port-forward closeout:
    • full Backup completed;
    • archive covered the target window;
    • Restore OpsRequest succeeded;
    • prepareData Restore completed in 1m33s with no 63-char API rejection;
    • postReady completed in 7m42s;
    • readback returned 1,2,3,4 and excluded row 5.
  • Naming behavior matched this PR's implementation contract:
    • Restore-derived BackupPolicy raw name length 65 -> 63-char shortened name with 8-char hash suffix.
    • Restore-derived BackupSchedule raw name length 67 -> 63-char shortened name with 8-char hash suffix.
    • The observed hash suffixes match constant.ShortenKubeName(raw, 63) for the raw BackupPolicy and BackupSchedule names.

Evidence from the addon validation lane:

  • evidence root SHA: 4ea2cb6214a29d6701ded841d4986f4c2d79ef30ce151817205accee8602f338
  • readback SHA: 3ddda085e6dc3595f64588ec88fdd6e4529b9268c7e7c259947cc58d9032e739
  • tar SHA: 3b242286...7777a0
  • attachment: 1b215e08-e256-4ad1-a417-47f15961ef0f

Boundary:

@weicao
Copy link
Copy Markdown
Contributor Author

weicao commented May 20, 2026

Closeout addendum for the 74-char OceanBase PITR validation lane:

  • Runtime PASS, readback PASS, and cleanup clean are now all closed for the patch-image scoped lane.
  • Cleanup required an owner-approved narrow finalizer patch for the known Backup finalizer cleanup-order issue already covered by PR fix(dataprotection): avoid worker setup before cleanup decision #10205. The cleanup patch was limited to 2 Backup CRs' dataprotection.kubeblocks.io/finalizer; it did not touch namespace finalizers, BackupRepo, DataProtection deployment, manager, or Spiderpool resources.
  • Final closeout SHA: d64b08bd89e66462ddac93e5e0465a2b1bcb479e0911e0c25a84ddf1a9c7bc7e
  • Final tar SHA: a3a747e3db44d62183048ec7e99f8fe403cedac06d0ec07603344b4b62fb9df4
  • Attachment: bcdef5b7-7a8b-4ff4-a4d6-e95a63428e67

Final boundary for this PR:

@apecloud-bot apecloud-bot added the approved PR Approved Test label May 21, 2026
@leon-ape leon-ape added the nopick Not auto cherry-pick when PR merged label May 22, 2026
@leon-ape
Copy link
Copy Markdown
Contributor

I think a cleaner naming scheme would be to drop the backup-policy / backup-schedule suffixes instead of keeping them and then shortening the full name. The resource kind already distinguishes BackupPolicy from BackupSchedule, so repeating the type in the object name is not necessary.

Using the same base name, e.g. <cluster>-<component>, for both resources should still be unambiguous because they are different CR types, and it also reduces the chance of hitting the Kubernetes name length limit and needing hash-based shortening.

If preserving the existing short-name output is intentional for compatibility, that tradeoff should be called out explicitly. Otherwise, I suggest switching to the suffix-free naming scheme and updating the tests accordingly.

@leon-ape
Copy link
Copy Markdown
Contributor

After looking more closely at the compatibility behavior, keeping the existing suffixes makes sense here because ShortenKubeName preserves all currently valid names unchanged and only affects names that were previously too long to be created.

Dropping the suffixes would be a broader naming change and would require migration/compatibility handling for existing objects. So my previous suggestion is more of a possible future cleanup, not a blocker for this PR.

@leon-ape leon-ape added pick-1.0 Auto cherry-pick to release-1.0 when PR merged pick-1.1 Auto cherry-pick to release-1.1 when PR merged and removed nopick Not auto cherry-pick when PR merged labels May 22, 2026
@leon-ape leon-ape merged commit 9546baf into main May 22, 2026
71 of 74 checks passed
@leon-ape leon-ape deleted the clara/dp-backup-policy-name-shortening branch May 22, 2026 07:06
@github-actions github-actions Bot added this to the Release 1.2.0 milestone May 22, 2026
@apecloud-bot
Copy link
Copy Markdown
Collaborator

/cherry-pick release-1.0

@apecloud-bot
Copy link
Copy Markdown
Collaborator

/cherry-pick release-1.1

@apecloud-bot
Copy link
Copy Markdown
Collaborator

🤖 says: cherry pick action finished successfully 🎉!
See: https://github.com/apecloud/kubeblocks/actions/runs/26273645727

@apecloud-bot
Copy link
Copy Markdown
Collaborator

🤖 says: cherry pick action finished successfully 🎉!
See: https://github.com/apecloud/kubeblocks/actions/runs/26273646469

apecloud-bot pushed a commit that referenced this pull request May 22, 2026
apecloud-bot pushed a commit that referenced this pull request May 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved PR Approved Test pick-1.0 Auto cherry-pick to release-1.0 when PR merged pick-1.1 Auto cherry-pick to release-1.1 when PR merged size/M Denotes a PR that changes 30-99 lines.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants