Skip to content

Commit

Permalink
Refactor the way Triggers creates child resources to follow Knative c…
Browse files Browse the repository at this point in the history 
…onventions.

Generally in Knative our controller structure follows the layout:
```
pkg/
  reconciler/
    {resource-name}/
      controller.go      # The *controller.Impl ctor (deals with informers and workqueues)
      {resource-name}.go # The typed reconciler (deals with listers and clients)
      resources/
        {child-resource-name}.go  # Logic for building this child resource.
        ...
        names/
          names.go # Optionally holds methods for naming child resources.
```

The main piece of this that this change start to move things towards is the
`resources/` sub-directory (I did not start a `names/` sub-directory).
  • Loading branch information
@mattmoor
mattmoor committed Aug 26, 2021
1 parent 43ff1e7 commit c3fd34b
Show file tree
Hide file tree
Showing 12 changed files with 743 additions and 397 deletions.
23 changes: 12 additions & 11 deletions cmd/controller/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"os"

"github.com/tektoncd/triggers/pkg/reconciler/clusterinterceptor"
elresources "github.com/tektoncd/triggers/pkg/reconciler/eventlistener/resources"

corev1 "k8s.io/api/core/v1"
"knative.dev/pkg/injection"
Expand All @@ -35,24 +36,24 @@ const (
)

var (
image = flag.String("el-image", eventlistener.DefaultImage, "The container image for the EventListener Pod.")
port = flag.Int("el-port", eventlistener.DefaultPort, "The container port for the EventListener to listen on.")
setSecurityContext = flag.Bool("el-security-context", eventlistener.DefaultSetSecurityContext, "Add a security context to the event listener deployment.")
readTimeOut = flag.Int64("el-readtimeout", eventlistener.DefaultReadTimeout, "The read timeout for EventListener Server.")
writeTimeOut = flag.Int64("el-writetimeout", eventlistener.DefaultWriteTimeout, "The write timeout for EventListener Server.")
idleTimeOut = flag.Int64("el-idletimeout", eventlistener.DefaultIdleTimeout, "The idle timeout for EventListener Server.")
timeOutHandler = flag.Int64("el-timeouthandler", eventlistener.DefaultTimeOutHandler, "The timeout for Timeout Handler of EventListener Server.")
periodSeconds = flag.Int("period-seconds", eventlistener.DefaultPeriodSeconds, "The Period Seconds for the EventListener Liveness and Readiness Probes.")
failureThreshold = flag.Int("failure-threshold", eventlistener.DefaultFailureThreshold, "The Failure Threshold for the EventListener Liveness and Readiness Probes.")
image = flag.String("el-image", elresources.DefaultImage, "The container image for the EventListener Pod.")
port = flag.Int("el-port", elresources.DefaultPort, "The container port for the EventListener to listen on.")
setSecurityContext = flag.Bool("el-security-context", elresources.DefaultSetSecurityContext, "Add a security context to the event listener deployment.")
readTimeOut = flag.Int64("el-readtimeout", elresources.DefaultReadTimeout, "The read timeout for EventListener Server.")
writeTimeOut = flag.Int64("el-writetimeout", elresources.DefaultWriteTimeout, "The write timeout for EventListener Server.")
idleTimeOut = flag.Int64("el-idletimeout", elresources.DefaultIdleTimeout, "The idle timeout for EventListener Server.")
timeOutHandler = flag.Int64("el-timeouthandler", elresources.DefaultTimeOutHandler, "The timeout for Timeout Handler of EventListener Server.")
periodSeconds = flag.Int("period-seconds", elresources.DefaultPeriodSeconds, "The Period Seconds for the EventListener Liveness and Readiness Probes.")
failureThreshold = flag.Int("failure-threshold", elresources.DefaultFailureThreshold, "The Failure Threshold for the EventListener Liveness and Readiness Probes.")

staticResourceLabels = eventlistener.DefaultStaticResourceLabels
staticResourceLabels = elresources.DefaultStaticResourceLabels
systemNamespace = os.Getenv("SYSTEM_NAMESPACE")
)

