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

feat: enhanced in-place update module to support vertical scaling #1353

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

LavenderQAQ
Copy link

@LavenderQAQ LavenderQAQ commented Aug 2, 2023

Ⅰ. Describe what this PR does

Enhanced the in-place update module to enable kruise's workload to modify resources without restarting the pod.

Ⅱ. Does this pull request fix one issue?

Fixes #1212

Ⅲ. Describe how to verify it

Need to kubernetes v1.27 version, and open feature gate InPlacePodVerticalScaling. Here is my kind configuration:

kind: Cluster
apiVersion: kind.x-k8s.io/v1alpha4
featureGates:
  "InPlacePodVerticalScaling": true
nodes:
- role: control-plane
  image: kindest/node:v1.27.3@sha256:3966ac761ae0136263ffdb6cfd4db23ef8a83cba8a463690e98317add2c9ba72
- role: worker
  image: kindest/node:v1.27.3@sha256:3966ac761ae0136263ffdb6cfd4db23ef8a83cba8a463690e98317add2c9ba72
- role: worker
  image: kindest/node:v1.27.3@sha256:3966ac761ae0136263ffdb6cfd4db23ef8a83cba8a463690e98317add2c9ba72
- role: worker
  image: kindest/node:v1.27.3@sha256:3966ac761ae0136263ffdb6cfd4db23ef8a83cba8a463690e98317add2c9ba72

Build a cloneset (or other workload that includes in-place updates) and set updateStrategy to InPlaceIfPossible. Here's an example:

apiVersion: apps.kruise.io/v1alpha1
kind: CloneSet
metadata:
  name: nginx-cloneset
spec:
  updateStrategy:
    type: InPlaceIfPossible
  replicas: 3
  selector:
    matchLabels:
      app: nginx
  template:
    metadata:
      labels:
        app: nginx
    spec:
      containers:
      - name: nginx-1
        image: nginx:1.21.1
        ports:
        - containerPort: 80
        resources:
          limits:
            cpu: "1"
            memory: "256Mi"
          requests:
            cpu: "0.5"
            memory: "128Mi"

Change the workload's resources, wait a while, and you'll find that the pod's resources will change, and the pod won't restart.

Ⅳ. Special notes for reviews

The control of Status for vertical scaling requires a higher version of the k8s api, which makes this pr need to wait for kruise version upgrade. It cannot be merged at this time because it is not yet complete.

@kruise-bot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign veophi for approval by writing /assign @veophi in a comment. For more information see:The Kubernetes Code Review Process.

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

Needs approval from an approver in each of these files:

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

@sonatype-lift
Copy link

sonatype-lift bot commented Aug 2, 2023

Sonatype Lift is retiring

Sonatype Lift will be retiring on Sep 12, 2023, with its analysis stopping on Aug 12, 2023. We understand that this news may come as a disappointment, and Sonatype is committed to helping you transition off it seamlessly. If you’d like to retain your data, please export your issues from the web console.
We are extremely grateful and thank you for your support over the years.

📖 Read about the impacts and timeline

@kruise-bot kruise-bot added the size/L size/L: 100-499 label Aug 2, 2023
@LavenderQAQ LavenderQAQ changed the title feat: enhanced in-place update module to support vertical scaling feat: enhanced inplace update module to support vertical scaling Aug 2, 2023
@LavenderQAQ LavenderQAQ changed the title feat: enhanced inplace update module to support vertical scaling feat: enhanced in-place update module to support vertical scaling Aug 2, 2023
@LavenderQAQ LavenderQAQ force-pushed the feat/inplace-workload-vertical-scaling branch 4 times, most recently from 08c47c7 to 04b5eb8 Compare August 7, 2023 03:04
@codecov-commenter
Copy link

codecov-commenter commented Aug 7, 2023

Codecov Report

Attention: Patch coverage is 4.59770% with 332 lines in your changes are missing coverage. Please review.

