From c27a72affa1ab2de4e493a10bc37102499700971 Mon Sep 17 00:00:00 2001 From: Narthana Epa Date: Fri, 28 Apr 2023 16:37:43 +1000 Subject: [PATCH 1/4] Change the ClusterRole to a Role Let's try to make everything live in the same namespace --- charts/agent-stack-k8s/templates/rbac.yaml.tpl | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/charts/agent-stack-k8s/templates/rbac.yaml.tpl b/charts/agent-stack-k8s/templates/rbac.yaml.tpl index f90d4bda..cb300294 100644 --- a/charts/agent-stack-k8s/templates/rbac.yaml.tpl +++ b/charts/agent-stack-k8s/templates/rbac.yaml.tpl @@ -1,7 +1,8 @@ apiVersion: rbac.authorization.k8s.io/v1 -kind: ClusterRole +kind: Role metadata: name: {{ .Release.Name }} + namespace: {{ .Release.Namespace }} rules: - apiGroups: - batch @@ -23,12 +24,12 @@ rules: - watch --- apiVersion: rbac.authorization.k8s.io/v1 -kind: ClusterRoleBinding +kind: RoleBinding metadata: name: {{ .Release.Name }} roleRef: apiGroup: rbac.authorization.k8s.io - kind: ClusterRole + kind: Role name: {{ .Release.Name }} subjects: - kind: ServiceAccount From 03e89d4cd9822b17adcb6cbb2a7e252f58f527b4 Mon Sep 17 00:00:00 2001 From: Narthana Epa Date: Sat, 29 Apr 2023 10:18:54 +1000 Subject: [PATCH 2/4] Add namespace to informer --- internal/controller/controller.go | 2 +- internal/controller/scheduler/scheduler.go | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/controller/controller.go b/internal/controller/controller.go index e33b728d..5862aa15 100644 --- a/internal/controller/controller.go +++ b/internal/controller/controller.go @@ -49,7 +49,7 @@ func Run( }) limiter := scheduler.NewLimiter(logger.Named("limiter"), sched, cfg.MaxInFlight) - informerFactory, err := scheduler.NewInformerFactory(k8sClient, cfg.Tags) + informerFactory, err := scheduler.NewInformerFactory(k8sClient, cfg.Namespace, cfg.Tags) if err != nil { logger.Fatal("failed to create informer", zap.Error(err)) } diff --git a/internal/controller/scheduler/scheduler.go b/internal/controller/scheduler/scheduler.go index 9f96172d..a9c9c9c2 100644 --- a/internal/controller/scheduler/scheduler.go +++ b/internal/controller/scheduler/scheduler.go @@ -48,6 +48,7 @@ func New(logger *zap.Logger, client kubernetes.Interface, cfg Config) *worker { // returns an informer factory configured to watch resources (pods, jobs) created by the scheduler func NewInformerFactory( k8s kubernetes.Interface, + namespace string, tags []string, ) (informers.SharedInformerFactory, error) { hasTag, err := labels.NewRequirement(config.TagLabel, selection.In, config.TagsToLabels(tags)) @@ -61,6 +62,7 @@ func NewInformerFactory( factory := informers.NewSharedInformerFactoryWithOptions( k8s, 0, + informers.WithNamespace(namespace), informers.WithTweakListOptions(func(opt *metav1.ListOptions) { opt.LabelSelector = labels.NewSelector().Add(*hasTag, *hasUUID).String() }), From d239ecd612eb0846f7ee6af90678737fd3ca8438 Mon Sep 17 00:00:00 2001 From: Narthana Epa Date: Sat, 29 Apr 2023 10:21:35 +1000 Subject: [PATCH 3/4] Move informer factor creation into controller package --- internal/controller/controller.go | 32 +++++++++++++++++++++- internal/controller/scheduler/scheduler.go | 28 ------------------- 2 files changed, 31 insertions(+), 29 deletions(-) diff --git a/internal/controller/controller.go b/internal/controller/controller.go index 5862aa15..2a2be600 100644 --- a/internal/controller/controller.go +++ b/internal/controller/controller.go @@ -2,6 +2,7 @@ package controller import ( "context" + "fmt" "net/http" _ "net/http/pprof" "time" @@ -10,6 +11,10 @@ import ( "github.com/buildkite/agent-stack-k8s/v2/internal/controller/monitor" "github.com/buildkite/agent-stack-k8s/v2/internal/controller/scheduler" "go.uber.org/zap" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/selection" + "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes" ) @@ -49,7 +54,7 @@ func Run( }) limiter := scheduler.NewLimiter(logger.Named("limiter"), sched, cfg.MaxInFlight) - informerFactory, err := scheduler.NewInformerFactory(k8sClient, cfg.Namespace, cfg.Tags) + informerFactory, err := NewInformerFactory(k8sClient, cfg.Namespace, cfg.Tags) if err != nil { logger.Fatal("failed to create informer", zap.Error(err)) } @@ -79,3 +84,28 @@ func Run( logger.Info("monitor failed", zap.Error(err)) } } + +// returns an informer factory configured to watch resources (pods, jobs) created by the scheduler +func NewInformerFactory( + k8s kubernetes.Interface, + namespace string, + tags []string, +) (informers.SharedInformerFactory, error) { + hasTag, err := labels.NewRequirement(config.TagLabel, selection.In, config.TagsToLabels(tags)) + if err != nil { + return nil, fmt.Errorf("failed to build tag label selector for job manager: %w", err) + } + hasUUID, err := labels.NewRequirement(config.UUIDLabel, selection.Exists, nil) + if err != nil { + return nil, fmt.Errorf("failed to build uuid label selector for job manager: %w", err) + } + factory := informers.NewSharedInformerFactoryWithOptions( + k8s, + 0, + informers.WithNamespace(namespace), + informers.WithTweakListOptions(func(opt *metav1.ListOptions) { + opt.LabelSelector = labels.NewSelector().Add(*hasTag, *hasUUID).String() + }), + ) + return factory, nil +} diff --git a/internal/controller/scheduler/scheduler.go b/internal/controller/scheduler/scheduler.go index a9c9c9c2..7f47864c 100644 --- a/internal/controller/scheduler/scheduler.go +++ b/internal/controller/scheduler/scheduler.go @@ -18,9 +18,6 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/labels" - "k8s.io/apimachinery/pkg/selection" - "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes" "k8s.io/utils/pointer" ) @@ -45,31 +42,6 @@ func New(logger *zap.Logger, client kubernetes.Interface, cfg Config) *worker { } } -// returns an informer factory configured to watch resources (pods, jobs) created by the scheduler -func NewInformerFactory( - k8s kubernetes.Interface, - namespace string, - tags []string, -) (informers.SharedInformerFactory, error) { - hasTag, err := labels.NewRequirement(config.TagLabel, selection.In, config.TagsToLabels(tags)) - if err != nil { - return nil, fmt.Errorf("failed to build tag label selector for job manager: %w", err) - } - hasUUID, err := labels.NewRequirement(config.UUIDLabel, selection.Exists, nil) - if err != nil { - return nil, fmt.Errorf("failed to build uuid label selector for job manager: %w", err) - } - factory := informers.NewSharedInformerFactoryWithOptions( - k8s, - 0, - informers.WithNamespace(namespace), - informers.WithTweakListOptions(func(opt *metav1.ListOptions) { - opt.LabelSelector = labels.NewSelector().Add(*hasTag, *hasUUID).String() - }), - ) - return factory, nil -} - type KubernetesPlugin struct { PodSpec *corev1.PodSpec GitEnvFrom []corev1.EnvFromSource From af973adf88b0f62adc1660d9883ab651d66f9637 Mon Sep 17 00:00:00 2001 From: Narthana Epa Date: Sat, 29 Apr 2023 16:28:32 +1000 Subject: [PATCH 4/4] Suffix RBAC names with `-controller` --- charts/agent-stack-k8s/templates/deployment.yaml.tpl | 2 +- charts/agent-stack-k8s/templates/rbac.yaml.tpl | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/charts/agent-stack-k8s/templates/deployment.yaml.tpl b/charts/agent-stack-k8s/templates/deployment.yaml.tpl index c01d63f4..626f62e7 100644 --- a/charts/agent-stack-k8s/templates/deployment.yaml.tpl +++ b/charts/agent-stack-k8s/templates/deployment.yaml.tpl @@ -15,7 +15,7 @@ spec: checksum/config: {{ include (print $.Template.BasePath "/config.yaml.tpl") . | sha256sum }} checksum/secrets: {{ include (print $.Template.BasePath "/secrets.yaml.tpl") . | sha256sum }} spec: - serviceAccountName: {{ .Release.Name }} + serviceAccountName: {{ .Release.Name }}-controller nodeSelector: {{ toYaml $.Values.nodeSelector | indent 8 }} containers: diff --git a/charts/agent-stack-k8s/templates/rbac.yaml.tpl b/charts/agent-stack-k8s/templates/rbac.yaml.tpl index cb300294..691f697e 100644 --- a/charts/agent-stack-k8s/templates/rbac.yaml.tpl +++ b/charts/agent-stack-k8s/templates/rbac.yaml.tpl @@ -1,7 +1,7 @@ apiVersion: rbac.authorization.k8s.io/v1 kind: Role metadata: - name: {{ .Release.Name }} + name: {{ .Release.Name }}-controller namespace: {{ .Release.Namespace }} rules: - apiGroups: @@ -30,14 +30,14 @@ metadata: roleRef: apiGroup: rbac.authorization.k8s.io kind: Role - name: {{ .Release.Name }} + name: {{ .Release.Name }}-controller subjects: - kind: ServiceAccount - name: {{ .Release.Name }} + name: {{ .Release.Name }}-controller namespace: {{ .Release.Namespace }} --- apiVersion: v1 kind: ServiceAccount metadata: - name: {{ .Release.Name }} + name: {{ .Release.Name }}-controller namespace: {{ .Release.Namespace }}