Migrate the TaskRun controller to genreconciler.

There are effectively two pieces to this change: 1. Generate and consume the base reconciler (dropping boilerplate) 2. Update tests to plumb certain things differently (the existing tests followed our bad example and used unsafe type cases to root around in the `controller.Reconciler.(*Reconciler)`) The changes in the pipeline run files are mostly for symmetry, or because a common utility changed.
tektoncd · Jun 9, 2020 · 9e8c654 · 9e8c654
1 parent 7df013b
commit 9e8c654
Show file tree

Hide file tree

Showing 14 changed files with 757 additions and 212 deletions.
diff --git a/pkg/apis/config/default_test.go b/pkg/apis/config/default_test.go
@@ -14,38 +14,39 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package config
+package config_test
 
 import (
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
+	"github.com/tektoncd/pipeline/pkg/apis/config"
 	"github.com/tektoncd/pipeline/pkg/apis/pipeline/pod"
 	test "github.com/tektoncd/pipeline/pkg/reconciler/testing"
 	"github.com/tektoncd/pipeline/test/diff"
 )
 
 func TestNewDefaultsFromConfigMap(t *testing.T) {
 	type testCase struct {
-		expectedConfig *Defaults
+		expectedConfig *config.Defaults
 		expectedError  bool
 		fileName       string
 	}
 
 	testCases := []testCase{
 		{
-			expectedConfig: &Defaults{
+			expectedConfig: &config.Defaults{
 				DefaultTimeoutMinutes:      50,
 				DefaultServiceAccount:      "tekton",
 				DefaultManagedByLabelValue: "something-else",
 			},
-			fileName: GetDefaultsConfigName(),
+			fileName: config.GetDefaultsConfigName(),
 		},
 		{
-			expectedConfig: &Defaults{
+			expectedConfig: &config.Defaults{
 				DefaultTimeoutMinutes:      50,
 				DefaultServiceAccount:      "tekton",
-				DefaultManagedByLabelValue: DefaultManagedByLabelValue,
+				DefaultManagedByLabelValue: config.DefaultManagedByLabelValue,
 				DefaultPodTemplate: &pod.Template{
 					NodeSelector: map[string]string{
 						"label": "value",
@@ -77,7 +78,7 @@ func TestNewDefaultsFromConfigMap(t *testing.T) {
 
 func TestNewDefaultsFromEmptyConfigMap(t *testing.T) {
 	DefaultsConfigEmptyName := "config-defaults-empty"
-	expectedConfig := &Defaults{
+	expectedConfig := &config.Defaults{
 		DefaultTimeoutMinutes:      60,
 		DefaultManagedByLabelValue: "tekton-pipelines",
 	}
@@ -87,8 +88,8 @@ func TestNewDefaultsFromEmptyConfigMap(t *testing.T) {
 func TestEquals(t *testing.T) {
 	testCases := []struct {
 		name     string
-		left     *Defaults
-		right    *Defaults
+		left     *config.Defaults
+		right    *config.Defaults
 		expected bool
 	}{
 		{
@@ -100,51 +101,51 @@ func TestEquals(t *testing.T) {
 		{
 			name:     "left nil",
 			left:     nil,
-			right:    &Defaults{},
+			right:    &config.Defaults{},
 			expected: false,
 		},
 		{
 			name:     "right nil",
-			left:     &Defaults{},
+			left:     &config.Defaults{},
 			right:    nil,
 			expected: false,
 		},
 		{
 			name:     "right and right default",
-			left:     &Defaults{},
-			right:    &Defaults{},
+			left:     &config.Defaults{},
+			right:    &config.Defaults{},
 			expected: true,
 		},
 		{
 			name: "different default timeout",
-			left: &Defaults{
+			left: &config.Defaults{
 				DefaultTimeoutMinutes: 10,
 			},
-			right: &Defaults{
+			right: &config.Defaults{
 				DefaultTimeoutMinutes: 20,
 			},
 			expected: false,
 		},
 		{
 			name: "same default timeout",
-			left: &Defaults{
+			left: &config.Defaults{
 				DefaultTimeoutMinutes: 20,
 			},
-			right: &Defaults{
+			right: &config.Defaults{
 				DefaultTimeoutMinutes: 20,
 			},
 			expected: true,
 		},
 		{
 			name: "different default pod template",
-			left: &Defaults{
+			left: &config.Defaults{
 				DefaultPodTemplate: &pod.Template{
 					NodeSelector: map[string]string{
 						"label": "value",
 					},
 				},
 			},
-			right: &Defaults{
+			right: &config.Defaults{
 				DefaultPodTemplate: &pod.Template{
 					NodeSelector: map[string]string{
 						"label2": "value",
@@ -155,14 +156,14 @@ func TestEquals(t *testing.T) {
 		},
 		{
 			name: "same default pod template",
-			left: &Defaults{
+			left: &config.Defaults{
 				DefaultPodTemplate: &pod.Template{
 					NodeSelector: map[string]string{
 						"label": "value",
 					},
 				},
 			},
-			right: &Defaults{
+			right: &config.Defaults{
 				DefaultPodTemplate: &pod.Template{
 					NodeSelector: map[string]string{
 						"label": "value",
@@ -183,9 +184,9 @@ func TestEquals(t *testing.T) {
 	}
 }
 
-func verifyConfigFileWithExpectedConfig(t *testing.T, fileName string, expectedConfig *Defaults) {
+func verifyConfigFileWithExpectedConfig(t *testing.T, fileName string, expectedConfig *config.Defaults) {
 	cm := test.ConfigMapFromTestFile(t, fileName)
-	if Defaults, err := NewDefaultsFromConfigMap(cm); err == nil {
+	if Defaults, err := config.NewDefaultsFromConfigMap(cm); err == nil {
 		if d := cmp.Diff(Defaults, expectedConfig); d != "" {
 			t.Errorf("Diff:\n%s", diff.PrintWantGot(d))
 		}
@@ -196,7 +197,7 @@ func verifyConfigFileWithExpectedConfig(t *testing.T, fileName string, expectedC
 
 func verifyConfigFileWithExpectedError(t *testing.T, fileName string) {
 	cm := test.ConfigMapFromTestFile(t, fileName)
-	if _, err := NewDefaultsFromConfigMap(cm); err == nil {
+	if _, err := config.NewDefaultsFromConfigMap(cm); err == nil {
 		t.Errorf("NewDefaultsFromConfigMap(actual) was expected to return an error")
 	}
 }
diff --git a/pkg/apis/config/feature_flags_test.go b/pkg/apis/config/feature_flags_test.go
@@ -14,32 +14,33 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package config
+package config_test
 
 import (
 	"os"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
+	"github.com/tektoncd/pipeline/pkg/apis/config"
 	test "github.com/tektoncd/pipeline/pkg/reconciler/testing"
 	"github.com/tektoncd/pipeline/test/diff"
 )
 
 func TestNewFeatureFlagsFromConfigMap(t *testing.T) {
 	type testCase struct {
-		expectedConfig *FeatureFlags
+		expectedConfig *config.FeatureFlags
 		fileName       string
 	}
 
 	testCases := []testCase{
 		{
-			expectedConfig: &FeatureFlags{
-				RunningInEnvWithInjectedSidecars: DefaultRunningInEnvWithInjectedSidecars,
+			expectedConfig: &config.FeatureFlags{
+				RunningInEnvWithInjectedSidecars: config.DefaultRunningInEnvWithInjectedSidecars,
 			},
-			fileName: GetFeatureFlagsConfigName(),
+			fileName: config.GetFeatureFlagsConfigName(),
 		},
 		{
-			expectedConfig: &FeatureFlags{
+			expectedConfig: &config.FeatureFlags{
 				DisableHomeEnvOverwrite:          true,
 				DisableWorkingDirOverwrite:       true,
 				DisableAffinityAssistant:         true,
@@ -56,7 +57,7 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) {
 
 func TestNewFeatureFlagsFromEmptyConfigMap(t *testing.T) {
 	FeatureFlagsConfigEmptyName := "feature-flags-empty"
-	expectedConfig := &FeatureFlags{
+	expectedConfig := &config.FeatureFlags{
 		RunningInEnvWithInjectedSidecars: true,
 	}
 	verifyConfigFileWithExpectedFeatureFlagsConfig(t, FeatureFlagsConfigEmptyName, expectedConfig)
@@ -84,7 +85,7 @@ func TestGetFeatureFlagsConfigName(t *testing.T) {
 			if tc.featureFlagEnvValue != "" {
 				os.Setenv("CONFIG_FEATURE_FLAGS_NAME", tc.featureFlagEnvValue)
 			}
-			got := GetFeatureFlagsConfigName()
+			got := config.GetFeatureFlagsConfigName()
 			want := tc.expected
 			if got != want {
 				t.Errorf("GetFeatureFlagsConfigName() = %s, want %s", got, want)
@@ -93,9 +94,9 @@ func TestGetFeatureFlagsConfigName(t *testing.T) {
 	}
 }
 
-func verifyConfigFileWithExpectedFeatureFlagsConfig(t *testing.T, fileName string, expectedConfig *FeatureFlags) {
+func verifyConfigFileWithExpectedFeatureFlagsConfig(t *testing.T, fileName string, expectedConfig *config.FeatureFlags) {
 	cm := test.ConfigMapFromTestFile(t, fileName)
-	if flags, err := NewFeatureFlagsFromConfigMap(cm); err == nil {
+	if flags, err := config.NewFeatureFlagsFromConfigMap(cm); err == nil {
 		if d := cmp.Diff(flags, expectedConfig); d != "" {
 			t.Errorf("Diff:\n%s", diff.PrintWantGot(d))
 		}

diff --git a/pkg/apis/config/store_test.go b/pkg/apis/config/store_test.go
@@ -14,13 +14,14 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package config
+package config_test
 
 import (
 	"context"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
+	"github.com/tektoncd/pipeline/pkg/apis/config"
 	test "github.com/tektoncd/pipeline/pkg/reconciler/testing"
 	"github.com/tektoncd/pipeline/test/diff"
 	logtesting "knative.dev/pkg/logging/testing"
@@ -30,21 +31,21 @@ func TestStoreLoadWithContext(t *testing.T) {
 	defaultConfig := test.ConfigMapFromTestFile(t, "config-defaults")
 	featuresConfig := test.ConfigMapFromTestFile(t, "feature-flags-all-flags-set")
 
-	expectedDefaults, _ := NewDefaultsFromConfigMap(defaultConfig)
-	expectedFeatures, _ := NewFeatureFlagsFromConfigMap(featuresConfig)
+	expectedDefaults, _ := config.NewDefaultsFromConfigMap(defaultConfig)
+	expectedFeatures, _ := config.NewFeatureFlagsFromConfigMap(featuresConfig)
 
-	expected := &Config{
+	expected := &config.Config{
 		Defaults:     expectedDefaults,
 		FeatureFlags: expectedFeatures,
 	}
 
-	store := NewStore(logtesting.TestLogger(t))
+	store := config.NewStore(logtesting.TestLogger(t))
 	store.OnConfigChanged(defaultConfig)
 	store.OnConfigChanged(featuresConfig)
 
-	config := FromContext(store.ToContext(context.Background()))
+	cfg := config.FromContext(store.ToContext(context.Background()))
 
-	if d := cmp.Diff(config, expected); d != "" {
+	if d := cmp.Diff(cfg, expected); d != "" {
 		t.Errorf("Unexpected config %s", diff.PrintWantGot(d))
 	}
 }
diff --git a/pkg/apis/pipeline/v1beta1/taskrun_types.go b/pkg/apis/pipeline/v1beta1/taskrun_types.go
@@ -306,6 +306,7 @@ type CloudEventDeliveryState struct {
 }
 
 // +genclient
+// +genreconciler
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
 
 // TaskRun represents a single execution of a Task. TaskRuns are how the steps