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
Show file tree
Hide file tree
Changes from 3 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
25 changes: 25 additions & 0 deletions pkg/apis/deployment/v1alpha/conditions.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
19 changes: 19 additions & 0 deletions pkg/apis/deployment/v1alpha/deployment_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Up @@ -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) ||
Expand Down
12 changes: 12 additions & 0 deletions pkg/apis/deployment/v1alpha/member_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
15 changes: 15 additions & 0 deletions pkg/apis/deployment/v1alpha/member_status_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
9 changes: 7 additions & 2 deletions pkg/deployment/deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -344,9 +344,8 @@ 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) {
if d.apiObject.Status.Equal(d.status.last) {
// Nothing has changed
return nil
}
Expand Down Expand Up @@ -390,6 +389,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
Expand Down