func main() {
cfg := injection.ParseAndGetRESTConfigOrDie()

c := eventlistener.Config{
c := elresources.Config{
Image: image,
Port: port,
SetSecurityContext: setSecurityContext,
Expand Down
3 changes: 2 additions & 1 deletion pkg/reconciler/eventlistener/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
eventlistenerinformer "github.com/tektoncd/triggers/pkg/client/injection/informers/triggers/v1beta1/eventlistener"
eventlistenerreconciler "github.com/tektoncd/triggers/pkg/client/injection/reconciler/triggers/v1beta1/eventlistener"
dynamicduck "github.com/tektoncd/triggers/pkg/dynamic"
"github.com/tektoncd/triggers/pkg/reconciler/eventlistener/resources"
"k8s.io/client-go/tools/cache"
duckinformer "knative.dev/pkg/client/injection/ducks/duck/v1/podspecable"
kubeclient "knative.dev/pkg/client/injection/kube/client"
Expand All @@ -38,7 +39,7 @@ import (
)

// NewController creates a new instance of an EventListener controller.
func NewController(config Config) func(context.Context, configmap.Watcher) *controller.Impl {
func NewController(config resources.Config) func(context.Context, configmap.Watcher) *controller.Impl {
return func(ctx context.Context, cmw configmap.Watcher) *controller.Impl {
logger := logging.FromContext(ctx)
dynamicclientset := dynamicclient.Get(ctx)
Expand Down
118 changes: 12 additions & 106 deletions pkg/reconciler/eventlistener/eventlistener.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import (
eventlistenerreconciler "github.com/tektoncd/triggers/pkg/client/injection/reconciler/triggers/v1beta1/eventlistener"
listers "github.com/tektoncd/triggers/pkg/client/listers/triggers/v1beta1"
dynamicduck "github.com/tektoncd/triggers/pkg/dynamic"
"github.com/tektoncd/triggers/pkg/reconciler/eventlistener/resources"
"golang.org/x/xerrors"
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
Expand All @@ -55,7 +56,6 @@ import (
"knative.dev/pkg/kmeta"
"knative.dev/pkg/logging"
"knative.dev/pkg/metrics"
"knative.dev/pkg/network"
"knative.dev/pkg/ptr"
pkgreconciler "knative.dev/pkg/reconciler"
)
Expand Down Expand Up @@ -101,7 +101,7 @@ type Reconciler struct {
serviceLister corev1lister.ServiceLister

// config is the configuration options that the Reconciler accepts.
config Config
config resources.Config
podspecableTracker dynamicduck.ListableTracker
onlyOnce sync.Once
}
Expand Down Expand Up @@ -183,36 +183,15 @@ func reconcileObjectMeta(existing *metav1.ObjectMeta, desired metav1.ObjectMeta)
}

func (r *Reconciler) reconcileService(ctx context.Context, el *v1beta1.EventListener) error {
// for backward compatibility with original behavior
var serviceType corev1.ServiceType
if el.Spec.Resources.KubernetesResource != nil && el.Spec.Resources.KubernetesResource.ServiceType != "" {
serviceType = el.Spec.Resources.KubernetesResource.ServiceType
}

servicePort := getServicePort(el, r.config)
metricsPort := corev1.ServicePort{
Name: eventListenerMetricsPortName,
Protocol: corev1.ProtocolTCP,
Port: int32(9000),
TargetPort: intstr.IntOrString{
IntVal: int32(eventListenerMetricsPort),
},
}
service := resources.MakeService(el, r.config)

service := &corev1.Service{
ObjectMeta: generateObjectMeta(el, r.config.StaticResourceLabels),
Spec: corev1.ServiceSpec{
Selector: GenerateResourceLabels(el.Name, r.config.StaticResourceLabels),
Type: serviceType,
Ports: []corev1.ServicePort{servicePort, metricsPort}},
}
existingService, err := r.serviceLister.Services(el.Namespace).Get(el.Status.Configuration.GeneratedResourceName)
switch {
case err == nil:
// Determine if reconciliation has to occur
updated := reconcileObjectMeta(&existingService.ObjectMeta, service.ObjectMeta)
el.Status.SetExistsCondition(v1beta1.ServiceExists, nil)
el.Status.SetAddress(listenerHostname(service.Name, el.Namespace, int(servicePort.Port)))
el.Status.SetAddress(resources.ListenerHostname(el, r.config))
if !reflect.DeepEqual(existingService.Spec.Selector, service.Spec.Selector) {
existingService.Spec.Selector = service.Spec.Selector
updated = true
Expand Down Expand Up @@ -243,7 +222,7 @@ func (r *Reconciler) reconcileService(ctx context.Context, el *v1beta1.EventList
logging.FromContext(ctx).Errorf("Error creating EventListener Service: %s", err)
return err
}
el.Status.SetAddress(listenerHostname(service.Name, el.Namespace, int(servicePort.Port)))
el.Status.SetAddress(resources.ListenerHostname(el, r.config))
logging.FromContext(ctx).Infof("Created EventListener Service %s in Namespace %s", service.Name, el.Namespace)
default:
logging.FromContext(ctx).Error(err)
Expand Down Expand Up @@ -510,7 +489,7 @@ func (r *Reconciler) reconcileCustomObject(ctx context.Context, el *v1beta1.Even
SuccessThreshold: 1,
}

podlabels := kmeta.UnionMaps(el.Labels, GenerateResourceLabels(el.Name, r.config.StaticResourceLabels))
podlabels := kmeta.UnionMaps(el.Labels, resources.GenerateLabels(el.Name, r.config.StaticResourceLabels))

podlabels = kmeta.UnionMaps(podlabels, customObjectData.Labels)

Expand Down Expand Up @@ -720,14 +699,14 @@ func (r *Reconciler) reconcileCustomObject(ctx context.Context, el *v1beta1.Even
return nil
}

func getDeployment(el *v1beta1.EventListener, container corev1.Container, c Config) *appsv1.Deployment {
func getDeployment(el *v1beta1.EventListener, container corev1.Container, c resources.Config) *appsv1.Deployment {
var (
tolerations []corev1.Toleration
nodeSelector, annotations, podlabels map[string]string
serviceAccountName string
securityContext corev1.PodSecurityContext
)
podlabels = kmeta.UnionMaps(el.Labels, GenerateResourceLabels(el.Name, c.StaticResourceLabels))
podlabels = kmeta.UnionMaps(el.Labels, resources.GenerateLabels(el.Name, c.StaticResourceLabels))

serviceAccountName = el.Spec.ServiceAccountName

Expand Down Expand Up @@ -781,11 +760,11 @@ func getDeployment(el *v1beta1.EventListener, container corev1.Container, c Conf
}

return &appsv1.Deployment{
ObjectMeta: generateObjectMeta(el, c.StaticResourceLabels),
ObjectMeta: resources.ObjectMeta(el, c.StaticResourceLabels),
Spec: appsv1.DeploymentSpec{
Replicas: replicas,
Selector: &metav1.LabelSelector{
MatchLabels: GenerateResourceLabels(el.Name, c.StaticResourceLabels),
MatchLabels: resources.GenerateLabels(el.Name, c.StaticResourceLabels),
},
Template: corev1.PodTemplateSpec{
ObjectMeta: metav1.ObjectMeta{
Expand All @@ -805,7 +784,7 @@ func getDeployment(el *v1beta1.EventListener, container corev1.Container, c Conf
}
}

func addCertsForSecureConnection(container corev1.Container, c Config) corev1.Container {
func addCertsForSecureConnection(container corev1.Container, c resources.Config) corev1.Container {
var elCert, elKey string
certEnv := map[string]*corev1.EnvVarSource{}
for i := range container.Env {
Expand Down Expand Up @@ -859,7 +838,7 @@ func addCertsForSecureConnection(container corev1.Container, c Config) corev1.Co
return container
}

func getContainer(el *v1beta1.EventListener, c Config, pod *duckv1.WithPod) corev1.Container {
func getContainer(el *v1beta1.EventListener, c resources.Config, pod *duckv1.WithPod) corev1.Container {
var resources corev1.ResourceRequirements
env := []corev1.EnvVar{}
if el.Spec.Resources.KubernetesResource != nil {
Expand Down Expand Up @@ -913,74 +892,6 @@ func getContainer(el *v1beta1.EventListener, c Config, pod *duckv1.WithPod) core
}
}

func getServicePort(el *v1beta1.EventListener, c Config) corev1.ServicePort {
var elCert, elKey string

servicePortName := eventListenerServicePortName
servicePortPort := *c.Port

certEnv := map[string]*corev1.EnvVarSource{}
if el.Spec.Resources.KubernetesResource != nil {
if len(el.Spec.Resources.KubernetesResource.Template.Spec.Containers) != 0 {
for i := range el.Spec.Resources.KubernetesResource.Template.Spec.Containers[0].Env {
certEnv[el.Spec.Resources.KubernetesResource.Template.Spec.Containers[0].Env[i].Name] =
el.Spec.Resources.KubernetesResource.Template.Spec.Containers[0].Env[i].ValueFrom
}
}
}

if v, ok := certEnv["TLS_CERT"]; ok {
elCert = v.SecretKeyRef.Key
} else {
elCert = ""
}
if v, ok := certEnv["TLS_KEY"]; ok {
elKey = v.SecretKeyRef.Key
} else {
elKey = ""
}

if elCert != "" && elKey != "" {
servicePortName = eventListenerServiceTLSPortName
if *c.Port == DefaultPort {
// We return port 8443 if TLS is enabled and the default HTTP port is set.
// This effectively makes 8443 the default HTTPS port unless a user explicitly sets a different port.
servicePortPort = 8443
}
}

return corev1.ServicePort{
Name: servicePortName,
Protocol: corev1.ProtocolTCP,
Port: int32(servicePortPort),
TargetPort: intstr.IntOrString{
IntVal: int32(eventListenerContainerPort),
},
}
}

// GenerateResourceLabels generates the labels to be used on all generated resources.
func GenerateResourceLabels(eventListenerName string, staticResourceLabels map[string]string) map[string]string {
resourceLabels := make(map[string]string, len(staticResourceLabels)+1)
for k, v := range staticResourceLabels {
resourceLabels[k] = v
}
resourceLabels["eventlistener"] = eventListenerName
return resourceLabels
}

// generateObjectMeta generates the object meta that should be used by all
// resources generated by the EventListener reconciler
func generateObjectMeta(el *v1beta1.EventListener, staticResourceLabels map[string]string) metav1.ObjectMeta {
return metav1.ObjectMeta{
Namespace: el.Namespace,
Name: el.Status.Configuration.GeneratedResourceName,
OwnerReferences: []metav1.OwnerReference{*kmeta.NewControllerRef(el)},
Labels: kmeta.UnionMaps(el.Labels, GenerateResourceLabels(el.Name, staticResourceLabels)),
Annotations: el.Annotations,
}
}

// wrapError wraps errors together. If one of the errors is nil, the other is
// returned.
func wrapError(err1, err2 error) error {
Expand All @@ -993,11 +904,6 @@ func wrapError(err1, err2 error) error {
return xerrors.Errorf("%s : %s", err1.Error(), err2.Error())
}

// listenerHostname returns the intended hostname for the EventListener service.
func listenerHostname(name, namespace string, port int) string {
return network.GetServiceHostname(name, namespace) + fmt.Sprintf(":%d", port)
}

func defaultLoggingConfigMap() *corev1.ConfigMap {
return &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{Name: eventListenerConfigMapName},
Expand Down
Loading

0 comments on commit c3fd34b

Please sign in to comment.