jtblin / kube2iam

kube2iam provides different AWS IAM roles for pods running on Kubernetes
BSD 3-Clause "New" or "Revised" License
1.97k stars 318 forks source link

[helm] Add maxUnavailable option for rolling updates #291

Closed hwoarang closed 3 years ago

hwoarang commented 3 years ago

Allow user to set maxUnavailable option for rolling updates in order to speed up deployment times on large clusters.

coveralls commented 3 years ago

Coverage Status

Coverage remained the same at 19.481% when pulling 56ee37cc0ac29eb0ac1c0c2004887d1412ee8957 on hwoarang:add-ds-update-strategy into c24c3c8feb7f798e045c5191c384da6f341de93c on jtblin:master.

jtblin commented 3 years ago

cc @mariusv @walkafwalka can you help reviewing helm charts changes pls?

walkafwalka commented 3 years ago

I prefer using something more flexible for updateStrategy but that would require a breaking change. We should take note of situations like this.

  updateStrategy: {}
  #  rollingUpdate:
  #    maxUnavailable: 1
  #  type: RollingUpdate

Besides the documentation, LGTM.

rdelpret commented 3 years ago

@walkafwalka I 100% agree, I reached out to @mariusv on slack about it to propose the same before I saw this PR but didn't get an answer.

rdelpret commented 3 years ago

Regardless of implementation, this one is urgent for us as large clusters need some pretty high timeouts set w/ helm to get through a rolling update with the hard-coded settings.

hwoarang commented 3 years ago

I have updated the default value and added the missing doc. I opted not to implement the braking change at this point. However, if you think we should do it then I can do that no problem