Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

new config-override feature doesn't work #454

Closed
arnoldyahad opened this issue May 22, 2024 · 7 comments
Closed

new config-override feature doesn't work #454

arnoldyahad opened this issue May 22, 2024 · 7 comments

Comments

@arnoldyahad
Copy link

trying to use the new feature introduced by @fhussonnois : #429
(thanks for this contribution! 🙏 )

im following the example i see here:
https://github.com/streamthoughts/jikkou/blob/main/docs/content/en/docs/Releases/release-v0.34.0.md?plain=1#L119-L128

and i have a file:

kind: "KafkaConnector"
metadata:
  name: "s3-sink-overrides"
  labels:
    kafka.jikkou.io/connect-cluster: "my-connect-cluster"
  annotations: 
    jikkou.io/config-override: |
      { "url": "http://localhost:8083" }

which returns:

java.lang.NullPointerException
	at io.streamthoughts.jikkou.kafka.connect.validation.KafkaConnectorResourceValidation.validate(KafkaConnectorResourceValidation.java:50)
	at io.streamthoughts.jikkou.kafka.connect.validation.KafkaConnectorResourceValidation.validate(KafkaConnectorResourceValidation.java:24)
	at io.streamthoughts.jikkou.core.validation.Validation.validate(Validation.java:43)
	at io.streamthoughts.jikkou.core.validation.ValidationChain.validate(ValidationChain.java:63)
	at io.streamthoughts.jikkou.core.validation.ValidationChain.validate(ValidationChain.java:52)
	at io.streamthoughts.jikkou.core.DefaultApi.validate(DefaultApi.java:495)
	at io.streamthoughts.jikkou.core.DefaultApi.getDiff(DefaultApi.java:538)
	at io.streamthoughts.jikkou.core.DefaultApi.reconcile(DefaultApi.java:396)
	at io.streamthoughts.jikkou.client.command.reconcile.BaseResourceCommand.call(BaseResourceCommand.java:54)
	at io.streamthoughts.jikkou.client.command.reconcile.BaseResourceCommand.call(BaseResourceCommand.java:30)
	at picocli.CommandLine.executeUserObject(CommandLine.java:2041)
	at picocli.CommandLine.access$1500(CommandLine.java:148)
	at picocli.CommandLine$RunLast.executeUserObjectOfLastSubcommandWithSameParent(CommandLine.java:2461)
	at picocli.CommandLine$RunLast.handle(CommandLine.java:2453)
	at picocli.CommandLine$RunLast.handle(CommandLine.java:2415)
	at picocli.CommandLine$AbstractParseResultHandler.execute(CommandLine.java:2273)
	at picocli.CommandLine$RunLast.execute(CommandLine.java:2417)
	at io.streamthoughts.jikkou.client.Jikkou.executionStrategy(Jikkou.java:150)
	at picocli.CommandLine.execute(CommandLine.java:2170)
	at io.streamthoughts.jikkou.client.Jikkou.execute(Jikkou.java:140)
	at io.streamthoughts.jikkou.client.Jikkou.main(Jikkou.java:128)
	at java.base@21.0.2/java.lang.invoke.LambdaForm$DMH/sa346b79c.invokeStaticInit(LambdaForm$DMH)

Error: NullPointerException: null

if i dont use the annotation:

apiVersion: "kafka.jikkou.io/v1beta1"
kind: "KafkaConnector"
metadata:
  name: "s3-sink"
  labels:
    kafka.jikkou.io/connect-cluster: "my-connect-cluster"

it works just fine.

To Reproduce
use the configuration above(use any spec you want)
and use this version:

jikkou --version
Jikkou version "0.34.0" 2024-05-01
JVM: 21.0.2 (GraalVM Community Substrate VM 21.0.2+13)

Expected behavior
for the configuration with override to work

Runtime environment

  • OS: mac
  • Jikkou: 0.34.0
  • Docker version:
docker --version
Docker version 24.0.6-rd, build da4c87c
@fhussonnois
Copy link
Member

Hi @arnoldyahad, thank you for reporting this issue. There is a NullPointerException if the spec field is missing from the resource definition.

