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

Support in-place update containers with launch priority #909

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
23 changes: 6 additions & 17 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,13 @@ undeploy: ## Undeploy controller from the K8s cluster specified in ~/.kube/confi
$(KUSTOMIZE) build config/default | kubectl delete -f -


CONTROLLER_GEN = $(shell pwd)/bin/controller-gen-0.7
CONTROLLER_GEN = $(shell pwd)/bin/controller-gen
controller-gen: ## Download controller-gen locally if necessary.
$(call go-get-tool-with-alias,$(CONTROLLER_GEN),sigs.k8s.io/controller-tools/cmd/controller-gen@v0.7.0,controller-gen)
ifeq ("$(shell $(CONTROLLER_GEN) --version 2> /dev/null)", "Version: v0.7.0")
else
rm -rf $(CONTROLLER_GEN)
$(call go-get-tool,$(CONTROLLER_GEN),sigs.k8s.io/controller-tools/cmd/controller-gen@v0.7.0)
endif

KUSTOMIZE = $(shell pwd)/bin/kustomize
kustomize: ## Download kustomize locally if necessary.
Expand All @@ -110,18 +114,3 @@ GOBIN=$(PROJECT_DIR)/bin go get $(2) ;\
rm -rf $$TMP_DIR ;\
}
endef

