Skip to content

Commit

Permalink
fix(group): Make group cluster scoped (#23)
Browse files Browse the repository at this point in the history
* Revert "fix(groups): Add group namespace field (#22)"

This reverts commit 8a23b56.

* fix(group): Make groups cluster scoped

We do not actually need to make the groups part of namespaces, they can
be cluster scoped, this way any ApiCheck in any namespace can read it.
It's a much better solution than specifying the namespace for each
group.

Tested locally, tests updated to not use a namespace for the group
resource.
  • Loading branch information
akosveres authored Jul 22, 2022
1 parent 8a23b56 commit 86b3883
Show file tree
Hide file tree
Showing 11 changed files with 7 additions and 55 deletions.
3 changes: 0 additions & 3 deletions apis/checkly/v1alpha1/apicheck_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,6 @@ type ApiCheckSpec struct {

// Group determines in which group does the check belong to
Group string `json:"group"`

// GroupNamespace determine in which namespace was the group defined
GroupNamespace string `json:"groupnamespace,omitempty"`
}

// ApiCheckStatus defines the observed state of ApiCheck
Expand Down
1 change: 1 addition & 0 deletions apis/checkly/v1alpha1/group_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ type GroupStatus struct {

//+kubebuilder:object:root=true
//+kubebuilder:subresource:status
//+kubebuilder:resource:scope=Cluster

// Group is the Schema for the groups API
type Group struct {
Expand Down
4 changes: 0 additions & 4 deletions config/crd/bases/k8s.checklyhq.com_apichecks.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,6 @@ spec:
description: Group determines in which group does the check belong
to
type: string
groupnamespace:
description: GroupNamespace determine in which namespace was the group
defined
type: string
maxresponsetime:
description: MaxResponseTime determines what the maximum number of
miliseconds can pass before the check fails, default 15000
Expand Down
2 changes: 1 addition & 1 deletion config/crd/bases/k8s.checklyhq.com_groups.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ spec:
listKind: GroupList
plural: groups
singular: group
scope: Namespaced
scope: Cluster
versions:
- name: v1alpha1
schema:
Expand Down
1 change: 0 additions & 1 deletion config/samples/checkly_v1alpha1_apicheck.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,3 @@ spec:
frequency: 10 # Default 5
muted: true # Default "false"
group: "group-sample"
groupnamespace: "default" # If not specified, the controller assumes the group is in the same namespace
8 changes: 1 addition & 7 deletions controllers/checkly/apicheck_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,13 +117,7 @@ func (r *ApiCheckReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
// Lookup group ID
// ////////////////////////////
group := &checklyv1alpha1.Group{}
var groupNamespace string
if apiCheck.Spec.GroupNamespace == "" {
groupNamespace = apiCheck.Namespace
} else {
groupNamespace = apiCheck.Spec.GroupNamespace
}
err = r.Get(ctx, types.NamespacedName{Name: apiCheck.Spec.Group, Namespace: groupNamespace}, group)
err = r.Get(ctx, types.NamespacedName{Name: apiCheck.Spec.Group}, group)
if err != nil {
if errors.IsNotFound(err) {
// The resource has been deleted
Expand Down
33 changes: 2 additions & 31 deletions controllers/checkly/apicheck_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,14 +60,12 @@ var _ = Describe("ApiCheck Controller", func() {
}

groupKey := types.NamespacedName{
Name: "test-apicheck-group",
Namespace: key.Namespace,
Name: "test-apicheck-group",
}

group := &checklyv1alpha1.Group{
ObjectMeta: metav1.ObjectMeta{
Name: groupKey.Name,
Namespace: groupKey.Namespace,
Name: groupKey.Name,
},
}

Expand Down Expand Up @@ -126,35 +124,8 @@ var _ = Describe("ApiCheck Controller", func() {
}
}, timeout, interval).Should(BeTrue())

// Update
groupUpdateNS := "kube-system"
groupUpdate := &checklyv1alpha1.Group{
ObjectMeta: metav1.ObjectMeta{
Name: groupKey.Name,
Namespace: groupUpdateNS,
},
}
Expect(k8sClient.Create(context.Background(), groupUpdate)).Should(Succeed())

apiCheckRaw := &checklyv1alpha1.ApiCheck{}
Expect(k8sClient.Get(context.Background(), key, apiCheckRaw)).Should(Succeed())
apiCheckRaw.Spec.GroupNamespace = groupUpdateNS
Expect(k8sClient.Update(context.Background(), apiCheckRaw)).Should(Succeed())

By("Expecting groupnamespace to change")
Eventually(func() bool {
f := &checklyv1alpha1.ApiCheck{}
err := k8sClient.Get(context.Background(), key, f)
if err == nil && f.Spec.GroupNamespace == groupUpdateNS {
return true
}
return false

}, timeout, interval).Should(BeTrue())

// Delete
Expect(k8sClient.Delete(context.Background(), group)).Should(Succeed())
Expect(k8sClient.Delete(context.Background(), groupUpdate)).Should(Succeed())

By("Expecting to delete successfully")
Eventually(func() error {
Expand Down
1 change: 0 additions & 1 deletion controllers/checkly/group_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,6 @@ func (r *GroupReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl
// Create internal Check type
internalCheck := external.Group{
Name: group.Name,
Namespace: group.Namespace,
Activated: group.Spec.Activated,
Locations: group.Spec.Locations,
AlertChannels: group.Spec.AlertChannels,
Expand Down
6 changes: 2 additions & 4 deletions controllers/checkly/group_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,12 @@ var _ = Describe("ApiCheck Controller", func() {

updatedLocations := []string{"eu-west-2", "eu-west-1"}
groupKey := types.NamespacedName{
Name: "test-group",
Namespace: "default",
Name: "test-group",
}

group := &checklyv1alpha1.Group{
ObjectMeta: metav1.ObjectMeta{
Name: groupKey.Name,
Namespace: groupKey.Namespace,
Name: groupKey.Name,
},
Spec: checklyv1alpha1.GroupSpec{
Locations: []string{"eu-west-1"},
Expand Down
2 changes: 0 additions & 2 deletions external/checkly/group.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (

type Group struct {
Name string
Namespace string
ID int64
Locations []string
Activated bool
Expand All @@ -37,7 +36,6 @@ func checklyGroup(group Group) (check checkly.Group) {

tags := getTags(group.Labels)
tags = append(tags, "checkly-operator")
tags = append(tags, group.Namespace)

alertSettings := checkly.AlertSettings{
EscalationType: checkly.RunBased,
Expand Down
1 change: 0 additions & 1 deletion external/checkly/group_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import "testing"
func TestChecklyGroup(t *testing.T) {
data := Group{
Name: "foo",
Namespace: "bar",
Locations: []string{"basement"},
}

Expand Down

0 comments on commit 86b3883

Please sign in to comment.