K8SPG-698: create serviceaccount before statefulset by pooknull · Pull Request #1125 · percona/percona-postgresql-operator

pooknull · 2025-04-09T21:07:57Z

https://perconadev.atlassian.net/browse/K8SPG-698

DESCRIPTION

Problem:
The cluster1-repo-host StatefulSet is created before its corresponding ServiceAccount cluster1-pgbackrest exists. This can cause failures in certain environments.

Solution:
Create ServiceAccount before the StatefulSet.

CHECKLIST

Jira

Is the Jira ticket created and referenced properly?
Does the Jira ticket have the proper statuses for documentation (Needs Doc) and QA (Needs QA)?
Does the Jira ticket link to the proper milestone (Fix Version field)?

Tests

Is an E2E test/test case added for the new feature/change?
Are unit tests added where appropriate?

Config/Logging/Testability

Are all needed new/changed options added to default YAML files?
Are all needed new/changed options added to the Helm Chart?
Did we add proper logging messages for operator actions?
Did we ensure compatibility with the previous version or cluster upgrade process?
Does the change support oldest and newest supported PG version?
Does the change support oldest and newest supported Kubernetes version?

https://perconadev.atlassian.net/browse/K8SPG-698

hors · 2025-04-10T16:10:49Z

-		result.Requeue = true
-		return result, nil
-	}
+	// K8SPG-698: it should happen earlier


can we removed comments

gkech · 2025-04-14T08:43:46Z