# go-get-tool will 'go get' any package $2 and install it to $1.
PROJECT_DIR := $(shell dirname $(abspath $(lastword $(MAKEFILE_LIST))))
define go-get-tool-with-alias
@[ -f $(1) ] || { \
set -e ;\
TMP_DIR=$$(mktemp -d) ;\
cd $$TMP_DIR ;\
go mod init tmp ;\
echo "Downloading $(2)" ;\
GOBIN=$(PROJECT_DIR)/bin go install $(2) ;\
rm -rf $$TMP_DIR ;\
mv $(PROJECT_DIR)/bin/$(3) $(1);\
}
endef
27 changes: 26 additions & 1 deletion apis/apps/pub/inplace_update.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ type InPlaceUpdateState struct {
// Revision is the updated revision hash.
Revision string `json:"revision"`

// UpdateTimestamp is the time when the in-place update happens.
// UpdateTimestamp is the start time when the in-place update happens.
UpdateTimestamp metav1.Time `json:"updateTimestamp"`

// LastContainerStatuses records the before-in-place-update container statuses. It is a map from ContainerName
Expand All @@ -61,6 +61,31 @@ type InPlaceUpdateState struct {

// UpdateEnvFromMetadata indicates there are envs from annotations/labels that should be in-place update.
UpdateEnvFromMetadata bool `json:"updateEnvFromMetadata,omitempty"`

// NextContainerImages is the containers with lower priority that waiting for in-place update images in next batch.
NextContainerImages map[string]string `json:"nextContainerImages,omitempty"`

// NextContainerRefMetadata is the containers with lower priority that waiting for in-place update labels/annotations in next batch.
NextContainerRefMetadata map[string]metav1.ObjectMeta `json:"nextContainerRefMetadata,omitempty"`
FillZpp marked this conversation as resolved.
Show resolved Hide resolved

// PreCheckBeforeNext is the pre-check that must pass before the next containers can be in-place update.
PreCheckBeforeNext *InPlaceUpdatePreCheckBeforeNext `json:"preCheckBeforeNext,omitempty"`

// ContainerBatchesRecord records the update batches that have patched in this revision.
ContainerBatchesRecord []InPlaceUpdateContainerBatch `json:"containerBatchesRecord,omitempty"`
}

// InPlaceUpdatePreCheckBeforeNext contains the pre-check that must pass before the next containers can be in-place update.
type InPlaceUpdatePreCheckBeforeNext struct {
ContainersRequiredReady []string `json:"containersRequiredReady,omitempty"`
}

// InPlaceUpdateContainerBatch indicates the timestamp and containers for a batch update
type InPlaceUpdateContainerBatch struct {
// Timestamp is the time for this update batch
Timestamp metav1.Time `json:"timestamp"`
// Containers is the name list of containers for this update batch
Containers []string `json:"containers"`
}

// InPlaceUpdateContainerStatus records the statuses of the container that are mainly used
Expand Down
71 changes: 70 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.

2 changes: 1 addition & 1 deletion pkg/controller/cloneset/sync/cloneset_update.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ func (c *realControl) refreshPodState(cs *appsv1alpha1.CloneSet, coreControl clo
var state appspub.LifecycleStateType
switch lifecycle.GetPodLifecycleState(pod) {
case appspub.LifecycleStateUpdating:
if opts.CheckUpdateCompleted(pod) == nil {
if opts.CheckPodUpdateCompleted(pod) == nil {
if cs.Spec.Lifecycle != nil && !lifecycle.IsPodHooked(cs.Spec.Lifecycle.InPlaceUpdate, pod) {
state = appspub.LifecycleStateUpdated
} else {
Expand Down
38 changes: 19 additions & 19 deletions pkg/controller/cloneset/sync/cloneset_update_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/clock"
intstrutil "k8s.io/apimachinery/pkg/util/intstr"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/client-go/kubernetes/scheme"
Expand Down Expand Up @@ -294,9 +295,10 @@ func TestUpdate(t *testing.T) {
appspub.LifecycleStateKey: string(appspub.LifecycleStateUpdating),
},
Annotations: map[string]string{appspub.InPlaceUpdateStateKey: util.DumpJSON(appspub.InPlaceUpdateState{
Revision: "rev_new",
UpdateTimestamp: now,
LastContainerStatuses: map[string]appspub.InPlaceUpdateContainerStatus{"c1": {ImageID: "image-id-xyz"}},
Revision: "rev_new",
UpdateTimestamp: now,
LastContainerStatuses: map[string]appspub.InPlaceUpdateContainerStatus{"c1": {ImageID: "image-id-xyz"}},
ContainerBatchesRecord: []appspub.InPlaceUpdateContainerBatch{{Timestamp: now, Containers: []string{"c1"}}},
})},
ResourceVersion: "2",
},
Expand Down Expand Up @@ -371,9 +373,8 @@ func TestUpdate(t *testing.T) {
},
Annotations: map[string]string{
appspub.InPlaceUpdateStateKey: util.DumpJSON(appspub.InPlaceUpdateState{
Revision: "rev_new",
UpdateTimestamp: now,
LastContainerStatuses: map[string]appspub.InPlaceUpdateContainerStatus{"c1": {ImageID: "image-id-xyz"}},
Revision: "rev_new",
UpdateTimestamp: now,
}),
appspub.InPlaceUpdateGraceKey: `{"revision":"rev_new","containerImages":{"c1":"foo2"},"graceSeconds":3630}`,
},
Expand Down Expand Up @@ -421,9 +422,8 @@ func TestUpdate(t *testing.T) {
Labels: map[string]string{apps.ControllerRevisionHashLabelKey: "rev_new", appsv1alpha1.CloneSetInstanceID: "id-0"},
Annotations: map[string]string{
appspub.InPlaceUpdateStateKey: util.DumpJSON(appspub.InPlaceUpdateState{
Revision: "rev_new",
UpdateTimestamp: metav1.NewTime(now.Add(-time.Second * 10)),
LastContainerStatuses: map[string]appspub.InPlaceUpdateContainerStatus{"c1": {ImageID: "image-id-xyz"}},
Revision: "rev_new",
UpdateTimestamp: metav1.NewTime(now.Add(-time.Second * 10)),
}),
appspub.InPlaceUpdateGraceKey: `{"revision":"rev_new","containerImages":{"c1":"foo2"},"graceSeconds":3630}`,
},
Expand Down Expand Up @@ -456,9 +456,8 @@ func TestUpdate(t *testing.T) {
},
Annotations: map[string]string{
appspub.InPlaceUpdateStateKey: util.DumpJSON(appspub.InPlaceUpdateState{
Revision: "rev_new",
UpdateTimestamp: metav1.NewTime(now.Add(-time.Second * 10)),
LastContainerStatuses: map[string]appspub.InPlaceUpdateContainerStatus{"c1": {ImageID: "image-id-xyz"}},
Revision: "rev_new",
UpdateTimestamp: metav1.NewTime(now.Add(-time.Second * 10)),
}),
appspub.InPlaceUpdateGraceKey: `{"revision":"rev_new","containerImages":{"c1":"foo2"},"graceSeconds":3630}`,
},
Expand Down Expand Up @@ -505,9 +504,8 @@ func TestUpdate(t *testing.T) {
Labels: map[string]string{apps.ControllerRevisionHashLabelKey: "rev_new", appsv1alpha1.CloneSetInstanceID: "id-0"},
Annotations: map[string]string{
appspub.InPlaceUpdateStateKey: util.DumpJSON(appspub.InPlaceUpdateState{
Revision: "rev_new",
UpdateTimestamp: metav1.NewTime(now.Add(-time.Minute)),
LastContainerStatuses: map[string]appspub.InPlaceUpdateContainerStatus{"c1": {ImageID: "image-id-xyz"}},
Revision: "rev_new",
UpdateTimestamp: metav1.NewTime(now.Add(-time.Minute)),
}),
appspub.InPlaceUpdateGraceKey: `{"revision":"rev_new","containerImages":{"c1":"foo2"},"graceSeconds":3630}`,
},
Expand Down Expand Up @@ -540,9 +538,10 @@ func TestUpdate(t *testing.T) {
},
Annotations: map[string]string{
appspub.InPlaceUpdateStateKey: util.DumpJSON(appspub.InPlaceUpdateState{
Revision: "rev_new",
UpdateTimestamp: metav1.NewTime(now.Add(-time.Minute)),
LastContainerStatuses: map[string]appspub.InPlaceUpdateContainerStatus{"c1": {ImageID: "image-id-xyz"}},
Revision: "rev_new",
UpdateTimestamp: metav1.NewTime(now.Add(-time.Minute)),
LastContainerStatuses: map[string]appspub.InPlaceUpdateContainerStatus{"c1": {ImageID: "image-id-xyz"}},
ContainerBatchesRecord: []appspub.InPlaceUpdateContainerBatch{{Timestamp: now, Containers: []string{"c1"}}},
}),
},
ResourceVersion: "1",
Expand All @@ -569,13 +568,14 @@ func TestUpdate(t *testing.T) {
},
}

inplaceupdate.Clock = clock.NewFakeClock(now.Time)
for _, mc := range cases {
initialObjs := mc.initial()
fakeClient := fake.NewClientBuilder().WithObjects(initialObjs...).Build()
ctrl := &realControl{
fakeClient,
lifecycle.New(fakeClient),
inplaceupdate.NewForTest(fakeClient, clonesetutils.RevisionAdapterImpl, func() metav1.Time { return now }),
inplaceupdate.New(fakeClient, clonesetutils.RevisionAdapterImpl),
record.NewFakeRecorder(10),
controllerfinder.NewControllerFinder(fakeClient),
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ import (
"strconv"
"time"

appspub "github.com/openkruise/kruise/apis/apps/pub"
"github.com/openkruise/kruise/pkg/util"
utilcontainerlaunchpriority "github.com/openkruise/kruise/pkg/util/containerlaunchpriority"
v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand All @@ -43,14 +43,6 @@ import (

const (
concurrentReconciles = 4

// priority string parse starting index
priorityStartIndex = 2

// this design is to accept any integer value
MaxInt = int(^uint(0) >> 1)
MinInt = -MaxInt - 1
minAcceptablePriority = MinInt
)

func Add(mgr manager.Manager) error {
Expand All @@ -77,12 +69,12 @@ func add(mgr manager.Manager, r *ReconcileContainerLaunchPriority) error {
CreateFunc: func(e event.CreateEvent) bool {
pod := e.Object.(*v1.Pod)
_, containersReady := podutil.GetPodCondition(&pod.Status, v1.ContainersReady)
return r.validate(pod) && containersReady != nil && containersReady.Status != v1.ConditionTrue
return utilcontainerlaunchpriority.ExistsPriorities(pod) && containersReady != nil && containersReady.Status != v1.ConditionTrue
},
UpdateFunc: func(e event.UpdateEvent) bool {
pod := e.ObjectNew.(*v1.Pod)
_, containersReady := podutil.GetPodCondition(&pod.Status, v1.ContainersReady)
return r.validate(pod) && containersReady != nil && containersReady.Status != v1.ConditionTrue
return utilcontainerlaunchpriority.ExistsPriorities(pod) && containersReady != nil && containersReady.Status != v1.ConditionTrue
},
DeleteFunc: func(e event.DeleteEvent) bool {
return false
Expand Down Expand Up @@ -144,10 +136,12 @@ func (r *ReconcileContainerLaunchPriority) Reconcile(_ context.Context, request
Name: pod.Name,
UID: pod.UID,
})
klog.V(4).Infof("Creating ConfigMap %s for Pod %s/%s", barrier.Name, pod.Namespace, pod.Name)
temErr := r.Client.Create(context.TODO(), barrier)
if temErr != nil {
if temErr != nil && !errors.IsAlreadyExists(temErr) {
return reconcile.Result{}, temErr
}
return reconcile.Result{Requeue: true}, nil
}
if err != nil {
return reconcile.Result{}, err
Expand All @@ -156,8 +150,11 @@ func (r *ReconcileContainerLaunchPriority) Reconcile(_ context.Context, request
// set next starting containers
_, containersReady := podutil.GetPodCondition(&pod.Status, v1.ContainersReady)
if containersReady != nil && containersReady.Status != v1.ConditionTrue {
var patchKey = r.findNextPatchKey(pod)
key := "p_" + strconv.Itoa(patchKey)
patchKey := r.findNextPatchKey(pod)
if patchKey == nil {
return reconcile.Result{}, nil
}
key := "p_" + strconv.Itoa(*patchKey)
if err = r.patchOnKeyNotExist(barrier, key); err != nil {
return reconcile.Result{}, err
}
Expand All @@ -166,21 +163,8 @@ func (r *ReconcileContainerLaunchPriority) Reconcile(_ context.Context, request
return reconcile.Result{}, nil
}

func (r *ReconcileContainerLaunchPriority) validate(pod *v1.Pod) bool {
if len(pod.Spec.Containers) == 0 {
return false
}
// Since priorityBarrier env is created by webhook on a all or none basis, we only need to check first container's env.
for _, v := range pod.Spec.Containers[0].Env {
if v.Name == appspub.ContainerLaunchBarrierEnvName {
return true
}
}
return false
}

func (r *ReconcileContainerLaunchPriority) findNextPatchKey(pod *v1.Pod) int {
var priority = minAcceptablePriority
func (r *ReconcileContainerLaunchPriority) findNextPatchKey(pod *v1.Pod) *int {
var priority *int
var containerPendingSet = make(map[string]bool)
for _, status := range pod.Status.ContainerStatuses {
if status.Ready {
Expand All @@ -190,25 +174,18 @@ func (r *ReconcileContainerLaunchPriority) findNextPatchKey(pod *v1.Pod) int {
}
for _, c := range pod.Spec.Containers {
if _, ok := containerPendingSet[c.Name]; ok {
p := r.getLaunchPriority(c)
if p > priority {
p := utilcontainerlaunchpriority.GetContainerPriority(&c)
if p == nil {
continue
}
if priority == nil || *p > *priority {
priority = p
}
}
}
return priority
}

func (r *ReconcileContainerLaunchPriority) getLaunchPriority(c v1.Container) int {
for _, e := range c.Env {
if e.Name == appspub.ContainerLaunchBarrierEnvName {
p, _ := strconv.Atoi(e.ValueFrom.ConfigMapKeyRef.Key[priorityStartIndex:])
return p
}
}
return minAcceptablePriority
}

func (r *ReconcileContainerLaunchPriority) patchOnKeyNotExist(barrier *v1.ConfigMap, key string) error {
if _, ok := barrier.Data[key]; !ok {
body := fmt.Sprintf(
Expand Down
Loading