konpyutaika / nifikop

The NiFiKop NiFi Kubernetes operator makes it easy to run Apache NiFi on Kubernetes. Apache NiFI is a free, open-source solution that support powerful and scalable directed graphs of data routing, transformation, and system mediation logic.
https://konpyutaika.github.io/nifikop/
Apache License 2.0
123 stars 40 forks source link

[Feature/NifiCluster] Add ability to set container port network protocol #320

Closed mh013370 closed 8 months ago

mh013370 commented 8 months ago
Q A
Bug fix? no
New feature? yes
API breaks? no
Deprecations? no
Related tickets fixes #319
License Apache 2.0

What's in this PR?

Adds the ability to set the NiFi container port network protocol. This field is new and optional, defaulting to TCP. If one wished to set the protocol for all container ports, you would use the following:

  listenersConfig:
    internalListeners:
      - type: "https"
        name: "https"
        containerPort: 8443
        protocol: "TCP"
      - type: "cluster"
        name: "cluster"
        containerPort: 6007
        protocol: "TCP"
      - type: "s2s"
        name: "s2s"
        containerPort: 10000
        protocol: "TCP"
      - type: "prometheus"
        name: "prometheus"
        containerPort: 9090
        protocol: "TCP"
      - type: "load-balance"
        name: "load-balance"
        containerPort: 6342
        protocol: "TCP"
      - name: "my-custom-listener-port"
        containerPort: 1234
        protocol: "UDP"
    sslSecrets:
      tlsSecretName: "test-nifikop"
      create: true

Why?

It's not currently possible to set anything other than TCP.

Checklist

mh013370 commented 8 months ago

@juldrixx : FYI This requires pushing a new build image, merging #321, and then rebasing this PR on master.

juldrixx commented 8 months ago

That still doesn't work. If you change the default value to UDP is still defaulting on TCP.

mh013370 commented 8 months ago

That still doesn't work. If you change the default value to UDP is still defaulting on TCP.

well isn't this a fun problem šŸ˜„

juldrixx commented 8 months ago

That still doesn't work. If you change the default value to UDP is still defaulting on TCP.

well isn't this a fun problem šŸ˜„

Even more when we have

// +kubebuilder:validation:Enum={"DO_NOT_LOAD_BALANCE","PARTITION_BY_ATTRIBUTE","ROUND_ROBIN","SINGLE"}
type ConnectionLoadBalanceStrategy string
// how to load balance the data in this Connection across the nodes in the cluster.
// +kubebuilder:default="DO_NOT_LOAD_BALANCE"
LoadBalanceStrategy ConnectionLoadBalanceStrategy `json:"loadBalanceStrategy,omitempty"

that gives us

loadBalanceStrategy:
 default: DO_NOT_LOAD_BALANCE
 enum:
 - DO_NOT_LOAD_BALANCE
 - PARTITION_BY_ATTRIBUTE
 - ROUND_ROBIN
 - SINGLE
 type: string

And I can change the default value

juldrixx commented 8 months ago

If you change the type from corev1.Protocol to string it works šŸ˜•

juldrixx commented 8 months ago

I guess, you can rollback to how it was before without default annotation. It's fine.

mh013370 commented 8 months ago

If you change the type from corev1.Protocol to string it works šŸ˜•

Hmm. I was hoping to use the k8s types for ease of use and to avoid converting to/from string and corev1.Protocol, but i think due to the // +enum annotation on corev1.Protocol we might not be able to.

I can either change it to a string or remove the default. Which would you prefer?

juldrixx commented 8 months ago

If you change the type from corev1.Protocol to string it works šŸ˜•

Hmm. I was hoping to use the k8s types for ease of use and to avoid converting to/from string and corev1.Protocol, but i think due to the // +enum annotation on corev1.Protocol we might not be able to.

I can either change it to a string or remove the default. Which would you prefer?

Remove the default, I asked on Kubebuilder slack how to do it. I'm still waiting for an answer, but depending on the response, I'll make the appropriate change in the future.

mh013370 commented 8 months ago

If you change the type from corev1.Protocol to string it works šŸ˜•

Hmm. I was hoping to use the k8s types for ease of use and to avoid converting to/from string and corev1.Protocol, but i think due to the // +enum annotation on corev1.Protocol we might not be able to. I can either change it to a string or remove the default. Which would you prefer?

Remove the default, I asked on Kubebuilder slack how to do it. I'm still waiting for an answer, but depending on the response, I'll make the appropriate change in the future.

pushed