From 22a1b474124e0b8a3f708b90ae980e031ed7c103 Mon Sep 17 00:00:00 2001 From: lamai93 Date: Fri, 26 Oct 2018 10:04:20 +0200 Subject: [PATCH 1/5] Check more carefully if an update is required to prevent update loops on PKS <3 --- pkg/apis/deployment/v1alpha/conditions.go | 25 +++++++++++++++++++ .../deployment/v1alpha/deployment_status.go | 19 ++++++++++++++ .../v1alpha/deployment_status_members.go | 10 ++++++++ pkg/apis/deployment/v1alpha/member_status.go | 12 +++++++++ .../deployment/v1alpha/member_status_list.go | 15 +++++++++++ pkg/deployment/context_impl.go | 2 +- pkg/deployment/deployment.go | 18 +++++++------ 7 files changed, 93 insertions(+), 8 deletions(-) diff --git a/pkg/apis/deployment/v1alpha/conditions.go b/pkg/apis/deployment/v1alpha/conditions.go index 4e8da01be..c8f1b710f 100644 --- a/pkg/apis/deployment/v1alpha/conditions.go +++ b/pkg/apis/deployment/v1alpha/conditions.go @@ -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++ { + 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 && + 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) diff --git a/pkg/apis/deployment/v1alpha/deployment_status.go b/pkg/apis/deployment/v1alpha/deployment_status.go index 280f3a661..f725cceed 100644 --- a/pkg/apis/deployment/v1alpha/deployment_status.go +++ b/pkg/apis/deployment/v1alpha/deployment_status.go @@ -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) && + 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) +} diff --git a/pkg/apis/deployment/v1alpha/deployment_status_members.go b/pkg/apis/deployment/v1alpha/deployment_status_members.go index 7344b7425..05fa9c0b5 100644 --- a/pkg/apis/deployment/v1alpha/deployment_status_members.go +++ b/pkg/apis/deployment/v1alpha/deployment_status_members.go @@ -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) || diff --git a/pkg/apis/deployment/v1alpha/member_status.go b/pkg/apis/deployment/v1alpha/member_status.go index a1c18e56a..5d6aee84a 100644 --- a/pkg/apis/deployment/v1alpha/member_status.go +++ b/pkg/apis/deployment/v1alpha/member_status.go @@ -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) diff --git a/pkg/apis/deployment/v1alpha/member_status_list.go b/pkg/apis/deployment/v1alpha/member_status_list.go index e47a9bb87..20fac390c 100644 --- a/pkg/apis/deployment/v1alpha/member_status_list.go +++ b/pkg/apis/deployment/v1alpha/member_status_list.go @@ -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 { diff --git a/pkg/deployment/context_impl.go b/pkg/deployment/context_impl.go index 173869d40..d0a4a06b6 100644 --- a/pkg/deployment/context_impl.go +++ b/pkg/deployment/context_impl.go @@ -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 diff --git a/pkg/deployment/deployment.go b/pkg/deployment/deployment.go index 21e1e8c15..f2f60927d 100644 --- a/pkg/deployment/deployment.go +++ b/pkg/deployment/deployment.go @@ -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 From 5987fa079471d95f4ca7e57529bb769ec541fbbe Mon Sep 17 00:00:00 2001 From: lamai93 Date: Fri, 26 Oct 2018 16:12:10 +0200 Subject: [PATCH 2/5] Reverted deletion of force parameter - still in use. --- pkg/deployment/context_impl.go | 2 +- pkg/deployment/deployment.go | 11 ++++++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/pkg/deployment/context_impl.go b/pkg/deployment/context_impl.go index d0a4a06b6..7ff0ea386 100644 --- a/pkg/deployment/context_impl.go +++ b/pkg/deployment/context_impl.go @@ -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(); err != nil { + if err := d.updateCRStatus(force); err != nil { return maskAny(err) } return nil diff --git a/pkg/deployment/deployment.go b/pkg/deployment/deployment.go index f2f60927d..8485e534f 100644 --- a/pkg/deployment/deployment.go +++ b/pkg/deployment/deployment.go @@ -343,11 +343,12 @@ func (d *Deployment) CreateEvent(evt *k8sutil.Event) { } // Update the status of the API object from the internal status -func (d *Deployment) updateCRStatus() error { - - if d.apiObject.Status.Equal(d.status.last) { - // Nothing has changed - return nil +func (d *Deployment) updateCRStatus(force ...bool) error { + if len(force) == 0 || !force[0] { + if d.apiObject.Status.Equal(d.status.last) { + // Nothing has changed + return nil + } } // Send update to API server From 9428447462c2d248ae18a8dd5579cb165a7de70f Mon Sep 17 00:00:00 2001 From: lamai93 Date: Fri, 26 Oct 2018 16:32:32 +0200 Subject: [PATCH 3/5] Make it compile. --- pkg/deployment/context_impl.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/deployment/context_impl.go b/pkg/deployment/context_impl.go index 7ff0ea386..173869d40 100644 --- a/pkg/deployment/context_impl.go +++ b/pkg/deployment/context_impl.go @@ -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(force...); err != nil { return maskAny(err) } return nil From f4db022647e1af2436258c0814430d490385a107 Mon Sep 17 00:00:00 2001 From: lamai93 Date: Wed, 31 Oct 2018 14:25:55 +0100 Subject: [PATCH 4/5] More manual tests for Equal. --- pkg/apis/deployment/v1alpha/conditions.go | 18 +++++--- .../deployment/v1alpha/deployment_spec.go | 7 +++ .../deployment/v1alpha/deployment_status.go | 14 +++--- pkg/apis/deployment/v1alpha/image_info.go | 35 +++++++++++++++ pkg/apis/deployment/v1alpha/member_status.go | 3 +- .../deployment/v1alpha/member_status_list.go | 13 ++++-- pkg/apis/deployment/v1alpha/plan.go | 29 ++++++++++++ pkg/apis/deployment/v1alpha/secret_hashes.go | 14 ++++++ pkg/deployment/deployment.go | 3 +- pkg/util/times.go | 45 +++++++++++++++++++ 10 files changed, 159 insertions(+), 22 deletions(-) create mode 100644 pkg/util/times.go diff --git a/pkg/apis/deployment/v1alpha/conditions.go b/pkg/apis/deployment/v1alpha/conditions.go index c8f1b710f..c15698ad6 100644 --- a/pkg/apis/deployment/v1alpha/conditions.go +++ b/pkg/apis/deployment/v1alpha/conditions.go @@ -23,6 +23,7 @@ package v1alpha import ( + "github.com/arangodb/kube-arangodb/pkg/util" "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -77,13 +78,18 @@ type Condition struct { type ConditionList []Condition // Equal checks for equality -func (cl ConditionList) Equal(other ConditionList) bool { - if len(cl) != len(other) { +func (list ConditionList) Equal(other ConditionList) bool { + if len(list) != len(other) { return false } - for i := 0; i < len(cl); i++ { - if !cl[i].Equal(other[i]) { + for i := 0; i < len(list); i++ { + c, found := other.Get(list[i].Type) + if !found { + return false + } + + if !list[i].Equal(c) { return false } } @@ -95,8 +101,8 @@ func (cl ConditionList) Equal(other ConditionList) bool { 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 && - c.LastTransitionTime.Time.Sub(other.LastTransitionTime.Time).Seconds() < 2 && + util.TimeCompareEqual(c.LastUpdateTime, other.LastUpdateTime) && + util.TimeCompareEqual(c.LastTransitionTime, other.LastTransitionTime) && c.Reason == other.Reason && c.Message == other.Message } diff --git a/pkg/apis/deployment/v1alpha/deployment_spec.go b/pkg/apis/deployment/v1alpha/deployment_spec.go index 5c51a4728..dee1f9e4d 100644 --- a/pkg/apis/deployment/v1alpha/deployment_spec.go +++ b/pkg/apis/deployment/v1alpha/deployment_spec.go @@ -23,6 +23,8 @@ package v1alpha import ( + "reflect" + "github.com/arangodb/kube-arangodb/pkg/util" "github.com/pkg/errors" "k8s.io/api/core/v1" @@ -69,6 +71,11 @@ type DeploymentSpec struct { Chaos ChaosSpec `json:"chaos"` } +// Equal compares two DeploymentSpec +func (s *DeploymentSpec) Equal(other *DeploymentSpec) bool { + return reflect.DeepEqual(s, other) +} + // GetMode returns the value of mode. func (s DeploymentSpec) GetMode() DeploymentMode { return ModeOrDefault(s.Mode) diff --git a/pkg/apis/deployment/v1alpha/deployment_status.go b/pkg/apis/deployment/v1alpha/deployment_status.go index f725cceed..ad175eab4 100644 --- a/pkg/apis/deployment/v1alpha/deployment_status.go +++ b/pkg/apis/deployment/v1alpha/deployment_status.go @@ -22,10 +22,6 @@ 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 @@ -68,11 +64,11 @@ func (ds *DeploymentStatus) Equal(other DeploymentStatus) bool { ds.Reason == other.Reason && ds.ServiceName == other.ServiceName && ds.SyncServiceName == other.SyncServiceName && - reflect.DeepEqual(ds.Images, other.Images) && - reflect.DeepEqual(ds.CurrentImage, other.CurrentImage) && + ds.Images.Equal(other.Images) && + ds.CurrentImage.Equal(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) + ds.Plan.Equal(other.Plan) && + ds.AcceptedSpec.Equal(other.AcceptedSpec) && + ds.SecretHashes.Equal(other.SecretHashes) } diff --git a/pkg/apis/deployment/v1alpha/image_info.go b/pkg/apis/deployment/v1alpha/image_info.go index 34c7f8a88..37a4addcb 100644 --- a/pkg/apis/deployment/v1alpha/image_info.go +++ b/pkg/apis/deployment/v1alpha/image_info.go @@ -71,3 +71,38 @@ func (l *ImageInfoList) AddOrUpdate(info ImageInfo) { // No existing entry found, add it *l = append(*l, info) } + +// Equal compares to ImageInfo +func (i *ImageInfo) Equal(other *ImageInfo) bool { + if i == other { + return true + } else if i == nil { + return false + } + + return i.ArangoDBVersion == other.ArangoDBVersion && + i.Enterprise == other.Enterprise && + i.Image == other.Image && + i.ImageID == other.ImageID +} + +// Equal compares to ImageInfoList +func (l ImageInfoList) Equal(other ImageInfoList) bool { + if len(l) != len(other) { + return false + } + + for i := 0; i < len(l); i++ { + ii, found := l.GetByImageID(l[i].ImageID) + + if !found { + return false + } + + if !l[i].Equal(&ii) { + return false + } + } + + return true +} diff --git a/pkg/apis/deployment/v1alpha/member_status.go b/pkg/apis/deployment/v1alpha/member_status.go index 5d6aee84a..5e16d9ff3 100644 --- a/pkg/apis/deployment/v1alpha/member_status.go +++ b/pkg/apis/deployment/v1alpha/member_status.go @@ -25,6 +25,7 @@ package v1alpha import ( "time" + "github.com/arangodb/kube-arangodb/pkg/util" "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -58,7 +59,7 @@ type MemberStatus struct { 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 && + util.TimeCompareEqual(s.CreatedAt, other.CreatedAt) && s.PersistentVolumeClaimName == other.PersistentVolumeClaimName && s.PodName == other.PodName && s.Conditions.Equal(other.Conditions) && diff --git a/pkg/apis/deployment/v1alpha/member_status_list.go b/pkg/apis/deployment/v1alpha/member_status_list.go index 20fac390c..670a76ff1 100644 --- a/pkg/apis/deployment/v1alpha/member_status_list.go +++ b/pkg/apis/deployment/v1alpha/member_status_list.go @@ -33,13 +33,18 @@ import ( type MemberStatusList []MemberStatus // Equal checks for equality -func (msl MemberStatusList) Equal(other MemberStatusList) bool { - if len(msl) != len(other) { +func (l MemberStatusList) Equal(other MemberStatusList) bool { + if len(l) != len(other) { return false } - for i := 0; i < len(msl); i++ { - if !msl[i].Equal(other[i]) { + for i := 0; i < len(l); i++ { + o, found := l.ElementByID(l[i].ID) + if !found { + return false + } + + if !l[i].Equal(o) { return false } } diff --git a/pkg/apis/deployment/v1alpha/plan.go b/pkg/apis/deployment/v1alpha/plan.go index 59a31afc4..201b74e4e 100644 --- a/pkg/apis/deployment/v1alpha/plan.go +++ b/pkg/apis/deployment/v1alpha/plan.go @@ -23,6 +23,7 @@ package v1alpha import ( + "github.com/arangodb/kube-arangodb/pkg/util" "github.com/dchest/uniuri" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -79,6 +80,18 @@ type Action struct { Image string `json:"image,omitempty"` } +// Equal compares two Actions +func (a Action) Equal(other Action) bool { + return a.ID == other.ID && + a.Type == other.Type && + a.MemberID == other.MemberID && + a.Group == other.Group && + util.TimeCompareEqual(a.CreationTime, other.CreationTime) && + util.TimeCompareEqualPointer(a.StartTime, other.StartTime) && + a.Reason == other.Reason && + a.Image == other.Image +} + // NewAction instantiates a new Action. func NewAction(actionType ActionType, group ServerGroup, memberID string, reason ...string) Action { a := Action{ @@ -105,3 +118,19 @@ func (a Action) SetImage(image string) Action { // Only 1 action is in progress at a time. The operator will wait for that // action to be completely and then remove the action. type Plan []Action + +// Equal compares two Plan +func (p Plan) Equal(other Plan) bool { + // For plan the order is relevant! + if len(p) != len(other) { + return false + } + + for i := 0; i < len(p); i++ { + if !p[i].Equal(other[i]) { + return false + } + } + + return true +} diff --git a/pkg/apis/deployment/v1alpha/secret_hashes.go b/pkg/apis/deployment/v1alpha/secret_hashes.go index 5860f363d..772ae972a 100644 --- a/pkg/apis/deployment/v1alpha/secret_hashes.go +++ b/pkg/apis/deployment/v1alpha/secret_hashes.go @@ -35,3 +35,17 @@ type SecretHashes struct { // SyncTLSCA contains the hash of the sync.tls.caSecretName secret SyncTLSCA string `json:"sync-tls-ca,omitempty"` } + +// Equal compares two SecretHashes +func (sh *SecretHashes) Equal(other *SecretHashes) bool { + if sh == other { + return true + } else if sh == nil { + return false + } + + return sh.AuthJWT == other.AuthJWT && + sh.RocksDBEncryptionKey == other.RocksDBEncryptionKey && + sh.TLSCA == other.TLSCA && + sh.SyncTLSCA == other.SyncTLSCA +} diff --git a/pkg/deployment/deployment.go b/pkg/deployment/deployment.go index 8485e534f..3a553023b 100644 --- a/pkg/deployment/deployment.go +++ b/pkg/deployment/deployment.go @@ -24,7 +24,6 @@ package deployment import ( "fmt" - "reflect" "sync" "sync/atomic" "time" @@ -390,7 +389,7 @@ func (d *Deployment) updateCRStatus(force ...bool) error { // On success, d.apiObject is updated. func (d *Deployment) updateCRSpec(newSpec api.DeploymentSpec) error { - if reflect.DeepEqual(d.apiObject.Spec, newSpec) { + if d.apiObject.Spec.Equal(&newSpec) { // Nothing to update return nil } diff --git a/pkg/util/times.go b/pkg/util/times.go new file mode 100644 index 000000000..1bdba1cb0 --- /dev/null +++ b/pkg/util/times.go @@ -0,0 +1,45 @@ +// +// DISCLAIMER +// +// Copyright 2018 ArangoDB GmbH, Cologne, Germany +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// Copyright holder is ArangoDB GmbH, Cologne, Germany +// +// Author Lars Maier +// + +package util + +import ( + "math" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// TimeCompareEqual compares two times, allowing an error of 1s +func TimeCompareEqual(a, b metav1.Time) bool { + return math.Abs(a.Time.Sub(b.Time).Seconds()) <= 1 +} + +// TimeCompareEqualPointer compares two times, allowing an error of 1s +func TimeCompareEqualPointer(a, b *metav1.Time) bool { + if a == b { + return true + } else if a == nil { + return false + } + + return TimeCompareEqual(*a, *b) +} From d5a54a2f1ba8ca8977e0a7ccce65ee6f157eb879 Mon Sep 17 00:00:00 2001 From: lamai93 Date: Wed, 31 Oct 2018 16:07:48 +0100 Subject: [PATCH 5/5] Replaced flawed null-ptr check by something better. :) --- pkg/apis/deployment/v1alpha/image_info.go | 6 +++--- pkg/apis/deployment/v1alpha/secret_hashes.go | 6 +++--- pkg/util/times.go | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/pkg/apis/deployment/v1alpha/image_info.go b/pkg/apis/deployment/v1alpha/image_info.go index 37a4addcb..2fcc4e307 100644 --- a/pkg/apis/deployment/v1alpha/image_info.go +++ b/pkg/apis/deployment/v1alpha/image_info.go @@ -74,10 +74,10 @@ func (l *ImageInfoList) AddOrUpdate(info ImageInfo) { // Equal compares to ImageInfo func (i *ImageInfo) Equal(other *ImageInfo) bool { - if i == other { - return true - } else if i == nil { + if i == nil || other == nil { return false + } else if i == other { + return true } return i.ArangoDBVersion == other.ArangoDBVersion && diff --git a/pkg/apis/deployment/v1alpha/secret_hashes.go b/pkg/apis/deployment/v1alpha/secret_hashes.go index 772ae972a..f495d4d05 100644 --- a/pkg/apis/deployment/v1alpha/secret_hashes.go +++ b/pkg/apis/deployment/v1alpha/secret_hashes.go @@ -38,10 +38,10 @@ type SecretHashes struct { // Equal compares two SecretHashes func (sh *SecretHashes) Equal(other *SecretHashes) bool { - if sh == other { - return true - } else if sh == nil { + if sh == nil || other == nil { return false + } else if sh == other { + return true } return sh.AuthJWT == other.AuthJWT && diff --git a/pkg/util/times.go b/pkg/util/times.go index 1bdba1cb0..ca074fdf2 100644 --- a/pkg/util/times.go +++ b/pkg/util/times.go @@ -35,10 +35,10 @@ func TimeCompareEqual(a, b metav1.Time) bool { // TimeCompareEqualPointer compares two times, allowing an error of 1s func TimeCompareEqualPointer(a, b *metav1.Time) bool { - if a == b { - return true - } else if a == nil { + if a == nil || b == nil { return false + } else if a == b { + return true } return TimeCompareEqual(*a, *b)