-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
[TEP-0137] Restructure customrun event controller
The events controllers for different resources (CustomRun, TaskRun and PipelineRun) will be almost identical, with only the resource type being different. This commit refactors the CustomRun events controller to factor up as much as possible of the reconciler, controller and test logic so that we can reuse it in the upcoming commits for the other resources. I've done a slight change of strategy in the unit test structure compared to what we do for the core controller tests. A set of tests verifies as much as possible of the shared functions, by mocking the event functionality away. These tests are independent of the specific target format of the events. Most of the functionality of the ReconcileKind functions is handled in reconciler tests, which do not need a controller object or a config map watcher to run, which reduces the complexity of these tests without sacrifying coverage. Finally, a smaller set of tests covers the controller -> reconciler logic, so verify that our controller works well when invoking the ReconcileKind indirectly through the generated package. Signed-off-by: Andrea Frittoli <andrea.frittoli@uk.ibm.com>
- Loading branch information
Showing
16 changed files
with
583 additions
and
125 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| /* | ||
| Copyright 2023 The Tekton 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 notifications | ||
|
|
||
| import ( | ||
| "context" | ||
|
|
||
| "github.com/tektoncd/pipeline/pkg/apis/config" | ||
| cacheclient "github.com/tektoncd/pipeline/pkg/reconciler/events/cache" | ||
| cloudeventclient "github.com/tektoncd/pipeline/pkg/reconciler/events/cloudevent" | ||
| "knative.dev/pkg/configmap" | ||
| "knative.dev/pkg/controller" | ||
| "knative.dev/pkg/logging" | ||
| ) | ||
|
|
||
| // ConfigStoreFromContext initialise the config store from the context | ||
| func ConfigStoreFromContext(ctx context.Context, cmw configmap.Watcher) *config.Store { | ||
| logger := logging.FromContext(ctx) | ||
| configStore := config.NewStore(logger.Named("config-store")) | ||
| configStore.WatchConfigs(cmw) | ||
| return configStore | ||
| } | ||
|
|
||
| // ReconcilerFromContext initialises a Reconciler from the context | ||
| func ReconcilerFromContext(ctx context.Context, c Reconciler) { | ||
| c.SetCloudEventsClient(cloudeventclient.Get(ctx)) | ||
| c.SetCacheClient(cacheclient.Get(ctx)) | ||
| } | ||
|
|
||
| // ControllerOptions returns a function that returns options for a controller implementation | ||
| func ControllerOptions(name string, store *config.Store) func(impl *controller.Impl) controller.Options { | ||
| return func(impl *controller.Impl) controller.Options { | ||
| return controller.Options{ | ||
| AgentName: name, | ||
| ConfigStore: store, | ||
| SkipStatusUpdates: true, | ||
| } | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
141 changes: 141 additions & 0 deletions
141
pkg/reconciler/notifications/customrun/controller_test.go
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,141 @@ | ||
| /* | ||
| Copyright 2019 The Tekton 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 customrun_test | ||
|
|
||
| import ( | ||
| "testing" | ||
|
|
||
| "github.com/google/go-cmp/cmp" | ||
| "github.com/google/go-cmp/cmp/cmpopts" | ||
| "github.com/tektoncd/pipeline/pkg/apis/config" | ||
| "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" | ||
| "github.com/tektoncd/pipeline/pkg/reconciler/events/cloudevent" | ||
| "github.com/tektoncd/pipeline/pkg/reconciler/notifications/customrun" | ||
| rtesting "github.com/tektoncd/pipeline/pkg/reconciler/testing" | ||
| "github.com/tektoncd/pipeline/test" | ||
| corev1 "k8s.io/api/core/v1" | ||
| metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
| "k8s.io/apimachinery/pkg/types" | ||
| "knative.dev/pkg/apis" | ||
| duckv1 "knative.dev/pkg/apis/duck/v1" | ||
| cminformer "knative.dev/pkg/configmap/informer" | ||
| pkgreconciler "knative.dev/pkg/reconciler" | ||
| "knative.dev/pkg/system" | ||
| _ "knative.dev/pkg/system/testing" // Setup system.Namespace() | ||
| ) | ||
|
|
||
| func InitializeTestController(t *testing.T, d test.Data, a test.Assets) test.Assets { | ||
| t.Helper() | ||
| configMapWatcher := cminformer.NewInformedWatcher(a.Clients.Kube, system.Namespace()) | ||
| ctl := customrun.NewController()(a.Ctx, configMapWatcher) | ||
| if err := configMapWatcher.Start(a.Ctx.Done()); err != nil { | ||
| t.Fatalf("error starting configmap watcher: %v", err) | ||
| } | ||
|
|
||
| if la, ok := ctl.Reconciler.(pkgreconciler.LeaderAware); ok { | ||
| la.Promote(pkgreconciler.UniversalBucket(), func(pkgreconciler.Bucket, types.NamespacedName) {}) | ||
| } | ||
| a.Controller = ctl | ||
| return a | ||
| } | ||
|
|
||
| // TestReconcileNewController runs reconcile with a cloud event sink configured | ||
| // to ensure that events are sent in different cases | ||
| func TestReconcileNewController(t *testing.T) { | ||
| ignoreResourceVersion := cmpopts.IgnoreFields(v1beta1.CustomRun{}, "ObjectMeta.ResourceVersion") | ||
|
|
||
| cms := []*corev1.ConfigMap{ | ||
| { | ||
| ObjectMeta: metav1.ObjectMeta{Name: config.GetEventsConfigName(), Namespace: system.Namespace()}, | ||
| Data: map[string]string{ | ||
| "sink": "http://synk:8080", | ||
| }, | ||
| }, { | ||
| ObjectMeta: metav1.ObjectMeta{Name: config.GetFeatureFlagsConfigName(), Namespace: system.Namespace()}, | ||
| Data: map[string]string{ | ||
| "send-cloudevents-for-runs": "true", | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| condition := &apis.Condition{ | ||
| Type: apis.ConditionSucceeded, | ||
| Status: corev1.ConditionTrue, | ||
| Reason: v1beta1.CustomRunReasonSuccessful.String(), | ||
| } | ||
| objectStatus := duckv1.Status{ | ||
| Conditions: []apis.Condition{}, | ||
| } | ||
| crStatusFields := v1beta1.CustomRunStatusFields{} | ||
| objectStatus.Conditions = append(objectStatus.Conditions, *condition) | ||
| customRun := v1beta1.CustomRun{ | ||
| ObjectMeta: metav1.ObjectMeta{ | ||
| Name: "test-customRun", | ||
| Namespace: "foo", | ||
| }, | ||
| Spec: v1beta1.CustomRunSpec{}, | ||
| Status: v1beta1.CustomRunStatus{ | ||
| Status: objectStatus, | ||
| CustomRunStatusFields: crStatusFields, | ||
| }, | ||
| } | ||
| customRuns := []*v1beta1.CustomRun{&customRun} | ||
| wantCloudEvents := []string{`(?s)dev.tekton.event.customrun.successful.v1.*test-customRun`} | ||
|
|
||
| d := test.Data{ | ||
| CustomRuns: customRuns, | ||
| ConfigMaps: cms, | ||
| ExpectedCloudEventCount: len(wantCloudEvents), | ||
| } | ||
| testAssets, cancel := rtesting.InitializeTestAssets(t, &d) | ||
| defer cancel() | ||
| clients := testAssets.Clients | ||
|
|
||
| // Initialise the controller. | ||
| // Verify that the config map watcher and reconciler setup works well | ||
| testAssets = InitializeTestController(t, d, testAssets) | ||
| c := testAssets.Controller | ||
|
|
||
| if err := c.Reconciler.Reconcile(testAssets.Ctx, rtesting.GetTestResourceName(&customRun)); err != nil { | ||
| t.Errorf("didn't expect an error, but got one: %v", err) | ||
| } | ||
|
|
||
| for _, a := range clients.Kube.Actions() { | ||
| aVerb := a.GetVerb() | ||
| if aVerb != "get" && aVerb != "list" && aVerb != "watch" { | ||
| t.Errorf("Expected only read actions to be logged in the kubeclient, got %s", aVerb) | ||
| } | ||
| } | ||
|
|
||
| crAfter, err := clients.Pipeline.TektonV1beta1().CustomRuns(customRun.Namespace).Get(testAssets.Ctx, customRun.Name, metav1.GetOptions{}) | ||
| if err != nil { | ||
| t.Fatalf("getting updated customRun: %v", err) | ||
| } | ||
|
|
||
| if d := cmp.Diff(&customRun, crAfter, ignoreResourceVersion); d != "" { | ||
| t.Fatalf("CustomRun should not have changed, got %v instead", d) | ||
| } | ||
|
|
||
| ceClient := clients.CloudEvents.(cloudevent.FakeClient) | ||
| ceClient.CheckCloudEventsUnordered(t, "controller test", wantCloudEvents) | ||
|
|
||
| // Try and reconcile again - expect no event | ||
| if err := c.Reconciler.Reconcile(testAssets.Ctx, rtesting.GetTestResourceName(&customRun)); err != nil { | ||
| t.Errorf("didn't expect an error, but got one: %v", err) | ||
| } | ||
| ceClient.CheckCloudEventsUnordered(t, "controller test", []string{}) | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.