+	}
+	serviceAccountName := sts.Spec.Template.Spec.ServiceAccountName
+	if serviceAccountName == "" {
+		panic("it's not expected to have empty service account name")


Since checkObject(...) returns an error and when used we do this

if err := sc.checkObject(ctx, obj); err != nil { panic(err) // should panic because reconciler can ignore the error }

I think this check can return an error and let the caller panic it. WDYT?

gkech · 2025-04-14T09:03:13Z

+func (sc *saTestClient) Update(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error {
+	if err := sc.checkObject(ctx, obj); err != nil {
+		panic(err) // should panic because reconciler can ignore the error
+	}
+	return sc.Client.Update(ctx, obj, opts...)
+}
+
+func (sc *saTestClient) Patch(ctx context.Context, obj client.Object, patch client.Patch, opts ...client.PatchOption) error {
+	if err := sc.checkObject(ctx, obj); err != nil {
+		panic(err) // should panic because reconciler can ignore the error
+	}
+	return sc.Client.Patch(ctx, obj, patch, opts...)
+}


Do we need these? Seems that they are not used in the test.

The Crunchy code uses a patch request to create the ServiceAccount instead of a create request:

percona-postgresql-operator/internal/controller/postgrescluster/apply.go

Lines 23 to 48 in 5dcd0e0

func (r *Reconciler) apply(ctx context.Context, object client.Object) error {

// Generate an apply-patch by comparing the object to its zero value.

zero := reflect.New(reflect.TypeOf(object).Elem()).Interface()

data, err := client.MergeFrom(zero.(client.Object)).Data(object)

apply := client.RawPatch(client.Apply.Type(), data)

// Keep a copy of the object before any API calls.

intent := object.DeepCopyObject()

patch := kubeapi.NewJSONPatch()

// Send the apply-patch with force=true.

if err == nil {

err = r.patch(ctx, object, apply, client.ForceOwnership)

}

// Some fields cannot be server-side applied correctly. When their outcome

// does not match the intent, send a json-patch to get really specific.

switch actual := object.(type) {

case *corev1.Service:

applyServiceSpec(patch, actual.Spec, intent.(*corev1.Service).Spec, "spec")

}

// Send the json-patch when necessary.

if err == nil && !patch.IsEmpty() {

err = r.patch(ctx, object, patch)

}

I will remove the update part

gkech · 2025-04-15T08:17:34Z

seems ok, let's check the golangci and the envtests why they are failing and then we can approve.

JNKPercona · 2025-04-15T09:59:53Z

Test name	Status
backup-enable-disable	passed
custom-extensions	passed
custom-tls	passed
demand-backup	passed
finalizers	passed
init-deploy	passed
monitoring	passed
monitoring-pmm3	passed
one-pod	passed
operator-self-healing	passed
pitr	passed
scaling	passed
scheduled-backup	passed
self-healing	passed
sidecars	passed
start-from-backup	passed
tablespaces	passed
telemetry-transfer	passed
upgrade-consistency	passed
upgrade-minor	passed
users	passed
We run 21 out of 21

commit: f24206a
image: perconalab/percona-postgresql-operator:PR-1125-f24206a27

pooknull added 2 commits April 10, 2025 00:07

K8SPG-698: create serviceaccount before statefulset

af4977a

https://perconadev.atlassian.net/browse/K8SPG-698

Merge remote-tracking branch 'origin/main' into K8SPG-698

5dcd0e0

pooknull marked this pull request as ready for review April 9, 2025 21:32

hors requested review from egegunes and gkech April 10, 2025 16:10

hors requested changes Apr 10, 2025

View reviewed changes

gkech reviewed Apr 14, 2025

View reviewed changes

pooknull added 2 commits April 14, 2025 16:09

small fixes

f1f4ce3

Merge remote-tracking branch 'origin/main' into K8SPG-698

3497874

pooknull requested a review from nmarukovich as a code owner April 14, 2025 13:10

pooknull and others added 2 commits April 14, 2025 16:10

remove update

da937df

Merge branch 'main' into K8SPG-698

d58f029

hors added this to the 2.7.0 milestone Apr 14, 2025

Merge branch 'main' into K8SPG-698

5eba903

fix lint

f24206a

pooknull requested review from gkech and hors April 15, 2025 11:18

gkech approved these changes Apr 15, 2025

View reviewed changes

egegunes approved these changes Apr 16, 2025

View reviewed changes

hors merged commit 56c9f76 into main Apr 16, 2025
18 checks passed

hors deleted the K8SPG-698 branch April 16, 2025 10:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

K8SPG-698: create serviceaccount before statefulset#1125

K8SPG-698: create serviceaccount before statefulset#1125
hors merged 8 commits intomainfrom
K8SPG-698

pooknull commented Apr 9, 2025 •

edited

Loading

Uh oh!

hors Apr 10, 2025

Uh oh!

pooknull Apr 14, 2025

Uh oh!

gkech Apr 14, 2025

Uh oh!

pooknull Apr 14, 2025

Uh oh!

gkech Apr 14, 2025 •

edited

Loading

Uh oh!

pooknull Apr 14, 2025

Uh oh!

pooknull Apr 14, 2025

Uh oh!

gkech commented Apr 15, 2025

Uh oh!

JNKPercona commented Apr 15, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

	func (r *Reconciler) apply(ctx context.Context, object client.Object) error {
	// Generate an apply-patch by comparing the object to its zero value.
	zero := reflect.New(reflect.TypeOf(object).Elem()).Interface()
	data, err := client.MergeFrom(zero.(client.Object)).Data(object)
	apply := client.RawPatch(client.Apply.Type(), data)

	// Keep a copy of the object before any API calls.
	intent := object.DeepCopyObject()
	patch := kubeapi.NewJSONPatch()

	// Send the apply-patch with force=true.
	if err == nil {
	err = r.patch(ctx, object, apply, client.ForceOwnership)
	}

	// Some fields cannot be server-side applied correctly. When their outcome
	// does not match the intent, send a json-patch to get really specific.
	switch actual := object.(type) {
	case *corev1.Service:
	applyServiceSpec(patch, actual.Spec, intent.(*corev1.Service).Spec, "spec")
	}

	// Send the json-patch when necessary.
	if err == nil && !patch.IsEmpty() {
	err = r.patch(ctx, object, patch)
	}

Conversation

pooknull commented Apr 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

DESCRIPTION

CHECKLIST

Uh oh!

hors Apr 10, 2025

Choose a reason for hiding this comment

Uh oh!

pooknull Apr 14, 2025

Choose a reason for hiding this comment

Uh oh!

gkech Apr 14, 2025

Choose a reason for hiding this comment

Uh oh!

pooknull Apr 14, 2025

Choose a reason for hiding this comment

Uh oh!

gkech Apr 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pooknull Apr 14, 2025

Choose a reason for hiding this comment

Uh oh!

pooknull Apr 14, 2025

Choose a reason for hiding this comment

Uh oh!

gkech commented Apr 15, 2025

Uh oh!

JNKPercona commented Apr 15, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

pooknull commented Apr 9, 2025 •

edited

Loading

gkech Apr 14, 2025 •

edited

Loading