-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
tests: Initial Integration Tests #144
base: main
Are you sure you want to change the base?
Conversation
Marked as draft, as a fork of |
440e6a4
to
aa903b8
Compare
By switching to the already existing Dockerfile, some permission difficulties have risen when Fixed by temporary setting an open umask, as |
aa903b8
to
16b1839
Compare
Why not just use chmod in both cases? If it just adds permissions, that's perfectly fine and nicer since the umask is process state. I have two other ideas that might also help here, but are probably out of scope here, but would be useful independent of testing:
By the way, it's not NAND, as this negates the output of AND, but to apply the umask, the umask input is negated (i.e. masked away). |
04cbbc5
to
2e770f1
Compare
Sounds fair. Now both cases are using chmod.
In theory, this is already possible: https://github.com/Icinga/icinga-testing/blob/1ef6e79ddf1ffaaae423a3f861774a24365f25e0/internal/services/notifications/docker.go#L91 I can change the logic to
I used the file (or file-like) approach as it was the simplest thing to implement I could think of. Furthermore, I had this as a real use case in mind, as, for SLA and other compliance thingies, one might want to keep a "hard copy" of all event logs lying around. However, this can also be changed to a more or less generic web hook, i.e., being configurable through Go's template engine. By doing so one can get rid of the other volume mount.
Ehm.. yes ._. |
Oh, that's not what I had in mind. I mean that you can start the container with variables similar to https://github.com/icinga/docker-icingadb/#usage. That's something we'll eventually want for the container image anyways. |
Then let's use the third way to configure dockerized software through environment variables - also fine for me. I would give this one a shot as this seems like a good small change to depend this PR on. |
What are the other two ways? Currently, the only option is to mount a config file into the container, but for me, that's more of a temporary workaround as there is nothing implemented to configure it via the environment, but once there is, that's the preferred way for me. What's the other remaining way you have in mind? |
The other two ways were mounting the configuration within the container (as the current code does) or deriving a new Docker image with an updated config (as I proposed earlier in #144 (comment)) Another topic: regarding a web request channel instead of a file channel. When establishing a connection from the dockerized icinga-notifications daemon to a web server, this web server also needs to be within the Docker network (or the icinga-notifications daemon must be bound to the host network). This breaks the current setup of having the test running on the host and the daemons within Docker. Of course, we could speak HTTP over an Unix domain socket, but this would require a mount again. |
Yes, that's part of why I consider this out of scope for now. Some notes on this:
|
2e770f1
to
ee9b049
Compare
ee9b049
to
479a43c
Compare
479a43c
to
ddf970a
Compare
I have made some structural changes to this PR and its linked PR Icinga/icinga-testing#27. As just written over there, this PR is now based on the environment variable configuration (#148) and the webhook channel (#149). A local web server to receive webhook messages will be launched on the host and is being bound to the host's address in the Docker Network, which is the Gateway address in Docker's lingo. After playing around with some approaches, this seems to be the easiest one to work out of the box without having to reconfigure Docker, as it is necessary, e.g., for using |
ddf970a
to
2ea4d30
Compare
FTR, I have also pushed my testing branch |
Heavily inspired by the already existing integration tests from Icinga DB, the icinga-testing project was extended to support Icinga Notifications[0]. Based on those changes, an initial integration test was implemented for Icinga Notifications, launching and configuring PostgreSQL, Icinga 2, and Icinga Notifications. Afterwards, a webhook channel is used to catch notifications. [0] Icinga/icinga-testing#27
2ea4d30
to
4992b13
Compare
Heavily inspired by the already existing integration tests from Icinga DB, the icinga-testing project was extended to support Icinga Notifications: Icinga/icinga-testing#26, Icinga/icinga-testing#27
Based on those changes, an initial integration test was implemented for Icinga Notifications, currently just launching PostgreSQL, Icinga 2, and Icinga Notifications, waiting for Icinga Notifications to scan the available channels and write them into the database.