Skip to content

Commit

Permalink
Move predicates into package and add tests
Browse files Browse the repository at this point in the history
Signed-off-by: Hidde Beydals <[email protected]>
  • Loading branch information
hiddeco committed May 6, 2022
1 parent 2612d17 commit f27d9e8
Show file tree
Hide file tree
Showing 8 changed files with 253 additions and 20 deletions.
18 changes: 10 additions & 8 deletions controllers/helmrelease_chart_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,6 @@ import (
"context"
"fmt"

"github.com/fluxcd/helm-controller/api/v2beta1"
intcmp "github.com/fluxcd/helm-controller/internal/cmp"
"github.com/fluxcd/pkg/runtime/acl"
"github.com/fluxcd/pkg/runtime/events"
"github.com/fluxcd/pkg/runtime/patch"
"github.com/fluxcd/pkg/runtime/predicates"
sourcev1 "github.com/fluxcd/source-controller/api/v1beta2"
"github.com/google/go-cmp/cmp"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand All @@ -40,7 +33,16 @@ import (
"sigs.k8s.io/controller-runtime/pkg/predicate"
"sigs.k8s.io/controller-runtime/pkg/ratelimiter"

"github.com/fluxcd/pkg/runtime/acl"
helper "github.com/fluxcd/pkg/runtime/controller"
"github.com/fluxcd/pkg/runtime/events"
"github.com/fluxcd/pkg/runtime/patch"
"github.com/fluxcd/pkg/runtime/predicates"
sourcev1 "github.com/fluxcd/source-controller/api/v1beta2"

"github.com/fluxcd/helm-controller/api/v2beta1"
intcmp "github.com/fluxcd/helm-controller/internal/cmp"
intpredicates "github.com/fluxcd/helm-controller/internal/predicates"
)

type HelmReleaseChartReconciler struct {
Expand All @@ -64,7 +66,7 @@ func (r *HelmReleaseChartReconciler) SetupWithManager(mgr ctrl.Manager) error {
func (r *HelmReleaseChartReconciler) SetupWithManagerAndOptions(mgr ctrl.Manager, opts HelmReleaseChartReconcilerOptions) error {
return ctrl.NewControllerManagedBy(mgr).
For(&v2beta1.HelmRelease{}).
WithEventFilter(predicate.Or(ChartTemplateChangePredicate{}, predicates.ReconcileRequestedPredicate{})).
WithEventFilter(predicate.Or(intpredicates.ChartTemplateChangePredicate{}, predicates.ReconcileRequestedPredicate{})).
WithOptions(controller.Options{
MaxConcurrentReconciles: opts.MaxConcurrentReconciles,
RateLimiter: opts.RateLimiter,
Expand Down
5 changes: 3 additions & 2 deletions controllers/helmrelease_chart_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@ import (
"testing"
"time"

"github.com/fluxcd/helm-controller/api/v2beta1"
sourcev1 "github.com/fluxcd/source-controller/api/v1beta2"
"github.com/go-logr/logr"
. "github.com/onsi/gomega"
apierrors "k8s.io/apimachinery/pkg/api/errors"
Expand All @@ -34,6 +32,9 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client/fake"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
"sigs.k8s.io/controller-runtime/pkg/reconcile"

"github.com/fluxcd/helm-controller/api/v2beta1"
sourcev1 "github.com/fluxcd/source-controller/api/v1beta2"
)

func TestHelmReleaseChartReconciler_Reconcile(t *testing.T) {
Expand Down
3 changes: 2 additions & 1 deletion controllers/helmrelease_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ import (
intchartutil "github.com/fluxcd/helm-controller/internal/chartutil"
"github.com/fluxcd/helm-controller/internal/kube"
"github.com/fluxcd/helm-controller/internal/loader"
intpredicates "github.com/fluxcd/helm-controller/internal/predicates"
"github.com/fluxcd/helm-controller/internal/runner"
"github.com/fluxcd/helm-controller/internal/util"
)
Expand Down Expand Up @@ -121,7 +122,7 @@ func (r *HelmReleaseReconciler) SetupWithManager(mgr ctrl.Manager, opts HelmRele
Watches(
&source.Kind{Type: &sourcev1.HelmChart{}},
handler.EnqueueRequestsFromMapFunc(r.requestsForHelmChartChange),
builder.WithPredicates(SourceRevisionChangePredicate{}),
builder.WithPredicates(intpredicates.SourceRevisionChangePredicate{}),
).
WithOptions(controller.Options{
MaxConcurrentReconciles: opts.MaxConcurrentReconciles,
Expand Down
5 changes: 3 additions & 2 deletions controllers/helmrelease_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,14 @@ import (
"context"
"testing"

v2 "github.com/fluxcd/helm-controller/api/v2beta1"
sourcev1 "github.com/fluxcd/source-controller/api/v1beta2"
. "github.com/onsi/gomega"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/client-go/tools/record"
"sigs.k8s.io/controller-runtime/pkg/client/fake"

v2 "github.com/fluxcd/helm-controller/api/v2beta1"
sourcev1 "github.com/fluxcd/source-controller/api/v1beta2"
)

func TestHelmReleaseReconciler_getHelmChart(t *testing.T) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

package controllers
package predicates

import (
"github.com/fluxcd/helm-controller/api/v2beta1"
Expand Down
142 changes: 142 additions & 0 deletions internal/predicates/chart_template_predicate_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
/*
Copyright 2022 The Flux authors
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 predicates

import (
"testing"

"github.com/onsi/gomega"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/event"

"github.com/fluxcd/helm-controller/api/v2beta1"
)

func TestChartTemplateChangePredicate_Create(t *testing.T) {
obj := &v2beta1.HelmRelease{Spec: v2beta1.HelmReleaseSpec{}}
suspended := &v2beta1.HelmRelease{Spec: v2beta1.HelmReleaseSpec{Suspend: true}}
not := &unstructured.Unstructured{}

tests := []struct {
name string
obj client.Object
want bool
}{
{name: "new", obj: obj, want: true},
{name: "suspended", obj: suspended, want: false},
{name: "not a HelmRelease", obj: not, want: false},
{name: "nil", obj: nil, want: false},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
g := gomega.NewWithT(t)

so := ChartTemplateChangePredicate{}
e := event.CreateEvent{
Object: tt.obj,
}
g.Expect(so.Create(e)).To(gomega.Equal(tt.want))
})
}
}

func TestChartTemplateChangePredicate_Update(t *testing.T) {
templateA := &v2beta1.HelmRelease{Spec: v2beta1.HelmReleaseSpec{
Chart: v2beta1.HelmChartTemplate{
Spec: v2beta1.HelmChartTemplateSpec{
Chart: "chart-name-a",
SourceRef: v2beta1.CrossNamespaceObjectReference{
Name: "repository",
Kind: "HelmRepository",
},
},
},
}}
templateB := &v2beta1.HelmRelease{Spec: v2beta1.HelmReleaseSpec{
Chart: v2beta1.HelmChartTemplate{
Spec: v2beta1.HelmChartTemplateSpec{
Chart: "chart-name-b",
SourceRef: v2beta1.CrossNamespaceObjectReference{
Name: "repository",
Kind: "HelmRepository",
},
},
},
}}
empty := &v2beta1.HelmRelease{}
suspended := &v2beta1.HelmRelease{Spec: v2beta1.HelmReleaseSpec{Suspend: true}}
not := &unstructured.Unstructured{}

tests := []struct {
name string
old client.Object
new client.Object
want bool
}{
{name: "same template", old: templateA, new: templateA, want: false},
{name: "diff template", old: templateA, new: templateB, want: true},
{name: "new with template", old: empty, new: templateA, want: true},
{name: "old with template", old: templateA, new: empty, want: true},
{name: "new suspended", old: templateA, new: suspended, want: false},
{name: "old suspended new template", old: suspended, new: templateA, want: true},
{name: "old not a HelmRelease", old: not, new: templateA, want: false},
{name: "new not a HelmRelease", old: templateA, new: not, want: false},
{name: "old nil", old: nil, new: templateA, want: false},
{name: "new nil", old: templateA, new: nil, want: false},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
g := gomega.NewWithT(t)

so := ChartTemplateChangePredicate{}
e := event.UpdateEvent{
ObjectOld: tt.old,
ObjectNew: tt.new,
}
g.Expect(so.Update(e)).To(gomega.Equal(tt.want))
})
}
}

func TestChartTemplateChangePredicate_Delete(t *testing.T) {
obj := &v2beta1.HelmRelease{Spec: v2beta1.HelmReleaseSpec{}}
suspended := &v2beta1.HelmRelease{Spec: v2beta1.HelmReleaseSpec{Suspend: true}}
not := &unstructured.Unstructured{}

tests := []struct {
name string
obj client.Object
want bool
}{
{name: "object", obj: obj, want: true},
{name: "suspended", obj: suspended, want: false},
{name: "not a HelmRelease", obj: not, want: false},
{name: "nil", obj: nil, want: false},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
g := gomega.NewWithT(t)

so := ChartTemplateChangePredicate{}
e := event.DeleteEvent{
Object: tt.obj,
}
g.Expect(so.Delete(e)).To(gomega.Equal(tt.want))
})
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

package controllers
package predicates

import (
"sigs.k8s.io/controller-runtime/pkg/event"
Expand All @@ -23,6 +23,8 @@ import (
sourcev1 "github.com/fluxcd/source-controller/api/v1beta2"
)

// SourceRevisionChangePredicate detects revision changes to the v1beta2.Artifact
// of a v1beta2.Source object.
type SourceRevisionChangePredicate struct {
predicate.Funcs
}
Expand All @@ -42,16 +44,18 @@ func (SourceRevisionChangePredicate) Update(e event.UpdateEvent) bool {
return false
}

if oldSource.GetArtifact() == nil && newSource.GetArtifact() != nil {
return true
oldHasArtifact := oldSource.GetArtifact() != nil
newHasArtifact := newSource.GetArtifact() != nil

if !newHasArtifact {
return false
}

if oldSource.GetArtifact() != nil && newSource.GetArtifact() != nil &&
oldSource.GetArtifact().Revision != newSource.GetArtifact().Revision {
if !oldHasArtifact {
return true
}

return false
return oldSource.GetArtifact().Revision != newSource.GetArtifact().Revision
}

func (SourceRevisionChangePredicate) Create(e event.CreateEvent) bool {
Expand Down
82 changes: 82 additions & 0 deletions internal/predicates/source_predicate_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
/*
Copyright 2022 The Flux authors
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 predicates

import (
"testing"
"time"

"github.com/onsi/gomega"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/event"

sourcev1 "github.com/fluxcd/source-controller/api/v1beta2"
)

func TestSourceRevisionChangePredicate_Update(t *testing.T) {
sourceA := &sourceMock{revision: "revision-a"}
sourceB := &sourceMock{revision: "revision-b"}
emptySource := &sourceMock{}
notASource := &unstructured.Unstructured{}

tests := []struct {
name string
old client.Object
new client.Object
want bool
}{
{name: "same artifact revision", old: sourceA, new: sourceA, want: false},
{name: "diff artifact revision", old: sourceA, new: sourceB, want: true},
{name: "new with artifact", old: emptySource, new: sourceA, want: true},
{name: "old with artifact", old: sourceA, new: emptySource, want: false},
{name: "old not a source", old: notASource, new: sourceA, want: false},
{name: "new not a source", old: sourceA, new: notASource, want: false},
{name: "old nil", old: nil, new: sourceA, want: false},
{name: "new nil", old: sourceA, new: nil, want: false},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
g := gomega.NewWithT(t)

so := SourceRevisionChangePredicate{}
e := event.UpdateEvent{
ObjectOld: tt.old,
ObjectNew: tt.new,
}
g.Expect(so.Update(e)).To(gomega.Equal(tt.want))
})
}
}

type sourceMock struct {
unstructured.Unstructured
revision string
}

func (m sourceMock) GetRequeueAfter() time.Duration {
return time.Second * 0
}

func (m *sourceMock) GetArtifact() *sourcev1.Artifact {
if m.revision != "" {
return &sourcev1.Artifact{
Revision: m.revision,
}
}
return nil
}

0 comments on commit f27d9e8

Please sign in to comment.