Use a Deployment instead of a StatefulSet for the controller#191
Conversation
✅ Deploy Preview for agent-sandbox canceled.
|
|
Hi @antonipp. Thanks for your PR. I'm waiting for a github.com member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions 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. |
|
/ok-to-test |
|
This is an acceptable change. We can switch back to sts later if needed. Would you check the docs and check if they need a change. |
barney-s
left a comment
There was a problem hiding this comment.
Thanks for the changes
aa548c2 to
1571613
Compare
|
/retest |
1571613 to
db992f2
Compare
|
/label release-note-action-required |
|
@janetkuo: The label(s) DetailsIn response to this:
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. |
db992f2 to
8fbc2d5
Compare
|
/retest |
|
@antonipp it seems that the e2e tests are broken, would you fix them? |
8fbc2d5 to
72fef4b
Compare
|
Right, looks like there was an issue reconciling an existing Service: I fixed the logic by ensuring that it's always reconciled, I think it should do the trick. |
72fef4b to
e7af63f
Compare
|
Looks like there was an RBAC issue after I enabled leader election as well, I fixed it too, the tests now seem to pass |
e7af63f to
adf9789
Compare
Signed-off-by: Anton Ippolitov <anton.ippolitov@datadoghq.com>
adf9789 to
91e6a31
Compare
|
/lgtm |
|
@aditya-shantanu: changing LGTM is restricted to collaborators DetailsIn response to this:
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. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: antonipp, janetkuo The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Description
While trying out the agent-sandbox project, I noticed that the main controller is deployed as a StatefulSet. I couldn't really find a justification for it in the commit history, so this looks like an oversight. The controller is stateless and doesn't require a stable network identity, so I don't really see a reason to use a much heavier abstraction here. Moreover, STS ordered rollout semantics are unnecessary for a stateless controller and could complicate scaling/updates if leader election is enabled for HA.
So my PR switches the controller to be deployed as a simple Deployment, I think it will make operations easier for everyone
Migration Guide
The controller changed from StatefulSet to Deployment and leader election is now enabled by default. Before you deploy the new charts, you need to clean-up the existing StatefulSet (this will cause a brief disruption to Sandbox state reconciliation):
Then deploy the new charts. Verify that the Deployment has been properly created:
If needed, the rollback steps are: