Added needed infra setup + pgcluster integration test base#1796
Added needed infra setup + pgcluster integration test base#1796M4KIF wants to merge 1 commit intofeature/database-controllersfrom
Conversation
|
CLA Assistant Lite bot: I have read the CLA Document and I hereby sign the CLA You can retrigger this bot by commenting recheck in this Pull Request |
| // +kubebuilder:scaffold:scheme | ||
|
|
||
| By("bootstrapping test environment") | ||
| cnpgCRDDirectory, err := getCNPGCRDDirectory() |
There was a problem hiding this comment.
we should not leak anything specific to cnpg to this file as cnpg is low level details.
There was a problem hiding this comment.
So If we need to setup the env for the pgcluster controller, ie. setup It's dependency we should try to place It into the cluster setup fixture. That maybe trivial, Is that possible? ie. isn't the state of the k8s api and test env decided before the we pass down to per test fixtures?
Ie. lets compare It to the database transaction, isn't the "state" transaction already committed when we would try and alter It? (state stabilised and we try to append cnpg "resource"?)
In general I agree and hope that's possible to do
| // +kubebuilder:scaffold:scheme | ||
|
|
||
| By("bootstrapping test environment") | ||
| cnpgCRDDirectory, err := getCNPGCRDDirectory() |
There was a problem hiding this comment.
why do we need this function?
There was a problem hiding this comment.
tbh, I haven't yet tried to minimise It. And tbh, I didn't exactly understand what happens here. So preparing cngp requires us to have the path of the cnpg(?).
Which is available as a go module, and the method tries to retrieve the path to cnpg "CRD"'s(?) which could or couldn't be available in the GOPATH. And statements in this method try to be as universal as possible by trying to consider every possible way this module is placed in the filesystem.
There was a problem hiding this comment.
after another round of reading I'm trying to use more domain specific language. Please correct me whenever I talk gibberish.
| }).SetupWithManager(k8sManager); err != nil { | ||
| Expect(err).NotTo(HaveOccurred()) | ||
| } | ||
| if err := (&IngestorClusterReconciler{ |
There was a problem hiding this comment.
my bad, was too focused on reaching sort of executability of the tests. Linked the cnpg and missed a change made.
| @@ -1,5 +1,5 @@ | |||
| /* | |||
There was a problem hiding this comment.
we dont own this file I'd leave it alone or just make the simplest changes possible
2 participants
Description
Key Changes
Testing and Verification
Related Issues
Jira tickets, GitHub issues, Support tickets...
PR Checklist