- 
                Notifications
    
You must be signed in to change notification settings  - Fork 4.2k
 
VPA: Implement in-place updates support #7673
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
          
     Closed
      
      
    
      
        
          +1,718
        
        
          −102
        
        
          
        
      
    
  
  
     Closed
                    Changes from all commits
      Commits
    
    
            Show all changes
          
          
            22 commits
          
        
        Select commit
          Hold shift + click to select a range
      
      dcea4ca
              
                VPA: Add UpdateModeInPlaceOrRecreate to types
              
              
                jkyros a4b91de
              
                VPA: Allow admission-controller to validate in-place spec
              
              
                maxcao13 0f5eb1d
              
                VPA: Stuck in-place resizes still require eviction
              
              
                jkyros 3451011
              
                VPA: Make eviction restriction in-place aware
              
              
                jkyros c5549fb
              
                VPA: Updater logic allows in-place scaling
              
              
                jkyros 88d64c3
              
                VPA: Add metrics gauges for in-place updates
              
              
                jkyros ec7863f
              
                VPA: Update mocks to accommodate in-place VPA changes
              
              
                jkyros 84588e4
              
                VPA: hack unit tests to account for in-place
              
              
                jkyros bfb0321
              
                VPA: Add e2e tests for in-place scaling
              
              
                jkyros d6a8ab4
              
                VPA: allow rule-breaking updates if disruptionless
              
              
                jkyros 94612ce
              
                VPA: only allow in-place if explicitly set
              
              
                jkyros cca0b60
              
                VPA: Allow VPA updater to actuate recommendations in-place
              
              
                maxcao13 8977802
              
                VPA: Enable InPlacePodVerticalScaling feature flag on e2e
              
              
                maxcao13 08ba2fb
              
                VPA: Add in-place actuation and admission-controller e2e tests
              
              
                maxcao13 6d8be83
              
                VPA: fix logs and cleanup TODOs according to review
              
              
                maxcao13 981ea57
              
                VPA: Revert changes that related to disruption/disruptionless changes
              
              
                maxcao13 acd5a15
              
                VPA: Fix in-place actuation tests to align with updated AEP
              
              
                maxcao13 742d694
              
                VPA: Add features gates; add InPlaceVerticalScaling feature gate
              
              
                maxcao13 ed61b77
              
                VPA: update updater unit tests
              
              
                maxcao13 aa8b3ff
              
                VPA: Revert using containerStatus resources to calculate update priority
              
              
                maxcao13 3825298
              
                VPA: updated in-place e2e tests to account for feature gate
              
              
                maxcao13 8044ed7
              
                VPA: add InPlaceVerticalScaling feature gate to admission-controller
              
              
                maxcao13 File filter
Filter by extension
Conversations
          Failed to load comments.   
        
        
          
      Loading
        
  Jump to
        
          Jump to file
        
      
      
          Failed to load files.   
        
        
          
      Loading
        
  Diff view
Diff view
There are no files selected for viewing
  
    
      This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
      Learn more about bidirectional Unicode characters
    
  
  
    
              
        
          
          
            11 changes: 11 additions & 0 deletions
          
          11 
        
  vertical-pod-autoscaler/deploy/admission-controller-service.yaml
  
  
      
      
   
        
      
      
    
  
    
      This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
      Learn more about bidirectional Unicode characters
    
  
  
    
              | Original file line number | Diff line number | Diff line change | 
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| apiVersion: v1 | ||
| kind: Service | ||
| metadata: | ||
| name: vpa-webhook | ||
| namespace: kube-system | ||
| spec: | ||
| ports: | ||
| - port: 443 | ||
| targetPort: 8000 | ||
| selector: | ||
| app: vpa-admission-controller | 
  
    
      This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
      Learn more about bidirectional Unicode characters
    
  
  
    
              
  
    
      This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
      Learn more about bidirectional Unicode characters
    
  
  
    
              | Original file line number | Diff line number | Diff line change | 
|---|---|---|
| 
          
            
          
           | 
    @@ -458,6 +458,7 @@ spec: | |
| - "Off" | ||
| - Initial | ||
| - Recreate | ||
| - InPlaceOrRecreate | ||
| - Auto | ||
| type: string | ||
| type: object | ||
| 
          
            
          
           | 
    ||
  
    
      This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
      Learn more about bidirectional Unicode characters
    
  
  
    
              
  
    
      This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
      Learn more about bidirectional Unicode characters
    
  
  
    
              
  
    
      This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
      Learn more about bidirectional Unicode characters
    
  
  
    
              
      
      Oops, something went wrong.
        
    
  
      
      Oops, something went wrong.
        
    
  
  Add this suggestion to a batch that can be applied as a single commit.
  This suggestion is invalid because no changes were made to the code.
  Suggestions cannot be applied while the pull request is closed.
  Suggestions cannot be applied while viewing a subset of changes.
  Only one suggestion per line can be applied in a batch.
  Add this suggestion to a batch that can be applied as a single commit.
  Applying suggestions on deleted lines is not supported.
  You must change the existing code in this line in order to create a valid suggestion.
  Outdated suggestions cannot be applied.
  This suggestion has been applied or marked resolved.
  Suggestions cannot be applied from pending reviews.
  Suggestions cannot be applied on multi-line comments.
  Suggestions cannot be applied while the pull request is queued to merge.
  Suggestion cannot be applied right now. Please check back later.
  
    
  
    
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
curious why we still need
patchon the pod itself, isn'tpods/resizesufficient?Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should be sufficient for resizing, but in order to patch the annotations onto the pod itself, I need that rule as well. Previously, when the admission-controller does it's own annotation (
vpaObservedContainers), it can just include the annotation in the webhook pod mangling, but the updater can't do that on its own.https://github.com/maxcao13/autoscaler/blob/maxcao13-inplace/vertical-pod-autoscaler/pkg/updater/eviction/pods_eviction_restriction.go#L544
Whether we want to use this new annotation or not is a different story though. It's purely for cosmetic reasons as noted in this comment: #7673 (comment), but the
vpaObservedContainersannotation is actually used inGetUpdatePriority. Curious what people think.