Skip to content

Commit 22a1b47

Browse files
author
lamai93
committed
Check more carefully if an update is required to prevent update loops on PKS <3
1 parent afd5dea commit 22a1b47

File tree

7 files changed

+93
-8
lines changed

7 files changed

+93
-8
lines changed

pkg/apis/deployment/v1alpha/conditions.go

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,31 @@ type Condition struct {
7676
// Each type is allowed only once.
7777
type ConditionList []Condition
7878

79+
// Equal checks for equality
80+
func (cl ConditionList) Equal(other ConditionList) bool {
81+
if len(cl) != len(other) {
82+
return false
83+
}
84+
85+
for i := 0; i < len(cl); i++ {
86+
if !cl[i].Equal(other[i]) {
87+
return false
88+
}
89+
}
90+
91+
return true
92+
}
93+
94+
// Equal checks for equality
95+
func (c Condition) Equal(other Condition) bool {
96+
return c.Type == other.Type &&
97+
c.Status == other.Status &&
98+
c.LastUpdateTime.Time.Sub(other.LastUpdateTime.Time).Seconds() < 2 &&
99+
c.LastTransitionTime.Time.Sub(other.LastTransitionTime.Time).Seconds() < 2 &&
100+
c.Reason == other.Reason &&
101+
c.Message == other.Message
102+
}
103+
79104
// IsTrue return true when a condition with given type exists and its status is `True`.
80105
func (list ConditionList) IsTrue(conditionType ConditionType) bool {
81106
c, found := list.Get(conditionType)

pkg/apis/deployment/v1alpha/deployment_status.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,10 @@
2222

2323
package v1alpha
2424

25+
import (
26+
"reflect"
27+
)
28+
2529
// DeploymentStatus contains the status part of a Cluster resource.
2630
type DeploymentStatus struct {
2731
// Phase holds the current lifetime phase of the deployment
@@ -57,3 +61,18 @@ type DeploymentStatus struct {
5761
// detect changes in secret values.
5862
SecretHashes *SecretHashes `json:"secret-hashes,omitempty"`
5963
}
64+
65+
// Equal checks for equality
66+
func (ds *DeploymentStatus) Equal(other DeploymentStatus) bool {
67+
return ds.Phase == other.Phase &&
68+
ds.Reason == other.Reason &&
69+
ds.ServiceName == other.ServiceName &&
70+
ds.SyncServiceName == other.SyncServiceName &&
71+
reflect.DeepEqual(ds.Images, other.Images) &&
72+
reflect.DeepEqual(ds.CurrentImage, other.CurrentImage) &&
73+
ds.Members.Equal(other.Members) &&
74+
ds.Conditions.Equal(other.Conditions) &&
75+
reflect.DeepEqual(ds.Plan, other.Plan) &&
76+
reflect.DeepEqual(ds.AcceptedSpec, other.AcceptedSpec) &&
77+
reflect.DeepEqual(ds.SecretHashes, other.SecretHashes)
78+
}

pkg/apis/deployment/v1alpha/deployment_status_members.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,16 @@ type DeploymentStatusMembers struct {
3636
SyncWorkers MemberStatusList `json:"syncworkers,omitempty"`
3737
}
3838

39+
// Equal checks for equality
40+
func (ds DeploymentStatusMembers) Equal(other DeploymentStatusMembers) bool {
41+
return ds.Single.Equal(other.Single) &&
42+
ds.Agents.Equal(other.Agents) &&
43+
ds.DBServers.Equal(other.DBServers) &&
44+
ds.Coordinators.Equal(other.Coordinators) &&
45+
ds.SyncMasters.Equal(other.SyncMasters) &&
46+
ds.SyncWorkers.Equal(other.SyncWorkers)
47+
}
48+
3949
// ContainsID returns true if the given set of members contains a member with given ID.
4050
func (ds DeploymentStatusMembers) ContainsID(id string) bool {
4151
return ds.Single.ContainsID(id) ||

pkg/apis/deployment/v1alpha/member_status.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,18 @@ type MemberStatus struct {
5454
CleanoutJobID string `json:"cleanout-job-id,omitempty"`
5555
}
5656

57+
// Equal checks for equality
58+
func (s MemberStatus) Equal(other MemberStatus) bool {
59+
return s.ID == other.ID &&
60+
s.Phase == other.Phase &&
61+
s.CreatedAt.Time.Sub(other.CreatedAt.Time).Seconds() < 2 &&
62+
s.PersistentVolumeClaimName == other.PersistentVolumeClaimName &&
63+
s.PodName == other.PodName &&
64+
s.Conditions.Equal(other.Conditions) &&
65+
s.IsInitialized == other.IsInitialized &&
66+
s.CleanoutJobID == other.CleanoutJobID
67+
}
68+
5769
// Age returns the duration since the creation timestamp of this member.
5870
func (s MemberStatus) Age() time.Duration {
5971
return time.Since(s.CreatedAt.Time)

pkg/apis/deployment/v1alpha/member_status_list.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,21 @@ import (
3232
// MemberStatusList is a list of MemberStatus entries
3333
type MemberStatusList []MemberStatus
3434

35+
// Equal checks for equality
36+
func (msl MemberStatusList) Equal(other MemberStatusList) bool {
37+
if len(msl) != len(other) {
38+
return false
39+
}
40+
41+
for i := 0; i < len(msl); i++ {
42+
if !msl[i].Equal(other[i]) {
43+
return false
44+
}
45+
}
46+
47+
return true
48+
}
49+
3550
// ContainsID returns true if the given list contains a member with given ID.
3651
func (l MemberStatusList) ContainsID(id string) bool {
3752
for _, x := range l {

pkg/deployment/context_impl.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ func (d *Deployment) UpdateStatus(status api.DeploymentStatus, lastVersion int32
112112
}
113113
d.status.version++
114114
d.status.last = *status.DeepCopy()
115-
if err := d.updateCRStatus(force...); err != nil {
115+
if err := d.updateCRStatus(); err != nil {
116116
return maskAny(err)
117117
}
118118
return nil

pkg/deployment/deployment.go

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -343,13 +343,11 @@ func (d *Deployment) CreateEvent(evt *k8sutil.Event) {
343343
}
344344

345345
// Update the status of the API object from the internal status
346-
func (d *Deployment) updateCRStatus(force ...bool) error {
347-
// TODO Remove force....
348-
if len(force) == 0 || !force[0] {
349-
if reflect.DeepEqual(d.apiObject.Status, d.status) {
350-
// Nothing has changed
351-
return nil
352-
}
346+
func (d *Deployment) updateCRStatus() error {
347+
348+
if d.apiObject.Status.Equal(d.status.last) {
349+
// Nothing has changed
350+
return nil
353351
}
354352

355353
// Send update to API server
@@ -390,6 +388,12 @@ func (d *Deployment) updateCRStatus(force ...bool) error {
390388
// to the given object, while preserving the status.
391389
// On success, d.apiObject is updated.
392390
func (d *Deployment) updateCRSpec(newSpec api.DeploymentSpec) error {
391+
392+
if reflect.DeepEqual(d.apiObject.Spec, newSpec) {
393+
// Nothing to update
394+
return nil
395+
}
396+
393397
// Send update to API server
394398
update := d.apiObject.DeepCopy()
395399
attempt := 0

0 commit comments

Comments
 (0)