Follow-up changes for WatcherAPI from PR 13 #26
Conversation
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/38cd80d695ca4a0b8e37128417e32d39 ✔️ noop SUCCESS in 0s |
cescgina
force-pushed
the
watcherapi_followup
branch
from
c006fd8 to
0b8f99e
Compare
| @@ -4,3 +4,4 @@ metadata: | |||
| name: watcherapi-kuttl | |||
| spec: | |||
| databaseInstance: "openstack" | |||
| secret: "osp-secret" | |||
There was a problem hiding this comment.
Given that the idea is that Watcher will create a secret with the minimum set of information required for the watcherapi, watcherdecisionengine and watcherapplier, we should use a different secret for watcherapi that the top level osp-secret this in kuttl tests containing only the required parameters instead of the entire osp-secret.
Also, in future, watcherapi may need some parameters which are not included in osp-secret.
There was a problem hiding this comment.
do you think it would be better to create some temporary secret in the kuttl tests and use that? By the time we integrate all the CRs into watcher we'll need to modify all these tests anyway
There was a problem hiding this comment.
Yes, that was my idea. That would also help us when creating the integration between first level watcher and sub-CRs to identify what information should be included in the secret provided from Watcher to WatcherAPIs and the rest.
There was a problem hiding this comment.
ack, makes sense to be explicit, let me update the PR with the change
There was a problem hiding this comment.
done, kuttl tests are passing locally, let's see if CI is green as well
cescgina
force-pushed
the
watcherapi_followup
branch
2 times, most recently
from
1a36fe9 to
11c9b64
Compare
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/f8bfb1c8c3d045c98a436aa4910f9656 ✔️ noop SUCCESS in 0s |
|
recheck |
|
/lgtm |
cescgina
force-pushed
the
watcherapi_followup
branch
from
11c9b64 to
12569a2
Compare
|
/lgtm |
Following the review on PR 13, there were a few changes suggested. This PR does 2 of them: 1. Make the Secret field on the Spec required, and remove the default from WatcherAPI. This means that the field is no longer shared between the Watcher and WatcherAPI kinds. 2. Add more functional tests to test scenarios where the secret or database are not present. To test the change 1, this commit also changes the watcherapi kuttl test to explicitely create a Secret, mimicking what we'll later do via the Watcher controller.
cescgina
force-pushed
the
watcherapi_followup
branch
from
12569a2 to
52053cb
Compare
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: raukadah The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/lgtm |
openshift-merge-bot
bot
merged commit eceb232
into
openstack-k8s-operators:main
Following the review on PR 13, there were a few changes suggested. This
PR does 2 of them:
Make the Secret field on the Spec required, and remove the default
from WatcherAPI. This means that the field is no longer shared
between the Watcher and WatcherAPI kinds.
Add more functional tests to test scenarios where the secret or
database are not present.
Related: OSPRH-11483