Skip to content
This repository has been archived by the owner on Oct 24, 2023. It is now read-only.

feat: add pod disruption budgets #218

Merged
merged 1 commit into from Jan 4, 2019
Merged

feat: add pod disruption budgets #218

merged 1 commit into from Jan 4, 2019

Conversation

sylr
Copy link
Contributor

@sylr sylr commented Jan 3, 2019

Reason for Change:

This allows the cluster-autoscaler to evict the pods of the concerned deployments.

Issue Fixed:

Requirements:

Notes: Add pod disruption budgets

@acs-bot acs-bot added the size/M label Jan 3, 2019
This allows the cluster-autoscaler to evict the pods of the concerned deployments.

Signed-off-by: Sylvain Rabot <s.rabot@lectra.com>
@codecov
Copy link

codecov bot commented Jan 4, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@208a8e0). Click here to learn what that means.
The diff coverage is n/a.

@@           Coverage Diff            @@
##             master    #218   +/-   ##
========================================
  Coverage          ?   53.2%           
========================================
  Files             ?      95           
  Lines             ?   14235           
  Branches          ?       0           
========================================
  Hits              ?    7574           
  Misses            ?    5995           
  Partials          ?     666

1 similar comment
@codecov
Copy link

codecov bot commented Jan 4, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@208a8e0). Click here to learn what that means.
The diff coverage is n/a.

@@           Coverage Diff            @@
##             master    #218   +/-   ##
========================================
  Coverage          ?   53.2%           
========================================
  Files             ?      95           
  Lines             ?   14235           
  Branches          ?       0           
========================================
  Hits              ?    7574           
  Misses            ?    5995           
  Partials          ?     666

@jackfrancis
Copy link
Member

@sylr Thanks for this! FYI I rebased/force pushed to enable our new E2E pipeline to run.

name: calico-typha
namespace: kube-system
spec:
minAvailable: 0%
Copy link
Member

Choose a reason for hiding this comment

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

This sort of seems like a no-op configuration, but I infer that cluster-autoscaler needs some value here in order to engage?

Copy link
Contributor Author

@sylr sylr Jan 4, 2019

Choose a reason for hiding this comment

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

Absolutely. The cluster-autoscaler will refuse to engage in a node scale down if there are non-mirrored pods which do not have a PDB that allows the pod to be killed.

@jackfrancis
Copy link
Member

/lgtm

@jackfrancis
Copy link
Member

@mboersma, could you spend a small amount of time reading up on the PodDisruptionBudget spec and sanity check the functional intent here. LGTM but I'd like a 2nd opinion. :)

@jackfrancis jackfrancis added this to In progress in backlog Jan 4, 2019
@mboersma
Copy link
Member

mboersma commented Jan 4, 2019

/lgtm based on my reading of the cluster-autoscaler FAQ and the CA docs.

@mboersma mboersma merged commit 8030f70 into Azure:master Jan 4, 2019
backlog automation moved this from In progress to Done Jan 4, 2019
@acs-bot
Copy link

acs-bot commented Jan 4, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jackfrancis, mboersma, sylr

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [jackfrancis,mboersma]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

juhacket pushed a commit to juhacket/aks-engine that referenced this pull request Mar 14, 2019
This allows the cluster-autoscaler to evict the pods of the concerned deployments.

Signed-off-by: Sylvain Rabot <s.rabot@lectra.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
No open projects
backlog
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants