Skip to content

Commit

Permalink
Dynamic Notification Policy Routes (#1800)
Browse files Browse the repository at this point in the history
* feat: run operator-sdk create api command

Adapted PROJECT to new go module version path and ran:

```
./bin/operator-sdk create api --group grafana --version v1beta1 --kind GrafanaNotificationPolicyRoute --controller false
```

* feat: add new fields to GrafanaNotificationPolicyRoute spec

* chore: run make manifests && make generate

* feat: add DiscoveredRoutes to GrafanaNotificationPolicy status

* refactor: switch to RouteSelector instead of InstanceSelector

- during the reconcile loop in notificationpolicy_controller.go, we have
  to fetch all matching GrafanaNotificationPolicyRoutes for the currently reconciled GrafanaNotificationPolicy
- this can be very easily achieved with a routeSelector, which will be a Kubernetes LabelSelector
- if we would go with instanceSelector, we would have to fetch all available
  GrafanaNotificationPolicyRoutes and then do some filtering afterwards,
  to see if the instanceSelector matches, which would be both more inefficient and more complex

* chore: run make

* feat: implement merging of NotificationPolicyRoutes

* test: add test for mergeNotificationPolicyRoutesWithRouteList

* chore: add samples for local GrafanaNotificationPolicyRoute testing

* feat: setup ownerReferences for GrafanaNotificationPolicyRoutes

* feat: replace owner references with watches to support cross-namespace

The GrafanaNotificationPolicy Controller now watches
GrafanaNoticationPolicyRoutes instead of using ownerReferences, as
ownerReferences do not support cross-namespace references.

We now also emit a event on the GrafanaNotificationPolicyRoute to
indicate that it has been merged into a specific policy.

* docs: add docs and example for new NotificationPolicyRoute

* refactor: move routeSelector to route object, make it mutually exclusive

* refactor: implement new assembleNotificationPolicyRoutes logic

* refactor: implement new dynamic route assembly and add tests

* refactor: remove priority

* chore: re-run make manifest and generate, fix go mods

* feat: add status condition for mutual exclusivity check

- adds validation for ensuring routes and routeSelector are mutual
  exclusive
- updates both GrafanaNotificationPolicy and
  GrafanaNotificationPolicyRoute status conditions accordingly

* docs: update examples and docs related to NotificationPolicies

* refactor: convert to member func and rename to selectorMutuallyExclusive

* chore: mention mutual exclusivity in comments

* test: fix test setup order and kind args

* refactor: inline Route object

* refactor: remove controller for GrafanaNotificationPolicyRoute

* refactor: implement NamespacedResource for NotificationPolicyRoute

* docs: switch to example image of rendered routes

* Update docs/docs/alerting/notification-policies.md

Co-authored-by: Dominik Süß <dominik@suess.wtf>

* Update docs/docs/alerting/notification-policies.md

Co-authored-by: Dominik Süß <dominik@suess.wtf>

* fix

* refactor: always sync applyErrors status condition on defer

* refactor: always sync all GrafanaNotificationPolicy on route changes

related GrafanaNotificationPolicies can now no longer easily retrieved
by comparing labels, as routes can be referenced transitively. Therefore
we simply sync all related policies now.

* Update docs/docs/alerting/notification-policies.md

Co-authored-by: Dominik Süß <dominik@suess.wtf>

* refactor: remove duplicate IsCrossNamespaceImportAllowed

* Update controllers/notificationpolicy_controller.go

Co-authored-by: Steffen Baarsgaard <steff.bpoulsen@gmail.com>

* refactor: move hasRouteSelector to struct function

* refactor: set invalid spec on mutual exclusivity constraint not met

* lint: fix test lint issues

* refactor: move namespace detection to assembleNotificationPolicyRoutes

* refactor: remove duplicate status condition set, already done on defer

* refactor: set specific condition and events for assembly errors

- reverted changes that moved building synchronized condition to defer
- added a specific error type for loop detected errors
- set condition for specific errors and emit events otherwise for
  assembly errors

* fix: formatting

* fix: return error when spec is invalid

* chore: fix repo in PROJECT

* refactor: fix linter issues

* chore: regenerate api docs

---------

Co-authored-by: Dominik Süß <dominik@suess.wtf>
Co-authored-by: Steffen Baarsgaard <steff.bpoulsen@gmail.com>
  • Loading branch information
3 people authored Jan 29, 2025
1 parent 1b0c72e commit ff3fb07
Show file tree
Hide file tree
Showing 32 changed files with 2,628 additions and 42 deletions.
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ bundle/redhat: bundle
# e2e
.PHONY: e2e-kind
e2e-kind:
ifeq (,$(shell kind get clusters $(KIND_CLUSTER_NAME)))
ifeq (,$(shell kind get clusters ))
$(KIND) --kubeconfig="${KUBECONFIG}" create cluster --image=kindest/node:v$(ENVTEST_K8S_VERSION) --config tests/e2e/kind.yaml
endif

Expand Down
9 changes: 9 additions & 0 deletions PROJECT
Original file line number Diff line number Diff line change
Expand Up @@ -64,4 +64,13 @@ resources:
kind: GrafanaContactPoint
path: github.com/grafana/grafana-operator/api/v1beta1
version: v1beta1
- api:
crdVersion: v1
namespaced: true
controller: false
domain: integreatly.org
group: grafana
kind: GrafanaNotificationPolicyRoute
path: github.com/grafana/grafana-operator/v5/api/v1beta1
version: v1beta1
version: "3"
54 changes: 51 additions & 3 deletions api/v1beta1/grafananotificationpolicy_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,11 @@ type Route struct {
// repeat interval
RepeatInterval string `json:"repeat_interval,omitempty"`

// routes
// selects GrafanaNotificationPolicyRoutes to merge in when specified
// mutually exclusive with Routes
RouteSelector *metav1.LabelSelector `json:"routeSelector,omitempty"`

// routes, mutually exclusive with RouteSelector
// +kubebuilder:pruning:PreserveUnknownFields
// +kubebuilder:validation:Schemaless
Routes []*Route `json:"routes,omitempty"`
Expand Down Expand Up @@ -126,6 +130,50 @@ func (r *Route) ToModelRoute() *models.Route {
return out
}

// selectorMutuallyExclusive checks if a single route satisfies the mutual exclusivity constraint
// for checking the entire route including nested routes, use IsRouteSelectorMutuallyExclusive
func (r *Route) selectorMutuallyExclusive() bool {
return !(r.RouteSelector != nil && len(r.Routes) > 0)
}

// IsRouteSelectorMutuallyExclusive returns true when the route and all its sub-routes
// satisfy the constraint of routes and routeSelector being mutually exclusive
func (r *Route) IsRouteSelectorMutuallyExclusive() bool {
if !r.selectorMutuallyExclusive() {
return false
}

// Recursively check all child routes
for _, childRoute := range r.Routes {
if !childRoute.IsRouteSelectorMutuallyExclusive() {
return false
}
}
return true
}

// HasRouteSelector checks if the given Route or any of its nested Routes has a RouteSelector
func (r *Route) HasRouteSelector() bool {
if r.RouteSelector != nil {
return true
}

for _, nestedRoute := range r.Routes {
if nestedRoute.HasRouteSelector() {
return true
}
}

return false
}

// GrafanaNotificationPolicyStatus defines the observed state of GrafanaNotificationPolicy
type GrafanaNotificationPolicyStatus struct {
GrafanaCommonStatus `json:",inline"`

DiscoveredRoutes *[]string `json:"discoveredRoutes,omitempty"`
}

//+kubebuilder:object:root=true
//+kubebuilder:subresource:status

Expand All @@ -137,8 +185,8 @@ type GrafanaNotificationPolicy struct {
metav1.TypeMeta `json:",inline"`
metav1.ObjectMeta `json:"metadata,omitempty"`

Spec GrafanaNotificationPolicySpec `json:"spec,omitempty"`
Status GrafanaCommonStatus `json:"status,omitempty"`
Spec GrafanaNotificationPolicySpec `json:"spec,omitempty"`
Status GrafanaNotificationPolicyStatus `json:"status,omitempty"`
}

func (np *GrafanaNotificationPolicy) NamespacedResource() string {
Expand Down
82 changes: 82 additions & 0 deletions api/v1beta1/grafananotificationpolicy_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@ package v1beta1

import (
"context"
"testing"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

Expand Down Expand Up @@ -74,3 +76,83 @@ var _ = Describe("NotificationPolicy type", func() {
})
})
})

