Skip to content

Commit

Permalink
Add ForceUpdateStrategy
Browse files Browse the repository at this point in the history
  • Loading branch information
@hex108
hex108 committed Nov 2, 2020
1 parent 3f8eff8 commit e0d8f77
Show file tree
Hide file tree
Showing 4 changed files with 135 additions and 14 deletions.
16 changes: 14 additions & 2 deletions pkg/apis/tappcontroller/v1/types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ const (

// DefaultMaxUnavailable is the default value for .Spec.UpdateStrategy.MaxUnavailable
DefaultMaxUnavailable = 1

// DefaultMaxUnavailable is the default value for .Spec.UpdateStrategy.ForceUpdateStrategy.MaxUnavailable
DefaultMaxUnavailableForceUpdate = "100%"
)

// +genclient
Expand Down Expand Up @@ -75,7 +78,7 @@ type TAppSpec struct {
// Templates stores instanceID --> template name
Templates map[string]string `json:"templates,omitempty"`

// UpdateStrategy indicates the StatefulSetUpdateStrategy that will be
// UpdateStrategy indicates the TappUpdateStrategy that will be
// employed to update Pods in the TApp
UpdateStrategy TAppUpdateStrategy `json:"updateStrategy,omitempty"`

Expand Down Expand Up @@ -111,12 +114,21 @@ type TAppSpec struct {
DefaultTemplateName string `json:"defaultTemplateName"`
}

// Only support rolling update now
// TApp update strategy
type TAppUpdateStrategy struct {
// Following fields are rolling update related configuration.
// Template is the rolling update template name
Template string `json:"template,omitempty"`
// MaxUnavailable is the max unavailable number when tapp is rolling update, default is 1.
MaxUnavailable *intstr.IntOrString `json:"maxUnavailable,omitempty"`

// Following fields are force update related configuration.
ForceUpdate ForceUpdateStrategy `json:"forceUpdate,omitempty"`
}

type ForceUpdateStrategy struct {
// MaxUnavailable is the max unavailable number when tapp is forced update, default is 100%.
MaxUnavailable *intstr.IntOrString `json:"maxUnavailable,omitempty"`
}

type InstanceStatus string
Expand Down
45 changes: 33 additions & 12 deletions pkg/tapp/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -488,6 +488,10 @@ func (c *Controller) setDefaultValue(tapp *tappv1.TApp) {
maxUnavailable := intstr.FromInt(tappv1.DefaultMaxUnavailable)
tapp.Spec.UpdateStrategy.MaxUnavailable = &maxUnavailable
}
if tapp.Spec.UpdateStrategy.ForceUpdate.MaxUnavailable == nil {
maxUnavailable := intstr.FromString(tappv1.DefaultMaxUnavailableForceUpdate)
tapp.Spec.UpdateStrategy.ForceUpdate.MaxUnavailable = &maxUnavailable
}
}

