unit tests for postgres cluster for helpers

[limak9182]

Description

What does this PR have in it?

Key Changes

Highlight the updates in specific files

Testing and Verification

How did you test these changes? What automated tests are added?

Related Issues

Jira tickets, GitHub issues, Support tickets...

PR Checklist

• Code changes adhere to the project's coding standards.
• Relevant unit and integration tests are included.
• Documentation has been updated accordingly.
• All tests pass locally.
• The PR description follows the project's guidelines.

Local testing:

# Creation
kubectl apply -f - <<'EOF'
  apiVersion: enterprise.splunk.com/v4
  kind: PostgresCluster
  metadata:
    name: test-cluster-2
    namespace: test1
  spec:
    class: postgresql-dev
    connectionPoolerEnabled: true
  EOF

kubectl apply -f - <<'EOF'
  apiVersion: enterprise.splunk.com/v4
  kind: PostgresDatabase
  metadata:
    name: myapp-db
    namespace: test1
  spec:
    cluster: test-cluster-2
    databases:
      - name: myapp
    users:
      - name: myapp_user
        passwordSecretRef:
          name: myapp-user-secret
  EOF

# validation
--- PostgresCluster status ---
NAME             CLASS            PHASE   AGE
test-cluster-2   postgresql-dev   Ready   5m19s

--- CNPG Cluster ---
NAME             AGE    INSTANCES   READY   STATUS                     PRIMARY
test-cluster-2   101s   1           1       Cluster in healthy state   test-cluster-2-1

--- Pods ---
NAME                                        READY   STATUS    RESTARTS   AGE
test-cluster-2-1                            1/1     Running   0          92s
test-cluster-2-pooler-ro-67d44b9757-8clcb   1/1     Running   0          81s
test-cluster-2-pooler-ro-67d44b9757-hnrwm   1/1     Running   0          81s
test-cluster-2-pooler-rw-9588d4b69-2mzfh    1/1     Running   0          81s
test-cluster-2-pooler-rw-9588d4b69-cpbg5    1/1     Running   0          81s

-- PostgresDatabase ---
NAME       CLUSTER          PHASE   AGE
cr-beta    test-cluster     Ready   2d19h
myapp-db   test-cluster-2   Ready   2m22s

[github-actions]

CLA Assistant Lite bot:
Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you sign our Contribution License Agreement before we can accept your contribution. You can sign the CLA by just posting a Pull Request Comment with the exact sentence copied from below.

---

I have read the CLA Document and I hereby sign the CLA

---

_{You can retrigger this bot by commenting recheck in this Pull Request}

[M4KIF]

1. nit: unit tests in go have a naming convention of <functionality_filename+test.go>, so It would be postgrescluster_controler_test.go. It exists in the package that It's meant to test, that == a unit test in go world.
2. nit: The tests could (if possible) use of assert package, to streamline the handling of string comparsion, bool predicates etc. so then instead of a if branch with string ==, or boolean conditions It would come down to a line with assert.Equal/Contains/True etc.(t, , )
3. Those tests do contain the "integrations" with pgbouncer logic, or k8s api's. So there's a breach between the business domain (the provisions and business operations that we want to set, provide to the cluster, let's say some rules, values for resource management, etc. other decision steps) logic with the dependencies (which are pgbouncer api's, k8s's api's etc.).
   But I don't know whether we want to follow with DDD and P&A. Nor do I know when We will split into business dependencies application level (integration of dependencies with business logic through ports). That's a discussion to be had?

In general, all above come to choice and the decisions that we've made and are focused on sticking too. I would suggest Michał to take a look too.

[mploski]

1. nit: unit tests in go have a naming convention of <functionality_filename+test.go>, so It would be postgrescluster_controler_test.go. It exists in the package that It's meant to test, that == a unit test in go world.
2. nit: The tests could (if possible) use of assert package, to streamline the handling of string comparsion, bool predicates etc. so then instead of a if branch with string ==, or boolean conditions It would come down to a line with assert.Equal/Contains/True etc.(t, , )
3. Those tests do contain the "integrations" with pgbouncer logic, or k8s api's. So there's a breach between the business domain (the provisions and business operations that we want to set, provide to the cluster, let's say some rules, values for resource management, etc. other decision steps) logic with the dependencies (which are pgbouncer api's, k8s's api's etc.).
   But I don't know whether we want to follow with DDD and P&A. Nor do I know when We will split into business dependencies application level (integration of dependencies with business logic through ports). That's a discussion to be had?

In general, all above come to choice and the decisions that we've made and are focused on sticking too. I would suggest Michał to take a look too.

2. Agree lets use https://pkg.go.dev/github.com/stretchr/testify/assert for comparision
3. Indeed we have pure unit tests and tests that tests integration with k8s api's/cnpg api. I think this is the time next week to sit down and draw the business vs integration separation logic

[DmytroPI-dev]

do we need to validate not only the basic CNPG cluster spec fields but also asserts propagation of class-level CNPG spec fields?

[limak9182]

I believe this unit test should be only responsible for testing this particular functionality, functional tests might cover the whole case of checking parameters between PostgresCluster and PostgresClusterClass.

[DmytroPI-dev]

do we need to check if delete returns an error?

[limak9182]

What I wanted to check here is the scenerio when the cluster was already deleted, we don't want an error here but just nil. In the future when this function might be modified we have an extra guard that our reconciliation logic relies on that.