Skip to content

Commit

Permalink
fix: correctly compare zero and nil `.spec.projectedVolumeTemplate.so…
Browse files Browse the repository at this point in the history
…urces` (cloudnative-pg#3647)

This patch prevents the rollout of the pod in case the current and
expected `.spec.projectedVolumeTemplate.sources` use different semantics
to describe a volume source not being present.

Previously, we required a strict equivalence between the current and
expected values of `.spec.projectedVolumeTemplate.sources`, which
resulted in the operator initiating rollouts even when comparing zero
and nil values.

Closes: cloudnative-pg#3623

Signed-off-by: YanniHu1996 <[email protected]>
Signed-off-by: Tao Li <[email protected]>
Signed-off-by: Armando Ruocco <[email protected]>
Co-authored-by: Tao Li <[email protected]>
Co-authored-by: Armando Ruocco <[email protected]>
  • Loading branch information
3 people authored Jan 5, 2024
1 parent 34f01d4 commit e304898
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 1 deletion.
11 changes: 10 additions & 1 deletion controllers/cluster_upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -385,10 +385,19 @@ func checkProjectedVolumeIsOutdated(
status postgres.PostgresqlStatus,
cluster *apiv1.Cluster,
) (rollout, error) {
isNilOrZero := func(vs *corev1.ProjectedVolumeSource) bool {
return vs == nil || len(vs.Sources) == 0
}

// Check if there is a change in the projected volume configuration
currentProjectedVolumeConfiguration := getProjectedVolumeConfigurationFromPod(*status.Pod)

desiredProjectedVolumeConfiguration := cluster.Spec.ProjectedVolumeTemplate.DeepCopy()

// we do not need to raise a rollout if the desired and current projected volume source equal to zero-length or nil
if isNilOrZero(desiredProjectedVolumeConfiguration) && isNilOrZero(currentProjectedVolumeConfiguration) {
return rollout{}, nil
}

if desiredProjectedVolumeConfiguration != nil && desiredProjectedVolumeConfiguration.DefaultMode == nil {
defaultMode := corev1.ProjectedVolumeSourceDefaultMode
desiredProjectedVolumeConfiguration.DefaultMode = &defaultMode
Expand Down
51 changes: 51 additions & 0 deletions controllers/cluster_upgrade_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -421,6 +421,57 @@ var _ = Describe("Pod upgrade", Ordered, func() {
Expect(rollout.required).To(BeTrue())
})
})

When("The projected volume changed", func() {
It("should not require rollout if projected volume is 0 length slice in cluster",
func(ctx SpecContext) {
cluster.Spec.ProjectedVolumeTemplate = &corev1.ProjectedVolumeSource{
Sources: []corev1.VolumeProjection{},
}
pod := specs.PodWithExistingStorage(cluster, 1)
status := postgres.PostgresqlStatus{
Pod: pod,
IsPodReady: true,
ExecutableHash: "test",
}

rollout := isPodNeedingRollout(ctx, status, &cluster)
Expect(rollout.reason).To(BeEmpty())
Expect(rollout.required).To(BeFalse())
})

It("should not require rollout if projected volume source is nil",
func(ctx SpecContext) {
cluster.Spec.ProjectedVolumeTemplate = &corev1.ProjectedVolumeSource{
Sources: nil,
}
pod := specs.PodWithExistingStorage(cluster, 1)
status := postgres.PostgresqlStatus{
Pod: pod,
IsPodReady: true,
ExecutableHash: "test",
}

rollout := isPodNeedingRollout(ctx, status, &cluster)
Expect(rollout.reason).To(BeEmpty())
Expect(rollout.required).To(BeFalse())
})

It("should not require rollout if projected volume is nil",
func(ctx SpecContext) {
cluster.Spec.ProjectedVolumeTemplate = nil
pod := specs.PodWithExistingStorage(cluster, 1)
status := postgres.PostgresqlStatus{
Pod: pod,
IsPodReady: true,
ExecutableHash: "test",
}

rollout := isPodNeedingRollout(ctx, status, &cluster)
Expect(rollout.reason).To(BeEmpty())
Expect(rollout.required).To(BeFalse())
})
})
})

var _ = Describe("Test pod rollout due to topology", func() {
Expand Down

0 comments on commit e304898

Please sign in to comment.