Skip to content

Commit ae3735a

Browse files
armruleonardoce
andauthored
fix: consider plugin changes while evaluating instance rollout (cloudnative-pg#7126)
Introduce the `EVALUATE` verb to enable a plugin to trigger a rollout if the desired Pod specification differs from the current one. Signed-off-by: Armando Ruocco <armando.ruocco@enterprisedb.com> Signed-off-by: Leonardo Cecchi <leonardo.cecchi@enterprisedb.com> Co-authored-by: Leonardo Cecchi <leonardo.cecchi@enterprisedb.com>
1 parent 135a6f9 commit ae3735a

File tree

13 files changed

+299
-84
lines changed

13 files changed

+299
-84
lines changed

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ require (
99
github.com/blang/semver v3.5.1+incompatible
1010
github.com/cheynewallace/tabby v1.1.1
1111
github.com/cloudnative-pg/barman-cloud v0.1.0
12-
github.com/cloudnative-pg/cnpg-i v0.1.0
12+
github.com/cloudnative-pg/cnpg-i v0.1.1-0.20250321093050-de4ab51537cb
1313
github.com/cloudnative-pg/machinery v0.1.0
1414
github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc
1515
github.com/evanphx/json-patch/v5 v5.9.11

go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,8 @@ github.com/cheynewallace/tabby v1.1.1 h1:JvUR8waht4Y0S3JF17G6Vhyt+FRhnqVCkk8l4Yr
2020
github.com/cheynewallace/tabby v1.1.1/go.mod h1:Pba/6cUL8uYqvOc9RkyvFbHGrQ9wShyrn6/S/1OYVys=
2121
github.com/cloudnative-pg/barman-cloud v0.1.0 h1:e/z52CehMBIh1LjZqNBJnncWJbS+1JYvRMBR8Js6Uiw=
2222
github.com/cloudnative-pg/barman-cloud v0.1.0/go.mod h1:rJUJO/f1yNckLZiVxHAyRmKY+4EPJkYRJsGbTZRJQSY=
23-
github.com/cloudnative-pg/cnpg-i v0.1.0 h1:QH2xTsrODMhEEc6B25GbOYe7ZIttDmSkYvXotfU5dfs=
24-
github.com/cloudnative-pg/cnpg-i v0.1.0/go.mod h1:G28BhgUEHqrxEyyQeHz8BbpMVAsGuLhJm/tHUbDi8Sw=
23+
github.com/cloudnative-pg/cnpg-i v0.1.1-0.20250321093050-de4ab51537cb h1:FPORwCxjZwlnKnF7dOkuOAz0GBSQ3Hrn+8lm4uMiWeM=
24+
github.com/cloudnative-pg/cnpg-i v0.1.1-0.20250321093050-de4ab51537cb/go.mod h1:n+kbHm3rzRCY5IJKuE1tGMbG6JaeYz8yycYoLt7BeKo=
2525
github.com/cloudnative-pg/machinery v0.1.0 h1:tjRmsqQmsO/OlaT0uFmkEtVqgr+SGPM88cKZOHYKLBo=
2626
github.com/cloudnative-pg/machinery v0.1.0/go.mod h1:0V3vm44FaIsY+x4pm8ORry7xCC3AJiO+ebfPNxeP5Ck=
2727
github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g=

internal/cnpi/plugin/mapping.go

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,15 +22,16 @@ import (
2222
"github.com/cloudnative-pg/cnpg-i/pkg/lifecycle"
2323
)
2424

25-
// The OperationVerb corresponds to the Kubernetes API method
25+
// The OperationVerb corresponds to the CNPG-I lifecycle operation verb
2626
type OperationVerb string
2727

28-
// A Kubernetes operation verb
28+
// A lifecycle operation verb
2929
const (
30-
OperationVerbPatch OperationVerb = "PATCH"
31-
OperationVerbUpdate OperationVerb = "UPDATE"
32-
OperationVerbCreate OperationVerb = "CREATE"
33-
OperationVerbDelete OperationVerb = "DELETE"
30+
OperationVerbPatch OperationVerb = "PATCH"
31+
OperationVerbUpdate OperationVerb = "UPDATE"
32+
OperationVerbCreate OperationVerb = "CREATE"
33+
OperationVerbDelete OperationVerb = "DELETE"
34+
OperationVerbEvaluate OperationVerb = "EVALUATE"
3435
)
3536

3637
// ToOperationType_Type converts an OperationVerb into a lifecycle.OperationType_Type
@@ -45,6 +46,8 @@ func (o OperationVerb) ToOperationType_Type() (lifecycle.OperatorOperationType_T
4546
return lifecycle.OperatorOperationType_TYPE_CREATE, nil
4647
case OperationVerbUpdate:
4748
return lifecycle.OperatorOperationType_TYPE_UPDATE, nil
49+
case OperationVerbEvaluate:
50+
return lifecycle.OperatorOperationType_TYPE_EVALUATE, nil
4851
}
4952

5053
return lifecycle.OperatorOperationType_Type(0), fmt.Errorf("unknown operation type: '%s'", o)

internal/controller/cluster_create.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1307,7 +1307,7 @@ func (r *ClusterReconciler) ensureInstancesAreCreated(
13071307
) (ctrl.Result, error) {
13081308
contextLogger := log.FromContext(ctx)
13091309

1310-
instanceToCreate, err := findInstancePodToCreate(cluster, instancesStatus, resources.pvcs.Items)
1310+
instanceToCreate, err := findInstancePodToCreate(ctx, cluster, instancesStatus, resources.pvcs.Items)
13111311
if err != nil {
13121312
return ctrl.Result{}, err
13131313
}
@@ -1394,6 +1394,7 @@ func (r *ClusterReconciler) ensureInstancesAreCreated(
13941394

13951395
// we elect a current instance that doesn't exist for creation
13961396
func findInstancePodToCreate(
1397+
ctx context.Context,
13971398
cluster *apiv1.Cluster,
13981399
instancesStatus postgres.PostgresqlStatusList,
13991400
pvcs []corev1.PersistentVolumeClaim,
@@ -1440,7 +1441,7 @@ func findInstancePodToCreate(
14401441
if err != nil {
14411442
return nil, err
14421443
}
1443-
return specs.PodWithExistingStorage(*cluster, serial)
1444+
return specs.NewInstance(ctx, *cluster, serial, true)
14441445
}
14451446

14461447
return nil, nil

internal/controller/cluster_upgrade.go

Lines changed: 40 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,7 @@ type rollout struct {
276276
}
277277

278278
type rolloutChecker func(
279+
ctx context.Context,
279280
pod *corev1.Pod,
280281
cluster *apiv1.Cluster,
281282
) (rollout, error)
@@ -335,7 +336,7 @@ func isPodNeedingRollout(
335336
contextLogger := log.FromContext(ctx)
336337
applyCheckers := func(checkers map[string]rolloutChecker) rollout {
337338
for message, check := range checkers {
338-
podRollout, err := check(pod, cluster)
339+
podRollout, err := check(ctx, pod, cluster)
339340
if err != nil {
340341
contextLogger.Error(err, "while checking if pod needs rollout")
341342
continue
@@ -380,10 +381,10 @@ func isPodNeedingRollout(
380381

381382
// These checks are subsumed by the PodSpec checker
382383
checkers = map[string]rolloutChecker{
383-
"pod environment is outdated": checkPodEnvironmentIsOutdated,
384-
"pod scheduler is outdated": checkSchedulerIsOutdated,
385-
"pod needs updated topology": checkPodNeedsUpdatedTopology,
386-
"pod init container is outdated": checkPodInitContainerIsOutdated,
384+
"pod environment is outdated": checkPodEnvironmentIsOutdated,
385+
"pod scheduler is outdated": checkSchedulerIsOutdated,
386+
"pod needs updated topology": checkPodNeedsUpdatedTopology,
387+
"pod bootstrap container is outdated": checkPodBootstrapImage,
387388
}
388389
podRollout = applyCheckers(checkers)
389390
if podRollout.required {
@@ -403,7 +404,7 @@ func hasValidPodSpec(pod *corev1.Pod) bool {
403404
return err == nil
404405
}
405406

406-
func checkHasResizingPVC(pod *corev1.Pod, cluster *apiv1.Cluster) (rollout, error) {
407+
func checkHasResizingPVC(_ context.Context, pod *corev1.Pod, cluster *apiv1.Cluster) (rollout, error) {
407408
if configuration.Current.EnableAzurePVCUpdates {
408409
for _, pvcName := range cluster.Status.ResizingPVC {
409410
// This code works on the assumption that the PVC begins with the name of the pod using it.
@@ -418,7 +419,7 @@ func checkHasResizingPVC(pod *corev1.Pod, cluster *apiv1.Cluster) (rollout, erro
418419
return rollout{}, nil
419420
}
420421

421-
func checkPodNeedsUpdatedTopology(pod *corev1.Pod, cluster *apiv1.Cluster) (rollout, error) {
422+
func checkPodNeedsUpdatedTopology(_ context.Context, pod *corev1.Pod, cluster *apiv1.Cluster) (rollout, error) {
422423
if reflect.DeepEqual(cluster.Spec.TopologySpreadConstraints, pod.Spec.TopologySpreadConstraints) {
423424
return rollout{}, nil
424425
}
@@ -432,7 +433,7 @@ func checkPodNeedsUpdatedTopology(pod *corev1.Pod, cluster *apiv1.Cluster) (roll
432433
}, nil
433434
}
434435

435-
func checkSchedulerIsOutdated(pod *corev1.Pod, cluster *apiv1.Cluster) (rollout, error) {
436+
func checkSchedulerIsOutdated(_ context.Context, pod *corev1.Pod, cluster *apiv1.Cluster) (rollout, error) {
436437
if cluster.Spec.SchedulerName == "" || cluster.Spec.SchedulerName == pod.Spec.SchedulerName {
437438
return rollout{}, nil
438439
}
@@ -447,7 +448,7 @@ func checkSchedulerIsOutdated(pod *corev1.Pod, cluster *apiv1.Cluster) (rollout,
447448
}, nil
448449
}
449450

450-
func checkProjectedVolumeIsOutdated(pod *corev1.Pod, cluster *apiv1.Cluster) (rollout, error) {
451+
func checkProjectedVolumeIsOutdated(_ context.Context, pod *corev1.Pod, cluster *apiv1.Cluster) (rollout, error) {
451452
isNilOrZero := func(vs *corev1.ProjectedVolumeSource) bool {
452453
return vs == nil || len(vs.Sources) == 0
453454
}
@@ -490,7 +491,7 @@ func getProjectedVolumeConfigurationFromPod(pod corev1.Pod) *corev1.ProjectedVol
490491
return nil
491492
}
492493

493-
func checkPodImageIsOutdated(pod *corev1.Pod, cluster *apiv1.Cluster) (rollout, error) {
494+
func checkPodImageIsOutdated(_ context.Context, pod *corev1.Pod, cluster *apiv1.Cluster) (rollout, error) {
494495
targetImageName := cluster.GetImageName()
495496

496497
pgCurrentImageName, err := specs.GetPostgresImageName(*pod)
@@ -510,7 +511,7 @@ func checkPodImageIsOutdated(pod *corev1.Pod, cluster *apiv1.Cluster) (rollout,
510511
}, nil
511512
}
512513

513-
func checkPodInitContainerIsOutdated(pod *corev1.Pod, _ *apiv1.Cluster) (rollout, error) {
514+
func checkPodBootstrapImage(_ context.Context, pod *corev1.Pod, _ *apiv1.Cluster) (rollout, error) {
514515
if configuration.Current.EnableInstanceManagerInplaceUpdates {
515516
return rollout{}, nil
516517
}
@@ -527,13 +528,13 @@ func checkPodInitContainerIsOutdated(pod *corev1.Pod, _ *apiv1.Cluster) (rollout
527528
// We need to apply a different version of the instance manager
528529
return rollout{
529530
required: true,
530-
reason: fmt.Sprintf("the instance is using an old init container image: %s -> %s",
531+
reason: fmt.Sprintf("the instance is using an old bootstrap container image: %s -> %s",
531532
opCurrentImageName, configuration.Current.OperatorImageName),
532533
needsChangeOperatorImage: true,
533534
}, nil
534535
}
535536

536-
func checkHasMissingPVCs(pod *corev1.Pod, cluster *apiv1.Cluster) (rollout, error) {
537+
func checkHasMissingPVCs(_ context.Context, pod *corev1.Pod, cluster *apiv1.Cluster) (rollout, error) {
537538
if persistentvolumeclaim.InstanceHasMissingMounts(cluster, pod) {
538539
return rollout{
539540
required: true,
@@ -544,7 +545,11 @@ func checkHasMissingPVCs(pod *corev1.Pod, cluster *apiv1.Cluster) (rollout, erro
544545
return rollout{}, nil
545546
}
546547

547-
func checkClusterHasDifferentRestartAnnotation(pod *corev1.Pod, cluster *apiv1.Cluster) (rollout, error) {
548+
func checkClusterHasDifferentRestartAnnotation(
549+
_ context.Context,
550+
pod *corev1.Pod,
551+
cluster *apiv1.Cluster,
552+
) (rollout, error) {
548553
// If the pod restart value doesn't match with the one contained in the cluster, restart the pod.
549554
if clusterRestart, ok := cluster.Annotations[utils.ClusterRestartAnnotationName]; ok {
550555
podRestart := pod.Annotations[utils.ClusterRestartAnnotationName]
@@ -560,7 +565,9 @@ func checkClusterHasDifferentRestartAnnotation(pod *corev1.Pod, cluster *apiv1.C
560565
return rollout{}, nil
561566
}
562567

563-
func checkPodEnvironmentIsOutdated(pod *corev1.Pod, cluster *apiv1.Cluster) (rollout, error) {
568+
// checkPodEnvironmentIsOutdated checks if the environment variables in the pod have changed.
569+
// Deprecated: this function doesn't take into account plugin changes, use PodSpec annotation.
570+
func checkPodEnvironmentIsOutdated(_ context.Context, pod *corev1.Pod, cluster *apiv1.Cluster) (rollout, error) {
564571
// Check if there is a change in the environment section
565572
envConfig := specs.CreatePodEnvConfig(*cluster, pod.Name)
566573

@@ -605,7 +612,11 @@ func checkPodEnvironmentIsOutdated(pod *corev1.Pod, cluster *apiv1.Cluster) (rol
605612
return rollout{}, nil
606613
}
607614

608-
func checkPodSpecIsOutdated(pod *corev1.Pod, cluster *apiv1.Cluster) (rollout, error) {
615+
func checkPodSpecIsOutdated(
616+
ctx context.Context,
617+
pod *corev1.Pod,
618+
cluster *apiv1.Cluster,
619+
) (rollout, error) {
609620
podSpecAnnotation, ok := pod.ObjectMeta.Annotations[utils.PodSpecAnnotationName]
610621
if !ok {
611622
return rollout{}, nil
@@ -616,10 +627,18 @@ func checkPodSpecIsOutdated(pod *corev1.Pod, cluster *apiv1.Cluster) (rollout, e
616627
if err != nil {
617628
return rollout{}, fmt.Errorf("while unmarshaling the pod resources annotation: %w", err)
618629
}
619-
envConfig := specs.CreatePodEnvConfig(*cluster, pod.Name)
620-
gracePeriod := int64(cluster.GetMaxStopDelay())
630+
621631
tlsEnabled := remote.GetStatusSchemeFromPod(pod).IsHTTPS()
622-
targetPodSpec := specs.CreateClusterPodSpec(pod.Name, *cluster, envConfig, gracePeriod, tlsEnabled)
632+
633+
serial, err := utils.GetClusterSerialValue(pod.Annotations)
634+
if err != nil {
635+
return rollout{}, fmt.Errorf("while getting the pod serial value: %w", err)
636+
}
637+
638+
targetPod, err := specs.NewInstance(ctx, *cluster, serial, tlsEnabled)
639+
if err != nil {
640+
return rollout{}, fmt.Errorf("while creating a new pod to check podSpec: %w", err)
641+
}
623642

624643
// the bootstrap init-container could change image after an operator upgrade.
625644
// If in-place upgrades of the instance manager are enabled, we don't need rollout.
@@ -631,17 +650,13 @@ func checkPodSpecIsOutdated(pod *corev1.Pod, cluster *apiv1.Cluster) (rollout, e
631650
!configuration.Current.EnableInstanceManagerInplaceUpdates {
632651
return rollout{
633652
required: true,
634-
reason: fmt.Sprintf("the instance is using an old init container image: %s -> %s",
653+
reason: fmt.Sprintf("the instance is using an old bootstrap container image: %s -> %s",
635654
opCurrentImageName, configuration.Current.OperatorImageName),
636655
needsChangeOperatorImage: true,
637656
}, nil
638657
}
639658

640-
// from here we don't care about drift in the init containers: avoid checking them
641-
storedPodSpec.InitContainers = nil
642-
targetPodSpec.InitContainers = nil
643-
644-
match, diff := specs.ComparePodSpecs(storedPodSpec, targetPodSpec)
659+
match, diff := specs.ComparePodSpecs(storedPodSpec, targetPod.Spec)
645660
if !match {
646661
return rollout{
647662
required: true,

0 commit comments

Comments
 (0)