func TestIsRouteSelectorMutuallyExclusive(t *testing.T) {
tests := []struct {
name string
route *Route
expected bool
}{
{
name: "Empty route",
route: &Route{},
expected: true,
},
{
name: "Route with only RouteSelector",
route: &Route{
RouteSelector: &metav1.LabelSelector{},
},
expected: true,
},
{
name: "Route with only sub-routes",
route: &Route{
Routes: []*Route{
{},
{},
},
},
expected: true,
},
{
name: "Route with both RouteSelector and sub-routes",
route: &Route{
RouteSelector: &metav1.LabelSelector{},
Routes: []*Route{
{},
},
},
expected: false,
},
{
name: "Nested routes with mutual exclusivity",
route: &Route{
Routes: []*Route{
{
RouteSelector: &metav1.LabelSelector{},
},
{
Routes: []*Route{
{},
},
},
},
},
expected: true,
},
{
name: "Nested routes without mutual exclusivity",
route: &Route{
Routes: []*Route{
{
RouteSelector: &metav1.LabelSelector{},
Routes: []*Route{
{},
},
},
},
},
expected: false,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result := tt.route.IsRouteSelectorMutuallyExclusive()
if result != tt.expected {
t.Errorf("IsRouteSelectorMutuallyExclusive() = %v, want %v", result, tt.expected)
}
})
}
}
64 changes: 64 additions & 0 deletions api/v1beta1/grafananotificationpolicyroute_types.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/*
Copyright 2022.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package v1beta1

import (
"fmt"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

// EDIT THIS FILE! THIS IS SCAFFOLDING FOR YOU TO OWN!
// NOTE: json tags are required. Any new fields you add must have json tags for the fields to be serialized.

// GrafanaNotificationPolicyRouteSpec defines the desired state of GrafanaNotificationPolicyRoute
type GrafanaNotificationPolicyRouteSpec struct {
// INSERT ADDITIONAL SPEC FIELDS - desired state of cluster
// Important: Run "make" to regenerate code after modifying this file

// Route for alerts to match against
Route `json:",inline"`
}

//+kubebuilder:object:root=true
//+kubebuilder:subresource:status

// GrafanaNotificationPolicyRoute is the Schema for the grafananotificationpolicyroutes API
type GrafanaNotificationPolicyRoute struct {
metav1.TypeMeta `json:",inline"`
metav1.ObjectMeta `json:"metadata,omitempty"`

Spec GrafanaNotificationPolicyRouteSpec `json:"spec,omitempty"`
Status GrafanaCommonStatus `json:"status,omitempty"`
}

func (r *GrafanaNotificationPolicyRoute) NamespacedResource() string {
return fmt.Sprintf("%v/%v/%v", r.ObjectMeta.Namespace, r.ObjectMeta.Name, r.ObjectMeta.UID)
}

//+kubebuilder:object:root=true

// GrafanaNotificationPolicyRouteList contains a list of GrafanaNotificationPolicyRoute
type GrafanaNotificationPolicyRouteList struct {
metav1.TypeMeta `json:",inline"`
metav1.ListMeta `json:"metadata,omitempty"`
Items []GrafanaNotificationPolicyRoute `json:"items"`
}

func init() {
SchemeBuilder.Register(&GrafanaNotificationPolicyRoute{}, &GrafanaNotificationPolicyRouteList{})
}
105 changes: 105 additions & 0 deletions api/v1beta1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit ff3fb07

Please sign in to comment.