From d0fa28601b7ebff35d9ce929af4b90c581b297ab Mon Sep 17 00:00:00 2001 From: Arash Deshmeh Date: Sat, 16 May 2020 21:59:13 +0000 Subject: [PATCH] Use configmap watcher for feature flags config Taskrun reconciler now uses a configmap watcher for `feature-flags` configmap to remove the need for an extra Kubernetes API call to get the configmap's value every time a pod is being created. Signed-off-by: Arash Deshmeh --- pkg/pod/pod.go | 35 +-- pkg/pod/pod_test.go | 64 ++--- pkg/reconciler/taskrun/controller.go | 3 + pkg/reconciler/taskrun/taskrun.go | 17 +- pkg/reconciler/taskrun/taskrun_test.go | 256 ++++++++++++++++-- test/controller.go | 12 + .../informers/core/v1/configmap/configmap.go | 52 ++++ .../informers/core/v1/configmap/fake/fake.go | 40 +++ vendor/modules.txt | 2 + 9 files changed, 388 insertions(+), 93 deletions(-) create mode 100644 vendor/knative.dev/pkg/client/injection/kube/informers/core/v1/configmap/configmap.go create mode 100644 vendor/knative.dev/pkg/client/injection/kube/informers/core/v1/configmap/fake/fake.go diff --git a/pkg/pod/pod.go b/pkg/pod/pod.go index 98ffae2c62a..beb20284a2a 100644 --- a/pkg/pod/pod.go +++ b/pkg/pod/pod.go @@ -17,14 +17,14 @@ limitations under the License. package pod import ( + "context" "fmt" - "os" "path/filepath" + "github.com/tektoncd/pipeline/pkg/apis/config" "github.com/tektoncd/pipeline/pkg/apis/pipeline" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" "github.com/tektoncd/pipeline/pkg/names" - "github.com/tektoncd/pipeline/pkg/system" "github.com/tektoncd/pipeline/pkg/version" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -32,15 +32,6 @@ import ( "k8s.io/client-go/kubernetes" ) -// GetFeatureFlagsConfigName returns the name of the configmap containing all -// customizations for the feature flags. -func GetFeatureFlagsConfigName() string { - if e := os.Getenv("CONFIG_FEATURE_FLAGS_NAME"); e != "" { - return e - } - return "feature-flags" -} - const ( // ResultsDir is the folder used by default to create the results file ResultsDir = "/tekton/results" @@ -86,7 +77,7 @@ var ( // MakePod converts TaskRun and TaskSpec objects to a Pod which implements the taskrun specified // by the supplied CRD. -func MakePod(images pipeline.Images, taskRun *v1beta1.TaskRun, taskSpec v1beta1.TaskSpec, kubeclient kubernetes.Interface, entrypointCache EntrypointCache, overrideHomeEnv bool) (*corev1.Pod, error) { +func MakePod(ctx context.Context, images pipeline.Images, taskRun *v1beta1.TaskRun, taskSpec v1beta1.TaskSpec, kubeclient kubernetes.Interface, entrypointCache EntrypointCache, overrideHomeEnv bool) (*corev1.Pod, error) { var initContainers []corev1.Container var volumes []corev1.Volume var volumeMounts []corev1.VolumeMount @@ -190,7 +181,7 @@ func MakePod(images pipeline.Images, taskRun *v1beta1.TaskRun, taskSpec v1beta1. // - sets container name to add "step-" prefix or "step-unnamed-#" if not specified. // TODO(#1605): Remove this loop and make each transformation in // isolation. - shouldOverrideWorkingDir := shouldOverrideWorkingDir(kubeclient) + shouldOverrideWorkingDir := shouldOverrideWorkingDir(ctx) for i, s := range stepContainers { if s.WorkingDir == "" && shouldOverrideWorkingDir { stepContainers[i].WorkingDir = pipeline.WorkspaceDir @@ -330,12 +321,9 @@ func getLimitRangeMinimum(namespace string, kubeclient kubernetes.Interface) (co // but this is planned to change in an upcoming release. // // For further reference see https://github.com/tektoncd/pipeline/issues/2013 -func ShouldOverrideHomeEnv(kubeclient kubernetes.Interface) bool { - configMap, err := kubeclient.CoreV1().ConfigMaps(system.GetNamespace()).Get(GetFeatureFlagsConfigName(), metav1.GetOptions{}) - if err == nil && configMap != nil && configMap.Data != nil && configMap.Data[featureFlagDisableHomeEnvKey] == "true" { - return false - } - return true +func ShouldOverrideHomeEnv(ctx context.Context) bool { + cfg := config.FromContextOrDefaults(ctx) + return !cfg.FeatureFlags.DisableHomeEnvOverwrite } // shouldOverrideWorkingDir returns a bool indicating whether a Pod should have its @@ -344,10 +332,7 @@ func ShouldOverrideHomeEnv(kubeclient kubernetes.Interface) bool { // if not specified by the user, but this is planned to change in an upcoming release. // // For further reference see https://github.com/tektoncd/pipeline/issues/1836 -func shouldOverrideWorkingDir(kubeclient kubernetes.Interface) bool { - configMap, err := kubeclient.CoreV1().ConfigMaps(system.GetNamespace()).Get(GetFeatureFlagsConfigName(), metav1.GetOptions{}) - if err == nil && configMap != nil && configMap.Data != nil && configMap.Data[featureFlagDisableWorkingDirKey] == "true" { - return false - } - return true +func shouldOverrideWorkingDir(ctx context.Context) bool { + cfg := config.FromContextOrDefaults(ctx) + return !cfg.FeatureFlags.DisableWorkingDirOverwrite } diff --git a/pkg/pod/pod_test.go b/pkg/pod/pod_test.go index 1a41b6ac942..96c1afec9b3 100644 --- a/pkg/pod/pod_test.go +++ b/pkg/pod/pod_test.go @@ -17,13 +17,14 @@ limitations under the License. package pod import ( + "context" "fmt" - "os" "path/filepath" "strings" "testing" "github.com/google/go-cmp/cmp" + "github.com/tektoncd/pipeline/pkg/apis/config" "github.com/tektoncd/pipeline/pkg/apis/pipeline" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" "github.com/tektoncd/pipeline/pkg/system" @@ -33,6 +34,7 @@ import ( "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" fakek8s "k8s.io/client-go/kubernetes/fake" + logtesting "knative.dev/pkg/logging/testing" ) var ( @@ -813,7 +815,7 @@ script-heredoc-randomly-generated-78c5n // No entrypoints should be looked up. entrypointCache := fakeCache{} - got, err := MakePod(images, tr, c.ts, kubeclient, entrypointCache, true) + got, err := MakePod(context.Background(), images, tr, c.ts, kubeclient, entrypointCache, true) if err != nil { t.Fatalf("MakePod: %v", err) } @@ -858,14 +860,14 @@ func TestShouldOverrideHomeEnv(t *testing.T) { }{{ description: "Default behaviour: A missing disable-home-env-overwrite flag should result in true", configMap: &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{Name: GetFeatureFlagsConfigName(), Namespace: system.GetNamespace()}, + ObjectMeta: metav1.ObjectMeta{Name: config.GetFeatureFlagsConfigName(), Namespace: system.GetNamespace()}, Data: map[string]string{}, }, expected: true, }, { description: "Setting disable-home-env-overwrite to false should result in true", configMap: &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{Name: GetFeatureFlagsConfigName(), Namespace: system.GetNamespace()}, + ObjectMeta: metav1.ObjectMeta{Name: config.GetFeatureFlagsConfigName(), Namespace: system.GetNamespace()}, Data: map[string]string{ featureFlagDisableHomeEnvKey: "false", }, @@ -874,7 +876,7 @@ func TestShouldOverrideHomeEnv(t *testing.T) { }, { description: "Setting disable-home-env-overwrite to true should result in false", configMap: &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{Name: GetFeatureFlagsConfigName(), Namespace: system.GetNamespace()}, + ObjectMeta: metav1.ObjectMeta{Name: config.GetFeatureFlagsConfigName(), Namespace: system.GetNamespace()}, Data: map[string]string{ featureFlagDisableHomeEnvKey: "true", }, @@ -882,47 +884,15 @@ func TestShouldOverrideHomeEnv(t *testing.T) { expected: false, }} { t.Run(tc.description, func(t *testing.T) { - kubeclient := fakek8s.NewSimpleClientset( - tc.configMap, - ) - if result := ShouldOverrideHomeEnv(kubeclient); result != tc.expected { + store := config.NewStore(logtesting.TestLogger(t)) + store.OnConfigChanged(tc.configMap) + if result := ShouldOverrideHomeEnv(store.ToContext(context.Background())); result != tc.expected { t.Errorf("Expected %t Received %t", tc.expected, result) } }) } } -func TestGetFeatureFlagsConfigName(t *testing.T) { - for _, tc := range []struct { - description string - featureFlagEnvValue string - expected string - }{{ - description: "Feature flags config value not set", - featureFlagEnvValue: "", - expected: "feature-flags", - }, { - description: "Feature flags config value set", - featureFlagEnvValue: "feature-flags-test", - expected: "feature-flags-test", - }} { - t.Run(tc.description, func(t *testing.T) { - original := os.Getenv("CONFIG_FEATURE_FLAGS_NAME") - defer t.Cleanup(func() { - os.Setenv("CONFIG_FEATURE_FLAGS_NAME", original) - }) - if tc.featureFlagEnvValue != "" { - os.Setenv("CONFIG_FEATURE_FLAGS_NAME", tc.featureFlagEnvValue) - } - got := GetFeatureFlagsConfigName() - want := tc.expected - if got != want { - t.Errorf("GetFeatureFlagsConfigName() = %s, want %s", got, want) - } - }) - } -} - func TestShouldOverrideWorkingDir(t *testing.T) { for _, tc := range []struct { description string @@ -931,14 +901,14 @@ func TestShouldOverrideWorkingDir(t *testing.T) { }{{ description: "Default behaviour: A missing disable-working-directory-overwrite flag should result in true", configMap: &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{Name: GetFeatureFlagsConfigName(), Namespace: system.GetNamespace()}, + ObjectMeta: metav1.ObjectMeta{Name: config.GetFeatureFlagsConfigName(), Namespace: system.GetNamespace()}, Data: map[string]string{}, }, expected: true, }, { description: "Setting disable-working-directory-overwrite to false should result in true", configMap: &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{Name: GetFeatureFlagsConfigName(), Namespace: system.GetNamespace()}, + ObjectMeta: metav1.ObjectMeta{Name: config.GetFeatureFlagsConfigName(), Namespace: system.GetNamespace()}, Data: map[string]string{ featureFlagDisableWorkingDirKey: "false", }, @@ -947,7 +917,7 @@ func TestShouldOverrideWorkingDir(t *testing.T) { }, { description: "Setting disable-working-directory-overwrite to true should result in false", configMap: &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{Name: GetFeatureFlagsConfigName(), Namespace: system.GetNamespace()}, + ObjectMeta: metav1.ObjectMeta{Name: config.GetFeatureFlagsConfigName(), Namespace: system.GetNamespace()}, Data: map[string]string{ featureFlagDisableWorkingDirKey: "true", }, @@ -955,10 +925,10 @@ func TestShouldOverrideWorkingDir(t *testing.T) { expected: false, }} { t.Run(tc.description, func(t *testing.T) { - kubeclient := fakek8s.NewSimpleClientset( - tc.configMap, - ) - if result := shouldOverrideWorkingDir(kubeclient); result != tc.expected { + store := config.NewStore(logtesting.TestLogger(t)) + store.OnConfigChanged(tc.configMap) + ctx := store.ToContext(context.Background()) + if result := shouldOverrideWorkingDir(ctx); result != tc.expected { t.Errorf("Expected %t Received %t", tc.expected, result) } }) diff --git a/pkg/reconciler/taskrun/controller.go b/pkg/reconciler/taskrun/controller.go index ec74030d28a..30cf7fab91e 100644 --- a/pkg/reconciler/taskrun/controller.go +++ b/pkg/reconciler/taskrun/controller.go @@ -20,6 +20,7 @@ import ( "context" "time" + "github.com/tektoncd/pipeline/pkg/apis/config" "github.com/tektoncd/pipeline/pkg/apis/pipeline" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" pipelineclient "github.com/tektoncd/pipeline/pkg/client/injection/client" @@ -105,6 +106,8 @@ func NewController(namespace string, images pipeline.Images) func(context.Contex Handler: controller.HandleAll(impl.EnqueueControllerOf), }) + c.configStore = config.NewStore(c.Logger.Named("config-store")) + c.configStore.WatchConfigs(cmw) return impl } } diff --git a/pkg/reconciler/taskrun/taskrun.go b/pkg/reconciler/taskrun/taskrun.go index 7636ba71189..b902418e910 100644 --- a/pkg/reconciler/taskrun/taskrun.go +++ b/pkg/reconciler/taskrun/taskrun.go @@ -47,6 +47,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/tools/cache" "knative.dev/pkg/apis" + "knative.dev/pkg/configmap" "knative.dev/pkg/controller" "knative.dev/pkg/tracker" ) @@ -56,6 +57,11 @@ const ( taskRunAgentName = "taskrun-controller" ) +type configStore interface { + ToContext(ctx context.Context) context.Context + WatchConfigs(w configmap.Watcher) +} + // Reconciler implements controller.Reconciler for Configuration resources. type Reconciler struct { *reconciler.Base @@ -71,6 +77,7 @@ type Reconciler struct { timeoutHandler *reconciler.TimeoutSet metrics *Recorder pvcHandler volumeclaim.PvcHandler + configStore configStore } // Check that our Reconciler implements controller.Reconciler @@ -88,6 +95,8 @@ func (c *Reconciler) Reconcile(ctx context.Context, key string) error { return nil } + ctx = c.configStore.ToContext(ctx) + // Get the Task Run resource with this namespace/name original, err := c.taskRunLister.TaskRuns(namespace).Get(name) if k8serrors.IsNotFound(err) { @@ -374,7 +383,7 @@ func (c *Reconciler) reconcile(ctx context.Context, tr *v1beta1.TaskRun, tr.Spec.Workspaces = taskRunWorkspaces } - pod, err = c.createPod(tr, rtr) + pod, err = c.createPod(ctx, tr, rtr) if err != nil { c.handlePodCreationError(tr, err) return nil @@ -533,7 +542,7 @@ func (c *Reconciler) failTaskRun(tr *v1beta1.TaskRun, reason, message string) er // createPod creates a Pod based on the Task's configuration, with pvcName as a volumeMount // TODO(dibyom): Refactor resource setup/substitution logic to its own function in the resources package -func (c *Reconciler) createPod(tr *v1beta1.TaskRun, rtr *resources.ResolvedTaskResources) (*corev1.Pod, error) { +func (c *Reconciler) createPod(ctx context.Context, tr *v1beta1.TaskRun, rtr *resources.ResolvedTaskResources) (*corev1.Pod, error) { ts := rtr.TaskSpec.DeepCopy() inputResources, err := resourceImplBinding(rtr.Inputs, c.Images) if err != nil { @@ -589,12 +598,12 @@ func (c *Reconciler) createPod(tr *v1beta1.TaskRun, rtr *resources.ResolvedTaskR } // Check if the HOME env var of every Step should be set to /tekton/home. - shouldOverrideHomeEnv := podconvert.ShouldOverrideHomeEnv(c.KubeClientSet) + shouldOverrideHomeEnv := podconvert.ShouldOverrideHomeEnv(ctx) // Apply creds-init path substitutions. ts = resources.ApplyCredentialsPath(ts, podconvert.CredentialsPath(shouldOverrideHomeEnv)) - pod, err := podconvert.MakePod(c.Images, tr, *ts, c.KubeClientSet, c.entrypointCache, shouldOverrideHomeEnv) + pod, err := podconvert.MakePod(ctx, c.Images, tr, *ts, c.KubeClientSet, c.entrypointCache, shouldOverrideHomeEnv) if err != nil { return nil, fmt.Errorf("translating TaskSpec to Pod: %w", err) } diff --git a/pkg/reconciler/taskrun/taskrun_test.go b/pkg/reconciler/taskrun/taskrun_test.go index c66d7a60f7f..5b4468f60b9 100644 --- a/pkg/reconciler/taskrun/taskrun_test.go +++ b/pkg/reconciler/taskrun/taskrun_test.go @@ -209,6 +209,12 @@ var ( }, }, } + credsVolume = corev1.Volume{ + Name: "tekton-creds-init-home", + VolumeSource: corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{Medium: corev1.StorageMediumMemory}, + }, + } getMkdirResourceContainer = func(name, dir, suffix string, ops ...tb.ContainerOp) tb.PodSpecOp { actualOps := []tb.ContainerOp{ @@ -255,6 +261,30 @@ func getRunName(tr *v1beta1.TaskRun) string { return strings.Join([]string{tr.Namespace, tr.Name}, "/") } +func ensureConfigurationConfigMapsExist(d *test.Data) { + var defaultsExists, featureFlagsExists bool + for _, cm := range d.ConfigMaps { + if cm.Name == config.GetDefaultsConfigName() { + defaultsExists = true + } + if cm.Name == config.GetFeatureFlagsConfigName() { + featureFlagsExists = true + } + } + if !defaultsExists { + d.ConfigMaps = append(d.ConfigMaps, &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: config.GetDefaultsConfigName(), Namespace: system.GetNamespace()}, + Data: map[string]string{}, + }) + } + if !featureFlagsExists { + d.ConfigMaps = append(d.ConfigMaps, &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: config.GetFeatureFlagsConfigName(), Namespace: system.GetNamespace()}, + Data: map[string]string{}, + }) + } +} + // getTaskRunController returns an instance of the TaskRun controller/reconciler that has been seeded with // d, where d represents the state of the system (existing resources) needed for the test. func getTaskRunController(t *testing.T, d test.Data) (test.Assets, func()) { @@ -265,12 +295,24 @@ func getTaskRunController(t *testing.T, d test.Data) (test.Assets, func()) { SendSuccessfully: true, } ctx = cloudevent.WithClient(ctx, &cloudEventClientBehaviour) + ensureConfigurationConfigMapsExist(&d) c, _ := test.SeedTestData(t, ctx, d) configMapWatcher := configmap.NewInformedWatcher(c.Kube, system.GetNamespace()) + + controller := NewController(namespace, images)(ctx, configMapWatcher) + + stopCh := make(chan struct{}) + if err := configMapWatcher.Start(stopCh); err != nil { + t.Fatalf("error starting configmap watcher: %v", err) + } + return test.Assets{ - Controller: NewController(namespace, images)(ctx, configMapWatcher), - Clients: c, - }, cancel + Controller: controller, + Clients: c, + }, func() { + close(stopCh) + cancel() + } } func checkEvents(fr *record.FakeRecorder, testName string, wantEvents []string) error { @@ -313,20 +355,21 @@ func TestReconcile_ExplicitDefaultSA(t *testing.T) { tb.TaskRunServiceAccountName("test-sa"), )) taskruns := []*v1beta1.TaskRun{taskRunSuccess, taskRunWithSaSuccess} + defaultSAName := "pipelines" d := test.Data{ TaskRuns: taskruns, Tasks: []*v1beta1.Task{simpleTask, saTask}, - } - - defaultSAName := "pipelines" - defaultCfg := &config.Config{ - Defaults: &config.Defaults{ - DefaultServiceAccount: defaultSAName, - DefaultTimeoutMinutes: 60, - DefaultManagedByLabelValue: "tekton-pipelines", + ConfigMaps: []*corev1.ConfigMap{ + { + ObjectMeta: metav1.ObjectMeta{Name: config.GetDefaultsConfigName(), Namespace: system.GetNamespace()}, + Data: map[string]string{ + "default-service-account": defaultSAName, + "default-timeout-minutes": "60", + "default-managed-by-label-value": "tekton-pipelines", + }, + }, }, } - for _, tc := range []struct { name string taskRun *v1beta1.TaskRun @@ -431,8 +474,7 @@ func TestReconcile_ExplicitDefaultSA(t *testing.T) { t.Fatal(err) } - ctx := config.ToContext(context.Background(), defaultCfg) - if err := c.Reconciler.Reconcile(ctx, getRunName(tc.taskRun)); err != nil { + if err := c.Reconciler.Reconcile(context.Background(), getRunName(tc.taskRun)); err != nil { t.Errorf("expected no error. Got error %v", err) } if len(clients.Kube.Actions()) == 0 { @@ -479,6 +521,186 @@ func TestReconcile_ExplicitDefaultSA(t *testing.T) { } } +// TestReconcile_FeatureFlags tests taskruns with and without feature flags set +// to ensure the 'feature-flags' config map can be used to disable the +// corresponding behavior. +func TestReconcile_FeatureFlags(t *testing.T) { + taskWithEnvVar := tb.Task("test-task-with-env-var", + tb.TaskSpec(tb.Step("foo", + tb.StepName("simple-step"), tb.StepCommand("/mycmd"), tb.StepEnvVar("foo", "bar"), + )), + tb.TaskNamespace("foo"), + ) + taskRunWithDisableHomeEnv := tb.TaskRun("test-taskrun-run-home-env", + tb.TaskRunNamespace("foo"), + tb.TaskRunSpec(tb.TaskRunTaskRef(taskWithEnvVar.Name)), + ) + taskRunWithDisableWorkingDirOverwrite := tb.TaskRun("test-taskrun-run-working-dir", + tb.TaskRunNamespace("foo"), + tb.TaskRunSpec(tb.TaskRunTaskRef(simpleTask.Name)), + ) + d := test.Data{ + TaskRuns: []*v1beta1.TaskRun{taskRunWithDisableHomeEnv, taskRunWithDisableWorkingDirOverwrite}, + Tasks: []*v1beta1.Task{simpleTask, taskWithEnvVar}, + } + for _, tc := range []struct { + name string + taskRun *v1beta1.TaskRun + featureFlag string + wantPod *corev1.Pod + }{{ + name: "disable-home-env-overwrite", + taskRun: taskRunWithDisableHomeEnv, + featureFlag: "disable-home-env-overwrite", + wantPod: tb.Pod("test-taskrun-run-home-env-pod-abcde", + tb.PodNamespace("foo"), + tb.PodAnnotation(podconvert.ReleaseAnnotation, podconvert.ReleaseAnnotationValue), + tb.PodLabel(taskNameLabelKey, "test-task-with-env-var"), + tb.PodLabel(taskRunNameLabelKey, "test-taskrun-run-home-env"), + tb.PodLabel("app.kubernetes.io/managed-by", "tekton-pipelines"), + tb.PodOwnerReference("TaskRun", "test-taskrun-run-home-env", + tb.OwnerReferenceAPIVersion(currentAPIVersion)), + tb.PodSpec( + tb.PodVolumes(workspaceVolume, homeVolume, resultsVolume, credsVolume, toolsVolume, downwardVolume), + tb.PodRestartPolicy(corev1.RestartPolicyNever), + getPlaceToolsInitContainer(), + tb.PodContainer("step-simple-step", "foo", + tb.Command(entrypointLocation), + tb.Args("-wait_file", + "/tekton/downward/ready", + "-wait_file_content", + "-post_file", + "/tekton/tools/0", + "-termination_path", + "/tekton/termination", + "-entrypoint", + "/mycmd", + "--", + ), + tb.WorkingDir(workspaceDir), + tb.EnvVar("foo", "bar"), + tb.VolumeMount("tekton-internal-tools", "/tekton/tools"), + tb.VolumeMount("tekton-internal-downward", "/tekton/downward"), + tb.VolumeMount("tekton-internal-workspace", workspaceDir), + tb.VolumeMount("tekton-internal-home", "/tekton/home"), + tb.VolumeMount("tekton-internal-results", "/tekton/results"), + tb.VolumeMount("tekton-creds-init-home", "/tekton/creds"), + tb.TerminationMessagePath("/tekton/termination"), + ), + ), + ), + }, { + name: "disable-working-dir-overwrite", + taskRun: taskRunWithDisableWorkingDirOverwrite, + featureFlag: "disable-working-directory-overwrite", + wantPod: tb.Pod("test-taskrun-run-working-dir-pod-abcde", + tb.PodNamespace("foo"), + tb.PodAnnotation(podconvert.ReleaseAnnotation, podconvert.ReleaseAnnotationValue), + tb.PodLabel(taskNameLabelKey, "test-task"), + tb.PodLabel(taskRunNameLabelKey, "test-taskrun-run-working-dir"), + tb.PodLabel("app.kubernetes.io/managed-by", "tekton-pipelines"), + tb.PodOwnerReference("TaskRun", "test-taskrun-run-working-dir", + tb.OwnerReferenceAPIVersion(currentAPIVersion)), + tb.PodSpec( + tb.PodVolumes(workspaceVolume, homeVolume, resultsVolume, toolsVolume, downwardVolume), + tb.PodRestartPolicy(corev1.RestartPolicyNever), + getPlaceToolsInitContainer(), + tb.PodContainer("step-simple-step", "foo", + tb.Command(entrypointLocation), + tb.Args("-wait_file", + "/tekton/downward/ready", + "-wait_file_content", + "-post_file", + "/tekton/tools/0", + "-termination_path", + "/tekton/termination", + "-entrypoint", + "/mycmd", + "--", + ), + tb.EnvVar("HOME", "/tekton/home"), + tb.VolumeMount("tekton-internal-tools", "/tekton/tools"), + tb.VolumeMount("tekton-internal-downward", "/tekton/downward"), + tb.VolumeMount("tekton-internal-workspace", workspaceDir), + tb.VolumeMount("tekton-internal-home", "/tekton/home"), + tb.VolumeMount("tekton-internal-results", "/tekton/results"), + tb.TerminationMessagePath("/tekton/termination"), + ), + ), + ), + }} { + t.Run(tc.name, func(t *testing.T) { + names.TestingSeed() + d.ConfigMaps = []*corev1.ConfigMap{ + { + ObjectMeta: metav1.ObjectMeta{Name: config.GetFeatureFlagsConfigName(), Namespace: system.GetNamespace()}, + Data: map[string]string{ + tc.featureFlag: "true", + }, + }, + } + testAssets, cancel := getTaskRunController(t, d) + defer cancel() + c := testAssets.Controller + clients := testAssets.Clients + saName := tc.taskRun.Spec.ServiceAccountName + if saName == "" { + saName = "default" + } + if _, err := clients.Kube.CoreV1().ServiceAccounts(tc.taskRun.Namespace).Create(&corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: saName, + Namespace: tc.taskRun.Namespace, + }, + }); err != nil { + t.Fatal(err) + } + if err := c.Reconciler.Reconcile(context.Background(), getRunName(tc.taskRun)); err != nil { + t.Errorf("expected no error. Got error %v", err) + } + if len(clients.Kube.Actions()) == 0 { + t.Errorf("Expected actions to be logged in the kubeclient, got none") + } + + namespace, name, err := cache.SplitMetaNamespaceKey(tc.taskRun.Name) + if err != nil { + t.Errorf("Invalid resource key: %v", err) + } + + tr, err := clients.Pipeline.TektonV1beta1().TaskRuns(namespace).Get(name, metav1.GetOptions{}) + if err != nil { + t.Fatalf("getting updated taskrun: %v", err) + } + condition := tr.Status.GetCondition(apis.ConditionSucceeded) + if condition == nil || condition.Status != corev1.ConditionUnknown { + t.Errorf("Expected invalid TaskRun to have in progress status, but had %v", condition) + } + if condition != nil && condition.Reason != podconvert.ReasonRunning { + t.Errorf("Expected reason %q but was %s", podconvert.ReasonRunning, condition.Reason) + } + + if tr.Status.PodName == "" { + t.Fatalf("Reconcile didn't set pod name") + } + + pod, err := clients.Kube.CoreV1().Pods(tr.Namespace).Get(tr.Status.PodName, metav1.GetOptions{}) + if err != nil { + t.Fatalf("Failed to fetch build pod: %v", err) + } + + if d := cmp.Diff(tc.wantPod.ObjectMeta, pod.ObjectMeta, ignoreRandomPodNameSuffix); d != "" { + t.Errorf("Pod metadata doesn't match %s", diff.PrintWantGot(d)) + } + + if d := cmp.Diff(tc.wantPod.Spec, pod.Spec, resourceQuantityCmp); d != "" { + t.Errorf("Pod spec doesn't match, %s", diff.PrintWantGot(d)) + } + if len(clients.Kube.Actions()) == 0 { + t.Fatalf("Expected actions to be logged in the kubeclient, got none") + } + }) + } +} func TestReconcile(t *testing.T) { taskRunSuccess := tb.TaskRun("test-taskrun-run-success", tb.TaskRunNamespace("foo"), tb.TaskRunSpec( tb.TaskRunTaskRef(simpleTask.Name, tb.TaskRefAPIVersion("a1")), @@ -1296,8 +1518,8 @@ func TestReconcileInvalidTaskRuns(t *testing.T) { // Check actions and events actions := clients.Kube.Actions() - if len(actions) != 1 || actions[0].Matches("namespaces", "list") { - t.Errorf("expected one action (list namespaces) created by the reconciler, got %d. Actions: %#v", len(actions), actions) + if len(actions) != 3 || actions[0].Matches("namespaces", "list") { + t.Errorf("expected 3 actions (first: list namespaces) created by the reconciler, got %d. Actions: %#v", len(actions), actions) } err = checkEvents(fr, tc.name, tc.wantEvents) @@ -1363,7 +1585,7 @@ func makePod(taskRun *v1beta1.TaskRun, task *v1beta1.Task) (*corev1.Pod, error) return nil, err } - return podconvert.MakePod(images, taskRun, task.Spec, kubeclient, entrypointCache, true) + return podconvert.MakePod(context.Background(), images, taskRun, task.Spec, kubeclient, entrypointCache, true) } func TestReconcilePodUpdateStatus(t *testing.T) { diff --git a/test/controller.go b/test/controller.go index a229b5704b3..3be038b249d 100644 --- a/test/controller.go +++ b/test/controller.go @@ -41,6 +41,7 @@ import ( coreinformers "k8s.io/client-go/informers/core/v1" fakekubeclientset "k8s.io/client-go/kubernetes/fake" fakekubeclient "knative.dev/pkg/client/injection/kube/client/fake" + fakeconfigmapinformer "knative.dev/pkg/client/injection/kube/informers/core/v1/configmap/fake" fakepodinformer "knative.dev/pkg/client/injection/kube/informers/core/v1/pod/fake" "knative.dev/pkg/controller" ) @@ -57,6 +58,7 @@ type Data struct { Conditions []*v1alpha1.Condition Pods []*corev1.Pod Namespaces []*corev1.Namespace + ConfigMaps []*corev1.ConfigMap } // Clients holds references to clients which are useful for reconciler tests. @@ -76,6 +78,7 @@ type Informers struct { PipelineResource resourceinformersv1alpha1.PipelineResourceInformer Condition informersv1alpha1.ConditionInformer Pod coreinformers.PodInformer + ConfigMap coreinformers.ConfigMapInformer } // Assets holds references to the controller, logs, clients, and informers. @@ -103,6 +106,7 @@ func SeedTestData(t *testing.T, ctx context.Context, d Data) (Clients, Informers PipelineResource: fakeresourceinformer.Get(ctx), Condition: fakeconditioninformer.Get(ctx), Pod: fakepodinformer.Get(ctx), + ConfigMap: fakeconfigmapinformer.Get(ctx), } for _, pr := range d.PipelineRuns { @@ -174,6 +178,14 @@ func SeedTestData(t *testing.T, ctx context.Context, d Data) (Clients, Informers t.Fatal(err) } } + for _, cm := range d.ConfigMaps { + if err := i.ConfigMap.Informer().GetIndexer().Add(cm); err != nil { + t.Fatal(err) + } + if _, err := c.Kube.CoreV1().ConfigMaps(cm.Namespace).Create(cm); err != nil { + t.Fatal(err) + } + } c.Pipeline.ClearActions() c.Kube.ClearActions() return c, i diff --git a/vendor/knative.dev/pkg/client/injection/kube/informers/core/v1/configmap/configmap.go b/vendor/knative.dev/pkg/client/injection/kube/informers/core/v1/configmap/configmap.go new file mode 100644 index 00000000000..17c854540c7 --- /dev/null +++ b/vendor/knative.dev/pkg/client/injection/kube/informers/core/v1/configmap/configmap.go @@ -0,0 +1,52 @@ +/* +Copyright 2020 The Knative 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. +*/ + +// Code generated by injection-gen. DO NOT EDIT. + +package configmap + +import ( + context "context" + + v1 "k8s.io/client-go/informers/core/v1" + factory "knative.dev/pkg/client/injection/kube/informers/factory" + controller "knative.dev/pkg/controller" + injection "knative.dev/pkg/injection" + logging "knative.dev/pkg/logging" +) + +func init() { + injection.Default.RegisterInformer(withInformer) +} + +// Key is used for associating the Informer inside the context.Context. +type Key struct{} + +func withInformer(ctx context.Context) (context.Context, controller.Informer) { + f := factory.Get(ctx) + inf := f.Core().V1().ConfigMaps() + return context.WithValue(ctx, Key{}, inf), inf.Informer() +} + +// Get extracts the typed informer from the context. +func Get(ctx context.Context) v1.ConfigMapInformer { + untyped := ctx.Value(Key{}) + if untyped == nil { + logging.FromContext(ctx).Panic( + "Unable to fetch k8s.io/client-go/informers/core/v1.ConfigMapInformer from context.") + } + return untyped.(v1.ConfigMapInformer) +} diff --git a/vendor/knative.dev/pkg/client/injection/kube/informers/core/v1/configmap/fake/fake.go b/vendor/knative.dev/pkg/client/injection/kube/informers/core/v1/configmap/fake/fake.go new file mode 100644 index 00000000000..73a324fa77d --- /dev/null +++ b/vendor/knative.dev/pkg/client/injection/kube/informers/core/v1/configmap/fake/fake.go @@ -0,0 +1,40 @@ +/* +Copyright 2020 The Knative 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. +*/ + +// Code generated by injection-gen. DO NOT EDIT. + +package fake + +import ( + context "context" + + configmap "knative.dev/pkg/client/injection/kube/informers/core/v1/configmap" + fake "knative.dev/pkg/client/injection/kube/informers/factory/fake" + controller "knative.dev/pkg/controller" + injection "knative.dev/pkg/injection" +) + +var Get = configmap.Get + +func init() { + injection.Fake.RegisterInformer(withInformer) +} + +func withInformer(ctx context.Context) (context.Context, controller.Informer) { + f := fake.Get(ctx) + inf := f.Core().V1().ConfigMaps() + return context.WithValue(ctx, configmap.Key{}, inf), inf.Informer() +} diff --git a/vendor/modules.txt b/vendor/modules.txt index 72f385462b0..d9fff81efeb 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -882,6 +882,8 @@ knative.dev/pkg/client/injection/kube/client knative.dev/pkg/client/injection/kube/client/fake knative.dev/pkg/client/injection/kube/informers/admissionregistration/v1beta1/mutatingwebhookconfiguration knative.dev/pkg/client/injection/kube/informers/admissionregistration/v1beta1/validatingwebhookconfiguration +knative.dev/pkg/client/injection/kube/informers/core/v1/configmap +knative.dev/pkg/client/injection/kube/informers/core/v1/configmap/fake knative.dev/pkg/client/injection/kube/informers/core/v1/pod knative.dev/pkg/client/injection/kube/informers/core/v1/pod/fake knative.dev/pkg/client/injection/kube/informers/factory