Skip to content

Retain leading zeros for Pod uid label - #802

Merged
mattlord merged 2 commits into
planetscale:mainfrom
absolutejam:fix-uid-label
Jul 18, 2026
Merged

Retain leading zeros for Pod uid label#802
mattlord merged 2 commits into
planetscale:mainfrom
absolutejam:fix-uid-label

Conversation

@absolutejam

@absolutejam absolutejam commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

@absolutejam

absolutejam commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

Just an FYI, I also have #800 open - so if either is accepted, the other will need to be rebased and fixed, but I didn't want to be presumptuous and based off the other branch.

@frouioui frouioui left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good to me, and good catch. Just one comment

Comment on lines +1 to +2
package vitessshard

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You will need a header comment here, similar to what pkg/controller/vitessshard/reconcile_tablets.go has.

@mattlord
mattlord self-requested a review July 8, 2026 15:22

@mhamza15 mhamza15 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Codex caught that we'll need to update test/endtoend/utils.sh as well:

uid=$((10#${uid}))
That drops the leading zeroes currently and won't match the selector anymore.

}
labels[planetscalev2.CellLabel] = tabletAlias.Cell
labels[planetscalev2.TabletUidLabel] = strconv.FormatUint(uint64(tabletAlias.Uid), 10)
labels[planetscalev2.TabletUidLabel] = fmt.Sprintf("%010d", tabletAlias.Uid)

@mhamza15 mhamza15 Jul 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: it might be nice to extract this to a helper (maybe pkg/operator/vttablet/uid.go#UIDString` or similar) that other parts of the code can use. For example, here's another use-case:

return fmt.Sprintf("%s/vt_%010d", vtDataRootPath, spec.Alias.Uid)

@absolutejam

absolutejam commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author
  • Removed the reformatting of uid in utils.sh
  • Created a helper and I've used that everywhere I can see the original, manual formatting was used
  • Added the license comment

@mattlord

Copy link
Copy Markdown
Collaborator

@absolutejam GitHub actions seems to be stuck in some odd state. Typically the only way to move forward is to push an empty to commit. The CI is the only thing blocking the merge at this point. Thanks!

Signed-off-by: James Booth <james@absolutejam.co.uk>
@absolutejam

Copy link
Copy Markdown
Contributor Author

Is it still the same? If so, I can push an empty commit (With sign-off, for DCO) - but will that get merged into the main branch or will it get squashed?

@mattlord

Copy link
Copy Markdown
Collaborator

@absolutejam I'm sorry, but we might need to merge in origin/main in order to get this: #805

Let's see...

@mattlord

Copy link
Copy Markdown
Collaborator

@absolutejam yeah, looks like that will be required. Sorry for the hassle.

@mattlord

Copy link
Copy Markdown
Collaborator

@absolutejam I'm going to merge in origin/main. I have way too many things in my review queue and I think that is all that is holding this one up.

@mattlord
mattlord merged commit 6b0a1b8 into planetscale:main Jul 18, 2026
12 checks passed
@absolutejam

Copy link
Copy Markdown
Contributor Author

Thanks for that - Apologies for the silence but I have been on leave

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants