fix unit tests and other minor changes

This commit is contained in:
Rafia Sabih 2020-11-04 07:56:34 +01:00
parent 2b44eff294
commit 9de57f0182
4 changed files with 82 additions and 73 deletions

View File

@ -185,8 +185,8 @@ class EndToEndTestCase(unittest.TestCase):
})
self.eventuallyEqual(lambda: k8s.get_deployment_replica_count(), 2, "Deployment replicas is 2 default")
self.eventuallyEqual(lambda: k8s.count_running_pods("connection-pooler-name=acid-minimal-cluster-pooler"), 2, "No pooler pods found")
self.eventuallyEqual(lambda: k8s.count_running_pods("connection-pooler-name=acid-minimal-cluster-pooler-repl"), 2, "No pooler replica pods found")
self.eventuallyEqual(lambda: k8s.count_running_pods("connection-pooler=acid-minimal-cluster-pooler"), 2, "No pooler pods found")
self.eventuallyEqual(lambda: k8s.count_running_pods("connection-pooler=acid-minimal-cluster-pooler-repl"), 2, "No pooler replica pods found")
self.eventuallyEqual(lambda: k8s.count_services_with_label('application=db-connection-pooler,cluster-name=acid-minimal-cluster'), 2, "No pooler service found")
#Turn off only master connection pooler
@ -200,9 +200,10 @@ class EndToEndTestCase(unittest.TestCase):
}
})
self.eventuallyEqual(lambda: k8s.get_operator_state(), {"0":"idle"}, "Operator does not get in sync")
self.eventuallyEqual(lambda: k8s.get_deployment_replica_count(), 2, "Deployment replicas is 2 default")
self.eventuallyEqual(lambda: k8s.count_running_pods("connection-pooler-name=acid-minimal-cluster-pooler"), 0, "Master pooler pods not deleted")
self.eventuallyEqual(lambda: k8s.count_running_pods("connection-pooler-name=acid-minimal-cluster-pooler-repl"), 2, "Pooler replica pods not found")
self.eventuallyEqual(lambda: k8s.count_running_pods("connection-pooler=acid-minimal-cluster-pooler"), 0, "Master pooler pods not deleted")
self.eventuallyEqual(lambda: k8s.count_running_pods("connection-pooler=acid-minimal-cluster-pooler-repl"), 2, "Pooler replica pods not found")
self.eventuallyEqual(lambda: k8s.count_services_with_label('application=db-connection-pooler,cluster-name=acid-minimal-cluster'), 1, "No pooler service found")
#Turn off only replica connection pooler
@ -216,9 +217,10 @@ class EndToEndTestCase(unittest.TestCase):
}
})
self.eventuallyEqual(lambda: k8s.get_operator_state(), {"0":"idle"}, "Operator does not get in sync")
self.eventuallyEqual(lambda: k8s.get_deployment_replica_count(), 2, "Deployment replicas is 2 default")
self.eventuallyEqual(lambda: k8s.count_running_pods("connection-pooler-name=acid-minimal-cluster-pooler"), 2, "Master pooler pods not found")
self.eventuallyEqual(lambda: k8s.count_running_pods("connection-pooler-name=acid-minimal-cluster-pooler-repl"), 0, "Pooler replica pods not deleted")
self.eventuallyEqual(lambda: k8s.count_running_pods("connection-pooler=acid-minimal-cluster-pooler"), 2, "Master pooler pods not found")
self.eventuallyEqual(lambda: k8s.count_running_pods("connection-pooler=acid-minimal-cluster-pooler-repl"), 0, "Pooler replica pods not deleted")
self.eventuallyEqual(lambda: k8s.count_services_with_label('application=db-connection-pooler,cluster-name=acid-minimal-cluster'), 1, "No pooler service found")
@ -235,7 +237,7 @@ class EndToEndTestCase(unittest.TestCase):
})
self.eventuallyEqual(lambda: k8s.get_deployment_replica_count(), 3, "Deployment replicas is scaled to 3")
self.eventuallyEqual(lambda: k8s.count_running_pods("connection-pooler-name=acid-minimal-cluster-pooler"), 3, "Scale up of pooler pods does not work")
self.eventuallyEqual(lambda: k8s.count_running_pods("connection-pooler=acid-minimal-cluster-pooler"), 3, "Scale up of pooler pods does not work")
# turn it off, keeping config should be overwritten by false
k8s.api.custom_objects_api.patch_namespaced_custom_object(
@ -249,7 +251,7 @@ class EndToEndTestCase(unittest.TestCase):
})
self.eventuallyEqual(lambda: k8s.count_running_pods("connection-pooler-name=acid-minimal-cluster-pooler"), 0, "Pooler pods not scaled down")
self.eventuallyEqual(lambda: k8s.count_running_pods("connection-pooler=acid-minimal-cluster-pooler"), 0, "Pooler pods not scaled down")
self.eventuallyEqual(lambda: k8s.count_services_with_label('application=db-connection-pooler,cluster-name=acid-minimal-cluster'), 0, "Pooler service not removed")
# Verify that all the databases have pooler schema installed.