The minimal resource for Kafka Connect is:

apiVersion: "kafka.jikkou.io/v1beta1"
kind: "KafkaConnector"
metadata:
  name: "s3-sink"
  labels:
    kafka.jikkou.io/connect-cluster: "my-connect-cluster"
spec:
 connectorClass: <connect_class_name>
 tasksMax: 1
``

@arnoldyahad
Copy link
Author

hey @fhussonnois thanks for the quick response!

i am using a spec, i forgot to mention it since i thought its not related and i wanted to keep the focus on the actual issue/
here is an example of what i use:

spec:
  connectorClass: "io.confluent.connect.s3.S3SinkConnector"
  tasksMax: 1
  config:
    name: s3-sink
    connector.class: io.confluent.connect.s3.S3SinkConnector
    s3.region: us-east-1
    partition.duration.ms: '3600000'
    flush.size: '150000'
    topics: mytopicname
    s3.bucket.name: my-bucket

(this works when not using the new config-override)

i see you made a new commit: b49e2cd

was this the issue?

@fhussonnois
Copy link
Member

Hi @arnoldyahad, yes the pre-release should be fixed. You should be able to use the docker image with tag main.

@arnoldyahad
Copy link
Author

hey @fhussonnois thanks a lot for fixing it.
I have downloaded the docker image with the main tag and i currently don't get the same error as before.

but I do get other errors:
when i want to use just the config-override without any cluster name i get this error:

 +----+------------------------------+------------------------------------------------------------+---------+                                                                          │
 |    | NAME                         | ERROR                                                      | DETAILS |                                                                          │
 +----+------------------------------+------------------------------------------------------------+---------+                                                                          │
 | #1 | KafkaConnectorResourceValida | Missing or empty field:                                    | {}      |                                                                          │
 |    | tion                         | 'metadata.labels.kafka.jikkou.io/connect-cluster'.         |         |                                                                          │
 +----+------------------------------+------------------------------------------------------------+---------+

and that was the whole point of #416 - to configure it dynamically without being dependent on the configuration file and be able to choose your cluster from the KafkaConnector CRD.

i tried to do a work around and just set a fake connect cluster, as im overwriting it anyway:

  apiVersion: kafka.jikkou.io/v1beta1
  kind: KafkaConnector
  metadata:
    name: collection-kafka-connect
    labels:
      kafka.jikkou.io/connect-cluster: use-override
  annotations:
    jikkou.io/config-override:
      url: "http://my-real-cluster:8083"

but i get this error:

Error: KafkaConnectClusterNotFoundException: No connect cluster configured for name 'use-override'                                                                                           

is there a way to use this feature as intended in the issue and not be dependant on the config file?
does something like this can work?

    jikkou.io/config-override:
      url: "http://my-real-cluster:8083"
      name: my-cluster

(i only saw example to URL so i dont know exactly which params can be changed)

thanks a lot for looking into this 🙏

@fhussonnois
Copy link
Member

Hi @arnoldyahad, the kafka.jikkou.io/connect-cluster is required, as you can only override some config properties. In addition, all connectors are grouped per cluster for the reconciliation process. But, yes I will provide a fix so that you don't need to configure a matching cluster in the Jikkou config.

here is an example of the use of the new annotation:

https://www.jikkou.io/docs/providers/kafka-connect/resources/connector/#jikkouioconfig-override-optional

@fhussonnois
Copy link
Member

fhussonnois commented Jun 3, 2024

Hi @arnoldyahad, I've did some checks and everything seem OK. But, I've figure out an issue in your configuration. annotations must be defined under metadata. Also, the configuration override must be passed in JSON (annotation values are expected to be primitive types).

Here is a valid config:

  apiVersion: kafka.jikkou.io/v1beta1
  kind: KafkaConnector
  metadata:
    name: collection-kafka-connect
    labels:
      kafka.jikkou.io/connect-cluster: use-override
    annotations:
      jikkou.io/config-override: |
        { "url": "http://my-real-cluster:8083" }

@arnoldyahad
Copy link
Author

thanks a lot @fhussonnois
problem was indentation in my KafkaConnector regarding annotations/labels. closing this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants