Add Pingdom Transaction Checks
Summary
This pull request addresses issue #553 #569 . The main changes are:
- Add new Provider
PingdomTransaction - Use Golang Client which supports Transaction Check
- Controller now supports more than one provider, before it was not possible to have more than one
- Refactor log messages
- Use pointer for service monitors for Reconciler to improve performance
- provide a secret template function to be able to read sensitive data from secrets
- Added Documentation and Examples
Detailed Feature List
Multi Provider Support:
It was not possible to add more than one provider. For example if you have provider config
providers:
- name: PingdomTransaction
apiURL: "https://api.pingdom.com/api/3.1"
apiToken: <API_TOKEN>
- name: Pingdom
apiURL: "https://api.pingdom.com/api/3.1"
apiToken: <API_TOKEN>
enableMonitorDeletion: true
If you add monitor regardless of the provider spec, the create and update events were fired to all available providers. This lead API errors and unwanted creation of monitors
We will now find the right service monitor proxy depending on the spec and sent create, update and delete events only to suitable providers. In case the provider could not be detected we use the first configured provider, which is default for endpoint monitors without specific provider spec.
Further more I added a enum for different providers
The existing spec does not assign a check to a service provider directly on the CRDs. We have check with either a provider config or without. This will lead to more code. It would be probably better to introduce a required field spec.monitorType to map each check explicitly to a provider. However that would introduce a breaking change (wanted to avoid that)
New Provider for Pingdom Transaction I created a new provider and package for Pingdom transaction provider since the fields are significantly different than Uptime checks. Also we need another client to do the API calls. So it's probably best if separate by packages
Using Pointer for Monitor Proxy The service monitor proxies are initialised on startup. It would make more sense for me if we use pointer here to avoid allocation of new objects. What do you think?
Refactor Logging Output
- in case the api of the pingdom failed to create a check it was not clear by which check it was caused -> adding check name
{"logger":"pingdom","msg":"Tag string should not contain spaces. Not applying tags for monitor: sapcl-group-bmw-us10-monitoring"}
- clear some log outputs that are not relevant to the user
Secret Template to retrieve sensitive Data
We have the requirement to set sensitive data such as password for some fields.
We would like to avoid to store them in endpoint monitor CRD. Instead we just define a secret reference e.g. value: '{{secret:my-secret-name:my-secret-key}}'.
The secret will be retrieved and is replacing the template before the post request is sent to Pingdom.
Minor fixes
- make envtest setup compatible with macos arm64
- validation on CRD level notified the user on apply phase about unmatching fields
The EndpointMonitor "manual-pingdom-transaction-check-karl" is invalid: spec.pingdomTransactionConfig.interval: Unsupported value: 22: supported values: "5", "10", "20", "60", "720", "1440"
Motivation
#569
Testing
- I did run my newly created test
pingdom-transaction-monitor_test.goto ensure a basic create and delete works - I did apply and tested the full spec for transaction check (see readme)
apiVersion: endpointmonitor.stakater.com/v1alpha1
kind: EndpointMonitor
metadata:
name: manual-pingdom-transaction-check-karl
spec:
url: https://www.google.com
pingdomTransactionConfig:
steps:
- function: go_to
args:
url: https://www.google.com
- function: fill
args:
input: textarea[name=q]
value: kubernetes
- function: submit
args:
form: form
- function: exists
args:
element: '#rso'
alertContacts: "14901866"
alertIntegrations: "132625"
#teamAlertContacts: "14901866"
custom_message: "This is a custom message"
paused: false
region: us-east
interval: 60
send_notification_when_down: 3
severity_level: low
tags:
- testing
- manual
---
apiVersion: endpointmonitor.stakater.com/v1alpha1
kind: EndpointMonitor
metadata:
name: manual-pingdom-uptime-check-without-spec-karl
spec:
url: "https://www.google.com"
---
apiVersion: endpointmonitor.stakater.com/v1alpha1
kind: EndpointMonitor
metadata:
name: manual-pingdom-uptime-check-with-spec-karl
spec:
url: "https://www.google.com"
pingdomConfig:
notifyWhenBackUp: true
alertContacts: "14901866"
alertIntegrations: "132625"
tags: "testing,manual"
- I ensured that check creation works even if you have multiple providers
- I tested that we have no breaking change - an endpoint monitor potentially can have no defined providerSpec. It will now use the first defined provider in case no spec was found
- I tested this version in my cluster and verified all functions and log outputs during my manual tests
Additional Notes
Include any additional notes or comments you have about the changes made.
closes #569
@karlderkaefer Image is available for testing. docker pull stakater/ingressmonitorcontroller:SNAPSHOT-PR-570-08fa1fdc
@karl-johan-grahn can you have look? I wanted to minimize changes but in the end still a lot. If splitting of the PR helps let me know
Hi @MuneebAijaz is there an update on the review status?
@karlderkaefer apologies for the delay, this is scheduled for next week
@MuneebAijaz I have answered all comments, can we get an update on the review?
@MuneebAijaz I appreciate the approval very much. Although we have tested these changes intensively, If there any bug related I'm happy to fix. you can mention me or @dennis-ge