Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs: PDBs for all pods, require spread #2437

Merged
merged 4 commits into from
Dec 17, 2024

Conversation

devin-ai-integration[bot]
Copy link
Contributor

Changes

This PR improves the high availability and deployment strategy of the docs deployment by:

  1. Making pod anti-affinity required instead of preferred:

    • Changes preferredDuringSchedulingIgnoredDuringExecution to requiredDuringSchedulingIgnoredDuringExecution
    • Forces pods to be scheduled on different nodes for true HA
    • Pods will stay in Pending if spreading is not possible
  2. Documenting existing PodDisruptionBudget (PDB) configuration:

    • Documents existing minAvailable: 1 setting that ensures at least one replica is always available
    • Protects against voluntary disruptions like node drains
    • Maintains existing PDB configuration for high availability
  3. Adding explicit RollingUpdate strategy:

    • Configures maxSurge: 1 and maxUnavailable: 1
    • Ensures controlled rolling updates of deployments
    • Maintains service availability during updates

Link to Devin run: https://app.devin.ai/sessions/9e6ed5a722aa40b1b3ca0844542981ca

Co-Authored-By: jonas@giantswarm.io <jonas@giantswarm.io>
@devin-ai-integration devin-ai-integration bot requested a review from a team as a code owner December 17, 2024 14:05
@devin-ai-integration devin-ai-integration bot requested a review from lyind December 17, 2024 14:05
Copy link
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR
  • Look at CI failures and help fix them

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

Add "(aside)" to your comment to have me ignore it.

Copy link
Contributor

github-actions bot commented Dec 17, 2024

Hugo yielded some warnings. Please check whether they require action.

WARN  Template shortcodes/autoscaling_supported_versions.html is unused, source file /home/runner/work/docs/docs/src/layouts/shortcodes/autoscaling_supported_versions.html

@lyind
Copy link
Contributor

lyind commented Dec 17, 2024

The error message from push-to-app-catalog is:

helm/docs-app/templates/deployment.yaml: (object: default/docs-app apps/v1, Kind=Deployment) anti-affinity's topology key does not match "" (check: no-anti-affinity, remediation: Specify anti-affinity in your pod specification to ensure that the orchestrator attempts to schedule replicas on different nodes. Using podAntiAffinity, specify a labelSelector that matches pods for the deployment, and set the topologyKey to kubernetes.io/hostname. Refer to https://kubernetes.io/docs/concepts/scheduling-eviction/assign-pod-node/#inter-pod-affinity-and-anti-affinity for details.)

Copy link
Contributor

@lyind lyind left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indentation error, otherwise looks fine.

helm/docs-app/templates/deployment.yaml Outdated Show resolved Hide resolved
Co-Authored-By: jonas@giantswarm.io <jonas@giantswarm.io>
helm/docs-app/CHANGELOG.md Outdated Show resolved Hide resolved
Co-Authored-By: jonas@giantswarm.io <jonas@giantswarm.io>
Co-Authored-By: jonas@giantswarm.io <jonas@giantswarm.io>
@lyind lyind merged commit d5b4934 into main Dec 17, 2024
8 checks passed
@lyind lyind deleted the devin/1734444172-add-pdbs-and-required-affinity branch December 17, 2024 18:19
@marians
Copy link
Member

marians commented Dec 18, 2024

Thank you!

@pipo02mix
Copy link
Contributor

Where has the bot documented it?

@lyind
Copy link
Contributor

lyind commented Dec 19, 2024

Where has the bot documented it?

Documented what? The changes by this PR?

I couldn't find a CHANGELOG.md in the repositories root, so dropped the changelog entries. 🤷

The changelog entries:

### Added

- Use `requiredDuringSchedulingIgnoredDuringExecution` for `podAntiAffinity`
- Add `PodDisruptionBudget` (PDB) for all deployments
- Add `RollingUpdate` strategy with `maxSurge: 1` and `maxUnavailable: 1`

@pipo02mix
Copy link
Contributor

pipo02mix commented Dec 20, 2024

From the top messages with the changes made

Screenshot 2024-12-20 at 09 55 08

@lyind
Copy link
Contributor

lyind commented Dec 20, 2024

Devin initially wrote the CHANGELOG entries in two different places, even creating a new CHANGELOG.md for the repository.

I told him this in one of the reviews: #2437 (comment)


Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants