refactor: merge operator changes

[charlie-haley]

This is a pretty big PR, so I'll try to summarise the changes here...

• Instead of trying to upgrade operator-sdk, I just rebuilt the project using native kubebuilder instead
• I've changed the API Group to streaming.benthos.dev/v1alpha1 to be more generic (in case we need to extend it in the future)
• Less permissive RBAC
• The controller watches ConfigMap's and Deployment's it creates and reconciles based on changes made to them.
• Deployment's` created by the operator will be rolled out if the config changes.
• Add status to the BenthosPipeline CR with printcolumn to pretty print these with kubectl
• Added a Tiltfile to make local development easier
• Removed the Helm chart in the repo (we should generate a new one as part of a separate PR)
• Map config from a string (ideally, it would be nice to use actual structs but at the moment the config structs in benthos are under internal)

Also, what do we think about changing the title of the project to benthos-operator so it's more consistent with other operators? (We can still keep the the kube blob 😃)

[mfamador]

Hey, @charlie-haley, thanks for your contribution.

This is a pretty big PR, so I'll try to summarise the changes here...

Indeed, overall it seems ok , I'll give it a more detailed look during the week, but I'll try to summarise my first thoughts.

Instead of trying to upgrade operator-sdk, I just rebuilt the project using native kubebuilder instead

First impression, nothing against it, I've used operator-sdk to kickstart the project faster, I'll take a look at kubebuilder to understand if it's a better alternative.

I've changed the API Group to streaming.benthos.dev/v1alpha1 to be more generic (in case we need to extend it in the future)

I'd prefer to keep a reference to captain, captain.benthos.dev should be ok. We can have pipeline that are not stream at all. And keep Pipeline since the api group already states benthos. Similar to Argo:

apiVersion: argoproj.io/v1alpha1
kind: Workflow

I'd keep:

apiVersion: captain.benthos.dev/v1alpha1
kind: Pipeline

Less permissive RBAC

lgtm

The controller watches ConfigMap's and Deployment's it creates and reconciles based on changes made to them.

lgtm

Deployment's` created by the operator will be rolled out if the config changes.

this is not the default behaviour of a kubernetes deployment with a config map, so we better think it over a little bit, my first impression would be not to do it.

Add status to the BenthosPipeline CR with printcolumn to pretty print these with kubectl

lgtm

Added a Tiltfile to make local development easier

lgtm

Removed the Helm chart in the repo (we should generate a new one as part of a separate PR)

let's keep it for now as a placeholder while we don't have another.

Map config from a string (ideally, it would be nice to use actual structs but at the moment the config structs in benthos are under internal)

lgtm

Also, what do we think about changing the title of the project to benthos-operator so it's more consistent with other operators? (We can still keep the the kube blob 😃)

let's keep benthos captain, there are others operators not following that pattern - like Argo wfs and Argo events

Thanks!

[charlie-haley]

First impression, nothing against it, I've used operator-sdk to kickstart the project faster, I'll take a look at kubebuilder to understand if it's a better alternative.

operator-sdk for Go projects is pretty much just a wrapper around kubebuilder nowadays, after v1.0.0 they moved away from their proprietary code and it just uses kubebuilder under the hood now. In my opinion, we don't really get any benefit from using it over kubebuilder

---

this is not the default behaviour of a kubernetes deployment with a config map, so we better think it over a little bit, my first impression would be not to do it.

It isn't the default behaviour, but, there isn't much point in building or using the operator without this feature, otherwise you may as well just deploy the manifests directly.

Also, it's worth noting that the benthos-helm-chart performs a roll out when the config changes by using the checksum/config annotation. I do think we should add configuration options for how this rollout is handled in the future though. e.g strategy, maxSurge, paused etc.

---

And keep Pipeline since the api group already states benthos.

I'd prefer to keep a reference to captain, captain.benthos.dev should be ok. We can have pipeline that are not stream at all.

let's keep it for now as a placeholder while we don't have another.

let's keep benthos captain, there are others operators not following that pattern - like Argo wfs and Argo events

I'm happy with all these, I'll push an update with those changes

[charlie-haley]

Just updated the PR with the feedback. Also, just to note I realised I could use the apiextensionsv1.JSON type when defining the struct for the Pipeline spec.

This means instead of having to use a multi-line YAML string for the Benthos config we can use an actual YAML object.

Old:

apiVersion: captain.benthos.dev/v1alpha1
kind: Pipeline
metadata:
  name: pipeline-sample
spec:
  replicas: 1
  config: |
    input:
      generate:
        mapping: root = "woof"
        interval: 5s
        count: 0

    pipeline:
      processors:
        - mapping: root = content().uppercase()

    output:
      stdout: {}

New:

apiVersion: captain.benthos.dev/v1alpha1
kind: Pipeline
metadata:
  name: pipeline-sample
spec:
  replicas: 1
  config:
    input:
      generate:
        mapping: root = "woof"
        interval: 5s
        count: 0

    pipeline:
      processors:
        - mapping: root = content().uppercase()

    output:
      stdout: {}

[mfamador]

That's great if we ever need to overlay a config on different environments.

[charlie-haley]

Hey @mfamador, just pinging to see if there's any extra feedback on this PR or if it's good to merge?