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

updated kyverno deployment strategy #2006

Merged
merged 4 commits into from
Aug 18, 2021

Conversation

vineethvanga18
Copy link
Contributor

@vineethvanga18 vineethvanga18 commented Jun 12, 2021

Signed-off-by: vineethvanga18 reddy.8@iitj.ac.in

Related issue

fixes #1982

What type of PR is this

/kind bug

Proposed Changes

kyverno rolling update strategy updated to maxSurge: 1, maxUnavailable: 40%

Proof Manifests

charts/kyverno/templates/deployment.yaml

kind: Deployment
metadata:
  name: {{ template "kyverno.fullname" . }}
  labels: {{ include "kyverno.labels" . | nindent 4 }}
    app: kyverno
  namespace: {{ template "kyverno.namespace" . }}
spec:
  ...
  ...
  {{- if .Values.updateStrategy }}
  strategy:
    {{ toYaml .Values.updateStrategy | nindent 4 | trim }}
  {{- end }}

charts/kyverno/values.yaml

updateStrategy:
  rollingUpdate:
    maxSurge: 1
    maxUnavailable: 40%
  type: RollingUpdate

definitions/manifest/deployment.yaml

kind: Deployment
metadata:
  labels:
    app: kyverno
    # do not remove
    app.kubernetes.io/name: kyverno
  namespace: kyverno
  name: kyverno
spec:
  ...
  ...
  strategy:
    type: RollingUpdate
    rollingUpdate:
      maxUnavailable: 40%
      maxSurge: 1

To test the changes

  1. Install kyverno
  2. Now update/change(image tag or any) this deployment
  3. As the maxSurge is 1 and maxUnavailable is 40%. The deployment is first scaled up and then old pod gets terminated ensuring availability.

Checklist

  • I have read the contributing guidelines.
  • [] I have added tests that prove my fix is effective or that my feature works.
  • [] My PR contains new or altered behavior to Kyverno and
    • [] I have added or changed the documentation myself in an existing PR and the link is:
    • [] I have raised an issue in kyverno/website to track the doc update and the link is:
    • [] I have read the PR documentation guide and followed the process including adding proof manifests to this PR.

Further Comments

Signed-off-by: vineethvanga18 <reddy.8@iitj.ac.in>
@treydock
Copy link
Member

@vineethvanga18 Maybe add something to Helm chart too? I had suggested such a change in #1985 but here might work too.

Signed-off-by: vineethvanga18 <reddy.8@iitj.ac.in>
@vyankyGH
Copy link
Contributor

vyankyGH commented Jul 1, 2021

@vineethvanga18 - Please add Proof manifest for the PR. What are the steps get followed to test this changes?

Signed-off-by: vineethvanga18 <reddy.8@iitj.ac.in>
@vyankyGH
Copy link
Contributor

vyankyGH commented Jul 8, 2021

Hi @vineethvanga18 can we make it configurable for the helm chat. we don't want something fix value, user can configure dynamically.

Signed-off-by: vineethvanga18 <reddy.8@iitj.ac.in>
@vyankyGH vyankyGH merged commit c7dbbe4 into kyverno:main Aug 18, 2021
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.

[Bug] Fix Kyverno Deployment updateStrategy
3 participants