View File

@ -331,7 +331,7 @@ func (c *Cluster) Create() error {
//
// Do not consider connection pooler as a strict requirement, and if
// something fails, report warning
c.createConnectionPooler()
c.createConnectionPooler(c.installLookupFunction)
return nil
}
@ -759,8 +759,7 @@ func (c *Cluster) Update(oldSpec, newSpec *acidv1.Postgresql) error {
// check which databases we need to process, but even repeating the whole
// installation process should be good enough.
if _, err := c.syncConnectionPooler(oldSpec, newSpec,
c.installLookupFunction); err != nil {
if _, err := c.syncConnectionPooler(oldSpec, newSpec, c.installLookupFunction); err != nil {
c.logger.Errorf("could not sync connection pooler: %v", err)
updateFailed = true
}

View File

@ -77,15 +77,15 @@ func needReplicaConnectionPoolerWorker(spec *acidv1.PostgresSpec) bool {
// have e.g. different `application` label, so that recreatePod operation will
// not interfere with it (it lists all the pods via labels, and if there would
// be no difference, it will recreate also pooler pods).
func (c *Cluster) connectionPoolerLabelsSelector(name string, role PostgresRole) *metav1.LabelSelector {
func (c *Cluster) connectionPoolerLabelsSelector(role PostgresRole) *metav1.LabelSelector {
connectionPoolerLabels := labels.Set(map[string]string{})
extraLabels := labels.Set(map[string]string{
"connection-pooler-name": name,
"application": "db-connection-pooler",
"role": string(role),
"cluster-name": c.Name,
"Namespace": c.Namespace,
"connection-pooler": c.connectionPoolerName(role),
"application": "db-connection-pooler",
"spilo-role": string(role),
"cluster-name": c.Name,
"Namespace": c.Namespace,
})
connectionPoolerLabels = labels.Merge(connectionPoolerLabels, c.labelsSet(false))
@ -108,12 +108,12 @@ func (c *Cluster) connectionPoolerLabelsSelector(name string, role PostgresRole)
// have connectionpooler name in the cp object to have it immutable name
// add these cp related functions to a new cp file
// opConfig, cluster, and database name
func (c *Cluster) createConnectionPooler() (SyncReason, error) {
func (c *Cluster) createConnectionPooler(LookupFunction InstallFunction) (SyncReason, error) {
var reason SyncReason
c.setProcessName("creating connection pooler")
//this is essentially sync with nil as oldSpec
if reason, err := c.syncConnectionPooler(nil, &c.Postgresql, c.installLookupFunction); err != nil {
if reason, err := c.syncConnectionPooler(nil, &c.Postgresql, LookupFunction); err != nil {
return reason, err
}
return reason, nil
@ -283,7 +283,7 @@ func (c *Cluster) generateConnectionPoolerPodTemplate(role PostgresRole) (
podTemplate := &v1.PodTemplateSpec{
ObjectMeta: metav1.ObjectMeta{
Labels: c.connectionPoolerLabelsSelector(c.connectionPoolerName(role), role).MatchLabels,
Labels: c.connectionPoolerLabelsSelector(role).MatchLabels,
Namespace: c.Namespace,
Annotations: c.generatePodAnnotations(spec),
},
@ -337,7 +337,7 @@ func (c *Cluster) generateConnectionPoolerDeployment(connectionPooler *Connectio
ObjectMeta: metav1.ObjectMeta{
Name: connectionPooler.Name,
Namespace: connectionPooler.Namespace,
Labels: c.connectionPoolerLabelsSelector(connectionPooler.Name, connectionPooler.Role).MatchLabels,
Labels: c.connectionPoolerLabelsSelector(connectionPooler.Role).MatchLabels,
Annotations: map[string]string{},
// make StatefulSet object its owner to represent the dependency.
// By itself StatefulSet is being deleted with "Orphaned"
@ -349,7 +349,7 @@ func (c *Cluster) generateConnectionPoolerDeployment(connectionPooler *Connectio
},
Spec: appsv1.DeploymentSpec{
Replicas: numberOfInstances,
Selector: c.connectionPoolerLabelsSelector(connectionPooler.Name, connectionPooler.Role),
Selector: c.connectionPoolerLabelsSelector(connectionPooler.Role),
Template: *podTemplate,
},
}
@ -380,7 +380,7 @@ func (c *Cluster) generateConnectionPoolerService(connectionPooler *ConnectionPo
},
Type: v1.ServiceTypeClusterIP,
Selector: map[string]string{
"connection-pooler": connectionPooler.Name,
"connection-pooler": c.connectionPoolerName(connectionPooler.Role),
},
}
@ -388,7 +388,7 @@ func (c *Cluster) generateConnectionPoolerService(connectionPooler *ConnectionPo
ObjectMeta: metav1.ObjectMeta{
Name: connectionPooler.Name,
Namespace: connectionPooler.Namespace,
Labels: c.connectionPoolerLabelsSelector(connectionPooler.Name, connectionPooler.Role).MatchLabels,
Labels: c.connectionPoolerLabelsSelector(connectionPooler.Role).MatchLabels,
Annotations: map[string]string{},
// make StatefulSet object its owner to represent the dependency.
// By itself StatefulSet is being deleted with "Orphaned"
@ -657,7 +657,7 @@ func makeDefaultConnectionPoolerResources(config *config.Config) acidv1.Resource
}
}
func (c *Cluster) syncConnectionPooler(oldSpec, newSpec *acidv1.Postgresql, installLookupFunction InstallFunction) (SyncReason, error) {
func (c *Cluster) syncConnectionPooler(oldSpec, newSpec *acidv1.Postgresql, LookupFunction InstallFunction) (SyncReason, error) {
var reason SyncReason
var err error
@ -695,6 +695,7 @@ func (c *Cluster) syncConnectionPooler(oldSpec, newSpec *acidv1.Postgresql, inst
Role: role,
}
}
if newNeedConnectionPooler {
// Try to sync in any case. If we didn't needed connection pooler before,
// it means we want to create it. If it was already present, still sync
@ -723,7 +724,7 @@ func (c *Cluster) syncConnectionPooler(oldSpec, newSpec *acidv1.Postgresql, inst
specUser,
c.OpConfig.ConnectionPooler.User)
if err = installLookupFunction(schema, user, role); err != nil {
if err = LookupFunction(schema, user, role); err != nil {
return NoSync, err
}
}

View File

@ -57,8 +57,19 @@ func TestConnectionPoolerCreationAndDeletion(t *testing.T) {
ConnectionPooler: &acidv1.ConnectionPooler{},
EnableReplicaConnectionPooler: boolToPointer(true),
}
reason, err := cluster.createConnectionPooler()
cluster.ConnectionPooler = map[PostgresRole]*ConnectionPoolerObjects{
Master: {
Deployment: nil,
Service: nil,
LookupFunction: true,
},
Replica: {
Deployment: nil,
Service: nil,
LookupFunction: true,
},
}
reason, err := cluster.createConnectionPooler(mockInstallLookupFunction)
if err != nil {
t.Errorf("%s: Cannot create connection pooler, %s, %+v",
@ -347,6 +358,21 @@ func TestConnectionPoolerSynchronization(t *testing.T) {
defaultInstances int32
check func(cluster *Cluster, err error, reason SyncReason) error
}{
{
subTest: "create from scratch",
oldSpec: &acidv1.Postgresql{
Spec: acidv1.PostgresSpec{},
},
newSpec: &acidv1.Postgresql{
Spec: acidv1.PostgresSpec{
ConnectionPooler: &acidv1.ConnectionPooler{},
},
},
cluster: clusterMissingObjects,
defaultImage: "pooler:1.0",
defaultInstances: 1,
check: MasterobjectsAreSaved,
},
{
subTest: "create if doesn't exist",
oldSpec: &acidv1.Postgresql{
@ -379,22 +405,6 @@ func TestConnectionPoolerSynchronization(t *testing.T) {
defaultInstances: 1,
check: MasterobjectsAreSaved,
},
{
subTest: "create replica if doesn't exist with a flag",
oldSpec: &acidv1.Postgresql{
Spec: acidv1.PostgresSpec{},
},
newSpec: &acidv1.Postgresql{
Spec: acidv1.PostgresSpec{
ConnectionPooler: &acidv1.ConnectionPooler{},
EnableReplicaConnectionPooler: boolToPointer(true),
},
},
cluster: clusterDirtyMock,
defaultImage: "pooler:1.0",
defaultInstances: 1,
check: ReplicaobjectsAreSaved,
},
{
subTest: "create no replica with flag",
oldSpec: &acidv1.Postgresql{
@ -411,19 +421,20 @@ func TestConnectionPoolerSynchronization(t *testing.T) {
check: objectsAreDeleted,
},
{
subTest: "create from scratch",
subTest: "create replica if doesn't exist with a flag",
oldSpec: &acidv1.Postgresql{
Spec: acidv1.PostgresSpec{},
},
newSpec: &acidv1.Postgresql{
Spec: acidv1.PostgresSpec{
ConnectionPooler: &acidv1.ConnectionPooler{},
ConnectionPooler: &acidv1.ConnectionPooler{},
EnableReplicaConnectionPooler: boolToPointer(true),
},
},
cluster: clusterMissingObjects,
cluster: clusterDirtyMock,
defaultImage: "pooler:1.0",
defaultInstances: 1,
check: MasterobjectsAreSaved,
check: ReplicaobjectsAreSaved,
},
{
subTest: "create both master and replica",
@ -443,19 +454,22 @@ func TestConnectionPoolerSynchronization(t *testing.T) {
check: objectsAreSaved,
},
{
subTest: "delete if not needed",
subTest: "delete only replica if not needed",
oldSpec: &acidv1.Postgresql{
Spec: acidv1.PostgresSpec{
ConnectionPooler: &acidv1.ConnectionPooler{},
EnableReplicaConnectionPooler: boolToPointer(true),
},
},
newSpec: &acidv1.Postgresql{
Spec: acidv1.PostgresSpec{
ConnectionPooler: &acidv1.ConnectionPooler{},
},
},
newSpec: &acidv1.Postgresql{
Spec: acidv1.PostgresSpec{},
},
cluster: clusterMock,
cluster: clusterDirtyMock,
defaultImage: "pooler:1.0",
defaultInstances: 1,
check: objectsAreDeleted,
check: OnlyReplicaDeleted,
},
{
subTest: "delete only master if not needed",
@ -476,22 +490,19 @@ func TestConnectionPoolerSynchronization(t *testing.T) {
check: OnlyMasterDeleted,
},
{
subTest: "delete only replica if not needed",
subTest: "delete if not needed",
oldSpec: &acidv1.Postgresql{
Spec: acidv1.PostgresSpec{
ConnectionPooler: &acidv1.ConnectionPooler{},
EnableReplicaConnectionPooler: boolToPointer(true),
},
},
newSpec: &acidv1.Postgresql{
Spec: acidv1.PostgresSpec{
ConnectionPooler: &acidv1.ConnectionPooler{},
},
},
cluster: clusterDirtyMock,
newSpec: &acidv1.Postgresql{
Spec: acidv1.PostgresSpec{},
},
cluster: clusterMock,
defaultImage: "pooler:1.0",
defaultInstances: 1,
check: OnlyReplicaDeleted,
check: objectsAreDeleted,
},
{
subTest: "cleanup if still there",
@ -742,7 +753,9 @@ func TestConnectionPoolerDeploymentSpec(t *testing.T) {
Master: {
Deployment: nil,
Service: nil,
LookupFunction: false,
LookupFunction: true,
Name: "",
Role: Master,
},
}
@ -835,9 +848,9 @@ func testResources(cluster *Cluster, podSpec *v1.PodTemplateSpec, role PostgresR
func testLabels(cluster *Cluster, podSpec *v1.PodTemplateSpec, role PostgresRole) error {
poolerLabels := podSpec.ObjectMeta.Labels["connection-pooler"]
if poolerLabels != cluster.connectionPoolerLabelsSelector(cluster.connectionPoolerName(role), role).MatchLabels["connection-pooler"] {
if poolerLabels != cluster.connectionPoolerLabelsSelector(role).MatchLabels["connection-pooler"] {
return fmt.Errorf("Pod labels do not match, got %+v, expected %+v",
podSpec.ObjectMeta.Labels, cluster.connectionPoolerLabelsSelector(cluster.connectionPoolerName(role), role).MatchLabels)
podSpec.ObjectMeta.Labels, cluster.connectionPoolerLabelsSelector(role).MatchLabels)
}
return nil
@ -845,7 +858,7 @@ func testLabels(cluster *Cluster, podSpec *v1.PodTemplateSpec, role PostgresRole
func testSelector(cluster *Cluster, deployment *appsv1.Deployment) error {
labels := deployment.Spec.Selector.MatchLabels
expected := cluster.connectionPoolerLabelsSelector(cluster.connectionPoolerName(Master), Master).MatchLabels
expected := cluster.connectionPoolerLabelsSelector(Master).MatchLabels
if labels["connection-pooler"] != expected["connection-pooler"] {
return fmt.Errorf("Labels are incorrect, got %+v, expected %+v",
@ -894,18 +907,12 @@ func TestConnectionPoolerServiceSpec(t *testing.T) {
Deployment: nil,
Service: nil,
LookupFunction: false,
Name: cluster.connectionPoolerName(Master),
ClusterName: cluster.ClusterName,
Namespace: cluster.Namespace,
Role: Master,
},
Replica: {
Deployment: nil,
Service: nil,
LookupFunction: false,
Name: cluster.connectionPoolerName(Replica),
ClusterName: cluster.ClusterName,
Namespace: cluster.Namespace,
Role: Replica,
},
}