Project coverage is 47.54%. Comparing base (5421ee7) to head (a74b455).
Report is 12 commits behind head on master.

❗ Current head a74b455 differs from pull request most recent head 8cdf30a. Consider uploading reports for the commit 8cdf30a to get more accurate results

Files Patch % Lines
pkg/util/inplaceupdate/inplace_update_defaults.go 4.28% 312 Missing and 1 partial ⚠️
pkg/util/inplaceupdate/inplace_update_vertical.go 0.00% 19 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1353      +/-   ##
==========================================
- Coverage   48.55%   47.54%   -1.01%     
==========================================
  Files         157      153       -4     
  Lines       22480    21805     -675     
==========================================
- Hits        10916    10368     -548     
+ Misses      10360    10301      -59     
+ Partials     1204     1136      -68     
Flag Coverage Δ
unittests 47.54% <4.59%> (-1.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@LavenderQAQ LavenderQAQ force-pushed the feat/inplace-workload-vertical-scaling branch from 0470ce2 to 4e13c0d Compare August 7, 2023 03:56
@kruise-bot kruise-bot added needs-rebase size/XL size/XL: 500-999 and removed size/L size/L: 100-499 labels Aug 8, 2023
@LavenderQAQ LavenderQAQ force-pushed the feat/inplace-workload-vertical-scaling branch from 35c4939 to 28406cb Compare August 8, 2023 11:46
@LavenderQAQ LavenderQAQ force-pushed the feat/inplace-workload-vertical-scaling branch 2 times, most recently from 48126ff to aca435f Compare August 8, 2023 12:04
@LavenderQAQ LavenderQAQ marked this pull request as ready for review August 8, 2023 13:16
@kruise-bot kruise-bot requested a review from veophi August 8, 2023 13:16
@LavenderQAQ
Copy link
Author

LavenderQAQ commented Aug 8, 2023

The complete logic needs to wait until the k8s api of kruise is upgraded.

@LavenderQAQ
Copy link
Author

/hold

@LavenderQAQ LavenderQAQ force-pushed the feat/inplace-workload-vertical-scaling branch 3 times, most recently from 2d14fc1 to e0b65be Compare August 15, 2023 09:34
@LavenderQAQ LavenderQAQ force-pushed the feat/inplace-workload-vertical-scaling branch 4 times, most recently from 248e6e7 to db22089 Compare August 16, 2023 08:43
@LavenderQAQ LavenderQAQ force-pushed the feat/inplace-workload-vertical-scaling branch from db22089 to 8fd6bad Compare August 16, 2023 08:46
@LavenderQAQ LavenderQAQ force-pushed the feat/inplace-workload-vertical-scaling branch 4 times, most recently from 70b03b2 to ff3e27e Compare August 19, 2023 07:39
Copy link

stale bot commented Nov 20, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix This will not be worked on label Nov 20, 2023
@LavenderQAQ LavenderQAQ force-pushed the feat/inplace-workload-vertical-scaling branch from cf9de87 to 50f632e Compare November 20, 2023 14:30
@stale stale bot removed the wontfix This will not be worked on label Nov 20, 2023
Signed-off-by: LavenderQAQ <lavenderqaq.cs@gmail.com>
Signed-off-by: LavenderQAQ <lavenderqaq.cs@gmail.com>
…tations

Signed-off-by: LavenderQAQ <lavenderqaq.cs@gmail.com>
Signed-off-by: LavenderQAQ <lavenderqaq.cs@gmail.com>
@LavenderQAQ LavenderQAQ force-pushed the feat/inplace-workload-vertical-scaling branch from 50f632e to 8cdf30a Compare January 31, 2024 02:24
@kruise-bot
Copy link

@LavenderQAQ: PR needs rebase.

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/test-infra repository.

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

Successfully merging this pull request may close these issues.

[feature request] In-place udpate support resources
3 participants