You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by "squakez (via GitHub)" <gi...@apache.org> on 2024/03/25 14:11:25 UTC
[PR] feat(strimzi): bind to either KafkaTopic name or topicName [camel-k]
squakez opened a new pull request, #5281:
URL: https://github.com/apache/camel-k/pull/5281
Closes #4759
<!-- Description -->
<!--
Enter your extended release note in the below block. If the PR requires
additional action from users switching to the new release, include the string
"action required". If no release note is required, write "NONE".
You can (optionally) mark this PR with labels "kind/bug" or "kind/feature" to make sure
the text is added to the right section of the release notes.
-->
**Release Note**
```release-note
feat(strimzi): bind to either KafkaTopic name or topicName
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(strimzi): bind to either KafkaTopic name or topicName [camel-k]
Posted by "lburgazzoli (via GitHub)" <gi...@apache.org>.
lburgazzoli commented on code in PR #5281:
URL: https://github.com/apache/camel-k/pull/5281#discussion_r1537675545
##########
addons/strimzi/duck/v1beta2/duck_types.go:
##########
@@ -40,6 +40,12 @@ const (
type KafkaTopic struct {
metav1.TypeMeta `json:",inline"`
metav1.ObjectMeta `json:"metadata,omitempty"`
+ Spec KafkaTopicSpec `json:"spec,omitempty"`
+}
+
+// KafkaTopicSpec is the duck of a KafkaTopic spec.
+type KafkaTopicSpec struct {
Review Comment:
I'm not familiar with the Kafka Topic CRD and the Strimzi Topic operator, but wonder if we should take the value from the status https://strimzi.io/docs/operators/latest/configuring.html#type-KafkaTopic-reference
@aboucham any idea ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(strimzi): bind to either KafkaTopic name or topicName [camel-k]
Posted by "lburgazzoli (via GitHub)" <gi...@apache.org>.
lburgazzoli commented on PR #5281:
URL: https://github.com/apache/camel-k/pull/5281#issuecomment-2018223817
I don't recall exactly what is the exact machinery so I wonder what happens when the resource is not found or what would happen if the [status](url) sub resource and/or related [topicName](url) is not set at the time of the binding materialization: does the operator re-try to deploy the resource ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(strimzi): bind to either KafkaTopic name or topicName [camel-k]
Posted by "lburgazzoli (via GitHub)" <gi...@apache.org>.
lburgazzoli commented on code in PR #5281:
URL: https://github.com/apache/camel-k/pull/5281#discussion_r1537697837
##########
addons/strimzi/duck/v1beta2/duck_types.go:
##########
@@ -40,6 +40,12 @@ const (
type KafkaTopic struct {
metav1.TypeMeta `json:",inline"`
metav1.ObjectMeta `json:"metadata,omitempty"`
+ Status KafkaTopicStatus `json:"status,omitempty"`
+}
+
+// KafkaTopicStatus is the duck of a KafkaTopic status.
+type KafkaTopicStatus struct {
+ TopicName string `json:"topicName,omitempty"`
Review Comment:
Not sure if we need to add a `status` subresource permission https://github.com/apache/camel-k/blob/main/pkg/resources/config/rbac/descoped/operator-cluster-role-strimzi.yaml
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(strimzi): bind to either KafkaTopic name or topicName [camel-k]
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on code in PR #5281:
URL: https://github.com/apache/camel-k/pull/5281#discussion_r1537693558
##########
addons/strimzi/duck/v1beta2/duck_types.go:
##########
@@ -40,6 +40,12 @@ const (
type KafkaTopic struct {
metav1.TypeMeta `json:",inline"`
metav1.ObjectMeta `json:"metadata,omitempty"`
+ Spec KafkaTopicSpec `json:"spec,omitempty"`
+}
+
+// KafkaTopicSpec is the duck of a KafkaTopic spec.
+type KafkaTopicSpec struct {
Review Comment:
Changed to `.status.topicName`. It seems to be the right thing to do, thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(strimzi): bind to either KafkaTopic name or topicName [camel-k]
Posted by "lburgazzoli (via GitHub)" <gi...@apache.org>.
lburgazzoli commented on PR #5281:
URL: https://github.com/apache/camel-k/pull/5281#issuecomment-2018277563
ok so I think we should probably follow up with some enhancements in case the operator does not retry.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(strimzi): bind to either KafkaTopic name or topicName [camel-k]
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on PR #5281:
URL: https://github.com/apache/camel-k/pull/5281#issuecomment-2018229684
> I don't recall exactly what is the exact machinery so I wonder what happens when the resource is not found or what would happen if the [status](url) sub resource and/or related [topicName](url) is not set at the time of the binding materialization: does the operator re-try to deploy the resource ?
For what I can see in the logic it just fail. I guess this is bubbled up into a Pipe error status.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(strimzi): bind to either KafkaTopic name or topicName [camel-k]
Posted by "lburgazzoli (via GitHub)" <gi...@apache.org>.
lburgazzoli commented on code in PR #5281:
URL: https://github.com/apache/camel-k/pull/5281#discussion_r1537689066
##########
addons/strimzi/duck/v1beta2/duck_types.go:
##########
@@ -40,6 +40,12 @@ const (
type KafkaTopic struct {
metav1.TypeMeta `json:",inline"`
metav1.ObjectMeta `json:"metadata,omitempty"`
+ Spec KafkaTopicSpec `json:"spec,omitempty"`
+}
+
+// KafkaTopicSpec is the duck of a KafkaTopic spec.
+type KafkaTopicSpec struct {
Review Comment:
> I thought the same but I noticed the gen crd was tagged with `noStatus` (tbh, not sure why), reason why I preferred to be conservative. I can definitely move to the status if somebody confirm it would be the best thing to do.
it seems this is for our own processing logic, maybe it is for the RBACs generation. In the CRD definition I see that the staus is defined https://github.com/strimzi/strimzi-kafka-operator/blob/main/install/cluster-operator/043-Crd-kafkatopic.yaml#L77-L110.
Maybe we should ask the Strimzi folks about what is the expected behavior.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(strimzi): bind to either KafkaTopic name or topicName [camel-k]
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on code in PR #5281:
URL: https://github.com/apache/camel-k/pull/5281#discussion_r1537679273
##########
addons/strimzi/duck/v1beta2/duck_types.go:
##########
@@ -40,6 +40,12 @@ const (
type KafkaTopic struct {
metav1.TypeMeta `json:",inline"`
metav1.ObjectMeta `json:"metadata,omitempty"`
+ Spec KafkaTopicSpec `json:"spec,omitempty"`
+}
+
+// KafkaTopicSpec is the duck of a KafkaTopic spec.
+type KafkaTopicSpec struct {
Review Comment:
I thought the same but I noticed the gen crd was tagged with `noStatus` (tbh, not sure why), reason why I preferred to be conservative. I can definitely move to the status if somebody confirm it would be the best thing to do.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(strimzi): bind to either KafkaTopic name or topicName [camel-k]
Posted by "lburgazzoli (via GitHub)" <gi...@apache.org>.
lburgazzoli commented on code in PR #5281:
URL: https://github.com/apache/camel-k/pull/5281#discussion_r1537687133
##########
addons/strimzi/duck/v1beta2/duck_types.go:
##########
@@ -40,6 +40,12 @@ const (
type KafkaTopic struct {
metav1.TypeMeta `json:",inline"`
metav1.ObjectMeta `json:"metadata,omitempty"`
+ Spec KafkaTopicSpec `json:"spec,omitempty"`
+}
+
+// KafkaTopicSpec is the duck of a KafkaTopic spec.
+type KafkaTopicSpec struct {
Review Comment:
> @lburgazzoli I would say `topicName` as this is the name used by the kafka broker to create the topic. kafkaTopic name is only the name of the CR that appears under the api.
>
> Hope that helps ?
I mean the CRD has two "topicName" one in the spec and one in the status.
I think the one in the status is populated only when the topic is actually created. SO I guess it can reflect either the CR name or the spec's `topicName`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(strimzi): bind to either KafkaTopic name or topicName [camel-k]
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez merged PR #5281:
URL: https://github.com/apache/camel-k/pull/5281
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(strimzi): bind to either KafkaTopic name or topicName [camel-k]
Posted by "aboucham (via GitHub)" <gi...@apache.org>.
aboucham commented on code in PR #5281:
URL: https://github.com/apache/camel-k/pull/5281#discussion_r1537682629
##########
addons/strimzi/duck/v1beta2/duck_types.go:
##########
@@ -40,6 +40,12 @@ const (
type KafkaTopic struct {
metav1.TypeMeta `json:",inline"`
metav1.ObjectMeta `json:"metadata,omitempty"`
+ Spec KafkaTopicSpec `json:"spec,omitempty"`
+}
+
+// KafkaTopicSpec is the duck of a KafkaTopic spec.
+type KafkaTopicSpec struct {
Review Comment:
@lburgazzoli I would say `topicName` as this is the name used by the kafka broker to create the topic. kafkaTopic name is only the name of the CR that appears under the api.
Hope that helps ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(strimzi): bind to either KafkaTopic name or topicName [camel-k]
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on code in PR #5281:
URL: https://github.com/apache/camel-k/pull/5281#discussion_r1537745850
##########
addons/strimzi/duck/v1beta2/duck_types.go:
##########
@@ -40,6 +40,12 @@ const (
type KafkaTopic struct {
metav1.TypeMeta `json:",inline"`
metav1.ObjectMeta `json:"metadata,omitempty"`
+ Status KafkaTopicStatus `json:"status,omitempty"`
+}
+
+// KafkaTopicStatus is the duck of a KafkaTopic status.
+type KafkaTopicStatus struct {
+ TopicName string `json:"topicName,omitempty"`
Review Comment:
Should be fixed now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(strimzi): bind to either KafkaTopic name or topicName [camel-k]
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on code in PR #5281:
URL: https://github.com/apache/camel-k/pull/5281#discussion_r1537708901
##########
addons/strimzi/duck/v1beta2/duck_types.go:
##########
@@ -40,6 +40,12 @@ const (
type KafkaTopic struct {
metav1.TypeMeta `json:",inline"`
metav1.ObjectMeta `json:"metadata,omitempty"`
+ Status KafkaTopicStatus `json:"status,omitempty"`
+}
+
+// KafkaTopicStatus is the duck of a KafkaTopic status.
+type KafkaTopicStatus struct {
+ TopicName string `json:"topicName,omitempty"`
Review Comment:
Good point. I'll have a look, thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org