Skip to content

Commit

Permalink
remove last resource in spec annotation
Browse files Browse the repository at this point in the history 
Signed-off-by: Abner-1 <[email protected]>
  • Loading branch information
@ABNER-1
ABNER-1 committed Nov 27, 2024
1 parent ee835ac commit 208c25a
Show file tree
Hide file tree
Showing 7 changed files with 57 additions and 399 deletions.
3 changes: 1 addition & 2 deletions apis/apps/pub/inplace_update.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,7 @@ 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"`
Resources v1.ResourceRequirements `json:"resources,omitempty"`
ImageID string `json:"imageID,omitempty"`
}

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

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

7 changes: 5 additions & 2 deletions pkg/util/inplaceupdate/inplace_update.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,12 +90,15 @@ type UpdateSpec struct {
MetaDataPatch []byte `json:"metaDataPatch,omitempty"`
UpdateEnvFromMetadata bool `json:"updateEnvFromMetadata,omitempty"`
GraceSeconds int32 `json:"graceSeconds,omitempty"`
VerticalUpdateOnly bool `json:"verticalUpdate,omitempty"`

OldTemplate *v1.PodTemplateSpec `json:"oldTemplate,omitempty"`
NewTemplate *v1.PodTemplateSpec `json:"newTemplate,omitempty"`
}

func (u *UpdateSpec) VerticalUpdateOnly() bool {
return len(u.ContainerResources) > 0 && len(u.ContainerImages) == 0 && !u.UpdateEnvFromMetadata
}

type realControl struct {
podAdapter podadapter.Adapter
revisionAdapter revisionadapter.Interface
Expand Down Expand Up @@ -338,7 +341,7 @@ func (c *realControl) updatePodInPlace(pod *v1.Pod, spec *UpdateSpec, opts *Upda
Revision: spec.Revision,
UpdateTimestamp: metav1.NewTime(Clock.Now()),
UpdateEnvFromMetadata: spec.UpdateEnvFromMetadata,
VerticalUpdateOnly: spec.VerticalUpdateOnly,
VerticalUpdateOnly: spec.VerticalUpdateOnly(),
UpdateResources: len(spec.ContainerResources) > 0,
}
inPlaceUpdateStateJSON, _ := json.Marshal(inPlaceUpdateState)
Expand Down
13 changes: 3 additions & 10 deletions pkg/util/inplaceupdate/inplace_update_defaults.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,6 @@ func defaultCalculateInPlaceUpdateSpec(oldRevision, newRevision *apps.Controller
ContainerResources: make(map[string]v1.ResourceRequirements),
ContainerRefMetadata: make(map[string]metav1.ObjectMeta),
GraceSeconds: opts.GracePeriodSeconds,
VerticalUpdateOnly: false,
}
if opts.GetRevision != nil {
updateSpec.Revision = opts.GetRevision(newRevision)
Expand Down Expand Up @@ -403,10 +402,6 @@ func defaultCalculateInPlaceUpdateSpec(oldRevision, newRevision *apps.Controller
}
updateSpec.MetaDataPatch = patchBytes
}
// Need to distinguish whether only resources have been updated
if len(updateSpec.ContainerResources) > 0 && len(updateSpec.ContainerImages) == 0 && !updateSpec.UpdateEnvFromMetadata {
updateSpec.VerticalUpdateOnly = true
}

return updateSpec
}
Expand Down Expand Up @@ -461,10 +456,8 @@ func defaultCheckContainersInPlaceUpdateCompleted(pod *v1.Pod, inPlaceUpdateStat
containers[c.Name] = c
}
for _, cs := range pod.Status.ContainerStatuses {
if oldStatus, ok := inPlaceUpdateState.LastContainerStatuses[cs.Name]; ok {
if !verticalUpdateImpl.IsContainerUpdateCompleted(pod, containers[cs.Name], &cs, oldStatus) {
return fmt.Errorf("container %s resources not changed", cs.Name)
}
if !verticalUpdateImpl.IsContainerUpdateCompleted(containers[cs.Name], &cs) {
return fmt.Errorf("container %s resources not changed", cs.Name)
}
}
}
Expand Down Expand Up @@ -595,7 +588,7 @@ const (
)

