Skip to content

Break PKS Loop #277

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

Merged
merged 6 commits into from
Oct 31, 2018
Merged
Changes from 1 commit
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
25 changes: 25 additions & 0 deletions pkg/apis/deployment/v1alpha/conditions.go
Original file line number Diff line number Diff line change
@@ -76,6 +76,31 @@ type Condition struct {
// Each type is allowed only once.
type ConditionList []Condition

// Equal checks for equality
func (cl ConditionList) Equal(other ConditionList) bool {
if len(cl) != len(other) {
return false
}

for i := 0; i < len(cl); i++ {
Copy link
Member

Choose a reason for hiding this comment

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

Might have to ignore the order.

if !cl[i].Equal(other[i]) {
return false
}
}

return true
}

// Equal checks for equality
func (c Condition) Equal(other Condition) bool {
return c.Type == other.Type &&
c.Status == other.Status &&
c.LastUpdateTime.Time.Sub(other.LastUpdateTime.Time).Seconds() < 2 &&
Copy link
Member

Choose a reason for hiding this comment

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

Use a function for time comparison. Also make sure we do not have to take the absolute value.

Copy link
Member

Choose a reason for hiding this comment

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

Add a test to make sure.

c.LastTransitionTime.Time.Sub(other.LastTransitionTime.Time).Seconds() < 2 &&
c.Reason == other.Reason &&
c.Message == other.Message
}

// IsTrue return true when a condition with given type exists and its status is `True`.
func (list ConditionList) IsTrue(conditionType ConditionType) bool {
c, found := list.Get(conditionType)
19 changes: 19 additions & 0 deletions pkg/apis/deployment/v1alpha/deployment_status.go
Original file line number Diff line number Diff line change
@@ -22,6 +22,10 @@

package v1alpha

import (
"reflect"
)

// DeploymentStatus contains the status part of a Cluster resource.
type DeploymentStatus struct {
// Phase holds the current lifetime phase of the deployment
@@ -57,3 +61,18 @@ type DeploymentStatus struct {
// detect changes in secret values.
SecretHashes *SecretHashes `json:"secret-hashes,omitempty"`
}

// Equal checks for equality
func (ds *DeploymentStatus) Equal(other DeploymentStatus) bool {
return ds.Phase == other.Phase &&
ds.Reason == other.Reason &&
ds.ServiceName == other.ServiceName &&
ds.SyncServiceName == other.SyncServiceName &&
reflect.DeepEqual(ds.Images, other.Images) &&
Copy link
Member

Choose a reason for hiding this comment

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

Get rid of DeepEqual altogether round here.

reflect.DeepEqual(ds.CurrentImage, other.CurrentImage) &&
ds.Members.Equal(other.Members) &&
ds.Conditions.Equal(other.Conditions) &&
reflect.DeepEqual(ds.Plan, other.Plan) &&
reflect.DeepEqual(ds.AcceptedSpec, other.AcceptedSpec) &&
reflect.DeepEqual(ds.SecretHashes, other.SecretHashes)
}
10 changes: 10 additions & 0 deletions pkg/apis/deployment/v1alpha/deployment_status_members.go
Original file line number Diff line number Diff line change
@@ -36,6 +36,16 @@ type DeploymentStatusMembers struct {
SyncWorkers MemberStatusList `json:"syncworkers,omitempty"`
}

// Equal checks for equality
func (ds DeploymentStatusMembers) Equal(other DeploymentStatusMembers) bool {
return ds.Single.Equal(other.Single) &&
ds.Agents.Equal(other.Agents) &&
ds.DBServers.Equal(other.DBServers) &&
ds.Coordinators.Equal(other.Coordinators) &&
ds.SyncMasters.Equal(other.SyncMasters) &&
ds.SyncWorkers.Equal(other.SyncWorkers)
}

// ContainsID returns true if the given set of members contains a member with given ID.
func (ds DeploymentStatusMembers) ContainsID(id string) bool {
return ds.Single.ContainsID(id) ||
12 changes: 12 additions & 0 deletions pkg/apis/deployment/v1alpha/member_status.go
Original file line number Diff line number Diff line change
@@ -54,6 +54,18 @@ type MemberStatus struct {
CleanoutJobID string `json:"cleanout-job-id,omitempty"`
}

// Equal checks for equality
func (s MemberStatus) Equal(other MemberStatus) bool {
return s.ID == other.ID &&
s.Phase == other.Phase &&
s.CreatedAt.Time.Sub(other.CreatedAt.Time).Seconds() < 2 &&
s.PersistentVolumeClaimName == other.PersistentVolumeClaimName &&
s.PodName == other.PodName &&
s.Conditions.Equal(other.Conditions) &&
s.IsInitialized == other.IsInitialized &&
s.CleanoutJobID == other.CleanoutJobID
}

// Age returns the duration since the creation timestamp of this member.
func (s MemberStatus) Age() time.Duration {
return time.Since(s.CreatedAt.Time)
15 changes: 15 additions & 0 deletions pkg/apis/deployment/v1alpha/member_status_list.go
Original file line number Diff line number Diff line change
@@ -32,6 +32,21 @@ import (
// MemberStatusList is a list of MemberStatus entries
type MemberStatusList []MemberStatus

// Equal checks for equality
func (msl MemberStatusList) Equal(other MemberStatusList) bool {
if len(msl) != len(other) {
return false
}

for i := 0; i < len(msl); i++ {
if !msl[i].Equal(other[i]) {
return false
}
}

return true
}

// ContainsID returns true if the given list contains a member with given ID.
func (l MemberStatusList) ContainsID(id string) bool {
for _, x := range l {
2 changes: 1 addition & 1 deletion pkg/deployment/context_impl.go
Original file line number Diff line number Diff line change
@@ -112,7 +112,7 @@ func (d *Deployment) UpdateStatus(status api.DeploymentStatus, lastVersion int32
}
d.status.version++
d.status.last = *status.DeepCopy()
if err := d.updateCRStatus(force...); err != nil {
if err := d.updateCRStatus(); err != nil {
return maskAny(err)
}
return nil
18 changes: 11 additions & 7 deletions pkg/deployment/deployment.go
Original file line number Diff line number Diff line change
@@ -343,13 +343,11 @@ func (d *Deployment) CreateEvent(evt *k8sutil.Event) {
}

// Update the status of the API object from the internal status
func (d *Deployment) updateCRStatus(force ...bool) error {
// TODO Remove force....
if len(force) == 0 || !force[0] {
if reflect.DeepEqual(d.apiObject.Status, d.status) {
// Nothing has changed
return nil
}
func (d *Deployment) updateCRStatus() error {

if d.apiObject.Status.Equal(d.status.last) {
// Nothing has changed
return nil
}

// Send update to API server
@@ -390,6 +388,12 @@ func (d *Deployment) updateCRStatus(force ...bool) error {
// to the given object, while preserving the status.
// On success, d.apiObject is updated.
func (d *Deployment) updateCRSpec(newSpec api.DeploymentSpec) error {

if reflect.DeepEqual(d.apiObject.Spec, newSpec) {
// Nothing to update
return nil
}

// Send update to API server
update := d.apiObject.DeepCopy()
attempt := 0