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.

Sign up for GitHub

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

Jump to bottom

Support inplace-update restart count #1097

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

Support inplace-update restart count #1097

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a36ff34
inplaceupdate restart count
diannaowa Mar 19, 2022
121eb28
move if statement into function
diannaowa Mar 24, 2022
b4bd74c
remove comment and handle the error of Unmarshal
diannaowa Mar 25, 2022
bc70823
make generate manifests
diannaowa Oct 7, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions apis/apps/pub/inplace_update.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,12 @@ const (
// RuntimeContainerMetaKey is a key in pod annotations. Kruise-daemon should report the
// states of runtime containers into its value, which is a structure JSON of RuntimeContainerMetaSet type.
RuntimeContainerMetaKey = "apps.kruise.io/runtime-containers-meta"

// InPlaceUpdatePodRestartKey records the count of pod restarts
InPlaceUpdatePodRestartKey = "apps.kruise.io/inplace-update-pod-restart-count"

// InPlaceUpdateContainersRestartKey records the count of containers restarts
InPlaceUpdateContainersRestartKey = "apps.kruise.io/inplace-update-containers-restart-count"
)

// InPlaceUpdateState records latest inplace-update state, including old statuses of containers.
Expand Down Expand Up @@ -101,6 +107,16 @@ type InPlaceUpdateStrategy struct {
GracePeriodSeconds int32 `json:"gracePeriodSeconds,omitempty"`
}

// InPlaceUpdateContainerRestartCount record the restart count of containers
type InPlaceUpdateContainerRestartCount struct {
// Revision is the updated revision hash.
Revision string `json:"revision,omitempty"`
// RestartCount is the container restart count
RestartCount int `json:"restartCount,omitempty"`
diannaowa marked this conversation as resolved.
Show resolved Hide resolved
// Timestamp is the time for this update
Timestamp metav1.Time `json:"timestamp,omitempty"`
}

