Skip to content

fix(webhook): validate the number of synchronous replicas#5985

Merged
gbartolini merged 4 commits intomainfrom
dev/5964
Oct 31, 2024
Merged

fix(webhook): validate the number of synchronous replicas#5985
gbartolini merged 4 commits intomainfrom
dev/5964

Conversation

@armru
Copy link
Member

@armru armru commented Oct 30, 2024

Closes #5964

Release Notes

Fixed a bug where the user could specify one or fewer instances along with the synchronous stanza. When specified together, these parameters would generate an incorrect replica configuration.

Note for the reviewers

While this could have been done with kubebuilder rule, I opted for the code solution given that it is easy to unit-test.

@armru armru requested a review from a team as a code owner October 30, 2024 13:34
@armru
Copy link
Member Author

armru commented Oct 30, 2024

/test limit=local

@github-actions
Copy link
Contributor

❗ By default, the pull request is configured to backport to all release branches.

  • To stop backporting this pr, remove the label: backport-requested ◀️ or add the label 'do not backport'
  • To stop backporting this pr to a certain release branch, remove the specific branch label: release-x.y

@github-actions
Copy link
Contributor

@armru, here's the link to the E2E on CNPG workflow run: https://github.com/cloudnative-pg/cloudnative-pg/actions/runs/11594332494

@armru armru changed the title feat(webhook): prohibit SynchornousReplicaConfiguration with one instance feat(webhook): prohibit .synchronous stanza while there is only one instance Oct 30, 2024
@armru armru changed the title feat(webhook): prohibit .synchronous stanza while there is only one instance feat(webhook): prohibit synchronous stanza while there is only one instance Oct 30, 2024
@armru armru changed the title feat(webhook): prohibit synchronous stanza while there is only one instance feat(webhook): prohibit synchronous stanza with one instance Oct 30, 2024
@armru armru force-pushed the dev/5964 branch 2 times, most recently from fb606f3 to 19be738 Compare October 30, 2024 15:11
@armru
Copy link
Member Author

armru commented Oct 30, 2024

/test limit=local

@github-actions
Copy link
Contributor

@armru, here's the link to the E2E on CNPG workflow run: https://github.com/cloudnative-pg/cloudnative-pg/actions/runs/11596148713

@armru armru force-pushed the dev/5964 branch 2 times, most recently from 9c21ee2 to 20fe47a Compare October 30, 2024 15:25
@armru
Copy link
Member Author

armru commented Oct 30, 2024

/test limit=local

@armru armru changed the title feat(webhook): prohibit synchronous stanza with one instance feat(webhook): prohibit synchronous stanza while having one or less instances Oct 30, 2024
@github-actions
Copy link
Contributor

@armru, here's the link to the E2E on CNPG workflow run: https://github.com/cloudnative-pg/cloudnative-pg/actions/runs/11596426784

@cnpg-bot cnpg-bot added the ok to merge 👌 This PR can be merged label Oct 30, 2024
@gbartolini
Copy link
Contributor

@armru can we make this more generic? See this comment for example: #5878 (comment)

@armru
Copy link
Member Author

armru commented Oct 31, 2024

@gbartolini can you validate if the changes are in line with the expectations?

@gbartolini gbartolini changed the title feat(webhook): prohibit synchronous stanza while having one or less instances fix(webhook): validate the number of synchronous replicas Oct 31, 2024
armru and others added 3 commits October 31, 2024 16:06
…ance

Closes #5964

Signed-off-by: Armando Ruocco <armando.ruocco@enterprisedb.com>
Signed-off-by: Armando Ruocco <armando.ruocco@enterprisedb.com>
Signed-off-by: Gabriele Bartolini <gabriele.bartolini@enterprisedb.com>
@mnencia mnencia added the do not merge 🙅 This PR cannot be merged (yet) label Oct 31, 2024
Signed-off-by: Marco Nenciarini <marco.nenciarini@enterprisedb.com>
@mnencia mnencia removed the do not merge 🙅 This PR cannot be merged (yet) label Oct 31, 2024
@gbartolini gbartolini merged commit 97f1e9e into main Oct 31, 2024
@gbartolini gbartolini deleted the dev/5964 branch October 31, 2024 16:33
cnpg-bot pushed a commit that referenced this pull request Oct 31, 2024
Closes #5964

## Release Notes

Fix an issue where the user could specify one or fewer instances along
with the `synchronous` stanza. When specified together, these parameters
would generate an incorrect replica configuration.

Signed-off-by: Armando Ruocco <armando.ruocco@enterprisedb.com>
Signed-off-by: Gabriele Bartolini <gabriele.bartolini@enterprisedb.com>
Signed-off-by: Marco Nenciarini <marco.nenciarini@enterprisedb.com>
Co-authored-by: Gabriele Bartolini <gabriele.bartolini@enterprisedb.com>
Co-authored-by: Marco Nenciarini <marco.nenciarini@enterprisedb.com>
(cherry picked from commit 97f1e9e)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport-requested ◀️ This pull request should be backported to all supported releases ok to merge 👌 This PR can be merged release-1.24

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: .spec.postgresql.synchronous and .spec.instances = 1 --> SyncRep

4 participants