Use deepcopy to propagate the spec to clusters.

Avoid sharing pointers to the same spec data between the informer
and the clusters. The only catch is that the error field is cleared
during deepcopy, since it is an interface that may contain private
fields that cannot be copied, however, the error is only used when
the manifest is parsed and before it is queued, therefore, we never
refer to that field in the cluster structure.
This commit is contained in:
Oleksii Kliukin 2018-05-17 16:05:12 +02:00
parent ebe50abccb
commit da4cc2705b
5 changed files with 42 additions and 22 deletions

14
glide.lock generated
View File

@ -1,8 +1,8 @@
hash: aa008e00a8cf34fa59a081dd67644319f9d5d077bf4de81aa4a25a2d5b8781a1 hash: 688e15147f1217da635b83ee33f20a3741a400a493787d79992d1650f6e4c514
updated: 2018-01-15T15:00:53.231516+01:00 updated: 2018-05-17T10:46:49.090929+02:00
imports: imports:
- name: github.com/aws/aws-sdk-go - name: github.com/aws/aws-sdk-go
version: 0cebc639926eb91b0192dae4b28bc808417e764c version: ee7b4b1162937cba700de23bd90acb742982e626
subpackages: subpackages:
- aws - aws
- aws/awserr - aws/awserr
@ -20,6 +20,8 @@ imports:
- aws/request - aws/request
- aws/session - aws/session
- aws/signer/v4 - aws/signer/v4
- internal/sdkio
- internal/sdkrand
- internal/shareddefaults - internal/shareddefaults
- private/protocol - private/protocol
- private/protocol/ec2query - private/protocol/ec2query
@ -97,6 +99,8 @@ imports:
- buffer - buffer
- jlexer - jlexer
- jwriter - jwriter
- name: github.com/mohae/deepcopy
version: c48cc78d482608239f6c4c92a4abd87eb8761c90
- name: github.com/motomux/pretty - name: github.com/motomux/pretty
version: b2aad2c9a95d14eb978f29baa6e3a5c3c20eef30 version: b2aad2c9a95d14eb978f29baa6e3a5c3c20eef30
- name: github.com/PuerkitoBio/purell - name: github.com/PuerkitoBio/purell
@ -104,7 +108,7 @@ imports:
- name: github.com/PuerkitoBio/urlesc - name: github.com/PuerkitoBio/urlesc
version: 5bd2802263f21d8788851d5305584c82a5c75d7e version: 5bd2802263f21d8788851d5305584c82a5c75d7e
- name: github.com/Sirupsen/logrus - name: github.com/Sirupsen/logrus
version: d682213848ed68c0a260ca37d6dd5ace8423f5ba version: c155da19408a8799da419ed3eeb0cb5db0ad5dbc
- name: github.com/spf13/pflag - name: github.com/spf13/pflag
version: 9ff6c6923cfffbcd502984b8e0c80539a94968b7 version: 9ff6c6923cfffbcd502984b8e0c80539a94968b7
- name: github.com/ugorji/go - name: github.com/ugorji/go
@ -153,7 +157,7 @@ imports:
- pkg/client/clientset/clientset/scheme - pkg/client/clientset/clientset/scheme
- pkg/client/clientset/clientset/typed/apiextensions/v1beta1 - pkg/client/clientset/clientset/typed/apiextensions/v1beta1
- name: k8s.io/apimachinery - name: k8s.io/apimachinery
version: 8ab5f3d8a330c2e9baaf84e39042db8d49034ae2 version: 1cb2cdd78d38df243e686d1b572b76e190469842
subpackages: subpackages:
- pkg/api/equality - pkg/api/equality
- pkg/api/errors - pkg/api/errors

View File

@ -44,3 +44,4 @@ import:
- tools/clientcmd - tools/clientcmd
- tools/remotecommand - tools/remotecommand
- package: gopkg.in/yaml.v2 - package: gopkg.in/yaml.v2
- package: github.com/mohae/deepcopy

View File

@ -348,10 +348,9 @@ func (c *Controller) mergeDeprecatedPostgreSQLSpecParameters(spec *spec.Postgres
func (c *Controller) queueClusterEvent(informerOldSpec, informerNewSpec *spec.Postgresql, eventType spec.EventType) { func (c *Controller) queueClusterEvent(informerOldSpec, informerNewSpec *spec.Postgresql, eventType spec.EventType) {
var ( var (
uid types.UID uid types.UID
clusterName spec.NamespacedName clusterName spec.NamespacedName
clusterError error clusterError error
oldSpec, newSpec *spec.Postgresql
) )
if informerOldSpec != nil { //update, delete if informerOldSpec != nil { //update, delete
@ -380,22 +379,13 @@ func (c *Controller) queueClusterEvent(informerOldSpec, informerNewSpec *spec.Po
// in the informer internal state, making it incohherent with the actual Kubernetes object (and, as a side // in the informer internal state, making it incohherent with the actual Kubernetes object (and, as a side
// effect, the modified state will be returned together with subsequent events). // effect, the modified state will be returned together with subsequent events).
if informerOldSpec != nil {
t := *informerOldSpec
oldSpec = &t
}
if informerNewSpec != nil {
t := *informerNewSpec
newSpec = &t
}
workerID := c.clusterWorkerID(clusterName) workerID := c.clusterWorkerID(clusterName)
clusterEvent := spec.ClusterEvent{ clusterEvent := spec.ClusterEvent{
EventTime: time.Now(), EventTime: time.Now(),
EventType: eventType, EventType: eventType,
UID: uid, UID: uid,
OldSpec: oldSpec, OldSpec: informerOldSpec.Clone(),
NewSpec: newSpec, NewSpec: informerNewSpec.Clone(),
WorkerID: workerID, WorkerID: workerID,
} }

View File

@ -3,6 +3,7 @@ package spec
import ( import (
"encoding/json" "encoding/json"
"fmt" "fmt"
"github.com/mohae/deepcopy"
"regexp" "regexp"
"strings" "strings"
"time" "time"
@ -138,6 +139,19 @@ var (
serviceNameRegex = regexp.MustCompile(serviceNameRegexString) serviceNameRegex = regexp.MustCompile(serviceNameRegexString)
) )
// Clone makes a deepcopy of the Postgresql structure. The Error field is nulled-out,
// as there is no guaratee that the actual implementation of the error interface
// will not contain any private fields not-reachable to deepcopy. This should be ok,
// since Error is never read from a Kubernetes object.
func (p *Postgresql) Clone() *Postgresql {
if p == nil {
return nil
}
c := deepcopy.Copy(p).(*Postgresql)
c.Error = nil
return c
}
func parseTime(s string) (time.Time, error) { func parseTime(s string) (time.Time, error) {
parts := strings.Split(s, ":") parts := strings.Split(s, ":")
if len(parts) != 2 { if len(parts) != 2 {

View File

@ -4,11 +4,10 @@ import (
"bytes" "bytes"
"encoding/json" "encoding/json"
"errors" "errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"reflect" "reflect"
"testing" "testing"
"time" "time"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
) )
var parseTimeTests = []struct { var parseTimeTests = []struct {
@ -546,3 +545,15 @@ func TestPostgresListMeta(t *testing.T) {
return return
} }
} }
func TestPostgresqlClone(t *testing.T) {
for _, tt := range unmarshalCluster {
cp := &tt.out
cp.Error = nil
clone := cp.Clone()
if !reflect.DeepEqual(clone, cp) {
t.Errorf("TestPostgresqlClone expected: \n%#v\n, got \n%#v", cp, clone)
}
}
}