func GetInPlaceUpdateState(obj metav1.Object) (string, bool) {
if v, ok := obj.GetAnnotations()[InPlaceUpdateStateKey]; ok {
return v, ok
Expand Down
16 changes: 16 additions & 0 deletions apis/apps/pub/zz_generated.deepcopy.go
View file
Open in desktop

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

2 changes: 0 additions & 2 deletions pkg/controller/cloneset/sync/cloneset_update.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ func (c *realControl) Update(cs *appsv1alpha1.CloneSet,

key := clonesetutils.GetControllerKey(cs)
coreControl := clonesetcore.New(cs)

// 1. refresh states for all pods
var modified bool
for _, pod := range pods {
Expand Down Expand Up @@ -249,7 +248,6 @@ func (c *realControl) updatePod(cs *appsv1alpha1.CloneSet, coreControl clonesetc
break
}
}

if c.inplaceControl.CanUpdateInPlace(oldRevision, updateRevision, coreControl.GetUpdateOptions()) {
switch state := lifecycle.GetPodLifecycleState(pod); state {
case "", appspub.LifecycleStatePreparingNormal, appspub.LifecycleStateNormal:
Expand Down
11 changes: 10 additions & 1 deletion pkg/controller/cloneset/sync/cloneset_update_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,12 @@ func TestUpdate(t *testing.T) {
UpdateTimestamp: now,
LastContainerStatuses: map[string]appspub.InPlaceUpdateContainerStatus{"c1": {ImageID: "image-id-xyz"}},
ContainerBatchesRecord: []appspub.InPlaceUpdateContainerBatch{{Timestamp: now, Containers: []string{"c1"}}},
})},
}),
appspub.InPlaceUpdatePodRestartKey: "1",
appspub.InPlaceUpdateContainersRestartKey: util.DumpJSON(map[string]appspub.InPlaceUpdateContainerRestartCount{
"c1": {RestartCount: 1, Revision: "rev_new", Timestamp: now},
}),
},
ResourceVersion: "2",
},
Spec: v1.PodSpec{
Expand Down Expand Up @@ -555,6 +560,10 @@ func TestUpdate(t *testing.T) {
LastContainerStatuses: map[string]appspub.InPlaceUpdateContainerStatus{"c1": {ImageID: "image-id-xyz"}},
ContainerBatchesRecord: []appspub.InPlaceUpdateContainerBatch{{Timestamp: now, Containers: []string{"c1"}}},
}),
appspub.InPlaceUpdatePodRestartKey: "1",
appspub.InPlaceUpdateContainersRestartKey: util.DumpJSON(map[string]appspub.InPlaceUpdateContainerRestartCount{
"c1": {RestartCount: 1, Revision: "rev_new", Timestamp: now},
}),
},
ResourceVersion: "1",
},
Expand Down
79 changes: 68 additions & 11 deletions pkg/util/inplaceupdate/inplace_update.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,10 @@ import (
"encoding/json"
"fmt"
"regexp"
"strconv"
"strings"
"time"

appspub "github.com/openkruise/kruise/apis/apps/pub"
"github.com/openkruise/kruise/pkg/util"
"github.com/openkruise/kruise/pkg/util/podadapter"
"github.com/openkruise/kruise/pkg/util/revisionadapter"
apps "k8s.io/api/apps/v1"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand All @@ -36,6 +33,11 @@ import (
"k8s.io/client-go/util/retry"
"k8s.io/klog/v2"
"sigs.k8s.io/controller-runtime/pkg/client"

appspub "github.com/openkruise/kruise/apis/apps/pub"
"github.com/openkruise/kruise/pkg/util"
"github.com/openkruise/kruise/pkg/util/podadapter"
"github.com/openkruise/kruise/pkg/util/revisionadapter"
)

var (
Expand Down Expand Up @@ -88,7 +90,6 @@ type UpdateSpec struct {
OldTemplate *v1.PodTemplateSpec `json:"oldTemplate,omitempty"`
NewTemplate *v1.PodTemplateSpec `json:"newTemplate,omitempty"`
}

type realControl struct {
podAdapter podadapter.Adapter
revisionAdapter revisionadapter.Interface
Expand Down Expand Up @@ -138,7 +139,6 @@ func (c *realControl) Refresh(pod *v1.Pod, opts *UpdateOptions) RefreshResult {
klog.V(5).Infof("Pod %s/%s in-place update pre-check not passed: %v", pod.Namespace, pod.Name, checkErr)
return RefreshResult{}
}

// do update the next containers
if updated, err := c.updateNextBatch(pod, opts); err != nil {
return RefreshResult{RefreshErr: err}
Expand Down Expand Up @@ -216,13 +216,13 @@ func (c *realControl) finishGracePeriod(pod *v1.Pod, opts *UpdateOptions) (time.
delayDuration = roundupSeconds(graceDuration - span)
return nil
}

if clone, err = opts.PatchSpecToPod(clone, &spec, &updateState); err != nil {
return err
}
// record restart count of the pod(containers)
recordPodAndContainersRestartCount(clone, &spec, updateState.Revision)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not an in-place upgrade here, and it doesn't feel like it should be judged

appspub.RemoveInPlaceUpdateGrace(clone)
}

_, err = c.podAdapter.UpdatePod(clone)
return err
})
Expand Down Expand Up @@ -258,7 +258,8 @@ func (c *realControl) updateNextBatch(pod *v1.Pod, opts *UpdateOptions) (bool, e
if clone, err = opts.PatchSpecToPod(clone, &spec, &state); err != nil {
return err
}

// record restart count of the pod(containers)
recordPodAndContainersRestartCount(clone, &spec, state.Revision)
updated = true
_, err = c.podAdapter.UpdatePod(clone)
return err
Expand All @@ -273,7 +274,6 @@ func (c *realControl) CanUpdateInPlace(oldRevision, newRevision *apps.Controller

func (c *realControl) Update(pod *v1.Pod, oldRevision, newRevision *apps.ControllerRevision, opts *UpdateOptions) UpdateResult {
opts = SetOptionsDefaults(opts)

// 1. calculate inplace update spec
spec := opts.CalculateSpec(oldRevision, newRevision, opts)
if spec == nil {
Expand Down Expand Up @@ -338,12 +338,13 @@ func (c *realControl) updatePodInPlace(pod *v1.Pod, spec *UpdateSpec, opts *Upda
if clone, err = opts.PatchSpecToPod(clone, spec, &inPlaceUpdateState); err != nil {
return err
}
// record restart count of the pod(containers)
recordPodAndContainersRestartCount(clone, spec, inPlaceUpdateState.Revision)
appspub.RemoveInPlaceUpdateGrace(clone)
} else {
inPlaceUpdateSpecJSON, _ := json.Marshal(spec)
clone.Annotations[appspub.InPlaceUpdateGraceKey] = string(inPlaceUpdateSpecJSON)
}

newPod, updateErr := c.podAdapter.UpdatePod(clone)
if updateErr == nil {
newResourceVersion = newPod.ResourceVersion
Expand Down Expand Up @@ -419,3 +420,59 @@ func hasEqualCondition(pod *v1.Pod, newCondition *v1.PodCondition) bool {
oldCondition.Reason == newCondition.Reason && oldCondition.Message == newCondition.Message
return isEqual
}

// recordPodAndContainersRestartCount record the count of pod(containers) restarts
func recordPodAndContainersRestartCount(pod *v1.Pod, spec *UpdateSpec, revision string) {
if spec.ContainerImages == nil && !spec.UpdateEnvFromMetadata {
return
}
var currentPodRestartCount int64
containersRestartCount := make(map[string]appspub.InPlaceUpdateContainerRestartCount)

if v, ok := pod.Annotations[appspub.InPlaceUpdatePodRestartKey]; ok {
currentPodRestartCount, _ = strconv.ParseInt(v, 10, 64)
}

if v, ok := pod.Annotations[appspub.InPlaceUpdateContainersRestartKey]; ok {
if err := json.Unmarshal([]byte(v), &containersRestartCount); err != nil {
return
}
}
calculateRestartCountFunc := func(cname string) {
containerRestartCount, ok := containersRestartCount[cname]
if ok && revision != containerRestartCount.Revision {
containerRestartCount.RestartCount += 1
containerRestartCount.Revision = revision
containerRestartCount.Timestamp = metav1.NewTime(Clock.Now())
currentPodRestartCount += 1 // pod restart
} else if !ok {
containerRestartCount = appspub.InPlaceUpdateContainerRestartCount{
Revision: revision,
RestartCount: 1,
Timestamp: metav1.NewTime(Clock.Now()),
}
currentPodRestartCount += 1 // pod restart
}
containersRestartCount[cname] = containerRestartCount // containers restart
}

containers := make(map[string]bool)
if spec.ContainerImages != nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This if judgment can be removed, just keep the for statement below

for cname := range spec.ContainerImages {
containers[cname] = true
calculateRestartCountFunc(cname)
}
}

if spec.UpdateEnvFromMetadata {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as above

for cname := range spec.ContainerRefMetadata {
if !containers[cname] {
calculateRestartCountFunc(cname)
}
}
}

containersRestartCountJson, _ := json.Marshal(containersRestartCount)
pod.Annotations[appspub.InPlaceUpdateContainersRestartKey] = string(containersRestartCountJson)
pod.Annotations[appspub.InPlaceUpdatePodRestartKey] = strconv.FormatInt(currentPodRestartCount, 10)
}
19 changes: 14 additions & 5 deletions pkg/util/inplaceupdate/inplace_update_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,18 +23,19 @@ import (
"testing"
"time"

appspub "github.com/openkruise/kruise/apis/apps/pub"
"github.com/openkruise/kruise/pkg/features"
"github.com/openkruise/kruise/pkg/util"
utilfeature "github.com/openkruise/kruise/pkg/util/feature"
"github.com/openkruise/kruise/pkg/util/revisionadapter"
apps "k8s.io/api/apps/v1"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/clock"
"sigs.k8s.io/controller-runtime/pkg/client/fake"

appspub "github.com/openkruise/kruise/apis/apps/pub"
"github.com/openkruise/kruise/pkg/features"
"github.com/openkruise/kruise/pkg/util"
utilfeature "github.com/openkruise/kruise/pkg/util/feature"
"github.com/openkruise/kruise/pkg/util/revisionadapter"
)

func TestCalculateInPlaceUpdateSpec(t *testing.T) {
Expand Down Expand Up @@ -451,6 +452,10 @@ func TestRefresh(t *testing.T) {
LastContainerStatuses: map[string]appspub.InPlaceUpdateContainerStatus{"c1": {ImageID: "img01"}},
ContainerBatchesRecord: []appspub.InPlaceUpdateContainerBatch{{Timestamp: aHourAgo, Containers: []string{"main"}}},
}),
appspub.InPlaceUpdatePodRestartKey: "1",
appspub.InPlaceUpdateContainersRestartKey: util.DumpJSON(map[string]appspub.InPlaceUpdateContainerRestartCount{
"main": {RestartCount: 1, Revision: "new-revision", Timestamp: aHourAgo},
}),
},
ResourceVersion: "1",
},
Expand Down Expand Up @@ -588,6 +593,10 @@ func TestRefresh(t *testing.T) {
LastContainerStatuses: map[string]appspub.InPlaceUpdateContainerStatus{"c1": {ImageID: "c1-img1-ID"}, "c2": {ImageID: "c2-img1-ID"}},
ContainerBatchesRecord: []appspub.InPlaceUpdateContainerBatch{{Timestamp: aHourAgo, Containers: []string{"c1"}}, {Timestamp: aHourAgo, Containers: []string{"c2"}}},
}),
appspub.InPlaceUpdatePodRestartKey: "1",
appspub.InPlaceUpdateContainersRestartKey: util.DumpJSON(map[string]appspub.InPlaceUpdateContainerRestartCount{
"c2": {RestartCount: 1, Revision: "new-revision", Timestamp: aHourAgo},
}),
},
},
Spec: v1.PodSpec{
Expand Down
Loading