From f63ea2f30c3dba555e86bf94781e8967a2167752 Mon Sep 17 00:00:00 2001 From: Daisuke Taniwaki Date: Tue, 12 Mar 2019 21:05:07 +0900 Subject: [PATCH 1/5] Set sensor namespace on trigger resources by default --- pkg/apis/sensor/v1alpha1/types.go | 2 +- sensors/event-handler_test.go | 14 ++++++++++-- sensors/trigger.go | 10 +++++--- sensors/trigger_test.go | 38 +++++++++++++++++++++++++++++++ 4 files changed, 58 insertions(+), 6 deletions(-) diff --git a/pkg/apis/sensor/v1alpha1/types.go b/pkg/apis/sensor/v1alpha1/types.go index 664a416204..5a8963fc08 100644 --- a/pkg/apis/sensor/v1alpha1/types.go +++ b/pkg/apis/sensor/v1alpha1/types.go @@ -261,7 +261,7 @@ type ResourceObject struct { GroupVersionKind `json:",inline" protobuf:"bytes,5,opt,name=groupVersionKind"` // Namespace in which to create this object - // defaults to the service account namespace + // defaults to the sensor namespace Namespace string `json:"namespace" protobuf:"bytes,1,opt,name=namespace"` // Source of the K8 resource file(s) diff --git a/sensors/event-handler_test.go b/sensors/event-handler_test.go index fb8e23a71d..18e4f1942b 100644 --- a/sensors/event-handler_test.go +++ b/sensors/event-handler_test.go @@ -100,10 +100,20 @@ func (m *mockHttpWriter) WriteHeader(statusCode int) { func getsensorExecutionCtx(sensor *v1alpha1.Sensor) *sensorExecutionCtx { kubeClientset := fake.NewSimpleClientset() + fakeDiscoveryClient := kubeClientset.Discovery().(*discoveryFake.FakeDiscovery) + resourceList := &metav1.APIResourceList{ + TypeMeta: metav1.TypeMeta{Kind: "Pod", APIVersion: "v1"}, + GroupVersion: metav1.GroupVersion{Group: "", Version: "v1"}.String(), + APIResources: []metav1.APIResource{{Kind: "Pod"}}, + } + clientPool := &FakeClientPool{ + kubeClientset.Fake, + } + fakeDiscoveryClient.Resources = append(fakeDiscoveryClient.Resources, resourceList) return &sensorExecutionCtx{ kubeClient: kubeClientset, - discoveryClient: kubeClientset.Discovery().(*discoveryFake.FakeDiscovery), - clientPool: NewFakeClientPool(), + discoveryClient: fakeDiscoveryClient, + clientPool: clientPool, log: common.GetLoggerContext(common.LoggerConf()).Logger(), sensorClient: sensorFake.NewSimpleClientset(), sensor: sensor, diff --git a/sensors/trigger.go b/sensors/trigger.go index 760e219970..e5fd0173b6 100644 --- a/sensors/trigger.go +++ b/sensors/trigger.go @@ -178,9 +178,13 @@ func (sec *sensorExecutionCtx) executeTrigger(trigger v1alpha1.Trigger) error { // createResourceObject creates K8s object for trigger func (sec *sensorExecutionCtx) createResourceObject(resource *v1alpha1.ResourceObject, obj *unstructured.Unstructured) error { - if resource.Namespace != "" { - obj.SetNamespace(resource.Namespace) + namespace := resource.Namespace + // Defaults to sensor's namespace + if namespace == "" { + namespace = sec.sensor.Namespace } + obj.SetNamespace(namespace) + if resource.Labels != nil { labels := obj.GetLabels() if labels != nil { @@ -225,7 +229,7 @@ func (sec *sensorExecutionCtx) createResourceObject(resource *v1alpha1.ResourceO } sec.log.Info().Str("api", apiResource.Name).Str("group-version", gvk.Version).Msg("created api resource") - reIf := client.Resource(apiResource, resource.Namespace) + reIf := client.Resource(apiResource, namespace) liveObj, err := reIf.Create(obj) if err != nil { return fmt.Errorf("failed to create resource object. err: %+v", err) diff --git a/sensors/trigger_test.go b/sensors/trigger_test.go index 1af95d1599..07c0cc6e5a 100644 --- a/sensors/trigger_test.go +++ b/sensors/trigger_test.go @@ -241,3 +241,41 @@ func TestProcessTrigger(t *testing.T) { convey.So(err, convey.ShouldNotBeNil) }) } + +func TestCreateResourceObject(t *testing.T) { + convey.Convey("Given a resource object", t, func() { + testSensor, err := getSensor() + convey.So(err, convey.ShouldBeNil) + soc := getsensorExecutionCtx(testSensor) + + rObj := testTrigger.Resource.DeepCopy() + + convey.Convey("Given a pod", func() { + namespace := "foo" + pod := &corev1.Pod{ + TypeMeta: metav1.TypeMeta{Kind: "Pod", APIVersion: "v1"}, + ObjectMeta: metav1.ObjectMeta{Namespace: namespace, Name: "my-pod"}, + } + uObj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(pod) + convey.So(err, convey.ShouldBeNil) + err = soc.createResourceObject(rObj, &unstructured.Unstructured{Object: uObj}) + convey.So(err, convey.ShouldBeNil) + pod, err = soc.kubeClient.CoreV1().Pods(namespace).Get(pod.Name, metav1.GetOptions{}) + convey.So(err, convey.ShouldBeNil) + convey.So(pod.Namespace, convey.ShouldEqual, namespace) + }) + convey.Convey("Given a pod without namespace, use sensor namespace", func() { + pod := &corev1.Pod{ + TypeMeta: metav1.TypeMeta{Kind: "Pod", APIVersion: "v1"}, + ObjectMeta: metav1.ObjectMeta{Name: "my-pod"}, + } + uObj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(pod) + convey.So(err, convey.ShouldBeNil) + err = soc.createResourceObject(rObj, &unstructured.Unstructured{Object: uObj}) + convey.So(err, convey.ShouldBeNil) + pod, err = soc.kubeClient.CoreV1().Pods(testSensor.Namespace).Get(pod.Name, metav1.GetOptions{}) + convey.So(err, convey.ShouldBeNil) + convey.So(pod.Namespace, convey.ShouldEqual, testSensor.Namespace) + }) + }) +} From 98c3698bc7261b66c5918c37fd5bc0f5e7e7b894 Mon Sep 17 00:00:00 2001 From: Daisuke Taniwaki Date: Tue, 12 Mar 2019 21:12:45 +0900 Subject: [PATCH 2/5] Update OpenAPI gen --- pkg/apis/sensor/v1alpha1/openapi_generated.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/pkg/apis/sensor/v1alpha1/openapi_generated.go b/pkg/apis/sensor/v1alpha1/openapi_generated.go index 250dbcfdba..71540cf4cd 100644 --- a/pkg/apis/sensor/v1alpha1/openapi_generated.go +++ b/pkg/apis/sensor/v1alpha1/openapi_generated.go @@ -159,7 +159,7 @@ func schema_pkg_apis_sensor_v1alpha1_DataFilter(ref common.ReferenceCallback) co }, "value": { SchemaProps: spec.SchemaProps{ - Description: "Value is the allowed string values for this key Booleans are pased using strconv.ParseBool() Numbers are parsed using as float64 using strconv.ParseFloat() Strings are taken as is Nils this value is ignored", + Description: "Value is the allowed string values for this key Booleans are passed using strconv.ParseBool() Numbers are parsed using as float64 using strconv.ParseFloat() Strings are taken as is Nils this value is ignored", Type: []string{"array"}, Items: &spec.SchemaOrArray{ Schema: &spec.Schema{ @@ -387,7 +387,7 @@ func schema_pkg_apis_sensor_v1alpha1_GitArtifact(ref common.ReferenceCallback) c }, "remote": { SchemaProps: spec.SchemaProps{ - Description: "Git remote to manage set of tracked repositories. Defaults to \"origin\". Refer https://git-scm.com/docs/git-remote", + Description: "Remote to manage set of tracked repositories. Defaults to \"origin\". Refer https://git-scm.com/docs/git-remote", Ref: ref("github.com/argoproj/argo-events/pkg/apis/sensor/v1alpha1.GitRemoteConfig"), }, }, @@ -430,7 +430,8 @@ func schema_pkg_apis_sensor_v1alpha1_GitRemoteConfig(ref common.ReferenceCallbac return common.OpenAPIDefinition{ Schema: spec.Schema{ SchemaProps: spec.SchemaProps{ - Type: []string{"object"}, + Description: "GitRemoteConfig contains the configuration of a Git remote", + Type: []string{"object"}, Properties: map[string]spec.Schema{ "name": { SchemaProps: spec.SchemaProps{ @@ -595,7 +596,7 @@ func schema_pkg_apis_sensor_v1alpha1_ResourceObject(ref common.ReferenceCallback }, "namespace": { SchemaProps: spec.SchemaProps{ - Description: "Namespace in which to create this object defaults to the service account namespace", + Description: "Namespace in which to create this object defaults to the sensor namespace", Type: []string{"string"}, Format: "", }, From ec19c5a8340b0a6e8f9d406f94d4eb7bd6840424 Mon Sep 17 00:00:00 2001 From: Daisuke Taniwaki Date: Tue, 12 Mar 2019 21:12:54 +0900 Subject: [PATCH 3/5] Update Gopkg.lock --- Gopkg.lock | 1 + 1 file changed, 1 insertion(+) diff --git a/Gopkg.lock b/Gopkg.lock index 4427aae777..8168877158 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -1553,6 +1553,7 @@ "github.com/nats-io/go-nats", "github.com/nats-io/go-nats-streaming", "github.com/nlopes/slack/slackevents", + "github.com/pkg/errors", "github.com/robfig/cron", "github.com/rs/zerolog", "github.com/satori/go.uuid", From eb01adcaf44e386a2d17b36f0346cc0b0464b5ec Mon Sep 17 00:00:00 2001 From: Daisuke Taniwaki Date: Wed, 13 Mar 2019 15:39:32 +0900 Subject: [PATCH 4/5] Fix tests --- Gopkg.lock | 4 ++- sensors/event-handler_test.go | 17 ++++++---- sensors/trigger_test.go | 63 ++++++++++++++++++++--------------- 3 files changed, 49 insertions(+), 35 deletions(-) diff --git a/Gopkg.lock b/Gopkg.lock index 8168877158..3959ca97a6 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -1291,12 +1291,13 @@ [[projects]] branch = "release-7.0" - digest = "1:6e37d35d68d68b1368b803617d346633830eb13b50120f68ead2c029c05eeea0" + digest = "1:7336dcb25cac9940ac84ea6c59c4c36d1b6b6ca4a3aba38d6d8c74bc53699a40" name = "k8s.io/client-go" packages = [ "discovery", "discovery/fake", "dynamic", + "dynamic/fake", "informers", "informers/admissionregistration", "informers/admissionregistration/v1alpha1", @@ -1596,6 +1597,7 @@ "k8s.io/client-go/discovery", "k8s.io/client-go/discovery/fake", "k8s.io/client-go/dynamic", + "k8s.io/client-go/dynamic/fake", "k8s.io/client-go/informers", "k8s.io/client-go/informers/core/v1", "k8s.io/client-go/kubernetes", diff --git a/sensors/event-handler_test.go b/sensors/event-handler_test.go index 18e4f1942b..e22fa1b3d0 100644 --- a/sensors/event-handler_test.go +++ b/sensors/event-handler_test.go @@ -37,6 +37,7 @@ import ( var sensorStr = `apiVersion: argoproj.io/v1alpha1 kind: Sensor metadata: + namespace: argo-events name: test-sensor labels: sensors.argoproj.io/sensor-controller-instanceid: argo-events @@ -78,6 +79,13 @@ spec: image: "docker/whalesay:latest" name: whalesay` +var podResourceList = metav1.APIResourceList{ + GroupVersion: metav1.GroupVersion{Group: "", Version: "v1"}.String(), + APIResources: []metav1.APIResource{ + {Kind: "Pod", Namespaced: true, Name: "pods", SingularName: "pod", Group: "", Version: "v1", Verbs: []string{"create", "get"}}, + }, +} + func getSensor() (*v1alpha1.Sensor, error) { var sensor v1alpha1.Sensor err := yaml.Unmarshal([]byte(sensorStr), &sensor) @@ -101,15 +109,10 @@ func (m *mockHttpWriter) WriteHeader(statusCode int) { func getsensorExecutionCtx(sensor *v1alpha1.Sensor) *sensorExecutionCtx { kubeClientset := fake.NewSimpleClientset() fakeDiscoveryClient := kubeClientset.Discovery().(*discoveryFake.FakeDiscovery) - resourceList := &metav1.APIResourceList{ - TypeMeta: metav1.TypeMeta{Kind: "Pod", APIVersion: "v1"}, - GroupVersion: metav1.GroupVersion{Group: "", Version: "v1"}.String(), - APIResources: []metav1.APIResource{{Kind: "Pod"}}, - } clientPool := &FakeClientPool{ - kubeClientset.Fake, + Fake: kubeClientset.Fake, } - fakeDiscoveryClient.Resources = append(fakeDiscoveryClient.Resources, resourceList) + fakeDiscoveryClient.Resources = append(fakeDiscoveryClient.Resources, &podResourceList) return &sensorExecutionCtx{ kubeClient: kubeClientset, discoveryClient: fakeDiscoveryClient, diff --git a/sensors/trigger_test.go b/sensors/trigger_test.go index 07c0cc6e5a..b52fd6dd8d 100644 --- a/sensors/trigger_test.go +++ b/sensors/trigger_test.go @@ -30,6 +30,7 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/watch" "k8s.io/client-go/dynamic" + dynamicfake "k8s.io/client-go/dynamic/fake" "k8s.io/client-go/kubernetes/fake" kTesting "k8s.io/client-go/testing" "k8s.io/client-go/util/flowcontrol" @@ -197,20 +198,14 @@ func (p *FakeClientPool) ClientForGroupVersionKind(kind schema.GroupVersionKind) } var testWf = ` -apiVersion: argoproj.io/v1alpha1 -kind: Workflow +apiVersion: v1 +kind: Pod metadata: generateName: hello-world- spec: -entrypoint: whalesay -templates: -- name: whalesay -container: - args: - - "hello world" - command: - - cowsay - image: "docker/whalesay:latest" + containers: + - name: whalesay + image: "docker/whalesay:latest" ` var testTrigger = v1alpha1.Trigger{ @@ -218,9 +213,8 @@ var testTrigger = v1alpha1.Trigger{ Resource: &v1alpha1.ResourceObject{ Namespace: corev1.NamespaceDefault, GroupVersionKind: v1alpha1.GroupVersionKind{ - Group: "argoproj.io", - Version: "v1alpha1", - Kind: "workflow", + Version: "v1", + Kind: "Pod", }, Source: v1alpha1.ArtifactLocation{ Inline: &testWf, @@ -247,35 +241,50 @@ func TestCreateResourceObject(t *testing.T) { testSensor, err := getSensor() convey.So(err, convey.ShouldBeNil) soc := getsensorExecutionCtx(testSensor) - - rObj := testTrigger.Resource.DeepCopy() + fakeclient := soc.clientPool.(*FakeClientPool).Fake + dynamicClient := dynamicfake.FakeResourceClient{Resource: schema.GroupVersionResource{Version: "v1", Resource: "pods"}, Fake: &fakeclient} convey.Convey("Given a pod", func() { - namespace := "foo" + rObj := testTrigger.Resource.DeepCopy() + rObj.Namespace = "foo" pod := &corev1.Pod{ TypeMeta: metav1.TypeMeta{Kind: "Pod", APIVersion: "v1"}, - ObjectMeta: metav1.ObjectMeta{Namespace: namespace, Name: "my-pod"}, + ObjectMeta: metav1.ObjectMeta{Namespace: rObj.Namespace, Name: "my-pod"}, } - uObj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(pod) + uObj, err := getUnstructuredPod(pod) convey.So(err, convey.ShouldBeNil) - err = soc.createResourceObject(rObj, &unstructured.Unstructured{Object: uObj}) + + err = soc.createResourceObject(rObj, uObj) convey.So(err, convey.ShouldBeNil) - pod, err = soc.kubeClient.CoreV1().Pods(namespace).Get(pod.Name, metav1.GetOptions{}) + + unstructuredPod, err := dynamicClient.Get(pod.Name, metav1.GetOptions{}) convey.So(err, convey.ShouldBeNil) - convey.So(pod.Namespace, convey.ShouldEqual, namespace) + convey.So(unstructuredPod.GetNamespace(), convey.ShouldEqual, rObj.Namespace) }) convey.Convey("Given a pod without namespace, use sensor namespace", func() { + rObj := testTrigger.Resource.DeepCopy() + rObj.Namespace = "" pod := &corev1.Pod{ TypeMeta: metav1.TypeMeta{Kind: "Pod", APIVersion: "v1"}, - ObjectMeta: metav1.ObjectMeta{Name: "my-pod"}, + ObjectMeta: metav1.ObjectMeta{Name: "my-pod-without-namespace"}, } - uObj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(pod) + uObj, err := getUnstructuredPod(pod) convey.So(err, convey.ShouldBeNil) - err = soc.createResourceObject(rObj, &unstructured.Unstructured{Object: uObj}) + + err = soc.createResourceObject(rObj, uObj) convey.So(err, convey.ShouldBeNil) - pod, err = soc.kubeClient.CoreV1().Pods(testSensor.Namespace).Get(pod.Name, metav1.GetOptions{}) + + unstructuredPod, err := dynamicClient.Get(pod.Name, metav1.GetOptions{}) convey.So(err, convey.ShouldBeNil) - convey.So(pod.Namespace, convey.ShouldEqual, testSensor.Namespace) + convey.So(unstructuredPod.GetNamespace(), convey.ShouldEqual, testSensor.Namespace) }) }) } + +func getUnstructuredPod(pod *corev1.Pod) (*unstructured.Unstructured, error) { + obj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(pod) + if err != nil { + return nil, err + } + return &unstructured.Unstructured{Object: obj}, nil +} From f62c07b48901385c69ad482745ebb6d0a423fb19 Mon Sep 17 00:00:00 2001 From: Daisuke Taniwaki Date: Wed, 13 Mar 2019 15:39:43 +0900 Subject: [PATCH 5/5] Make condition easier to read --- sensors/trigger.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sensors/trigger.go b/sensors/trigger.go index e5fd0173b6..9d9a1fdfcb 100644 --- a/sensors/trigger.go +++ b/sensors/trigger.go @@ -235,7 +235,7 @@ func (sec *sensorExecutionCtx) createResourceObject(resource *v1alpha1.ResourceO return fmt.Errorf("failed to create resource object. err: %+v", err) } sec.log.Info().Str("kind", liveObj.GetKind()).Str("name", liveObj.GetName()).Msg("created object") - if !errors.IsAlreadyExists(err) { + if err == nil || !errors.IsAlreadyExists(err) { return err } liveObj, err = reIf.Get(obj.GetName(), metav1.GetOptions{})