Fix failing pgupgrade if names are too long#1649
Open
Kajot-dev wants to merge 2 commits into
Open
Conversation
Signed-off-by: jjaruszewski <jjaruszewski@man.poznan.pl>
Kajot-dev
requested review from
egegunes,
gkech,
hors,
mayankshah1607,
nmarukovich,
oksana-grishchenko and
pooknull
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses pgupgrade Jobs silently failing when generated Kubernetes Job names exceed the DNS-1123 label limit (63 chars), by introducing DNS-safe naming helpers and switching pgupgrade Jobs to use a truncated/unique name approach.
Changes:
- Added
SafeDNSName/SafeDNSUniqueNamehelpers ininternal/naming/dns.goto enforce the 63-character DNS label limit. - Updated pgupgrade and remove-data Job name generation to use the new DNS-safe unique naming helper.
- Updated pgupgrade reconciliation to locate the upgrade Job by labels (instead of by deterministic name), and added a unit test for long upgrade names.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| internal/naming/dns.go | Introduces helpers to truncate DNS label names and generate “unique” names under the 63-char limit. |
| internal/controller/pgupgrade/pgupgrade_controller.go | Changes upgrade Job lookup logic to search Jobs by role label. |
| internal/controller/pgupgrade/jobs.go | Switches pgupgrade/remove-data Job naming to use SafeDNSUniqueName. |
| internal/controller/pgupgrade/jobs_test.go | Adds a unit test validating long-name Job generation stays within DNS limits. |
Signed-off-by: jjaruszewski <jjaruszewski@man.poznan.pl>
Kajot-dev
force-pushed
the
fix/failing-pg-upgrade
branch
from
f50582a to
57ffb08
Compare
Comment on lines
+39
to
+61
| // SafeDNSUniqueName ensures the name fits within the 63-character DNS-1123 label limit. | ||
| // If the name exceeds the limit, it truncates to 58 characters and appends a 4-character | ||
| // deterministic suffix based on the input name to maintain consistency across reconciles. | ||
| // It also ensures the name doesn't end with a hyphen, which is invalid for DNS labels. | ||
| // This is useful for resources that need unique names like Jobs or Pods. | ||
| func SafeDNSUniqueName(name string) string { | ||
| if len(name) <= maxDNSSafeLength { | ||
| return name | ||
| } | ||
|
|
||
| // Reserve 5 characters for the dash + 4 char suffix | ||
| prefix := name[:maxDNSSafeLength-5] | ||
| // Strip trailing hyphens from the truncated prefix | ||
| prefix = strings.TrimRight(prefix, "-") | ||
|
|
||
| // Use a deterministic suffix based on the full name (not random!) | ||
| // This ensures the same name always produces the same output across reconciles | ||
| hash := fnv.New32a() | ||
| hash.Write([]byte(name)) | ||
| suffix := rand.SafeEncodeString(fmt.Sprint(hash.Sum32()))[:4] | ||
|
|
||
| return prefix + "-" + suffix | ||
| } |
|
|
||
| // Verify the job name fits within DNS limits and has the correct format | ||
| assert.Assert(t, len(longJob.Name) <= 63, "job name %q exceeds 63 characters", longJob.Name) | ||
| assert.Assert(t, len(longJob.Name) == 63, "truncated job name %q should be exactly 63 characters", longJob.Name) |
Collaborator
commit: 57ffb08 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
CHANGE DESCRIPTION
Problem:
When the pgupgrade name/cluster name is too long the generated job names for pgupgrade will exceed 63 characters and upgrade will silently fail (visible in the logs, but not visible by the user, which sees the upgrade as indefinietely progressing)
Solution:
We need to trim the name (and preserve uniqueness) to
CHECKLIST
Jira
Needs Doc) and QA (Needs QA)?Tests
Config/Logging/Testability