func defaultCheckPodNeedsBeUnready(pod *v1.Pod, spec *UpdateSpec) bool {
if !utilfeature.DefaultFeatureGate.Enabled(features.InPlaceWorkloadVerticalScaling) || !spec.VerticalUpdateOnly {
if !utilfeature.DefaultFeatureGate.Enabled(features.InPlaceWorkloadVerticalScaling) || !spec.VerticalUpdateOnly() {
return containsReadinessGate(pod)
}

Expand Down
121 changes: 26 additions & 95 deletions pkg/util/inplaceupdate/inplace_update_defaults_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,14 +126,6 @@ func TestDefaultPatchUpdateSpecToPod(t *testing.T) {
ContainerBatchesRecord: []appspub.InPlaceUpdateContainerBatch{{Timestamp: metav1.NewTime(now), Containers: []string{"c1"}}},
},
expectedPatch: map[string]interface{}{
//"metadata": map[string]interface{}{
// "annotations": map[string]interface{}{
// appspub.InPlaceUpdateStateKey: util.DumpJSON(appspub.InPlaceUpdateState{
// LastContainerStatuses: map[string]appspub.InPlaceUpdateContainerStatus{"c1": {ImageID: "containerd://c1-img"}},
// ContainerBatchesRecord: []appspub.InPlaceUpdateContainerBatch{{Timestamp: metav1.NewTime(now), Containers: []string{"c1"}}},
// }),
// },
//},
"spec": map[string]interface{}{
"containers": []map[string]interface{}{
{
Expand Down Expand Up @@ -1068,7 +1060,6 @@ func TestDefaultCalculateInPlaceUpdateSpec(t *testing.T) {
},
},
},
VerticalUpdateOnly: true,
},
},
{
Expand All @@ -1089,7 +1080,6 @@ func TestDefaultCalculateInPlaceUpdateSpec(t *testing.T) {
},
},
},
VerticalUpdateOnly: false,
},
},
{
Expand Down Expand Up @@ -1129,7 +1119,6 @@ func TestDefaultCalculateInPlaceUpdateSpec(t *testing.T) {
},
},
},
VerticalUpdateOnly: true,
},
},
{
Expand All @@ -1156,7 +1145,6 @@ func TestDefaultCalculateInPlaceUpdateSpec(t *testing.T) {
},
},
},
VerticalUpdateOnly: false,
},
},
{
Expand Down Expand Up @@ -1311,20 +1299,6 @@ func TestDefaultPatchUpdateSpecToPod_Resource(t *testing.T) {
},
state: &appspub.InPlaceUpdateState{},
expectedState: &appspub.InPlaceUpdateState{
LastContainerStatuses: map[string]appspub.InPlaceUpdateContainerStatus{
"c1": {
Resources: v1.ResourceRequirements{
Requests: v1.ResourceList{
v1.ResourceMemory: resource.MustParse("1Gi"),
v1.ResourceCPU: resource.MustParse("1"),
},
Limits: v1.ResourceList{
v1.ResourceMemory: resource.MustParse("2Gi"),
v1.ResourceCPU: resource.MustParse("2"),
},
},
},
},
ContainerBatchesRecord: []appspub.InPlaceUpdateContainerBatch{{Timestamp: metav1.NewTime(now), Containers: []string{"c1"}}},
},
expectedPatch: map[string]interface{}{
Expand Down Expand Up @@ -1362,16 +1336,6 @@ func TestDefaultPatchUpdateSpecToPod_Resource(t *testing.T) {
LastContainerStatuses: map[string]appspub.InPlaceUpdateContainerStatus{
"c1": {
ImageID: "containerd://c1-img",
Resources: v1.ResourceRequirements{
Requests: v1.ResourceList{
v1.ResourceMemory: resource.MustParse("1Gi"),
v1.ResourceCPU: resource.MustParse("1"),
},
Limits: v1.ResourceList{
v1.ResourceMemory: resource.MustParse("2Gi"),
v1.ResourceCPU: resource.MustParse("2"),
},
},
},
},
ContainerBatchesRecord: []appspub.InPlaceUpdateContainerBatch{{Timestamp: metav1.NewTime(now), Containers: []string{"c1"}}},
Expand Down Expand Up @@ -1418,16 +1382,6 @@ func TestDefaultPatchUpdateSpecToPod_Resource(t *testing.T) {
LastContainerStatuses: map[string]appspub.InPlaceUpdateContainerStatus{
"c1": {
ImageID: "containerd://c1-img",
Resources: v1.ResourceRequirements{
Requests: v1.ResourceList{
v1.ResourceMemory: resource.MustParse("1Gi"),
v1.ResourceCPU: resource.MustParse("1"),
},
Limits: v1.ResourceList{
v1.ResourceMemory: resource.MustParse("2Gi"),
v1.ResourceCPU: resource.MustParse("2"),
},
},
},
},
NextContainerResources: map[string]v1.ResourceRequirements{
Expand Down Expand Up @@ -1479,16 +1433,6 @@ func TestDefaultPatchUpdateSpecToPod_Resource(t *testing.T) {
LastContainerStatuses: map[string]appspub.InPlaceUpdateContainerStatus{
"c1": {
ImageID: "containerd://c2-img",
Resources: v1.ResourceRequirements{
Requests: v1.ResourceList{
v1.ResourceMemory: resource.MustParse("1Gi"),
v1.ResourceCPU: resource.MustParse("1"),
},
Limits: v1.ResourceList{
v1.ResourceMemory: resource.MustParse("2Gi"),
v1.ResourceCPU: resource.MustParse("2"),
},
},
},
},
NextContainerResources: map[string]v1.ResourceRequirements{
Expand All @@ -1509,29 +1453,9 @@ func TestDefaultPatchUpdateSpecToPod_Resource(t *testing.T) {
LastContainerStatuses: map[string]appspub.InPlaceUpdateContainerStatus{
"c1": {
ImageID: "containerd://c2-img",
Resources: v1.ResourceRequirements{
Requests: v1.ResourceList{
v1.ResourceMemory: resource.MustParse("1Gi"),
v1.ResourceCPU: resource.MustParse("1"),
},
Limits: v1.ResourceList{
v1.ResourceMemory: resource.MustParse("2Gi"),
v1.ResourceCPU: resource.MustParse("2"),
},
},
},
"c2": {
ImageID: "containerd://c2-img",
Resources: v1.ResourceRequirements{
Requests: v1.ResourceList{
v1.ResourceMemory: resource.MustParse("1Gi"),
v1.ResourceCPU: resource.MustParse("1"),
},
Limits: v1.ResourceList{
v1.ResourceMemory: resource.MustParse("2Gi"),
v1.ResourceCPU: resource.MustParse("2"),
},
},
},
},
ContainerBatchesRecord: []appspub.InPlaceUpdateContainerBatch{{Timestamp: metav1.NewTime(now), Containers: []string{"c1"}}, {Timestamp: metav1.NewTime(now), Containers: []string{"c2"}}},
Expand Down Expand Up @@ -1652,12 +1576,6 @@ func createFakePod(imageInject, resourceInject, stateInject bool, num, imageOKNu
},
}
}
lastStatus.Resources = v1.ResourceRequirements{
Requests: map[v1.ResourceName]resource.Quantity{
v1.ResourceCPU: lastCPU,
v1.ResourceMemory: lastMem,
},
}
}
state.LastContainerStatuses[pod.Spec.Containers[i].Name] = lastStatus
}
Expand Down Expand Up @@ -1821,7 +1739,7 @@ func TestDefaultCheckPodNeedsBeUnready(t *testing.T) {
},
},
spec: &UpdateSpec{
VerticalUpdateOnly: true,

ContainerResources: map[string]v1.ResourceRequirements{},
},
expected: true,
Expand All @@ -1835,7 +1753,7 @@ func TestDefaultCheckPodNeedsBeUnready(t *testing.T) {
},
},
spec: &UpdateSpec{
VerticalUpdateOnly: true,

ContainerResources: map[string]v1.ResourceRequirements{},
},
expected: false,
Expand All @@ -1852,8 +1770,14 @@ func TestDefaultCheckPodNeedsBeUnready(t *testing.T) {
},
},
spec: &UpdateSpec{
VerticalUpdateOnly: true,
ContainerResources: map[string]v1.ResourceRequirements{},
ContainerResources: map[string]v1.ResourceRequirements{
"c2": {
Requests: v1.ResourceList{
v1.ResourceMemory: resource.MustParse("2Gi"),
v1.ResourceCPU: resource.MustParse("2"),
},
},
},
},
expected: false,
vpaEnabled: true,
Expand All @@ -1869,7 +1793,6 @@ func TestDefaultCheckPodNeedsBeUnready(t *testing.T) {
},
},
spec: &UpdateSpec{
VerticalUpdateOnly: false,
ContainerResources: map[string]v1.ResourceRequirements{},
},
expected: true,
Expand All @@ -1883,7 +1806,7 @@ func TestDefaultCheckPodNeedsBeUnready(t *testing.T) {
},
},
spec: &UpdateSpec{
VerticalUpdateOnly: true,

ContainerResources: map[string]v1.ResourceRequirements{},
},
expected: false,
Expand All @@ -1897,7 +1820,7 @@ func TestDefaultCheckPodNeedsBeUnready(t *testing.T) {
},
},
spec: &UpdateSpec{
VerticalUpdateOnly: false,

ContainerResources: map[string]v1.ResourceRequirements{},
},
expected: false,
Expand All @@ -1914,15 +1837,24 @@ func TestDefaultCheckPodNeedsBeUnready(t *testing.T) {
{ResourceName: v1.ResourceCPU, RestartPolicy: v1.RestartContainer},
},
},
{
Name: "c2",
},
},
ReadinessGates: []v1.PodReadinessGate{
{ConditionType: appspub.InPlaceUpdateReady},
},
},
},
spec: &UpdateSpec{
VerticalUpdateOnly: true,
ContainerResources: map[string]v1.ResourceRequirements{},
ContainerResources: map[string]v1.ResourceRequirements{
"c2": {
Requests: v1.ResourceList{
v1.ResourceMemory: resource.MustParse("2Gi"),
v1.ResourceCPU: resource.MustParse("2"),
},
},
},
},
expected: false,
vpaEnabled: true,
Expand All @@ -1945,7 +1877,7 @@ func TestDefaultCheckPodNeedsBeUnready(t *testing.T) {
},
},
spec: &UpdateSpec{
VerticalUpdateOnly: true,

ContainerResources: map[string]v1.ResourceRequirements{
"11": {
Requests: map[v1.ResourceName]resource.Quantity{
Expand Down Expand Up @@ -1975,7 +1907,7 @@ func TestDefaultCheckPodNeedsBeUnready(t *testing.T) {
},
},
spec: &UpdateSpec{
VerticalUpdateOnly: true,

ContainerResources: map[string]v1.ResourceRequirements{
"11": {
Requests: map[v1.ResourceName]resource.Quantity{
Expand Down Expand Up @@ -2006,7 +1938,6 @@ func TestDefaultCheckPodNeedsBeUnready(t *testing.T) {
},
},
spec: &UpdateSpec{
VerticalUpdateOnly: true,
ContainerResources: map[string]v1.ResourceRequirements{
"11": {
Requests: map[v1.ResourceName]resource.Quantity{
Expand Down Expand Up @@ -2037,7 +1968,7 @@ func TestDefaultCheckPodNeedsBeUnready(t *testing.T) {
},
},
spec: &UpdateSpec{
VerticalUpdateOnly: true,

ContainerResources: map[string]v1.ResourceRequirements{
"11": {
Requests: map[v1.ResourceName]resource.Quantity{
Expand Down
Loading

0 comments on commit 208c25a

Please sign in to comment.