Skip to content

Commit

Permalink
refactor: define vertically updated interfaces for different implemen…
Browse files Browse the repository at this point in the history
…tations

Signed-off-by: LavenderQAQ <lavenderqaq.cs@gmail.com>
  • Loading branch information
LavenderQAQ committed Aug 16, 2023
1 parent bef6a4e commit a74b455
Show file tree
Hide file tree
Showing 4 changed files with 126 additions and 13 deletions.
3 changes: 2 additions & 1 deletion apis/apps/pub/inplace_update.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,8 @@ type InPlaceUpdateContainerBatch struct {
// InPlaceUpdateContainerStatus records the statuses of the container that are mainly used
// to determine whether the InPlaceUpdate is completed.
type InPlaceUpdateContainerStatus struct {
ImageID string `json:"imageID,omitempty"`
ImageID string `json:"imageID,omitempty"`
Resource v1.ResourceRequirements `json:"resource,omitempty"`
}

// InPlaceUpdateStrategy defines the strategies for in-place update.
Expand Down
3 changes: 2 additions & 1 deletion apis/apps/pub/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

42 changes: 31 additions & 11 deletions pkg/util/inplaceupdate/inplace_update_defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ func SetOptionsDefaults(opts *UpdateOptions) *UpdateOptions {
}

if utilfeature.DefaultFeatureGate.Enabled(features.InPlaceWorkloadVerticalScaling) {
registerVerticalUpdate()

Check warning on line 48 in pkg/util/inplaceupdate/inplace_update_defaults.go

View check run for this annotation

Codecov / codecov/patch

pkg/util/inplaceupdate/inplace_update_defaults.go#L48

Added line #L48 was not covered by tests

if opts.CalculateSpec == nil {
opts.CalculateSpec = defaultCalculateInPlaceUpdateSpecWithVerticalUpdate
}

Check warning on line 52 in pkg/util/inplaceupdate/inplace_update_defaults.go

View check run for this annotation

Codecov / codecov/patch

pkg/util/inplaceupdate/inplace_update_defaults.go#L50-L52

Added lines #L50 - L52 were not covered by tests
Expand All @@ -60,7 +62,6 @@ func SetOptionsDefaults(opts *UpdateOptions) *UpdateOptions {
if opts.CheckContainersUpdateCompleted == nil {
opts.CheckContainersUpdateCompleted = defaultCheckContainersInPlaceUpdateCompletedWithVerticalUpdate
}

Check warning on line 64 in pkg/util/inplaceupdate/inplace_update_defaults.go

View check run for this annotation

Codecov / codecov/patch

pkg/util/inplaceupdate/inplace_update_defaults.go#L62-L64

Added lines #L62 - L64 were not covered by tests

} else {
if opts.CalculateSpec == nil {
opts.CalculateSpec = defaultCalculateInPlaceUpdateSpec
Expand All @@ -77,7 +78,6 @@ func SetOptionsDefaults(opts *UpdateOptions) *UpdateOptions {
if opts.CheckContainersUpdateCompleted == nil {
opts.CheckContainersUpdateCompleted = defaultCheckContainersInPlaceUpdateCompleted
}

}

return opts
Expand Down Expand Up @@ -498,6 +498,8 @@ func checkAllContainersHashConsistent(pod *v1.Pod, runtimeContainerMetaSet *apps
func defaultPatchUpdateSpecToPodWithVerticalUpdate(pod *v1.Pod, spec *UpdateSpec, state *appspub.InPlaceUpdateState) (*v1.Pod, error) {
klog.V(5).Infof("Begin to in-place update pod %s/%s with update spec %v, state %v", pod.Namespace, pod.Name, util.DumpJSON(spec), util.DumpJSON(state))

registerVerticalUpdate()

state.NextContainerImages = make(map[string]string)
state.NextContainerRefMetadata = make(map[string]metav1.ObjectMeta)
state.NextContainerResources = make(map[string]v1.ResourceRequirements)
Expand Down Expand Up @@ -566,29 +568,39 @@ func defaultPatchUpdateSpecToPodWithVerticalUpdate(pod *v1.Pod, spec *UpdateSpec
containersImageChanged.Insert(c.Name)
}
if resourceExists {
for key, quantity := range newResource.Limits {
c.Resources.Limits[key] = quantity
}
for key, quantity := range newResource.Requests {
c.Resources.Requests[key] = quantity
}
verticalUpdateOperator.UpdateContainerResource(c, &newResource)
containersResourceChanged.Insert(c.Name)
}
} else {
state.NextContainerImages[c.Name] = newImage
state.NextContainerResources[c.Name] = newResource
}

Check warning on line 577 in pkg/util/inplaceupdate/inplace_update_defaults.go

View check run for this annotation

Codecov / codecov/patch

pkg/util/inplaceupdate/inplace_update_defaults.go#L565-L577

Added lines #L565 - L577 were not covered by tests
}

// This provides a hook for vertical updates,
// so that internal enterprise implementations can update pod resources here at once
verticalUpdateOperator.UpdatePodResource(pod)

for _, c := range pod.Status.ContainerStatuses {
if containersImageChanged.Has(c.Name) {
if state.LastContainerStatuses == nil {
state.LastContainerStatuses = map[string]appspub.InPlaceUpdateContainerStatus{}
}
state.LastContainerStatuses[c.Name] = appspub.InPlaceUpdateContainerStatus{ImageID: c.ImageID}
if cs, ok := state.LastContainerStatuses[c.Name]; !ok {
state.LastContainerStatuses[c.Name] = appspub.InPlaceUpdateContainerStatus{ImageID: c.ImageID}
} else {
cs.ImageID = c.ImageID
}

Check warning on line 593 in pkg/util/inplaceupdate/inplace_update_defaults.go

View check run for this annotation

Codecov / codecov/patch

pkg/util/inplaceupdate/inplace_update_defaults.go#L582-L593

Added lines #L582 - L593 were not covered by tests
}
if containersResourceChanged.Has(c.Name) {
verticalUpdateOperator.SyncContainerResource(&c, state)
}

Check warning on line 597 in pkg/util/inplaceupdate/inplace_update_defaults.go

View check run for this annotation

Codecov / codecov/patch

pkg/util/inplaceupdate/inplace_update_defaults.go#L595-L597

Added lines #L595 - L597 were not covered by tests
// TODO(LavenderQAQ): The status of resource needs to be printed
}

// This provides a hook for vertical updates,
// so that internal enterprise implementations can sync pod resources here at once
verticalUpdateOperator.SyncPodResource(pod, state)

// update annotations and labels for the containers to update
for cName, objMeta := range spec.ContainerRefMetadata {
if containersToUpdate.Has(cName) {
Expand Down Expand Up @@ -780,6 +792,8 @@ func defaultCalculateInPlaceUpdateSpecWithVerticalUpdate(oldRevision, newRevisio
// If the imageID in containerStatuses has not been changed, we assume that kubelet has not updated
// containers in Pod.
func DefaultCheckInPlaceUpdateCompletedWithVerticalUpdate(pod *v1.Pod) error {
registerVerticalUpdate()

if _, isInGraceState := appspub.GetInPlaceUpdateGrace(pod); isInGraceState {
return fmt.Errorf("still in grace period of in-place update")
}

Check warning on line 799 in pkg/util/inplaceupdate/inplace_update_defaults.go

View check run for this annotation

Codecov / codecov/patch

pkg/util/inplaceupdate/inplace_update_defaults.go#L794-L799

Added lines #L794 - L799 were not covered by tests
Expand All @@ -798,6 +812,8 @@ func DefaultCheckInPlaceUpdateCompletedWithVerticalUpdate(pod *v1.Pod) error {
}

func defaultCheckContainersInPlaceUpdateCompletedWithVerticalUpdate(pod *v1.Pod, inPlaceUpdateState *appspub.InPlaceUpdateState) error {
registerVerticalUpdate()

runtimeContainerMetaSet, err := appspub.GetRuntimeContainerMetaSet(pod)
if err != nil {
return err
Expand Down Expand Up @@ -840,7 +856,11 @@ func defaultCheckContainersInPlaceUpdateCompletedWithVerticalUpdate(pod *v1.Pod,
return fmt.Errorf("container %s imageID not changed", cs.Name)
}

Check warning on line 857 in pkg/util/inplaceupdate/inplace_update_defaults.go

View check run for this annotation

Codecov / codecov/patch

pkg/util/inplaceupdate/inplace_update_defaults.go#L851-L857

Added lines #L851 - L857 were not covered by tests
}
// TODO(LavenderQAQ): Check the vertical updating status of the container
// Determine whether the vertical update was successful by the resource values in the pod's spec and status
// TODO(LavenderQAQ): The third parameter here should be passed to the resources value in the status field of all containers and will need to be modified after the k8s api upgrade.
if !verticalUpdateOperator.IsUpdateCompleted(pod, containerResources[cs.Name], containerResources[cs.Name], inPlaceUpdateState.LastContainerStatuses[cs.Name]) {
return fmt.Errorf("container %s resources not changed", cs.Name)
}
delete(inPlaceUpdateState.LastContainerStatuses, cs.Name)

Check warning on line 864 in pkg/util/inplaceupdate/inplace_update_defaults.go

View check run for this annotation

Codecov / codecov/patch

pkg/util/inplaceupdate/inplace_update_defaults.go#L861-L864

Added lines #L861 - L864 were not covered by tests
}
}
Expand Down
91 changes: 91 additions & 0 deletions pkg/util/inplaceupdate/inplace_update_vertical.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
/*
Copyright 2023 The Kruise Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package inplaceupdate

import (
appspub "github.com/openkruise/kruise/apis/apps/pub"
v1 "k8s.io/api/core/v1"
)

// For In-place workload vertical scaling
type VerticalUpdateInterface interface {
// Get the expected resource values of the container and its current status
SyncContainerResource(container *v1.ContainerStatus, state *appspub.InPlaceUpdateState)
// Pass in the container to be modified and the expected resource values.
UpdateContainerResource(container *v1.Container, resource *v1.ResourceRequirements)
// Get the expected resource values of all containers in the pod and their current status
SyncPodResource(pod *v1.Pod, state *appspub.InPlaceUpdateState)
// All containers of a pod can be updated at once within this interface.
UpdatePodResource(pod *v1.Pod)
// To determine whether the container has been successfully vertical updated,
// pass in the expected resources of the container and its current status,
// as well as the information for the entire pod for compatibility with some internal vertical scaling implementations.
IsUpdateCompleted(pod *v1.Pod, containerResourcesInSpec v1.ResourceRequirements, containerResourcesInStatus v1.ResourceRequirements, lastContainerStatus appspub.InPlaceUpdateContainerStatus) bool
}

var verticalUpdateOperator VerticalUpdateInterface = nil

// To register vertical update operations,
// you can register different vertical update implementations here
func registerVerticalUpdate() {
if verticalUpdateOperator == nil {
verticalUpdateOperator = &VerticalUpdate{}
}

Check warning on line 47 in pkg/util/inplaceupdate/inplace_update_vertical.go

View check run for this annotation

Codecov / codecov/patch

pkg/util/inplaceupdate/inplace_update_vertical.go#L44-L47

Added lines #L44 - L47 were not covered by tests
}

// VerticalUpdate represents the vertical scaling of k8s standard
type VerticalUpdate struct{}

var _ VerticalUpdateInterface = &VerticalUpdate{}

// Get the resource status from the container and synchronize it to state
func (v *VerticalUpdate) SyncContainerResource(container *v1.ContainerStatus, state *appspub.InPlaceUpdateState) {
// TODO(LavenderQAQ): Need to write the status synchronization module after api upgrade

Check warning on line 57 in pkg/util/inplaceupdate/inplace_update_vertical.go

View check run for this annotation

Codecov / codecov/patch

pkg/util/inplaceupdate/inplace_update_vertical.go#L56-L57

Added lines #L56 - L57 were not covered by tests
}

// UpdateResource implements vertical updates by directly modifying the container's resources,
// conforming to the k8s community standard
func (v *VerticalUpdate) UpdateContainerResource(container *v1.Container, newResource *v1.ResourceRequirements) {
for key, quantity := range newResource.Limits {
container.Resources.Limits[key] = quantity
}
for key, quantity := range newResource.Requests {
container.Resources.Requests[key] = quantity
}

Check warning on line 68 in pkg/util/inplaceupdate/inplace_update_vertical.go

View check run for this annotation

Codecov / codecov/patch

pkg/util/inplaceupdate/inplace_update_vertical.go#L62-L68

Added lines #L62 - L68 were not covered by tests
}

// Get the resource status from the pod and synchronize it to state
func (v *VerticalUpdate) SyncPodResource(pod *v1.Pod, state *appspub.InPlaceUpdateState) {
// TODO(LavenderQAQ): Need to write the status synchronization module after api upgrade

Check warning on line 73 in pkg/util/inplaceupdate/inplace_update_vertical.go

View check run for this annotation

Codecov / codecov/patch

pkg/util/inplaceupdate/inplace_update_vertical.go#L72-L73

Added lines #L72 - L73 were not covered by tests
}

// For the community-standard vertical scale-down implementation,
// there is no need to do anything here because the container has already been updated in the UpdateContainerResource interface
func (v *VerticalUpdate) UpdatePodResource(pod *v1.Pod) {
return

Check warning on line 79 in pkg/util/inplaceupdate/inplace_update_vertical.go

View check run for this annotation

Codecov / codecov/patch

pkg/util/inplaceupdate/inplace_update_vertical.go#L78-L79

Added lines #L78 - L79 were not covered by tests
}

// IsUpdateCompleted directly determines whether the current container is vertically updated by the spec and status of the container,
// which conforms to the k8s community standard
func (v *VerticalUpdate) IsUpdateCompleted(pod *v1.Pod, containerResourcesInSpec v1.ResourceRequirements, containerResourcesInStatus v1.ResourceRequirements, lastContainerStatus appspub.InPlaceUpdateContainerStatus) bool {
return true

Check warning on line 85 in pkg/util/inplaceupdate/inplace_update_vertical.go

View check run for this annotation

Codecov / codecov/patch

pkg/util/inplaceupdate/inplace_update_vertical.go#L84-L85

Added lines #L84 - L85 were not covered by tests
}

// Internal implementation of vertical updates
// type VerticalUpdateInternal struct{}

// var _ VerticalUpdateInterface = &VerticalUpdateInternal{}

0 comments on commit a74b455

Please sign in to comment.