func (c *Controller) removeUnusedTemplate(tapp *tappv1.TApp) error {
Expand Down Expand Up @@ -784,7 +788,7 @@ func (c *Controller) transformPodActions(tapp *tappv1.TApp, podActions map[strin
desiredRunningPods sets.String) (add, del, forceDel, update []*Instance) {
var rollingUpdateIds []string
availablePods := getAvailablePods(podMap, desiredRunningPods)
// Delete pods

for p, a := range podActions {
pod := podMap[p]
switch a {
Expand All @@ -799,34 +803,55 @@ func (c *Controller) transformPodActions(tapp *tappv1.TApp, podActions map[strin
del = append(del, ins)
}
availablePods.Delete(p)
break
case createPod:
if instance, err := newInstance(tapp, p); err == nil {
add = append(add, instance)
}
break
}
}

maxUnavailableForceUpdate := intstr.FromString(tappv1.DefaultMaxUnavailableForceUpdate)
if tapp.Spec.UpdateStrategy.ForceUpdate.MaxUnavailable != nil {
maxUnavailableForceUpdate = *tapp.Spec.UpdateStrategy.ForceUpdate.MaxUnavailable
}
v, err := intstr.GetValueFromIntOrPercent(&maxUnavailableForceUpdate, desiredRunningPods.Len(), true)
if err != nil {
klog.Errorf("invalid value for MaxUnavailable: %v", err)
return
}
minAvailablePods := desiredRunningPods.Len() - v

for p, a := range podActions {
pod := podMap[p]
switch a {
case updatePod:
if !isInRollingUpdate(tapp, p) {
if len(availablePods) <= minAvailablePods {
klog.V(3).Infof("Skip %v pod %v because available pods are not enough: %v(current) vs %v(min)",
a, getPodFullName(pod), len(availablePods), minAvailablePods)
break
}
if instance, err := newInstance(tapp, p); err == nil {
update = append(update, instance)
availablePods.Delete(p)
}
} else {
rollingUpdateIds = append(rollingUpdateIds, p)
}
break
case recreatePod:
if !isInRollingUpdate(tapp, p) {
if len(availablePods) <= minAvailablePods {
klog.V(3).Infof("Skip %v pod %v because available pods are not enough: %v(current) vs %v(min)",
a, getPodFullName(pod), len(availablePods), minAvailablePods)
break
}
if instance, err := newInstanceWithPod(tapp, pod); err == nil {
del = append(del, instance)
availablePods.Delete(p)
}
} else {
rollingUpdateIds = append(rollingUpdateIds, p)
}
break
default:
klog.Errorf("Unknown pod action %v for pod %v", a, getPodFullName(pod))
}
}

Expand All @@ -839,7 +864,7 @@ func (c *Controller) transformPodActions(tapp *tappv1.TApp, podActions map[strin
return
}
}
minAvailablePods := desiredRunningPods.Len() - maxUnavailable
minAvailablePods = desiredRunningPods.Len() - maxUnavailable

// First sort ids.
sort.Slice(rollingUpdateIds, func(i, j int) bool {
Expand All @@ -862,15 +887,11 @@ func (c *Controller) transformPodActions(tapp *tappv1.TApp, podActions map[strin
update = append(update, instance)
availablePods.Delete(p)
}
break
case recreatePod:
if instance, err := newInstanceWithPod(tapp, pod); err == nil {
del = append(del, instance)
availablePods.Delete(p)
}
break
default:
klog.Errorf("Unknown pod action %v for pod %v", action, getPodFullName(pod))
}
}

Expand Down
27 changes: 27 additions & 0 deletions pkg/tapp/fakes.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package tapp

import (
"fmt"
"sync"
"time"

corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -48,10 +49,13 @@ type fakeInstanceClient struct {
InstancesCreated, InstancesDeleted, InstanceForceDeleted, InstancesUpdated int
recorder record.EventRecorder
InstanceHealthChecker
sync.Mutex
}

// Delete fakes Instance client deletion.
func (f *fakeInstanceClient) Delete(p *Instance, options *metav1.DeleteOptions) error {
f.Lock()
defer f.Unlock()
if _, ok := f.Instances[p.id]; ok {
delete(f.Instances, p.id)
f.recorder.Eventf(p.parent, corev1.EventTypeNormal, "SuccessfulDelete", "Instance: %v", p.pod.Name)
Expand All @@ -68,6 +72,8 @@ func (f *fakeInstanceClient) Delete(p *Instance, options *metav1.DeleteOptions)

// Get fakes getting Instances.
func (f *fakeInstanceClient) Get(p *Instance) (*Instance, bool, error) {
f.Lock()
defer f.Unlock()
if instance, ok := f.Instances[p.id]; ok {
return instance, true, nil
}
Expand All @@ -76,17 +82,28 @@ func (f *fakeInstanceClient) Get(p *Instance) (*Instance, bool, error) {

// Create fakes Instance creation.
func (f *fakeInstanceClient) Create(p *Instance) error {
f.Lock()
defer f.Unlock()
if _, ok := f.Instances[p.id]; ok {
return fmt.Errorf("failedl to create: instance %v already exists", p.id)
}
f.recorder.Eventf(p.parent, corev1.EventTypeNormal, "SuccessfulCreate", "Instance: %v", p.pod.Name)
pod := p.pod
pod.Status.Phase = corev1.PodRunning
pod.Status.Conditions = make([]corev1.PodCondition, 1)
pod.Status.Conditions[0] = corev1.PodCondition{
Type: corev1.PodReady,
Status: corev1.ConditionTrue,
}
f.Instances[p.id] = p
f.InstancesCreated++
return nil
}

// Update fakes Instance updates.
func (f *fakeInstanceClient) Update(expected, wanted *Instance) error {
f.Lock()
defer f.Unlock()
if _, ok := f.Instances[wanted.id]; !ok {
return fmt.Errorf("failed to update: Instance %v not found", wanted.id)
}
Expand All @@ -96,6 +113,8 @@ func (f *fakeInstanceClient) Update(expected, wanted *Instance) error {
}

func (f *fakeInstanceClient) getPodList() []*corev1.Pod {
f.Lock()
defer f.Unlock()
p := []*corev1.Pod{}
for i, Instance := range f.Instances {
if Instance.pod == nil {
Expand All @@ -108,6 +127,8 @@ func (f *fakeInstanceClient) getPodList() []*corev1.Pod {

// Delete fakes Instance client deletion.
func (f *fakeInstanceClient) DeleteInstance(id string) error {
f.Lock()
defer f.Unlock()
if _, ok := f.Instances[id]; ok {
delete(f.Instances, id)
} else {
Expand All @@ -117,6 +138,8 @@ func (f *fakeInstanceClient) DeleteInstance(id string) error {
}

func (f *fakeInstanceClient) setDeletionTimestamp(id string, t time.Time) error {
f.Lock()
defer f.Unlock()
if _, ok := f.Instances[id]; !ok {
return fmt.Errorf("instance %v not found", id)
}
Expand All @@ -125,6 +148,8 @@ func (f *fakeInstanceClient) setDeletionTimestamp(id string, t time.Time) error
}

func (f *fakeInstanceClient) setPodStatus(id string, status corev1.PodPhase) error {
f.Lock()
defer f.Unlock()
if _, ok := f.Instances[id]; !ok {
return fmt.Errorf("instance %v not found", id)
}
Expand All @@ -133,6 +158,8 @@ func (f *fakeInstanceClient) setPodStatus(id string, status corev1.PodPhase) err
}

func (f *fakeInstanceClient) setPodReason(id string, reason string) error {
f.Lock()
defer f.Unlock()
if _, ok := f.Instances[id]; !ok {
return fmt.Errorf("instance %v not found", id)
}
Expand Down
61 changes: 61 additions & 0 deletions pkg/tapp/tapp_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,67 @@ func TestUpdateInstance(t *testing.T) {
checkInstances("TestUpdateInstance3_2", tapp, oldCreate+1, oldDelete+1, 0, oldUpdate, client, t)
}

func TestForceUpdateStrategy(t *testing.T) {
tests := []struct {
name string
forceUpdate intstr.IntOrString
expectedForceUpdate int
}{
{
name: "TestForceUpdateStrategy1",
forceUpdate: intstr.FromInt(1),
expectedForceUpdate: 1,
},
{
name: "TestForceUpdateStrategy1",
forceUpdate: intstr.FromInt(5),
expectedForceUpdate: 5,
},
{
name: "TestForceUpdateStrategy1",
forceUpdate: intstr.FromInt(20),
expectedForceUpdate: 10, // because the number of forced pods is 10
},
{
name: "TestForceUpdateStrategy1",
forceUpdate: intstr.FromString("10%"),
expectedForceUpdate: 1,
},
{
name: "TestForceUpdateStrategy1",
forceUpdate: intstr.FromString("50%"),
expectedForceUpdate: 5,
},
{
name: "TestForceUpdateStrategy1",
forceUpdate: intstr.FromString("100%"),
expectedForceUpdate: 10, // because the number of forced pods is 10
},
}

for _, test := range tests {
controller, client := newFakeTAppController()
tapp := testutil.CreateValidTApp(10)
syncTApp(t, tapp, controller, client)
template := testutil.CreateValidPodTemplate()
image := template.Spec.Containers[0].Image
template.Spec.Containers[0].Image = image + "update"
template0 := "template0"
if err := testutil.AddPodTemplate(tapp, template0, template); err != nil {
t.Errorf("add pod template failed: %v", err)
}
for i := 0; i < 10; i++ {
testutil.UpdateInstanceTemplate(tapp, strconv.Itoa(i), template0)
}
tapp.Spec.UpdateStrategy.ForceUpdate.MaxUnavailable = &test.forceUpdate
oldCreate := client.InstancesCreated
oldDelete := client.InstancesDeleted
oldUpdate := client.InstancesUpdated
syncTApp(t, tapp, controller, client)
checkInstances(test.name, tapp, oldCreate, oldDelete, 0, oldUpdate+test.expectedForceUpdate, client, t)
}
}

func TestInstanceFailed(t *testing.T) {
onFail := func() *corev1.PodTemplateSpec {
template := testutil.CreateValidPodTemplate()
Expand Down

0 comments on commit e0d8f77

Please # to comment.