diff --git a/api/v1/mongodbcommunity_types.go b/api/v1/mongodbcommunity_types.go index 652e5093a..7a6d68577 100644 --- a/api/v1/mongodbcommunity_types.go +++ b/api/v1/mongodbcommunity_types.go @@ -3,6 +3,7 @@ package v1 import ( "encoding/json" "fmt" + "k8s.io/apimachinery/pkg/runtime" "net/url" "strings" @@ -17,7 +18,6 @@ import ( "github.com/mongodb/mongodb-kubernetes-operator/pkg/automationconfig" "github.com/mongodb/mongodb-kubernetes-operator/pkg/util/scale" - "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" @@ -350,9 +350,13 @@ func (m *MongodConfiguration) UnmarshalJSON(data []byte) error { } func (m *MongodConfiguration) DeepCopy() *MongodConfiguration { - return &MongodConfiguration{ - Object: runtime.DeepCopyJSON(m.Object), + if m != nil && m.Object != nil { + return &MongodConfiguration{ + Object: runtime.DeepCopyJSON(m.Object), + } } + c := NewMongodConfiguration() + return &c } // NewMongodConfiguration returns an empty MongodConfiguration diff --git a/cmd/readiness/testdata/health-status-no-replication.json b/cmd/readiness/testdata/health-status-no-replication.json index 0398e8e4e..325d4a3b4 100644 --- a/cmd/readiness/testdata/health-status-no-replication.json +++ b/cmd/readiness/testdata/health-status-no-replication.json @@ -1,81 +1,81 @@ { - "mmsStatus": { - "bar": { - "errorString": "", - "errorCode": 0, - "plans": [ + "mmsStatus": { + "bar": { + "errorString": "", + "errorCode": 0, + "plans": [ + { + "moves": [ + { + "steps": [ { - "moves": [ - { - "steps": [ - { - "result": "success", - "completed": "2019-09-11T14:20:55.645615846Z", - "started": "2019-09-11T14:20:40.631404367Z", - "isWaitStep": false, - "stepDoc": "Download mongodb binaries (may take a while)", - "step": "Download" - } - ], - "moveDoc": "Download mongodb binaries", - "move": "Download" - }, - { - "steps": [ - { - "result": "success", - "completed": "2019-09-11T14:20:59.325129842Z", - "started": "2019-09-11T14:20:55.645743003Z", - "isWaitStep": false, - "stepDoc": "Start a mongo instance (start fresh)", - "step": "StartFresh" - } - ], - "moveDoc": "Start the process", - "move": "Start" - }, - { - "steps": [ - { - "result": "wait", - "completed": null, - "started": "2019-09-11T14:20:59.325272608Z", - "isWaitStep": true, - "stepDoc": "Wait for the replica set to be initialized by another member", - "step": "WaitRsInit" - } - ], - "moveDoc": "Wait for the replica set to be initialized by another member", - "move": "WaitRsInit" - }, - { - "steps": [ - { - "result": "", - "completed": null, - "started": null, - "isWaitStep": true, - "stepDoc": "Wait for featureCompatibilityVersion to be right", - "step": "WaitFeatureCompatibilityVersionCorrect" - } - ], - "moveDoc": "Wait for featureCompatibilityVersion to be right", - "move": "WaitFeatureCompatibilityVersionCorrect" - } - ], - "completed": "2019-09-11T14:21:42.034934358Z", - "started": "2019-09-11T14:20:40.631348806Z" + "result": "success", + "completed": "2019-09-11T14:20:55.645615846Z", + "started": "2019-09-11T14:20:40.631404367Z", + "isWaitStep": false, + "stepDoc": "Download mongodb binaries (may take a while)", + "step": "Download" } - ], - "lastGoalVersionAchieved": 5, - "name": "bar" - } - }, - "statuses": { - "bar": { - "ExpectedToBeUp": true, - "LastMongoUpTime": 1568222195, - "IsInGoalState": true + ], + "moveDoc": "Download mongodb binaries", + "move": "Download" + }, + { + "steps": [ + { + "result": "success", + "completed": "2019-09-11T14:20:59.325129842Z", + "started": "2019-09-11T14:20:55.645743003Z", + "isWaitStep": false, + "stepDoc": "Start a mongo instance (start fresh)", + "step": "StartFresh" + } + ], + "moveDoc": "Start the process", + "move": "Start" + }, + { + "steps": [ + { + "result": "wait", + "completed": null, + "started": "2019-09-11T14:20:59.325272608Z", + "isWaitStep": true, + "stepDoc": "Wait for the replica set to be initialized by another member", + "step": "WaitRsInit" + } + ], + "moveDoc": "Wait for the replica set to be initialized by another member", + "move": "WaitRsInit" + }, + { + "steps": [ + { + "result": "", + "completed": null, + "started": null, + "isWaitStep": true, + "stepDoc": "Wait for featureCompatibilityVersion to be right", + "step": "WaitFeatureCompatibilityVersionCorrect" + } + ], + "moveDoc": "Wait for featureCompatibilityVersion to be right", + "move": "WaitFeatureCompatibilityVersionCorrect" + } + ], + "completed": "2019-09-11T14:21:42.034934358Z", + "started": "2019-09-11T14:20:40.631348806Z" } + ], + "lastGoalVersionAchieved": 5, + "name": "bar" + } + }, + "statuses": { + "bar": { + "ExpectedToBeUp": true, + "LastMongoUpTime": 1568222195, + "IsInGoalState": true } + } } diff --git a/cmd/readiness/testdata/health-status-ok-no-replica-status.json b/cmd/readiness/testdata/health-status-ok-no-replica-status.json index caaf67f56..fbf69490c 100644 --- a/cmd/readiness/testdata/health-status-ok-no-replica-status.json +++ b/cmd/readiness/testdata/health-status-ok-no-replica-status.json @@ -1,82 +1,82 @@ { - "mmsStatus": { - "bar": { - "errorString": "", - "errorCode": 0, - "plans": [ + "mmsStatus": { + "bar": { + "errorString": "", + "errorCode": 0, + "plans": [ + { + "moves": [ + { + "steps": [ { - "moves": [ - { - "steps": [ - { - "result": "success", - "completed": "2019-09-11T14:20:55.645615846Z", - "started": "2019-09-11T14:20:40.631404367Z", - "isWaitStep": false, - "stepDoc": "Download mongodb binaries (may take a while)", - "step": "Download" - } - ], - "moveDoc": "Download mongodb binaries", - "move": "Download" - }, - { - "steps": [ - { - "result": "success", - "completed": "2019-09-11T14:20:59.325129842Z", - "started": "2019-09-11T14:20:55.645743003Z", - "isWaitStep": false, - "stepDoc": "Start a mongo instance (start fresh)", - "step": "StartFresh" - } - ], - "moveDoc": "Start the process", - "move": "Start" - }, - { - "steps": [ - { - "result": "wait", - "completed": null, - "started": "2019-09-11T14:20:59.325272608Z", - "isWaitStep": true, - "stepDoc": "Wait for the replica set to be initialized by another member", - "step": "WaitRsInit" - } - ], - "moveDoc": "Wait for the replica set to be initialized by another member", - "move": "WaitRsInit" - }, - { - "steps": [ - { - "result": "", - "completed": null, - "started": null, - "isWaitStep": true, - "stepDoc": "Wait for featureCompatibilityVersion to be right", - "step": "WaitFeatureCompatibilityVersionCorrect" - } - ], - "moveDoc": "Wait for featureCompatibilityVersion to be right", - "move": "WaitFeatureCompatibilityVersionCorrect" - } - ], - "completed": "2019-09-11T14:21:42.034934358Z", - "started": "2019-09-11T14:20:40.631348806Z" + "result": "success", + "completed": "2019-09-11T14:20:55.645615846Z", + "started": "2019-09-11T14:20:40.631404367Z", + "isWaitStep": false, + "stepDoc": "Download mongodb binaries (may take a while)", + "step": "Download" } - ], - "lastGoalVersionAchieved": 5, - "name": "bar" - } - }, - "statuses": { - "bar": { - "ReplicationStatus": null, - "ExpectedToBeUp": true, - "LastMongoUpTime": 1568222195, - "IsInGoalState": true + ], + "moveDoc": "Download mongodb binaries", + "move": "Download" + }, + { + "steps": [ + { + "result": "success", + "completed": "2019-09-11T14:20:59.325129842Z", + "started": "2019-09-11T14:20:55.645743003Z", + "isWaitStep": false, + "stepDoc": "Start a mongo instance (start fresh)", + "step": "StartFresh" + } + ], + "moveDoc": "Start the process", + "move": "Start" + }, + { + "steps": [ + { + "result": "wait", + "completed": null, + "started": "2019-09-11T14:20:59.325272608Z", + "isWaitStep": true, + "stepDoc": "Wait for the replica set to be initialized by another member", + "step": "WaitRsInit" + } + ], + "moveDoc": "Wait for the replica set to be initialized by another member", + "move": "WaitRsInit" + }, + { + "steps": [ + { + "result": "", + "completed": null, + "started": null, + "isWaitStep": true, + "stepDoc": "Wait for featureCompatibilityVersion to be right", + "step": "WaitFeatureCompatibilityVersionCorrect" + } + ], + "moveDoc": "Wait for featureCompatibilityVersion to be right", + "move": "WaitFeatureCompatibilityVersionCorrect" + } + ], + "completed": "2019-09-11T14:21:42.034934358Z", + "started": "2019-09-11T14:20:40.631348806Z" } + ], + "lastGoalVersionAchieved": 5, + "name": "bar" + } + }, + "statuses": { + "bar": { + "ReplicationStatus": null, + "ExpectedToBeUp": true, + "LastMongoUpTime": 1568222195, + "IsInGoalState": true } + } } diff --git a/controllers/replicaset_controller_test.go b/controllers/replicaset_controller_test.go index df98cb80d..7017e71bd 100644 --- a/controllers/replicaset_controller_test.go +++ b/controllers/replicaset_controller_test.go @@ -715,7 +715,7 @@ func TestAutomationConfig_CustomMongodConfig(t *testing.T) { mdb := newTestReplicaSet() mongodConfig := objx.New(map[string]interface{}{}) - mongodConfig.Set("net.port", 1000) + mongodConfig.Set("net.port", float64(1000)) mongodConfig.Set("storage.other", "value") mongodConfig.Set("arbitrary.config.path", "value") mdb.Spec.AdditionalMongodConfig.Object = mongodConfig diff --git a/pkg/kube/annotations/annotations.go b/pkg/kube/annotations/annotations.go index f4d923738..9f508b990 100644 --- a/pkg/kube/annotations/annotations.go +++ b/pkg/kube/annotations/annotations.go @@ -34,8 +34,9 @@ func GetAnnotation(object client.Object, key string) string { return value } +// SetAnnotations updates the objects.Annotation with the supplied annotation and does the same with the object backed in kubernetes. func SetAnnotations(object client.Object, annotations map[string]string, kubeClient client.Client) error { - currentObject := object + currentObject := object.DeepCopyObject().(client.Object) err := kubeClient.Get(context.TODO(), types.NamespacedName{Name: object.GetName(), Namespace: object.GetNamespace()}, currentObject) if err != nil { return err @@ -67,7 +68,11 @@ func SetAnnotations(object client.Object, annotations map[string]string, kubeCli } patch := client.RawPatch(types.JSONPatchType, data) - return kubeClient.Patch(context.TODO(), object, patch) + if err = kubeClient.Patch(context.TODO(), currentObject, patch); err != nil { + return err + } + object.SetAnnotations(currentObject.GetAnnotations()) + return nil } func UpdateLastAppliedMongoDBVersion(mdb Versioned, kubeClient client.Client) error { diff --git a/pkg/kube/client/client_test.go b/pkg/kube/client/client_test.go index fcaeccd31..0a8b5c726 100644 --- a/pkg/kube/client/client_test.go +++ b/pkg/kube/client/client_test.go @@ -2,6 +2,8 @@ package client import ( "context" + "github.com/mongodb/mongodb-kubernetes-operator/pkg/kube/annotations" + "github.com/mongodb/mongodb-kubernetes-operator/pkg/kube/service" "testing" "k8s.io/apimachinery/pkg/types" @@ -81,3 +83,33 @@ func TestDeleteConfigMap(t *testing.T) { _, err = client.GetConfigMap(types.NamespacedName{Name: "config-map", Namespace: "default"}) assert.Equal(t, err, notFoundError()) } + +// TestSetAnnotationsDoesNotChangeSuppliedObject verifies that the supplied object for annotations.SetAnnotations is not overridden due being a shallow copy. +// the function lies here, otherwise it will lead to import cycles. +func TestSetAnnotationsDoesNotChangeSuppliedObject(t *testing.T) { + c := NewClient(NewMockedClient()) + backedService := service.Builder(). + SetName("some-name"). + SetNamespace("some-namespace"). + SetAnnotations(map[string]string{"one": "annotation"}). + SetClusterIP("123"). + Build() + err := service.CreateOrUpdateService(c, backedService) + assert.NoError(t, err) + + serviceWithoutAnnotation := service.Builder(). + SetName("some-name"). + SetNamespace("some-namespace"). + Build() + + // make sure this method only changes the annotations locally and in kube + err = annotations.SetAnnotations(&serviceWithoutAnnotation, map[string]string{"new": "something"}, c) + assert.NoError(t, err) + assert.Len(t, serviceWithoutAnnotation.Annotations, 2) + assert.Equal(t, "", serviceWithoutAnnotation.Spec.ClusterIP) + + err = c.Get(context.TODO(), types.NamespacedName{Name: serviceWithoutAnnotation.GetName(), Namespace: serviceWithoutAnnotation.GetNamespace()}, &serviceWithoutAnnotation) + assert.NoError(t, err) + assert.Len(t, serviceWithoutAnnotation.Annotations, 2) + assert.Equal(t, "123", serviceWithoutAnnotation.Spec.ClusterIP) +} diff --git a/pkg/readiness/health/health.go b/pkg/readiness/health/health.go index 8adf5ebf0..53eefe2ab 100644 --- a/pkg/readiness/health/health.go +++ b/pkg/readiness/health/health.go @@ -61,7 +61,7 @@ type StepStatus struct { Result string `json:"result"` } -// isReadyState will return true, meaning a *ready state* in the sense that this Process can +// IsReadyState will return true, meaning a *ready state* in the sense that this Process can // accept read operations. // It returns true if the managed process is mongos or standalone (replicationStatusUndefined) // or if the agent doesn't publish the replica status (older agents)