Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 2 additions & 11 deletions internal/controller/pgupgrade/jobs.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,6 @@ import (

// Upgrade job

// pgUpgradeJob returns the ObjectMeta for the pg_upgrade Job utilized to
// upgrade from one major PostgreSQL version to another
func pgUpgradeJob(upgrade *v1beta1.PGUpgrade) metav1.ObjectMeta {
return metav1.ObjectMeta{
Namespace: upgrade.Namespace,
Name: upgrade.Name + "-pgdata",
}
}

// upgradeCommand returns an entrypoint that prepares the filesystem for
// and performs a PostgreSQL major version upgrade using pg_upgrade.
func upgradeCommand(oldVersion, newVersion int, fetchKeyCommand string, availableCPUs int) []string {
Expand Down Expand Up @@ -142,7 +133,7 @@ func (r *PGUpgradeReconciler) generateUpgradeJob(
job.SetGroupVersionKind(batchv1.SchemeGroupVersion.WithKind("Job"))

job.Namespace = upgrade.Namespace
job.Name = pgUpgradeJob(upgrade).Name
job.Name = naming.SafeDNSUniqueName(upgrade.Name + "-pgdata")
Comment thread
Kajot-dev marked this conversation as resolved.

job.Labels = Merge(upgrade.Spec.Metadata.GetLabelsOrNil(),
commonLabels(pgUpgrade, upgrade), //FIXME role pgupgrade
Expand Down Expand Up @@ -302,7 +293,7 @@ func (r *PGUpgradeReconciler) generateRemoveDataJob(
job.SetGroupVersionKind(batchv1.SchemeGroupVersion.WithKind("Job"))

job.Namespace = upgrade.Namespace
job.Name = upgrade.Name + "-" + sts.Name
job.Name = naming.SafeDNSUniqueName(upgrade.Name + "-" + sts.Name)
Comment thread
Kajot-dev marked this conversation as resolved.

job.Labels = labels.Merge(upgrade.Spec.Metadata.GetLabelsOrNil(),
commonLabels(removeData, upgrade)) //FIXME role removedata
Expand Down
21 changes: 21 additions & 0 deletions internal/controller/pgupgrade/jobs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package pgupgrade
import (
"context"
"os"
"regexp"
"strings"
"testing"

Expand Down Expand Up @@ -240,6 +241,26 @@ spec:
status: {}
`))

t.Run("LongName", func(t *testing.T) {
// Test with an intentionally long name that exceeds 63 characters
longNameUpgrade := &v1beta1.PGUpgrade{}
longNameUpgrade.Name = "very-long-upgrade-name-that-will-exceed-the-maximum-dns-label-limit"
longNameUpgrade.Namespace = "ns1"
longNameUpgrade.Spec.PostgresClusterName = "very-long-cluster-name-that-also-exceeds-limits"
longNameUpgrade.Spec.FromPostgresVersion = 14
longNameUpgrade.Spec.ToPostgresVersion = 15

longJob := reconciler.generateUpgradeJob(ctx, longNameUpgrade, startup, "")

// 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)
Comment thread
Kajot-dev marked this conversation as resolved.
// Verify the name ends with a dash followed by 4 alphanumeric characters (the random suffix)
// Pattern: <prefix>-<4 alphanumeric chars>
assert.Assert(t, regexp.MustCompile(`-[a-zA-Z0-9]{4}$`).MatchString(longJob.Name),
"job name %q should end with -<4 alphanumeric chars>", longJob.Name)
})

t.Run(feature.PGUpgradeCPUConcurrency+"Enabled", func(t *testing.T) {
gate := feature.NewGate()
assert.NilError(t, gate.SetFromMap(map[string]bool{
Expand Down
13 changes: 11 additions & 2 deletions internal/controller/pgupgrade/pgupgrade_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,16 @@ func (r *PGUpgradeReconciler) Reconcile(ctx context.Context, req ctrl.Request) (

// Get the status version and check the jobs to see if this upgrade has completed
statusVersion := int64(world.Cluster.Status.PostgresVersion)
upgradeJob := world.Jobs[pgUpgradeJob(upgrade).Name]

// Find the upgrade job by its role label
var upgradeJob *batchv1.Job
for _, job := range world.Jobs {
if job.GetLabels()[LabelRole] == pgUpgrade {
upgradeJob = job
break
}
}
Comment thread
Kajot-dev marked this conversation as resolved.

upgradeJobComplete := upgradeJob != nil &&
jobCompleted(upgradeJob)
upgradeJobFailed := upgradeJob != nil &&
Expand Down Expand Up @@ -502,7 +511,7 @@ func (r *PGUpgradeReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
func setStatusToProgressingIfReasonWas(reason string, upgrade *v1beta1.PGUpgrade) {
progressing := meta.FindStatusCondition(upgrade.Status.Conditions,
ConditionPGUpgradeProgressing)
if progressing == nil || (progressing != nil && progressing.Reason == reason) {
if progressing == nil || progressing.Reason == reason {
meta.SetStatusCondition(&upgrade.Status.Conditions, metav1.Condition{
ObservedGeneration: upgrade.GetGeneration(),
Type: ConditionPGUpgradeProgressing,
Expand Down
35 changes: 35 additions & 0 deletions internal/naming/dns.go

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.

it'd be great to have unit tests for new functions here

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added tests

Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,43 @@ import (
"github.com/pkg/errors"
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/util/rand"
)

// maxDNSSafeLength is the maximum length for a Kubernetes DNS-1123 label (63 characters).
// This is the universal limit for resource names that get used in DNS.
// https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#dns-label-names
const maxDNSSafeLength = 63

// SafeDNSName truncates a name to fit within the Kubernetes DNS-1123 label limit of 63 characters.
// It also ensures the name doesn't end with a hyphen, which is invalid for DNS labels.
// This should be used for any resource name that may be used in DNS (Pods, Services, Jobs, etc.).
func SafeDNSName(name string) string {
if len(name) <= maxDNSSafeLength {
return name
}
name = name[:maxDNSSafeLength]
// Strip trailing hyphens which are invalid in DNS labels
return strings.TrimRight(name, "-")
}
Comment thread
Kajot-dev marked this conversation as resolved.

// 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
// random suffix (e.g., "-a1b2") to maintain uniqueness.
// 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 random chars
name = name[:maxDNSSafeLength-5]
// Strip trailing hyphens from the truncated prefix
name = strings.TrimRight(name, "-")
return name + "-" + rand.String(4)
}
Comment thread
Kajot-dev marked this conversation as resolved.

// InstancePodDNSNames returns the possible DNS names for instance. The first
// name is the fully qualified domain name (FQDN).
func InstancePodDNSNames(ctx context.Context, instance *appsv1.StatefulSet, dnsSuffix string) []string {
Expand Down
Loading