statefulset: Use stateful sets for posgres deployments (PROJQUAY-6672) #927
Conversation
jonathankingfc
force-pushed
the
PROJQUAY-6672
branch
10 times, most recently
from
7e3cab8 to
fb9e6ba
Compare
jonathankingfc
force-pushed
the
PROJQUAY-6672
branch
from
fb9e6ba to
84e5ac2
Compare
| @@ -1,5 +1,5 @@ | |||
| apiVersion: apps/v1 | |||
| kind: Deployment | |||
| kind: StatefulSet | |||
There was a problem hiding this comment.
how will this work with existing deployments? On upgrade will it replace the deployment with a statefulset?
There was a problem hiding this comment.
Yes, it will replace the deployment with a stateful set. This is so that we do not have issues with overlapping deployments that would cause issues on the upgrade job.
| replicas: 1 | ||
| strategy: | ||
| type: Recreate |
There was a problem hiding this comment.
This will switch the deployment to a rolling upgrade, which could cause the old and new pod to run at the same time. Will that cause issues?
There was a problem hiding this comment.
Since it is a stateful set, it will only run one pod at a time
There was a problem hiding this comment.
In theory, issues can occur where Quay is operating on a Clair that has a split-brain during a deployment (i.e. the security worker requests the indexing state from a new pod, sees it has changed then makes an indexing request but it goes to an old pod). In practice, this is how it works in production and we've never seen problems so I think it's probably fine but something to document.
| @@ -1,5 +1,5 @@ | |||
| apiVersion: apps/v1 | |||
| kind: Deployment | |||
| kind: StatefulSet | |||
There was a problem hiding this comment.
The original issue seems to be caused by the old and the new db pods being matched by the same service, making the requests against the db go to either or randomly. Does switching to stateful sets fix this issue?
There was a problem hiding this comment.
Yes, since their will be only one underlying pod attached to the service
|
/retest |
1 similar comment
|
/retest |
- Swap Postgres and Clair Postgres Deployments to StatefulSets
jonathankingfc
force-pushed
the
PROJQUAY-6672
branch
from
6bb30c0 to
0ba46ee
Compare
|
@jonathankingfc: The following test failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
@sourcery-ai summary |
|
PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
@sourcery-ai review |
|
@sourcery-ai guide |
Reviewer's Guide by SourceryThis pull request migrates the Postgres and Clair Postgres deployments to StatefulSets for improved database management and persistence. The Kustomize configurations and patches have been updated to reflect these changes. The test command in the Makefile has been simplified, and the Kubebuilder installation process in the CI workflow has been updated. Finally, the StatefulSet kind was added to the kustomize package. Updated class diagram for Kustomize ModelFor functionclassDiagram
class client.Object {
}
class apps.StatefulSet {
}
client.Object <|-- apps.StatefulSet
note for apps.StatefulSet "Added StatefulSet type to ModelFor function"
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @jonathankingfc - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider adding podManagementPolicy: Parallel to the StatefulSet spec to speed up deployments.
- Consider adding updateStrategy to the StatefulSet spec to control the update process.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Can you provide an example of the proposed changes? |
@sourcery-ai how did you evaluate tests? |
|
@sourcery-ai plan |
|
@sourcery-ai review |
|
Sure! I'm generating a new review now. |
There was a problem hiding this comment.
Hey @jonathankingfc - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider adding a comment explaining why the Kubebuilder installation was removed from the CI workflow.
- Ensure that the
volumeClaimTemplatesare properly configured for the StatefulSets to manage persistent storage.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Hey @jonathankingfc - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider adding a comment explaining why the Kubebuilder installation was removed from the CI workflow.
- Ensure that the
volumeClaimTemplatesare properly configured for the StatefulSets to manage persistent storage.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
Hey @deshpandevlab, I've posted a new review for you! |
|
@sourcery-ai help |
6 participants
Summary by Sourcery
Convert Postgres deployments for Quay and Clair from Deployments to StatefulSets to improve database management and persistence
Enhancements:
CI:
Chores: